A "placed" field was added to the commerce_order table as part of #2888407: Backport the "placed" timestamp from 2.x, but that column wasn't added to the revision table, it'd definitely help to add it.
I've experienced weird issues on a project where the order revision was reverted to a previous one and since we don't store the placed timestamp as part of the revision, that value is lost and there's no way to recover it...
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 3052201_6.patch | 1.51 KB | khiminrm |
| #6 | interdiff_2_6.txt | 831 bytes | khiminrm |
| #2 | 3052201_2.patch | 1.31 KB | khiminrm |
Comments
Comment #2
khiminrm commentedHi @jsacksick, I've created patch for adding column 'placed' to 'commerce_order_revision' table. Should we also create similar drush command as 'drush_commerce_order_update_placed_timestamp' fo updating values for 'placed' in 'commerce_order_revision' table?
Comment #3
khiminrm commentedComment #4
jsacksick commentedNo I don't think we need a drush command, perhaps just a MYSQL query in the update hook. But wondering if it's ok to just update million of rows for people with big amount of orders.
Comment #5
rszrama commentedHmm, yeah. If I'm not mistaken, in another such update instance, what we did was we permitted site maintainers to set a variable in settings.php prior to running the update hook that would cause it to ignore the update. So, for example, you could set $conf['skip_order_revision_update'] to FALSE, and when you run update.php, the system table would still increment the module version number but not actually process the update. We'd need to document this in the docblock of the update hook and in a change notice / release notes, and we'd need to point folks who skip this to the query or other code needed to run the update.
The idea here is that the site could be placed in maintenance mode, the update performed, and then an asynchronous process utilized to perform the necessary database updates before the site exits maintenance mode. @jsacksick, any way we have to test this?
Comment #6
khiminrm commented@rszrama I've improved the patch.
Comment #7
khiminrm commentedComment #8
khiminrm commenteduploaded missed patch file for #6
Comment #9
danmer commentedTested. Patch from comment #8 works as expected.
Comment #10
rszrama commentedThanks, guys! I've revised the comment a little bit for clarity and committed.