Merge pull request #4280 from craftcms/luke/fix-4279

[5.x] forgetCart() should prevent getCart() from using the cookie information again

Author: lukeholder

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- Fixed a bug where `forgetCart()` was ineffective when `getCart()` was called later in the same request. ([#4279](https://github.com/craftcms/commerce/issues/4279))
 - Fixed a bug where variants weren’t being saved for product types with a Variant SKU Format that could cause duplicate SKUs. ([#4249](https://github.com/craftcms/commerce/issues/4249))
 - Fixed a PHP error that occurred when marking an inventory transfer as pending. ([#4267](https://github.com/craftcms/commerce/issues/4267))
 - Improved the performance of migrations when upgrading to 5.x. ([#4277](https://github.com/craftcms/commerce/issues/4277))

src/services/Carts.php

@@ -505,6 +505,11 @@ protected function loadCookie(): void
             $this->cartCookie['name'] = md5(sprintf('Craft.%s.%s.%s', self::class, Craft::$app->id, $currentStore->handle)) . '_commerce_cart';
         }
 
+        // Don't restore from cookie if the cart was explicitly forgotten this request.
+        if ($this->_cartNumber === false) {
+            return;
+        }
+
         $request = Craft::$app->getRequest();
         if (!$request->getIsConsoleRequest()) {
             $this->cartCookie = Craft::cookieConfig($this->cartCookie);

tests/unit/services/CartsTest.php

@@ -13,6 +13,7 @@
 use craft\commerce\Plugin;
 use craft\commerce\services\Carts;
 use craft\commerce\services\Stores;
+use craft\web\Request;
 use craftcommercetests\fixtures\CustomerAddressFixture;
 use craftcommercetests\fixtures\CustomerFixture;
 use UnitTester;
@@ -108,6 +109,93 @@ public function getCartDataProvider(): array
         ];
     }
 
+    /**
+     * Tests that calling forgetCart() followed by getCart() in the same request returns a new
+     * cart with a different number — verifying the fix in loadCookie() that respects the `false`
+     * set by forgetCart() and prevents the cookie from restoring the forgotten cart.
+     *
+     * @see https://github.com/craftcms/commerce/issues/4279
+     */
+    public function testForgetCartPreventsCartRestoration(): void
+    {
+        $cartsService = Plugin::getInstance()->getCarts();
+
+        // First call generates an in-memory cart number and returns a new cart.
+        $initialCart = $cartsService->getCart();
+        $originalNumber = $initialCart->number;
+
+        // forgetCart() sets the private $_cartNumber sentinel to `false`.
+        $cartsService->forgetCart();
+
+        // A subsequent getCart() must generate a completely new number and return a fresh cart.
+        $newCart = $cartsService->getCart();
+
+        self::assertNotEquals(
+            $originalNumber,
+            $newCart->number,
+            'After forgetCart(), getCart() should return a cart with a new number.',
+        );
+    }
+
+    /**
+     * Demonstrates that without the fix, a web request whose cookie still carries the forgotten
+     * cart number causes getCart() to reuse that number — exactly as the old loadCookie() would
+     * have behaved before the `$this->_cartNumber === false` guard was introduced.
+     *
+     * @see https://github.com/craftcms/commerce/issues/4279
+     */
+    public function testForgetCartWithRestoredCartNumberReturnsSameNumber(): void
+    {
+        $carts = Plugin::getInstance()->getCarts();
+
+        // Get an initial cart and number.
+        $initialCart = $carts->getCart();
+        $originalNumber = $initialCart->number;
+
+        // Forget the cart — $_cartNumber is now `false`.
+        $carts->forgetCart();
+
+        $cookieName = 'test_commerce_cart';
+        $carts->cartCookie = ['name' => $cookieName];
+
+        // Simulate the pre-fix state: set $_cartNumber to `null`.
+        // In old code the `$this->_cartNumber === false` guard didn't exist, so even after
+        // forgetCart() wrote `false`, loadCookie() would proceed and silently overwrite it
+        // with whatever value was in the request cookie.
+        $reflection = new \ReflectionClass($carts);
+        $cartNumberProp = $reflection->getProperty('_cartNumber');
+        $cartNumberProp->setAccessible(true);
+        $cartNumberProp->setValue($carts, null);
+
+        $requestCookies = new \yii\web\CookieCollection();
+        $requestCookies->add(new \yii\web\Cookie([
+            'name' => $cookieName,
+            'value' => $originalNumber,
+        ]));
+
+        $originalRequest = \Craft::$app->getRequest();
+
+        // Create a mock request class to return test data
+        $requestMock = $this->make(Request::class, [
+            'getIsConsoleRequest' => false,
+            'getCookies' => $requestCookies,
+        ]);
+
+        Craft::$app->set('request', $requestMock);
+
+        try {
+            $restoredCart = $carts->getCart();
+
+            self::assertEquals(
+                $originalNumber,
+                $restoredCart->number,
+                'Without the false guard in loadCookie(), the request cookie restores the forgotten cart number.',
+            );
+        } finally {
+            \Craft::$app->set('request', $originalRequest);
+        }
+    }
+
     public function testGetCartSwitchCustomer(): void
     {
         $cartNumber = Plugin::getInstance()->getCarts()->generateCartNumber();