After created an refund order , trying to update I checked the order don't save or lose some information as owner uid, created date, etc. I checked the form structure and we are not loading the correct created time.
Also I checked the implementation and you have part of your code similar with DrupalCommerceOrder, as the save function, but in other parts, as to the forms validation and submit you are using a different a approach. Which implementation pattern are you using for this module? If you let me know It will be easier to me create a patch that you can accept for fix those issues.
Thanks,
Comments
Comment #1
dhernandes CreditAttribution: dhernandes commentedComment #2
dhernandes CreditAttribution: dhernandes commentedHi did some changes in the module to fix the problems I was folding regarding the issue reported. I am attached the patch here in case you think you could use it.
The changes I've implemented are:
Comment #3
GoZ CreditAttribution: GoZ commentedI can't reproduce. Can you explain how to reproduce issue ?
What i check :
- Create client account. Make and order with client account.
- With admin (super admin), update order to complete status.
- With client, create return order.
- With admin, check order (all is all right), update order (client name of shipping informations. All datas are corrects.
Make same test creating return with admin, and no issues.
Comment #4
agoradesign CreditAttribution: agoradesign commentedI can reproduce this very easily:
I'm using Entity API 1.5 with Drupal 7.34 and the current dev version (7.x-2.0-rc2+15-dev) of Commerce RMA.
I did have problems with dev and stable, when I tried the module about one month ago. The patch seemed to solve my problems. I'm not sure, which properties were lost after update. However, today I've tried Commerce RMA again and installed the current dev version. I did the following steps:
1. create a return order with the client.
2. As for my installation "pending" status is also among the allowed ones, I went directly as admin to the edit page of the return entity and simply just pressed the save button - one time I tried with and one time without creating a revision.
3. No matter, if I chosse to create a revision or not, the create date of the return entity is always lost (to be exact: the value of the create date is set to '8')
The patch from #2 unfortunately does not apply anymore...
PS: it doesn't make a difference, if I leave the date field of the edit form empty or not
Comment #5
agoradesign CreditAttribution: agoradesign commentedI've re-rolled the patch from #2 to work with the latest dev, and modified a few things. Here are my changes (in comparison to the original patch):
I dropped this for several reasons:
1. I couldn't reproduce this at all
2. Even if I could, this wouldn't be an appropriate fix at all. Without saving the return explicitely, nothing - no return, no line items, etc - should be saved!
I dropped this because this has nothing to do with the original issue. You should open a new issue instead. Mainly, this is a architectural decision, the module owners have to decide. Imho there's nothing wrong about having the possibility to create more than one return per order. And even if you want to prevent this, running a query within commerce_return_new_order_wrapper() function is definitely the wrong place. Because if you change it like that, the function won't longer do what it should do.
In the form validation, the original patch accidentially dropped the call to field_attach_form_validate(), which I re-included (this is also included in the commerce_order module code)
An open question are the removed lines in commerce_return_form() (in commerce_return.admin.inc). On the one side, commerce_order also doesn't have this else-branch (that's, why the patch removes it), on the other side, I don't think it's bad or harmful. Nevertheless, my patch removes the lines, like it was in the original patch.
Summarized, this patch now only adjusts code parts (form validation, submit + a small change in an if condition inside the save function) to match exactly the approach, how these things are handled in commerce_order. This helps losing date information on updating an order.
Comment #6
jkuma CreditAttribution: jkuma as a volunteer commentedOk, fair enough, thanks for the hard work here, the code looks cleaner and it fixes up the raised issue. I've remove some trailing whitespace and some line codes that are no longer needed. I'm updating this issue to RTBC.
Comment #8
jkuma CreditAttribution: jkuma as a volunteer commentedCommitted, thank you guys!