Maybe I'm missing something, but it looks like the e-mail is set initially in Order::preSave() but only for a new order and then when you log in or register, the e-mail is not updated, the result is that no confirmation mail is sent at the end.

I think it would make sense to set the E-Mail when the customer is set, so that those are kept in sync. Not sure if it needs special cases for anonymous users or that maybe getEmail() should even automatically use to the user e-mail if he's registered, which would take care of issues like #2873526: Updating email in user account, doesn't update email in order (cart) mail property. Then the order email field would only be used for anonymous users.

CommentFileSizeAuthor
#5 order_e_mail_is_not_set-2873685-5.patch1.91 KBmglaman
#2 order-update-email-on-customer-change-2873685-2.patch499 bytesBerdir
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir created an issue. See original summary.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Active » Needs review
FileSize
order-update-email-on-customer-change-2873685-2.patch499 bytes
sumanthkumarc’s picture

sumanthkumarc CreditAttribution: sumanthkumarc at Azri Solutions commented
Related issues: +#2873526: Updating email in user account, doesn't update email in order (cart) mail property

Added Related issue and also should we call setCustomer() in orderRefresh?? otherwise the above solution might not help with the related issue.

mglaman’s picture

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

sumanthkumarc, so you're suggesting that on order refresh we check if the order has a customer. If it does, then always sync that address. Which fixes issues if an order is re-assigned.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
FileSize
order_e_mail_is_not_set-2873685-5.patch1.91 KB

Here is my fix. Instead of running setEmail the logic to set the order email to the customer's (if order email empty) out of the isNew check. This fixes the issue for all anonymous orders that convert to an authenticated user due to login at checkout, or registration.

I feel #2873526: Updating email in user account, doesn't update email in order (cart) mail property requires a different fix. Where on user save if the email has changed any related carts are updated.

The PR https://github.com/drupalcommerce/commerce/pull/727 has two commits: one with the failing test to prove what Berdir reported, and a second to provide the fix.

EDIT:

The real bug seems to be the register pane form. When a user is logged in we run the following in order assignment:

    $order->setCustomer($account);
    $order->setEmail($account->getEmail());

However, I think the proposed fix will help catch any of these type of accidents where someone implementing forgets to explicitly set the order email after setting the customer. Especially when using setCustomerId.

  • bojanz committed e93fdfc on 8.x-2.x authored by mglaman 
    Issue #2873685 by Berdir, mglaman: Order E-Mail is not set when logging...
bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Needs review » Fixed

This PR landed while I was on vacation, and I completely overlooked it. Sorry about that.

Status: Fixed » Closed (fixed)

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