From fca6cca294dc276a196ae37cb79563de51df9db9 Mon Sep 17 00:00:00 2001 From: Alec Ritson Date: Mon, 8 Jul 2024 13:06:32 +0100 Subject: [PATCH] Fix cart id hanging around in the session when not cleared (#1854) Currently if a cart has a completed order but wasn't cleared from the session it will seize up and cause problems when users try to interact with it. This also causes data issues as the cart no longer maps to the completed order correctly. --------- Co-authored-by: alecritson Co-authored-by: Glenn Jacobs --- docs/core/reference/carts.md | 10 +- docs/core/upgrading.md | 13 +++ packages/core/config/cart.php | 23 ---- packages/core/config/cart_session.php | 38 +++++++ packages/core/src/LunarServiceProvider.php | 1 + .../core/src/Managers/CartSessionManager.php | 25 +++-- packages/core/src/Models/Cart.php | 5 +- .../Unit/Managers/CartSessionManagerTest.php | 100 ++++++++++++++---- 8 files changed, 160 insertions(+), 55 deletions(-) create mode 100644 packages/core/config/cart_session.php diff --git a/docs/core/reference/carts.md b/docs/core/reference/carts.md index c8a9e2f3a7..1aa386b648 100644 --- a/docs/core/reference/carts.md +++ b/docs/core/reference/carts.md @@ -220,11 +220,17 @@ Configuration for your cart is handled in `lunar/cart.php` | Field | Description | Default | |:--------------|:---------------------------------------------------------------------------------------|:-------------| -| `session_key` | What key to use when storing the cart id in the session | `lunar_cart` | -| `auto_create` | If no current basket exists, should we create one in the database? | `false` | | `auth_policy` | When a user logs in, how should we handle merging of the basket? | `merge` | | `eager_load` | Which relationships should be eager loaded by default when calculating the cart totals | +There is additional, separate, config specifically for when using the `CartSession` located in `lunar/cart_session.php`. + +| Field | Description | Default | +|:---------------------------------|:-------------------------------------------------------------------|:-------------| +| `session_key` | What key to use when storing the cart id in the session | `lunar_cart` | +| `auto_create` | If no current basket exists, should we create one in the database? | `false` | +| `allow_multiple_orders_per_cart` | Whether carts can have multiple orders associated to them. | `false` | + ### Getting the cart session instance You can either use the facade or inject the `CartSession` into your code. diff --git a/docs/core/upgrading.md b/docs/core/upgrading.md index 025f5622a3..5068ef30a1 100644 --- a/docs/core/upgrading.md +++ b/docs/core/upgrading.md @@ -18,6 +18,19 @@ php artisan migrate Lunar currently provides bug fixes and security updates for only the latest minor release, e.g. `0.8`. +## 1.0.0-alpha.31 + +### High Impact + +Certain parts of `config/cart.php` which are more specific to when you are interacting with carts via the session have been relocated to a new `config/cart_session.php` file. + +```php +// Move to config/cart_session.php +'session_key' => 'lunar_cart', +'auto_create' => false, +``` + +You should also check this file for any new config values you may need to add. ## 1.0.0-alpha.29 diff --git a/packages/core/config/cart.php b/packages/core/config/cart.php index 00b5244ef7..371ab00698 100644 --- a/packages/core/config/cart.php +++ b/packages/core/config/cart.php @@ -3,17 +3,6 @@ use Lunar\Actions\Carts\GenerateFingerprint; return [ - - /* - |-------------------------------------------------------------------------- - | Session Key - |-------------------------------------------------------------------------- - | - | Specify the session key used when fetching the cart. - | - */ - 'session_key' => 'lunar_cart', - /* |-------------------------------------------------------------------------- | Fingerprint Generator @@ -24,18 +13,6 @@ */ 'fingerprint_generator' => GenerateFingerprint::class, - /* - |-------------------------------------------------------------------------- - | Auto create a cart when none exists for user. - |-------------------------------------------------------------------------- - | - | Determines whether you want to automatically create a cart for a user if - | they do not currently have one in the session. By default this is false - | to minimise the amount of cart lines added to the database. - | - */ - 'auto_create' => false, - /* |-------------------------------------------------------------------------- | Authentication policy diff --git a/packages/core/config/cart_session.php b/packages/core/config/cart_session.php new file mode 100644 index 0000000000..849235881c --- /dev/null +++ b/packages/core/config/cart_session.php @@ -0,0 +1,38 @@ + 'lunar_cart', + + /* + |-------------------------------------------------------------------------- + | Auto create a cart when none exists for user. + |-------------------------------------------------------------------------- + | + | Determines whether you want to automatically create a cart for a user if + | they do not currently have one in the session. By default, this is false + | to minimise the amount of carts added to the database. + | + */ + 'auto_create' => false, + + /* + |-------------------------------------------------------------------------- + | Allow Carts to have multiple orders associated. + |-------------------------------------------------------------------------- + | + | Determines whether the same cart instance will be returned if there is already + | a completed order associated to the cart which is retrieved in the session. + | When set to false, if a cart has a completed order, then a new instance + | of a cart will be returned, even if auto_create is set to false + | + */ + 'allow_multiple_orders_per_cart' => false, +]; diff --git a/packages/core/src/LunarServiceProvider.php b/packages/core/src/LunarServiceProvider.php index eccc85e345..3b1596a4cf 100644 --- a/packages/core/src/LunarServiceProvider.php +++ b/packages/core/src/LunarServiceProvider.php @@ -92,6 +92,7 @@ class LunarServiceProvider extends ServiceProvider { protected $configFiles = [ 'cart', + 'cart_session', 'database', 'media', 'orders', diff --git a/packages/core/src/Managers/CartSessionManager.php b/packages/core/src/Managers/CartSessionManager.php index 346dc8b3b4..d79b918147 100644 --- a/packages/core/src/Managers/CartSessionManager.php +++ b/packages/core/src/Managers/CartSessionManager.php @@ -25,13 +25,18 @@ public function __construct( // } + public function allowsMultipleOrdersPerCart(): bool + { + return config('lunar.cart_session.allow_multiple_per_order', false); + } + /** * {@inheritDoc} */ public function current(bool $estimateShipping = false, bool $calculate = true): ?Cart { return $this->fetchOrCreate( - config('lunar.cart.auto_create', false), + config('lunar.cart_session.auto_create', false), estimateShipping: $estimateShipping, calculate: $calculate, ); @@ -135,6 +140,10 @@ private function fetchOrCreate(bool $create = false, bool $estimateShipping = fa config('lunar.cart.eager_load', []) )->find($cartId); + if ($cart->hasCompletedOrders() && ! $this->allowsMultipleOrdersPerCart()) { + return $this->createNewCart(); + } + if (! $cart) { return $create ? $this->createNewCart() : null; } @@ -173,7 +182,7 @@ public function estimateShipping(): void */ public function getSessionKey(): string { - return config('lunar.cart.session_key'); + return config('lunar.cart_session.session_key'); } /** @@ -232,12 +241,12 @@ public function getShippingOptions(): Collection /** * Create an order from a cart instance. - * - * @param bool $forget */ - public function createOrder($forget = true): Order + public function createOrder(bool $forget = true): Order { - $order = $this->manager()->createOrder(); + $order = $this->manager()->createOrder( + allowMultipleOrders: $this->allowsMultipleOrdersPerCart() + ); if ($forget) { $this->forget(); @@ -248,10 +257,8 @@ public function createOrder($forget = true): Order /** * Create a new cart instance. - * - * @return \Lunar\Models\Cart */ - protected function createNewCart() + protected function createNewCart(): Cart { $user = $this->authManager->user(); diff --git a/packages/core/src/Models/Cart.php b/packages/core/src/Models/Cart.php index 0523d60f94..d2efd64bc8 100644 --- a/packages/core/src/Models/Cart.php +++ b/packages/core/src/Models/Cart.php @@ -43,6 +43,7 @@ use Lunar\Pipelines\Cart\Calculate; use Lunar\Validation\Cart\ValidateCartForOrderCreation; use Lunar\Validation\CartLine\CartLineStock; +use Throwable; /** * @property int $id @@ -465,6 +466,8 @@ public function clear(): Cart /** * Associate a user to the cart + * + * @throws Exception */ public function associate(User $user, string $policy = 'merge', bool $refresh = true): Cart { @@ -643,7 +646,7 @@ public function fingerprint(): string /** * Check whether a given fingerprint matches the one being generated for the cart. * - * @throws FingerprintMismatchException + * @throws FingerprintMismatchException|Throwable */ public function checkFingerprint(string $fingerprint): bool { diff --git a/tests/core/Unit/Managers/CartSessionManagerTest.php b/tests/core/Unit/Managers/CartSessionManagerTest.php index 2b8f29fb71..2fbdc92890 100644 --- a/tests/core/Unit/Managers/CartSessionManagerTest.php +++ b/tests/core/Unit/Managers/CartSessionManagerTest.php @@ -1,6 +1,6 @@ group('cart_session'); use Illuminate\Support\Facades\Config; use Illuminate\Support\Facades\Session; @@ -12,7 +12,8 @@ use Lunar\Models\Currency; use Lunar\Models\Order; -use function Pest\Laravel\{actingAs}; +use function Pest\Laravel\actingAs; +use function Pest\Laravel\assertDatabaseMissing; uses(\Illuminate\Foundation\Testing\RefreshDatabase::class); @@ -32,19 +33,19 @@ 'default' => true, ]); - Config::set('lunar.cart.auto_create', false); + Config::set('lunar.cart_session.auto_create', false); $cart = $manager->current(); expect($cart)->toBeNull(); - Config::set('lunar.cart.auto_create', true); + Config::set('lunar.cart_session.auto_create', true); $cart = $manager->current(); expect($cart)->toBeInstanceOf(Cart::class); - $sessionCart = Session::get(config('lunar.cart.session_key')); + $sessionCart = Session::get(config('lunar.cart_session.session_key')); expect($sessionCart)->not->toBeNull(); expect($sessionCart)->toEqual($cart->id); @@ -59,7 +60,7 @@ 'default' => true, ]); - Config::set('lunar.cart.auto_create', true); + Config::set('lunar.cart_session.auto_create', true); $cart = CartSession::current(); @@ -76,7 +77,7 @@ $cart->setShippingAddress($shipping); $cart->setBillingAddress($billing); - $sessionCart = Session::get(config('lunar.cart.session_key')); + $sessionCart = Session::get(config('lunar.cart_session.session_key')); expect($sessionCart)->not->toBeNull(); expect($sessionCart)->toEqual($cart->id); @@ -86,7 +87,7 @@ expect($order)->toBeInstanceOf(Order::class); expect($cart->id)->toEqual($order->cart_id); - expect(Session::get(config('lunar.cart.session_key')))->toBeNull(); + expect(Session::get(config('lunar.cart_session.session_key')))->toBeNull(); }); test('can create order from session cart and retain cart', function () { @@ -98,7 +99,7 @@ 'default' => true, ]); - Config::set('lunar.cart.auto_create', true); + Config::set('lunar.cart_session.auto_create', true); $cart = CartSession::current(); @@ -115,7 +116,7 @@ $cart->setShippingAddress($shipping); $cart->setBillingAddress($billing); - $sessionCart = Session::get(config('lunar.cart.session_key')); + $sessionCart = Session::get(config('lunar.cart_session.session_key')); expect($sessionCart)->not->toBeNull(); expect($sessionCart)->toEqual($cart->id); @@ -127,7 +128,7 @@ expect($order)->toBeInstanceOf(Order::class); expect($cart->id)->toEqual($order->cart_id); - expect(Session::get(config('lunar.cart.session_key')))->toEqual($cart->id); + expect(Session::get(config('lunar.cart_session.session_key')))->toEqual($cart->id); }); test('can fetch authenticated users cart and set in session', function () { @@ -139,11 +140,11 @@ 'default' => true, ]); - Config::set('lunar.cart.auto_create', false); + Config::set('lunar.cart_session.auto_create', false); $cart = CartSession::current(); - $sessionCart = Session::get(config('lunar.cart.session_key')); + $sessionCart = Session::get(config('lunar.cart_session.session_key')); expect($sessionCart)->toBeNull(); expect($cart)->toBeNull(); @@ -157,7 +158,7 @@ ]); $cart = CartSession::current(); - $sessionCart = Session::get(config('lunar.cart.session_key')); + $sessionCart = Session::get(config('lunar.cart_session.session_key')); expect($cart)->not->toBeNull(); expect($cart->id)->toBe($userCart->id); @@ -175,7 +176,7 @@ 'default' => true, ]); - Config::set('lunar.cart.auto_create', true); + Config::set('lunar.cart_session.auto_create', true); $cart = CartSession::current(); @@ -192,7 +193,7 @@ $cart->setShippingAddress($shipping); $cart->setBillingAddress($billing); - $sessionCart = Session::get(config('lunar.cart.session_key')); + $sessionCart = Session::get(config('lunar.cart_session.session_key')); expect($sessionCart) ->not @@ -203,7 +204,7 @@ CartSession::forget(); expect( - Session::get(config('lunar.cart.session_key')) + Session::get(config('lunar.cart_session.session_key')) ) ->toBeNull() ->and($cart->refresh()->deleted_at) @@ -221,7 +222,7 @@ 'default' => true, ]); - Config::set('lunar.cart.auto_create', true); + Config::set('lunar.cart_session.auto_create', true); $cart = CartSession::current(); @@ -238,7 +239,7 @@ $cart->setShippingAddress($shipping); $cart->setBillingAddress($billing); - $sessionCart = Session::get(config('lunar.cart.session_key')); + $sessionCart = Session::get(config('lunar.cart_session.session_key')); expect($sessionCart) ->not @@ -249,7 +250,7 @@ CartSession::forget(delete: false); expect( - Session::get(config('lunar.cart.session_key')) + Session::get(config('lunar.cart_session.session_key')) ) ->toBeNull() ->and($cart->refresh()->deleted_at) @@ -266,3 +267,62 @@ expect($meta)->toBeArray(); expect($meta['postcode'])->toEqual('NP1 1TX'); }); + +test('can return new instance when current cart has completed order', function () { + Currency::factory()->create([ + 'default' => true, + ]); + + Channel::factory()->create([ + 'default' => true, + ]); + + Config::set('lunar.cart_session.auto_create', true); + Config::set('lunar.cart_session.allow_multiple_orders_per_cart', false); + + $cart = CartSession::current(); + + $shipping = CartAddress::factory()->create([ + 'cart_id' => $cart->id, + 'type' => 'shipping', + ]); + + $billing = CartAddress::factory()->create([ + 'cart_id' => $cart->id, + 'type' => 'billing', + ]); + + $cart->setShippingAddress($shipping); + $cart->setBillingAddress($billing); + + $sessionCart = Session::get(config('lunar.cart_session.session_key')); + + expect($sessionCart)->not->toBeNull(); + expect($sessionCart)->toEqual($cart->id); + + $order = CartSession::createOrder( + forget: false + ); + + expect($order) + ->toBeInstanceOf(Order::class) + ->and($cart->id) + ->toEqual($order->cart_id) + ->and(Session::get(config('lunar.cart_session.session_key'))) + ->toEqual($cart->id); + + $order->update([ + 'placed_at' => now(), + ]); + + $cart = CartSession::current()->calculate(); + + expect($order->cart_id)->not->toBe($cart->id) + ->and($cart->subTotal->value)->toBe(0) + ->and(Session::get(config('lunar.cart_session.session_key'))) + ->toBe($cart->id); + + assertDatabaseMissing(Order::class, [ + 'cart_id' => $cart->id, + ]); +});