The commerce order normally saves the REQUEST_TIME of the server in both create time as well as changed time. But this time is the time where the request has been sent. In a single request, there can be a multiple order updates to an order via commerce_order_save(). In those cases, it the $order->changed should not save the created time rather it should pick the time when the order is updated.

CommentFileSizeAuthor
#12 commerce_order_changed_time_update_issue-2941961-12465612.patch1.22 KBpoornachandran
#2 commerce_order_changed_time_update_issue-2941961-12465612.patch1.22 KBpoornachandran
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Poornachandran created an issue. See original summary.

poornachandran’s picture

poornachandran CreditAttribution: poornachandran as a volunteer and at Capgemini commented
FileSize
commerce_order_changed_time_update_issue-2941961-12465612.patch1.22 KB

Created a patch which will fix the issue..

poornachandran’s picture

poornachandran CreditAttribution: poornachandran as a volunteer and at Capgemini commented
Status: Active » Needs review
poornachandran’s picture

poornachandran CreditAttribution: poornachandran as a volunteer and at Capgemini commented
Issue summary: View changes
poornachandran’s picture

poornachandran CreditAttribution: poornachandran as a volunteer and at Capgemini commented
Related issues: +#2593983: Create a utility method to compare an orders changed date against the database

Status: Needs review » Needs work

The last submitted patch, 2: commerce_order_changed_time_update_issue-2941961-12465612.patch, failed testing. View results

poornachandran’s picture

poornachandran CreditAttribution: poornachandran as a volunteer and at Capgemini commented
Version: 7.x-1.x-dev » 7.x-1.14
BramDriesen’s picture

BramDriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
CreditAttribution: BramDriesen as a volunteer and at Capgemini commented

Any motivation of why this is needed? Personally I don't see an issue with using the first timestamp that started the the request to change the order.

Also tests failed to run so re-scheduled them.

poornachandran’s picture

poornachandran CreditAttribution: poornachandran as a volunteer and at Capgemini commented

@BramDriesen Actually the logic used in commerce_order_save applies only if there is only one order save per request. But if in a single request if we create and update the order, then it should update the changed time as well (indeed I have a requirement like that :) ). But that is not happening here..That is why I created this.

poornachandran’s picture

poornachandran CreditAttribution: poornachandran as a volunteer and at Capgemini commented
Status: Needs work » Needs review
mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented

Any motivation of why this is needed? Personally I don't see an issue with using the first timestamp that started the the request to change the order.

Long-running command line requests. The REQUEST_TIME is set from the parent process. Any fork carries the same constant. So an hour-long process uses that initial timestamp, forever.

poornachandran’s picture

poornachandran CreditAttribution: poornachandran as a volunteer and at Capgemini commented
FileSize
commerce_order_changed_time_update_issue-2941961-12465612.patch1.22 KB
poornachandran’s picture

poornachandran CreditAttribution: poornachandran as a volunteer and at Capgemini commented

Status: Needs review » Needs work

The last submitted patch, 12: commerce_order_changed_time_update_issue-2941961-12465612.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

poornachandran’s picture

poornachandran CreditAttribution: poornachandran as a volunteer and at Capgemini commented

The test passed with PHP 5.4

poornachandran’s picture

poornachandran CreditAttribution: poornachandran as a volunteer and at Capgemini commented
Status: Needs work » Needs review
czigor’s picture

czigor CreditAttribution: czigor at Centarro commented
Version: 7.x-1.14 » 7.x-1.x-dev
Status: Needs review » Reviewed & tested by the community

I like that the patch in #12 still uses REQUEST_TIME for new orders and time() for order updates. This is ready to be committed.

rszrama’s picture

rszrama CreditAttribution: rszrama at Centarro commented
Assigned: poornachandran » Unassigned
Priority: Major » Normal
Status: Reviewed & tested by the community » Fixed

Thanks for the review, committed!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.