Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent behaviour resaving elements #1345

Closed
johnnynotsolucky opened this issue Mar 30, 2020 · 3 comments
Closed

Inconsistent behaviour resaving elements #1345

johnnynotsolucky opened this issue Mar 30, 2020 · 3 comments
Labels

Comments

@johnnynotsolucky
Copy link
Contributor

johnnynotsolucky commented Mar 30, 2020

Description

Orders resave action doesn't update elements.dateUpdated, but updates orders.dateUpdated. Therefore using the .dateUpdated() filter can cause orders to be removed from the result set if there's orders which have been resaved using ./craft resave/orders.

Resaving entries does not update the dateUpdated field in either table.

It's not clear whether dateUpdated should be updated on resave for all elements (which would probably put this as a cms bug), or whether the orders resave is updating incorrectly.

Steps to reproduce

  1. Find an older order:
select elements.id, type, elements.dateCreated, elements.dateUpdated as elementUpdated, orders.dateUpdated as orderUpdated from craft_commerce_orders orders join craft_elements elements on orders.id = elements.id where elements.id = 25;
+-------+-------------------------------+---------------------+---------------------+---------------------+
| id    | type                          | dateCreated         | elementUpdated      | orderUpdated        |
+-------+-------------------------------+---------------------+---------------------+---------------------+
| 25    | craft\commerce\elements\Order | 2019-08-28 00:16:16 | 2019-08-28 00:16:16 | 2019-08-28 00:16:16 |
+-------+-------------------------------+---------------------+---------------------+---------------------+
  1. Run ./craft resave/orders --element-id 25 and rerun the query and observe that only the orders.dateUpdated field has changed:
+-------+-------------------------------+---------------------+---------------------+---------------------+
| id    | type                          | dateCreated         | elementUpdated      | orderUpdated        |
+-------+-------------------------------+---------------------+---------------------+---------------------+
| 25    | craft\commerce\elements\Order | 2019-08-28 00:16:16 | 2019-08-28 00:16:16 | 2020-03-30 12:08:54 |
+-------+-------------------------------+---------------------+---------------------+---------------------+
  1. Try find the order in code:
>>> $orders->dateUpdated(['and', '> 2020-03-30 00:00', '< 2020-04-01 00:00'])->one();
=> null
  1. Save the order via the CP, and try again:
