$order->setCustomer($account);
    $order->setEmail($account->getEmail());
    // Notify other modules.
    $event = new OrderAssignEvent($order, $account);
    $this->eventDispatcher->dispatch(OrderEvents::ORDER_ASSIGN, $event);

Our current flow is:
1) Modify order
2) Dispatch event
3) Save order

That means that event subscribers aren't able to determine the original customer/email. This is often needed, some actions are only performed when assigning an anonymous order (VS reassigning an authenticated one). This oversight is a result of OrderAssignment initially only being used for anonymous orders, but that's no longer true today (due to OrderReassignForm).

Plan

1. Clean up the service and event by replacing $account with $customer, deprecating the old event getter.
2. Fire the event before the order is modified.
3. Fix the event docs to mention in which cases the event is fired, and what the order contains

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bojanz created an issue. See original summary.

bojanz’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
7.32 KB

Let's make it explicit, by adding a new method to the event.

Also, let's rename $account to $customer.
$account / getAccount() is from the time orders had owners instead of customers. For consistency's sake it makes sense to call this parameter $customer.

bojanz’s picture

Title: OrderAssignElement doesn't have access to the original customer/email » OrderAssignElement doesn't have access to the original customer

Status: Needs review » Needs work

The last submitted patch, 2: 3047875-2-order-assign.patch, failed testing. View results

bojanz’s picture

Title: OrderAssignElement doesn't have access to the original customer » OrderAssignEvent doesn't have access to the original customer/email
Issue summary: View changes

Reverting title back, since we do need to care about both the original customer and the original email.

Adding an implementation plan to the issue summary.

Decided to remove getOriginalCustomer() and ensure that the order given to the event is the unchanged one. Didn't feel like adding both getOriginalCustomer() and getOriginalEmail() to the event for what is essentially a corner case.

bojanz’s picture

Status: Needs work » Needs review
FileSize
7.01 KB

Updated patch.

  • bojanz committed 940da84 on 8.x-2.x
    Issue #3047875 by bojanz: OrderAssignEvent doesn't have access to the...
bojanz’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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