Orders with multiple order items generate multiple emails

this is the output of the query after this MR

SELECT "o"."order_id" AS "order_id", "o"."mail" AS "mail"
FROM
{commerce_order} "o"
LEFT OUTER JOIN {commerce_abandoned_carts} "a" ON o.order_id = a.order_id
INNER JOIN {commerce_order_item} "l" ON o.order_id = l.order_id
WHERE ("o"."mail" != '') AND ("a"."status" IS NULL) AND ("o"."changed" <= '1702846318') AND ("o"."changed" >= '1701550378') AND ("o"."state" = 'draft')
GROUP BY "o"."order_id", "o"."mail"
ORDER BY "o"."changed" ASC

I have tested this and this fixes the issue.
I have bumped this to critical because anyone using this module where users have more than one item type in cart will get duplicates.

This other issue seems to solve in a slightly different way: https://www.drupal.org/project/commerce_abandoned_carts/issues/3212480 🐛 SQL error if we have orders with multiple line items Needs review

This needs a test to get in. The test should cover both this issue as 🐛 SQL error if we have orders with multiple line items Needs review .

The test is also extra important because in ✨ Use queue for sending emails Needs review I'm working on a major overhaul of the module. And I want to make sure that this bug isn't in there.

Also a translated string should not be logged.

I've worked on test coverage for this issue. It does not cover the SQL error yet that is reported in 🐛 SQL error if we have orders with multiple line items Needs review .

I also haven't checked yet if the provided fix makes the test pass. I only had a few spare minutes to write a test today.

I think if you're moving to a queue since you can't rely on the sent status for the cart you need to check the queue as well in case an email gets in the queue more than once for the same cart.

So you might want to manually add the same cart to the queue twice and make sure the email is only sent once. That will cover any other instances in addition to the ones you already caught here.

I found out that the SQL error did occur, but it does not get reported by the test. Probably because the cron runner catches and logs that exception, so the test does not see it.

I've updated MR 4 (3366213-groupby-ordder) with the following changes:

• The test from the tests_only branch is added.
• The logged message is no longer a translation (addresses #8).
• An additional fix for 🐛 SQL error if we have orders with multiple line items Needs review is included, a merge query is used instead of an issert query. This avoids the SQL error.

If this passes tests, then I think it's ready.

@nicxvan
Note: ✨ Use queue for sending emails Needs review is not yet updated. Good idea to expand the test by checking the number of queue items created there. However, if you add the same task twice to a queue, I think two mails will still be sent, because the one queue item does not know about the existance of the other queue item.

Yes, your note is the issue I was concerned about, the test won't catch multiple emails that get queued, you need to add a check before sending to see if an email has been sent for the cart, and this test would have to be updated in order to catch multi queue items.

I think the MR looks good, thanks both of you!

MR is failing because it is pointed to the old 8.x-1.x and that branch does not have this fix: https://git.drupalcode.org/project/commerce_abandoned_carts/-/commit/522... . Other than that tests seems to be green. I suppose this will go into 2.0.x.

The MR is merged!

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