diff --git a/CHANGELOG.md b/CHANGELOG.md index 65846a0854..06336a357b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/controllers/CartController.php b/src/controllers/CartController.php index 61af99d9bd..e2836ad29c 100644 --- a/src/controllers/CartController.php +++ b/src/controllers/CartController.php @@ -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()); } diff --git a/src/elements/Order.php b/src/elements/Order.php index a38c40a3b8..1bf2b6b9ca 100644 --- a/src/elements/Order.php +++ b/src/elements/Order.php @@ -1138,20 +1138,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() @@ -1177,7 +1163,7 @@ class Order extends Element * {{ order.customer }} * ``` */ - private User|null|false $_customer; + private User|null|false $_customer = null; /** * @var float|null @@ -1356,7 +1342,6 @@ public function attributes(): array $names[] = 'customerId'; $names[] = 'paymentCurrency'; $names[] = 'paymentAmount'; - $names[] = 'email'; $names[] = 'isPaid'; $names[] = 'itemSubtotal'; $names[] = 'itemTotal'; @@ -1440,6 +1425,7 @@ public function fields(): array }; } + $fields['email'] = 'email'; $fields['paidStatusHtml'] = 'paidStatusHtml'; $fields['customerLinkHtml'] = 'customerLinkHtml'; $fields['orderStatusHtml'] = 'orderStatusHtml'; @@ -1511,7 +1497,6 @@ protected function defineRules(): array [['paymentSourceId'], 'number', 'integerOnly' => true], [['paymentSourceId'], 'validatePaymentSourceId'], - [['email'], 'email'], [['number', 'user', 'orderCompletedEmail', 'saveBillingAddressOnOrderComplete', 'saveShippingAddressOnOrderComplete'], 'safe'], ]); @@ -2312,10 +2297,6 @@ public function getCustomer(): ?User } } - if ($this->_customer) { - $this->_email = $this->_customer->email; - } - return $this->_customer ?: null; } @@ -2329,7 +2310,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; } } @@ -2338,7 +2320,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(); } @@ -2347,37 +2329,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; 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; diff --git a/src/elements/db/OrderQuery.php b/src/elements/db/OrderQuery.php index 49eacced74..9b03a170c0 100644 --- a/src/elements/db/OrderQuery.php +++ b/src/elements/db/OrderQuery.php @@ -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); @@ -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', @@ -1538,7 +1549,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 diff --git a/src/services/Carts.php b/src/services/Carts.php index f055185c2c..6a5f763582 100644 --- a/src/services/Carts.php +++ b/src/services/Carts.php @@ -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; diff --git a/src/services/Customers.php b/src/services/Customers.php index 4459d805fa..ee13c74797 100644 --- a/src/services/Customers.php +++ b/src/services/Customers.php @@ -380,75 +380,72 @@ private function _saveAddressesFromOrder(Order $order): void */ 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__); } } } diff --git a/tests/unit/elements/order/OrderCustomerTest.php b/tests/unit/elements/order/OrderCustomerTest.php new file mode 100644 index 0000000000..8404d5030f --- /dev/null +++ b/tests/unit/elements/order/OrderCustomerTest.php @@ -0,0 +1,112 @@ + + * @since 4.3.0 + */ +class OrderCustomerTest extends Unit +{ + /** + * @var UnitTester + */ + protected $tester; + + /** + * @return array + */ + public function _fixtures(): array + { + return [ + 'orders' => [ + 'class' => OrdersFixture::class, + ], + ]; + } + + /** + * @param string $email + * @return void + * @throws Exception + * @throws InvalidConfigException + * @dataProvider emailDataProvider + */ + public function testSetEmail(string $email): void + { + \Craft::$app->set('deprecator', $this->make(Deprecator::class, [ + 'log' => function(string $key, string $message, ?string $file = null, ?int $line = null) { + self::once(); + self::assertEquals(Order::class . '::setEmail', $key); + }, + ])); + + $order = new Order(); + $order->setEmail($email); + + self::assertEquals($email, $order->getEmail()); + self::assertNotNull($order->getCustomer()); + self::assertEquals($email, $order->getCustomer()->email); + } + + /** + * @return array[] + */ + public function emailDataProvider(): array + { + return [ + 'existing-credentialed-user' => ['email' => 'customer1@crafttest.com'], + 'existing-inactive-user' => ['email' => 'inactive.user@crafttest.com'], + ]; + } + + /** + * @param string $email + * @return void + * @dataProvider customerDataProvider + */ + public function testSetCustomer(string $email): void + { + $user = \Craft::$app->getUsers()->getUserByUsernameOrEmail($email); + $order = new Order(); + $order->setCustomer($user); + + self::assertEquals($email, $order->getEmail()); + self::assertNotNull($order->getCustomer()); + self::assertEquals($email, $order->getCustomer()->email); + self::assertEquals($user->id, $order->getCustomer()->id); + self::assertEquals($user->id, $order->getCustomerId()); + + // Test remove customer + $order->setCustomer(); + self::assertNull($order->getCustomer()); + self::assertNull($order->getCustomerId()); + self::assertNull($order->getEmail()); + } + + /** + * @return array[] + */ + public function customerDataProvider(): array + { + return [ + 'existing-credentialed-user' => ['email' => 'customer1@crafttest.com'], + 'existing-inactive-user' => ['email' => 'inactive.user@crafttest.com'], + ]; + } +} diff --git a/tests/unit/elements/order/OrderMarkAsCompleteTest.php b/tests/unit/elements/order/OrderMarkAsCompleteTest.php index 76c3b0f805..60a3e9a6da 100644 --- a/tests/unit/elements/order/OrderMarkAsCompleteTest.php +++ b/tests/unit/elements/order/OrderMarkAsCompleteTest.php @@ -60,7 +60,8 @@ public function testUpdatedProperties(): void { $order = new Order(); $email = 'test@newemailaddress.xyz'; - $order->setEmail($email); + $user = \Craft::$app->getUsers()->ensureUserByEmail($email); + $order->setCustomer($user); /** @var Order $order */ $completedOrder = $this->tester->grabFixture('orders')->getElement('completed-new'); $lineItem = $completedOrder->getLineItems()[0]; @@ -80,7 +81,7 @@ public function testUpdatedProperties(): void self::assertEquals($email, $order->orderCompletedEmail); $this->_deleteElementIds[] = $order->id; - $this->_deleteElementIds[] = $order->getCustomerId(); + $this->_deleteElementIds[] = $user->id; } /** diff --git a/tests/unit/services/CartsTest.php b/tests/unit/services/CartsTest.php index ceedb49fb9..aac96a20af 100644 --- a/tests/unit/services/CartsTest.php +++ b/tests/unit/services/CartsTest.php @@ -13,6 +13,7 @@ use craft\commerce\Plugin; use craft\commerce\services\Carts; use craftcommercetests\fixtures\CustomerAddressFixture; +use craftcommercetests\fixtures\CustomerFixture; use UnitTester; /** @@ -31,6 +32,9 @@ class CartsTest extends Unit public function _fixtures(): array { return [ + 'customer' => [ + 'class' => CustomerFixture::class, + ], 'customerAddresses' => [ 'class' => CustomerAddressFixture::class, ], @@ -60,17 +64,16 @@ public function testGetCartAutoSetAddresses(string $email, bool $autoSet, bool $ }, ])); + $user = Craft::$app->getUsers()->getUserByUsernameOrEmail($email); if ($loggedIn) { - $user = Craft::$app->getUsers()->getUserByUsernameOrEmail($email); Craft::$app->getUser()->setIdentity($user); Craft::$app->getUser()->getIdentity()->password = $user->password; } $newCart = new Order(); - $newCart->setEmail($email); + $newCart->setCustomer($user); $newCart->number = $cartNumber; Craft::$app->getElements()->saveElement($newCart, false); - // Plugin::getInstance()->getCarts()->__set('cart', $newCart); $cart = Plugin::getInstance()->getCarts()->getCart(); @@ -97,4 +100,39 @@ public function getCartDataProvider(): array 'logged-in-user-auto-set-addresses' => ['cred.user@crafttest.com', true, true, true, true], ]; } + + public function testGetCartSwitchCustomer(): void + { + $cartNumber = Plugin::getInstance()->getCarts()->generateCartNumber(); + Plugin::getInstance()->set('carts', $this->make(Carts::class, [ + 'getSessionCartNumber' => function() use ($cartNumber) { + return $cartNumber; + }, + ])); + + $inactiveUser = $this->tester->grabFixture('customer')->getElement('inactive-user'); + $credUser = $this->tester->grabFixture('customer')->getElement('credentialed-user'); + $originalIdentity = Craft::$app->getUser()->getIdentity(); + Craft::$app->getUser()->setIdentity($credUser); + Craft::$app->getUser()->getIdentity()->password = $credUser->password; + + + $order = new Order(); + $order->number = $cartNumber; + $order->setCustomer($inactiveUser); + + Craft::$app->getElements()->saveElement($order, false); + self::assertEquals($inactiveUser->id, $order->getCustomerId()); + + $cart = Plugin::getInstance()->getCarts()->getCart(); + + // assert customer has changed; + self::assertNotEquals($inactiveUser->id, $cart->getCustomerId()); + self::assertEquals($credUser->id, $cart->getCustomerId()); + self::assertEquals($credUser->email, $cart->getEmail()); + + // Reset data + Craft::$app->getUser()->setIdentity($originalIdentity); + Craft::$app->getElements()->deleteElement($cart, true); + } } diff --git a/tests/unit/services/OrdersTest.php b/tests/unit/services/OrdersTest.php index d7ce3c5fb1..5b3e9a08f8 100644 --- a/tests/unit/services/OrdersTest.php +++ b/tests/unit/services/OrdersTest.php @@ -98,12 +98,15 @@ public function testGetOrdersByCustomer(): void public function testGetOrdersByEmail(): void { - $orders = $this->service->getOrdersByEmail($this->fixtureData->getElement('completed-new')->email); + /** @var Order $orderFixture */ + $orderFixture = $this->fixtureData->getElement('completed-new'); + $email = $orderFixture->getEmail(); + $orders = $this->service->getOrdersByEmail($email); self::assertIsArray($orders); self::assertCount(3, $orders); foreach ($orders as $order) { - self::assertEquals($this->fixtureData->getElement('completed-new')->email, $order->email); + self::assertEquals($email, $order->getEmail()); } } }