>>> $orders->dateUpdated(['and', '> 2020-03-30 00:00', '< 2020-04-01 00:00'])->one();
=> craft\commerce\elements\Order {
     +id: "25",
     +number: "<redacted>",
     +reference: "<redacted>",
     +couponCode: null,
     +isCompleted: "1",
     +dateOrdered: DateTime @1566942720 {
       date: 2019-08-27 14:52:00.0 America/Los_Angeles (-07:00),
     },
     +datePaid: DateTime @1566967948 {
       date: 2019-08-27 21:52:28.0 America/Los_Angeles (-07:00),
     },
     +currency: "USD",
     ...
     +dateCreated: DateTime @1566951376 {
       date: 2019-08-27 17:16:16.0 America/Los_Angeles (-07:00),
     },
     +dateUpdated: DateTime @1585570743 {
       date: 2020-03-30 05:19:03.0 America/Los_Angeles (-07:00),
     },

Additional info

The missing results are due to the dateUpdated filter being applied to both orders and elements tables:

>>> $orders->dateUpdated(['and', '> 2020-03-30 00:00', '< 2020-04-01 00:00'])->getRawSql();
=> """
   SELECT ... 
   FROM (SELECT `elements`.`id` AS `elementsId`, `elements_sites`.`id` AS `elementsSitesId`, `content`.`id` AS `contentId`
   FROM `craft_elements` `elements`
   INNER JOIN `craft_commerce_orders` `commerce_orders` ON `commerce_orders`.`id` = `elements`.`id`
   LEFT JOIN `craft_commerce_addresses` `billing_address` ON billing_address.id = `commerce_orders`.`billingAddressId`
   LEFT JOIN `craft_commerce_addresses` `shipping_address` ON shipping_address.id = `commerce_orders`.`shippingAddressId`
   INNER JOIN `craft_elements_sites` `elements_sites` ON `elements_sites`.`elementId` = `elements`.`id`
   INNER JOIN `craft_content` `content` ON `content`.`elementId` = `elements`.`id`
   WHERE (`commerce_orders`.`dateUpdated` > '2020-03-30 07:00:00') AND (`commerce_orders`.`dateUpdated` < '2020-04-01 07:00:00') AND (`elements`.`archived`=FALSE) AND (`elements`.`dateDeleted` IS NULL) AND ((`elements`.`dateUpdated` > '2020-03-30 07:00:00') AND (`elements`.`dateUpdated` < '2020-04-01 07:00:00')) AND (`elements_sites`.`enabled`=TRUE) AND (`elements`.`draftId` IS NULL) AND (`elements`.`revisionId` IS NULL)
   ORDER BY `commerce_orders`.`id`) `subquery`
   INNER JOIN `craft_commerce_orders` `commerce_orders` ON `commerce_orders`.`id` = `subquery`.`elementsId`
   LEFT JOIN `craft_commerce_addresses` `billing_address` ON billing_address.id = `commerce_orders`.`billingAddressId`
   LEFT JOIN `craft_commerce_addresses` `shipping_address` ON shipping_address.id = `commerce_orders`.`shippingAddressId`
   INNER JOIN `craft_elements` `elements` ON `elements`.`id` = `subquery`.`elementsId`
   INNER JOIN `craft_elements_sites` `elements_sites` ON `elements_sites`.`id` = `subquery`.`elementsSitesId`
   INNER JOIN `craft_content` `content` ON `content`.`id` = `subquery`.`contentId`
   ORDER BY `commerce_orders`.`id`
   """
>>>
  • Craft version: 3.4.9
  • Commerce version: 3.0.1
  • PHP version: 7.3.14-6+ubuntu18.04.1+deb.sury.org+1
  • Database driver & version: 10.1.44-MariaDB-0ubuntu0.18.04.1, 10.3.22-MariaDB-0ubuntu0.19.10.1
lukeholder added a commit that referenced this issue Apr 1, 2020
@lukeholder
Copy link
Member

Thanks for reporting this has been fixed in da8d20b which will be out this week in Commerce 3.1

@johnnynotsolucky
Copy link
Contributor Author

@lukeholder I'm not sure if da8d20b resolves the issue. Removing the commerce_orders.dateUpdated means that the date range is applied to elements.dateUpdated, which is fine, but resaving an order which was created on 2020-02-01 will update the the order record with the new date, and the corresponding elements record will keep the original date. So applying the date range assuming that the orders dateUpdated is at 2020-04-02, will not return the expected results.

@lukeholder
Copy link
Member

lukeholder commented Apr 16, 2020

Hi @johnnynotsolucky

The elements.dateUpdated and commerce_orders.dateUpdated columns should definitely updated when you save an Order.

Do you mean the resave/orders CLI command?

If so, that command specifically does not make changes to the elements elements.dateUpdated on purpose since there are no new values (changes) being made by a user, so updating dateUpdated wouldn’t really add value. That was a https://github.com/craftcms/cms decision, you can submit a ticket there if you would like to see an option added to turn that off in the command.

My change (da8d20b ) means that we never use the commerce_orders.dateUpdated value anywhere in the code, so the date in the orders table will never be used and we have a consistent understanding of the date updated (This is the same as entry elements etc).

Having said that I can see it is a little confusing to see the dates as different in the DB and when using the resave/orders cli command, and I have gone ahead and made sure they stay in sync from now on for all commerce elements: 38b196cc870d4db35add006cb23e8feb633fef6

I you want to fix them on mass, you would need to write your own SQL command to update them.

Something like?:

UPDATE craft_commerce_orders orders
INNER JOIN craft_elements elements	
ON orders.id = elements.id
SET orders.dateUpdated = elements.dateUpdated

Confirm the dates are the same with:

SELECT orders.dateUpdated orderDate, elements.dateUpdated elementDate, elements.id
FROM craft_commerce_orders orders
INNER JOIN craft_elements elements
ON elements.id = orders.id

This will be included in the next release. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants