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.

Sign up for GitHub

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

Jump to bottom

Deprecate email as an attribute on the Order element #3201

Merged
merged 2 commits into from
Jul 17, 2023
Merged

Deprecate email as an attribute on the Order element #3201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d6c376
Deprecated `setEmail` and `$email` as an attribute on the `Order` ele…
nfourtythree Jun 22, 2023
1f3d659
Merge branch '4.3' into bugfix/email-property-on-order
nfourtythree Jul 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## 4.3.0 - Unreleased

- Guest customers registering during checkout now have their addresses saved to their account.
- Deprecated `craft\commerce\elements\Order::setEmail()`. `Order::setCustomer()` should be used instead.

## Unreleased

Expand Down
3 changes: 2 additions & 1 deletion src/controllers/CartController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,8 @@ public function actionUpdateCart(): ?Response
$email = $this->request->getParam('email');
if ($email && ($this->_cart->getEmail() === null || $this->_cart->getEmail() != $email)) {
try {
$this->_cart->setEmail($email);
$user = Craft::$app->getUsers()->ensureUserByEmail($email);
$this->_cart->setCustomer($user);
} catch (\Exception $e) {
$this->_cart->addError('email', $e->getMessage());
}
Expand Down
46 changes: 14 additions & 32 deletions src/elements/Order.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1109,20 +1109,6 @@ class Order extends Element
*/
private ?string $_paymentCurrency = null;

/**
* @var string|null
* @see Order::setEmail() To set the order email
* @see Order::getEmail() To get the email
* ---
* ```php
* echo $order->email;
* ```
* ```twig
* {{ order.email }}
* ```
*/
private ?string $_email = null;

/**
* @var Transaction[]|null
* @see Order::getTransactions()
Expand All @@ -1148,7 +1134,7 @@ class Order extends Element
* {{ order.customer }}
* ```
*/
private User|null|false $_customer;
private User|null|false $_customer = null;

/**
* @var float|null
Expand Down Expand Up @@ -1327,7 +1313,6 @@ public function attributes(): array
$names[] = 'customerId';
$names[] = 'paymentCurrency';
$names[] = 'paymentAmount';
$names[] = 'email';
$names[] = 'isPaid';
$names[] = 'itemSubtotal';
$names[] = 'itemTotal';
Expand Down Expand Up @@ -1411,6 +1396,7 @@ public function fields(): array
};
}

$fields['email'] = 'email';
$fields['paidStatusHtml'] = 'paidStatusHtml';
$fields['customerLinkHtml'] = 'customerLinkHtml';
$fields['orderStatusHtml'] = 'orderStatusHtml';
Expand Down Expand Up @@ -1482,7 +1468,6 @@ protected function defineRules(): array

[['paymentSourceId'], 'number', 'integerOnly' => true],
[['paymentSourceId'], 'validatePaymentSourceId'],
[['email'], 'email'],

[['number', 'user', 'orderCompletedEmail'], 'safe'],
]);
Expand Down Expand Up @@ -2281,10 +2266,6 @@ public function getCustomer(): ?User
}
}

if ($this->_customer) {
$this->_email = $this->_customer->email;
}

return $this->_customer ?: null;
}

Expand All @@ -2298,7 +2279,8 @@ public function setCustomer(?User $customer = null): void
$this->_customer = $customer;
if ($this->_customer) {
$this->_customerId = $this->_customer->id;
$this->_email = $this->_customer->email;
} else {
$this->_customerId = null;
}
}

Expand All @@ -2307,7 +2289,7 @@ public function setCustomer(?User $customer = null): void
*/
public function getUser(): ?User
{
Craft::$app->getDeprecator()->log('Order::getUser()', 'The `Order::getUser()` is deprecated, use the `Order::getCustomer()` instead.');
Craft::$app->getDeprecator()->log('Order::getUser()', 'The `Order::getUser()` is deprecated, use `Order::getCustomer()` instead.');
return $this->getCustomer();
}

Expand All @@ -2316,37 +2298,37 @@ public function getUser(): ?User
*
* @param string|null $email
* @throws Exception
* @deprecated in 4.3.0. Use [[setCustomer()]] instead.
*/
public function setEmail(?string $email): void
{
Craft::$app->getDeprecator()->log(__METHOD__, '`Order::setEmail()` has been deprecated use `Order::setCustomer()` instead.');
if (!$email) {
$this->_customer = null;
$this->_customerId = null;
$this->_email = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason not to clear the this->_email ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where have you seen that, I can't see it in the code?

return;
}

if ($this->_email === $email) {
if ($this->_customer && $this->_customer->email === $email) {
return;
}

$user = Craft::$app->getUsers()->ensureUserByEmail($email);
$this->_email = $email;
$this->setCustomer($user);
}

/**
* Returns the email for this order. Will always be the registered users email if the order's customer is related to a user.
* Returns the email for this order. Will always be the customer's email if they exist.
* @return string|null
*/
public function getEmail(): ?string
{
if ($user = $this->getCustomer()) {
$this->_email = $user->email;
}

return $this->_email ?? null;
return $this->getCustomer()?->email ?? null;
}

/**
* @return bool
*/
public function getIsPaid(): bool
{
return !$this->hasOutstandingBalance() && $this->isCompleted;
Expand Down
15 changes: 14 additions & 1 deletion src/elements/db/OrderQuery.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1412,6 +1412,14 @@ public function withTransactions(bool $value = true): OrderQuery
*/
public function populate($rows): array
{
// @TODO remove at next breaking change
// Remove `email` key from each row.
array_walk($rows, function(&$row) {
if (array_key_exists('email', $row)) {
unset($row['email']);
}
});

/** @var Order[] $orders */
$orders = parent::populate($rows);

Expand Down Expand Up @@ -1463,7 +1471,10 @@ protected function beforePrepare(): bool
'commerce_orders.couponCode',
'commerce_orders.orderStatusId',
'commerce_orders.dateOrdered',

// @TODO remove at next breaking change
'commerce_orders.email',

'commerce_orders.isCompleted',
'commerce_orders.datePaid',
'commerce_orders.currency',
Expand Down Expand Up @@ -1536,7 +1547,9 @@ protected function beforePrepare(): bool
}

if (isset($this->email) && $this->email) {
$this->subQuery->andWhere(Db::parseParam('commerce_orders.email', $this->email, '=', true));
// Join and search the users table for email address
$this->subQuery->leftJoin(CraftTable::USERS . ' users', '[[users.id]] = [[commerce_orders.customerId]]');
$this->subQuery->andWhere(Db::parseParam('users.email', $this->email, '=', true));
}

// Allow true ot false but not null
Expand Down
7 changes: 3 additions & 4 deletions src/services/Carts.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,9 @@ public function getCart(bool $forceSave = false): Order
$this->_cart->paymentCurrency = $this->_getCartPaymentCurrencyIso();
$this->_cart->origin = Order::ORIGIN_WEB;

if ($currentUser) {
if ($this->_cart->getCustomer() === null || ($currentUser->email && $currentUser->email !== $this->_cart->email)) {
$this->_cart->setEmail($currentUser->email); // Will ensure the customer is also set
}
// Switch the cart customer if needed
if ($currentUser && ($this->_cart->getCustomer() === null || ($currentUser->email && $currentUser->email !== $this->_cart->getEmail()))) {
$this->_cart->setCustomer($currentUser);
}

$hasIpChanged = $originalIp != $this->_cart->lastIp;
Expand Down
97 changes: 47 additions & 50 deletions src/services/Customers.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,75 +312,72 @@ public function transferCustomerData(User $fromCustomer, User $toCustomer): bool
*/
private function _activateUserFromOrder(Order $order): void
{
if (!$order->email) {
$user = $order->getCustomer();
if (!$user || $user->getIsCredentialed()) {
return;
}

$user = Craft::$app->getUsers()->ensureUserByEmail($order->email);
$billingAddress = $order->getBillingAddress();
$shippingAddress = $order->getShippingAddress();

if (!$user->getIsCredentialed()) {
$billingAddress = $order->getBillingAddress();
$shippingAddress = $order->getShippingAddress();

if (!$user->fullName) {
$user->fullName = $billingAddress?->fullName ?? $shippingAddress?->fullName ?? '';
}
if (!$user->fullName) {
$user->fullName = $billingAddress?->fullName ?? $shippingAddress?->fullName ?? '';
}

$user->username = $order->email;
$user->pending = true;
$user->setScenario(Element::SCENARIO_ESSENTIALS);
$user->username = $order->getEmail();
$user->pending = true;
$user->setScenario(Element::SCENARIO_ESSENTIALS);

if (Craft::$app->getElements()->saveElement($user)) {
Craft::$app->getUsers()->assignUserToDefaultGroup($user);
$emailSent = Craft::$app->getUsers()->sendActivationEmail($user);
if (Craft::$app->getElements()->saveElement($user)) {
Craft::$app->getUsers()->assignUserToDefaultGroup($user);
$emailSent = Craft::$app->getUsers()->sendActivationEmail($user);

if (!$emailSent) {
Craft::warning('"registerUserOnOrderComplete" used to create the user, but couldn’t send an activation email. Check your email settings.', __METHOD__);
}
if (!$emailSent) {
Craft::warning('"registerUserOnOrderComplete" used to create the user, but couldn’t send an activation email. Check your email settings.', __METHOD__);
}

if ($billingAddress || $shippingAddress) {
$newAttributes = ['ownerId' => $user->id];
if ($billingAddress || $shippingAddress) {
$newAttributes = ['ownerId' => $user->id];

// If there is only one address make sure we don't add duplicates to the user
if ($order->hasMatchingAddresses()) {
$newAttributes['title'] = Craft::t('commerce', 'Address');
$shippingAddress = null;
}
// If there is only one address make sure we don't add duplicates to the user
if ($order->hasMatchingAddresses()) {
$newAttributes['title'] = Craft::t('commerce', 'Address');
$shippingAddress = null;
}

// Copy addresses to user
if ($billingAddress) {
$newBillingAddress = Craft::$app->getElements()->duplicateElement($billingAddress, $newAttributes);
// Copy addresses to user
if ($billingAddress) {
$newBillingAddress = Craft::$app->getElements()->duplicateElement($billingAddress, $newAttributes);

/**
* Because we are cloning from an order address the `CustomerAddressBehavior` hasn't been instantiated
* therefore we are unable to simply set the `isPrimaryBilling` property when specifying the new attributes during duplication.
*/
if (!$newBillingAddress->hasErrors()) {
$this->savePrimaryBillingAddressId($user, $newBillingAddress->id);
/**
* Because we are cloning from an order address the `CustomerAddressBehavior` hasn't been instantiated
* therefore we are unable to simply set the `isPrimaryBilling` property when specifying the new attributes during duplication.
*/
if (!$newBillingAddress->hasErrors()) {
$this->savePrimaryBillingAddressId($user, $newBillingAddress->id);

if ($order->hasMatchingAddresses()) {
$this->savePrimaryShippingAddressId($user, $newBillingAddress->id);
}
if ($order->hasMatchingAddresses()) {
$this->savePrimaryShippingAddressId($user, $newBillingAddress->id);
}
}
}

if ($shippingAddress) {
$newShippingAddress = Craft::$app->getElements()->duplicateElement($shippingAddress, $newAttributes);
if ($shippingAddress) {
$newShippingAddress = Craft::$app->getElements()->duplicateElement($shippingAddress, $newAttributes);

/**
* Because we are cloning from an order address the `CustomerAddressBehavior` hasn't been instantiated
* therefore we are unable to simply set the `isPrimaryShipping` property when specifying the new attributes during duplication.
*/
if (!$newShippingAddress->hasErrors()) {
$this->savePrimaryShippingAddressId($user, $newShippingAddress->id);
}
/**
* Because we are cloning from an order address the `CustomerAddressBehavior` hasn't been instantiated
* therefore we are unable to simply set the `isPrimaryShipping` property when specifying the new attributes during duplication.
*/
if (!$newShippingAddress->hasErrors()) {
$this->savePrimaryShippingAddressId($user, $newShippingAddress->id);
}
}
} else {
$errors = $user->getErrors();
Craft::warning('Could not create user on order completion.', __METHOD__);
Craft::warning($errors, __METHOD__);
}
} else {
$errors = $user->getErrors();
Craft::warning('Could not create user on order completion.', __METHOD__);
Craft::warning($errors, __METHOD__);
}
}
}
Loading
Loading