From cbb4b849485b19973d768d00964d9f6426115d9f Mon Sep 17 00:00:00 2001 From: "Fabian @ Blax Software" Date: Mon, 5 Jan 2026 09:07:09 +0100 Subject: [PATCH] BF cart/pool/booking --- .github/kaizen.md | 40 +++++++++++++- .github/models.md | 13 ++++- .../create_blax_shop_tables.php.stub | 1 + docs/ProductTypes/02-pool-products.md | 24 +++++---- src/Models/Cart.php | 52 +++++++++---------- src/Models/CartItem.php | 40 ++++++++++---- src/Traits/MayBePoolProduct.php | 5 +- .../CartItemAvailabilityValidationTest.php | 3 -- tests/Feature/Pool/PoolProductPriceIdTest.php | 7 +-- tests/Feature/Pool/PoolProductionBugTest.php | 10 ++-- .../Feature/Pool/PoolSmartAllocationTest.php | 6 ++- 11 files changed, 136 insertions(+), 65 deletions(-) diff --git a/.github/kaizen.md b/.github/kaizen.md index d7b48fd..91a8598 100644 --- a/.github/kaizen.md +++ b/.github/kaizen.md @@ -49,4 +49,42 @@ if ($singlePrice !== null) { - `src/Traits/MayBePoolProduct.php` - removed pricing strategy comparison - `src/Models/Cart.php` - removed pricing strategy comparison -**Key Learning:** ALWAYS verify understanding of business logic before implementing. Pool pricing strategy is about allocation order, not price comparison. \ No newline at end of file +**Key Learning:** ALWAYS verify understanding of business logic before implementing. Pool pricing strategy is about allocation order, not price comparison. + +### 2026-01-05: Cart Item Price/Currency Fixes + +**Issues Fixed:** +1. Pool singles bookings should show `unit_amount` when added (not 0), even without dates +2. Bug: Date range adjustment was showing wrong price (5000 instead of 1755) when singles had no price +3. Added `currency` column to cart_items table to store currency from selected price +4. Removed obsolete `allocated_single_item_name` from meta (replaced by `product_id` column) + +**Root Cause of Price Bug:** +- `updateDates()` was calling `$allocatedSingle->defaultPrice()->first()` instead of using `$this->price()->first()` +- When single has no price, `reallocatePoolItems` sets `price_id` to the pool's price model +- `updateDates()` was ignoring this and going back to the single's (non-existent) price + +**Fix Applied:** +```php +// In CartItem::updateDates() +// IMPORTANT: Use the price_id relationship first, as it was set by reallocatePoolItems +$priceModel = $this->price_id ? $this->price()->first() : null; +if ($priceModel) { + $pricePerDay = $priceModel->getCurrentPrice(...); +} else { + // Fallback: Get price from the allocated single, with fallback to pool price + ... +} +``` + +**New CartItem Fields:** +- `currency`: Currency from the selected price model (e.g., 'USD', 'EUR') + +**Removed:** +- `meta->allocated_single_item_name` - use `$cartItem->product->name` instead via the `product_id` relationship + +**Files Modified:** +- `src/Models/CartItem.php` - added currency, fixed updateDates price resolution +- `src/Models/Cart.php` - added currency to addToCart and reallocatePoolItems +- `src/Traits/MayBePoolProduct.php` - added currency to getNextAvailablePoolItemWithPrice return +- `database/migrations/create_blax_shop_tables.php.stub` - added currency column \ No newline at end of file diff --git a/.github/models.md b/.github/models.md index 6f7b2d0..c697cfb 100644 --- a/.github/models.md +++ b/.github/models.md @@ -40,8 +40,17 @@ The goal of this file is to not miss any important model traits, relationships, ### CartItem - An item within a Cart. -- Links a `Product` and a specific `ProductPrice`. -- **Key Attributes**: `quantity`, `dates` (for bookings), `configuration`. +- Links a `Product` (purchasable) and a specific `ProductPrice`. +- **Key Attributes**: + - `purchasable_id`, `purchasable_type`: The product being purchased + - `product_id`: For pool items, the allocated single item; otherwise null + - `price_id`: The selected price model + - `currency`: Currency from the selected price + - `quantity`: Number of items + - `unit_amount`: Base price per unit (per day for bookings) + - `price`: Calculated price (unit_amount × days for bookings, same as unit_amount for simple) + - `subtotal`: Total (price × quantity) + - `from`, `until`: Booking date range (for booking products) ## Order Management ### Order diff --git a/database/migrations/create_blax_shop_tables.php.stub b/database/migrations/create_blax_shop_tables.php.stub index 78ebdce..8807784 100644 --- a/database/migrations/create_blax_shop_tables.php.stub +++ b/database/migrations/create_blax_shop_tables.php.stub @@ -285,6 +285,7 @@ return new class extends Migration $table->foreignUuid('purchase_id')->nullable()->constrained(config('shop.tables.product_purchases', 'product_purchases'))->nullOnDelete(); $table->foreignUuid('price_id')->nullable()->constrained(config('shop.tables.product_prices', 'product_prices'))->nullOnDelete(); $table->integer('quantity')->default(1); + $table->string('currency', 3)->nullable(); // Currency from the selected price $table->integer('price')->nullable(); // Stored in cents, null = unavailable $table->integer('regular_price')->nullable(); // Stored in cents $table->integer('unit_amount')->nullable(); // Base unit price for 1 quantity, 1 day (in cents) diff --git a/docs/ProductTypes/02-pool-products.md b/docs/ProductTypes/02-pool-products.md index 1c4eca6..48398be 100644 --- a/docs/ProductTypes/02-pool-products.md +++ b/docs/ProductTypes/02-pool-products.md @@ -305,13 +305,14 @@ $cartItem = $cart->addToCart($parkingPool, $quantity = 2, [], $from, $until); - Claims 1 unit from each: `$spot->claimStock(1, $cartItem, $from, $until)` 3. **Store Claimed Items** - - Cart item metadata stores which single items were claimed - - Metadata: `claimed_single_items: [spot1_id, spot2_id]` + - Cart item's `product_id` column stores which single item was allocated + - Each cart item is linked to one specific single item 4. **Calculate Price** - Gets price from available single items (using pricing strategy) - - Multiplies by number of days - - Stores in cart item + - If single has no price, falls back to pool's price + - Multiplies by number of days for booking products + - Stores in cart item (unit_amount, price, subtotal) ### Manual Stock Operations @@ -399,11 +400,15 @@ $cartItem = $cart->addToCart($parkingPool, $quantity = 1, [], $from, $until); // Cart item properties: // - purchasable_id: Pool Product ID // - purchasable_type: Product::class -// - product_id: Allocated Single Item ID (NEW!) +// - product_id: Allocated Single Item ID +// - price_id: Price used (from single or pool fallback) +// - currency: Currency from the selected price // - quantity: 1 // - from: 2025-01-15 // - until: 2025-01-17 -// - price: (unit_amount × 2 days) +// - unit_amount: Price per day (in cents) +// - price: unit_amount × days (calculated booking price) +// - subtotal: price × quantity ``` ### Product ID Column @@ -687,13 +692,12 @@ $price = $pool->getLowestAvailablePoolPrice($from, $until); ### Single Items Not Released After Cart Deletion -**Cause:** Metadata not properly storing claimed items +**Cause:** Cart item's `product_id` not properly tracking claimed single **Solution:** ```php -// Ensure cart item has metadata -$meta = $cartItem->getMeta(); -$claimedItems = $meta->claimed_single_items ?? []; +// Check the cart item's product_id +$allocatedSingle = $cartItem->product; // Manually release if needed $pool->releasePoolStock($cartItem); diff --git a/src/Models/Cart.php b/src/Models/Cart.php index 22d80f7..2f4786b 100644 --- a/src/Models/Cart.php +++ b/src/Models/Cart.php @@ -686,6 +686,7 @@ class Cart extends Model 'single' => $single, 'price' => $price, 'price_id' => $priceModel?->id, + 'currency' => $priceModel?->currency, 'available' => $effectiveAvailable, ]; } @@ -707,7 +708,6 @@ class Cart extends Model 'subtotal' => null, 'unit_amount' => null, ]); - $cartItem->updateMetaKey('allocated_single_item_name', null); } continue; } @@ -751,12 +751,10 @@ class Cart extends Model if ($singleInfo['price_id'] && $singleInfo['price_id'] !== $cartItem->price_id) { $updates['price_id'] = $singleInfo['price_id']; } - $cartItem->update($updates); - $cartItem->updateMetaKey('allocated_single_item_name', $single->name); - - // Legacy: update price_id if changed (now handled in the update above) - if (false) { + if ($singleInfo['currency']) { + $updates['currency'] = $singleInfo['currency']; } + $cartItem->update($updates); // Track usage $singleUsage[$single->id] = $usedFromSingle + $neededQty; @@ -795,21 +793,19 @@ class Cart extends Model if ($singleInfo['price_id'] && $singleInfo['price_id'] !== $cartItem->price_id) { $updates['price_id'] = $singleInfo['price_id']; } + if ($singleInfo['currency']) { + $updates['currency'] = $singleInfo['currency']; + } $cartItem->update($updates); $cartItem->refresh(); // Ensure model reflects database state - $cartItem->updateMetaKey('allocated_single_item_name', $single->name); $firstAllocation = false; } else { // Create a new cart item for the additional quantity - // Get price from the single - $priceModel = $single->defaultPrice()->first(); - $singlePrice = $priceModel?->getCurrentPrice($single->isOnSale()); - - if ($singlePrice === null && $poolProduct->hasPrice()) { - $priceModel = $poolProduct->defaultPrice()->first(); - $singlePrice = $priceModel?->getCurrentPrice($poolProduct->isOnSale()); - } + // Use the price info from singleInfo (already calculated with pool fallback) + $singlePrice = $singleInfo['price']; + $priceId = $singleInfo['price_id']; + $currency = $singleInfo['currency']; $days = $this->calculateBookingDays($from, $until); $pricePerUnit = (int) round($singlePrice * $days); @@ -818,8 +814,9 @@ class Cart extends Model 'purchasable_id' => $cartItem->purchasable_id, 'purchasable_type' => $cartItem->purchasable_type, 'product_id' => $single->id, - 'price_id' => $priceModel?->id, + 'price_id' => $priceId, 'quantity' => $qtyToAllocate, + 'currency' => $currency, 'price' => $pricePerUnit, 'regular_price' => $pricePerUnit, 'unit_amount' => (int) round($singlePrice), @@ -828,8 +825,6 @@ class Cart extends Model 'from' => $from, 'until' => $until, ]); - - $newCartItem->updateMetaKey('allocated_single_item_name', $single->name); } $singleUsage[$single->id] = $usedFromSingle + $qtyToAllocate; @@ -847,7 +842,6 @@ class Cart extends Model 'subtotal' => null, 'unit_amount' => null, ]); - $cartItem->updateMetaKey('allocated_single_item_name', null); } else { // Partial allocation - the cart item was already updated with what we could allocate // The remaining quantity is lost (over-capacity) @@ -1263,7 +1257,7 @@ class Cart extends Model $poolPriceId = $priceModel?->id; // Still try to find a single item for allocation even with pool's direct price - // This ensures allocated_single_item_name is always set for pool items + // This ensures product_id is always set for pool items if (!$poolSingleItem) { $singleItems = $cartable->singleProducts; foreach ($singleItems as $single) { @@ -1338,20 +1332,26 @@ class Cart extends Model return $existingItem->fresh(); } - // Determine price_id for the cart item + // Determine price_id and currency for the cart item $priceId = null; + $currency = null; if ($cartable instanceof Product) { - // For pool products, use the single item's price_id + // For pool products, use the single item's price_id and currency if ($is_pool && $poolPriceId) { $priceId = $poolPriceId; + // Get currency from poolItemData if available + $poolItemData = $cartable->getNextAvailablePoolItemWithPrice($this, null, $from, $until); + $currency = $poolItemData['currency'] ?? null; } else { // Get the default price for the product $defaultPrice = $cartable->defaultPrice()->first(); $priceId = $defaultPrice?->id; + $currency = $defaultPrice?->currency; } } elseif ($cartable instanceof \Blax\Shop\Models\ProductPrice) { - // If adding a ProductPrice directly, use its ID + // If adding a ProductPrice directly, use its ID and currency $priceId = $cartable->id; + $currency = $cartable->currency; } // Create new cart item @@ -1361,6 +1361,7 @@ class Cart extends Model 'product_id' => ($cartable instanceof Product && $cartable->isPool() && $poolSingleItem) ? $poolSingleItem->id : null, 'price_id' => $priceId, 'quantity' => $quantity, + 'currency' => $currency, 'price' => $pricePerUnit, // Price per unit for the period 'regular_price' => $regularPricePerUnit, 'unit_amount' => $unitAmount, // Base price for 1 quantity, 1 day (in cents) @@ -1370,11 +1371,6 @@ class Cart extends Model 'until' => ($is_booking) ? $until : null, ]); - // For pool products, store the single item name in meta for display purposes - if ($cartable instanceof Product && $cartable->isPool() && $poolSingleItem) { - $cartItem->updateMetaKey('allocated_single_item_name', $poolSingleItem->name); - } - // Touch activity timestamp $this->touchActivity(); diff --git a/src/Models/CartItem.php b/src/Models/CartItem.php index aef9fb2..02090aa 100644 --- a/src/Models/CartItem.php +++ b/src/Models/CartItem.php @@ -21,6 +21,7 @@ class CartItem extends Model 'product_id', 'price_id', 'quantity', + 'currency', 'price', 'regular_price', 'unit_amount', @@ -34,6 +35,7 @@ class CartItem extends Model protected $casts = [ 'quantity' => 'integer', + 'currency' => 'string', 'price' => 'integer', 'regular_price' => 'integer', 'unit_amount' => 'integer', @@ -503,22 +505,37 @@ class CartItem extends Model // This ensures consistency when reallocatePoolItems has already assigned a specific single // The product_id column stores the actual single product being purchased $allocatedSingleItemId = $this->product_id; + $currency = null; if ($product->isPool() && $allocatedSingleItemId) { // Get the allocated single item from the product_id column $allocatedSingle = $this->product; if ($allocatedSingle) { - // Get price from the allocated single, with fallback to pool price - $priceModel = $allocatedSingle->defaultPrice()->first(); - $pricePerDay = $priceModel?->getCurrentPrice($allocatedSingle->isOnSale()); - $regularPricePerDay = $priceModel?->getCurrentPrice(false) ?? $pricePerDay; + // IMPORTANT: Use the price_id relationship first, as it was set by reallocatePoolItems + // to the correct price (either single's price or pool's fallback price). + // Only fall back to defaultPrice() if price_id is not set. + $priceModel = $this->price_id ? $this->price()->first() : null; - // Fallback to pool price if single has no price - if ($pricePerDay === null && $product->hasPrice()) { - $poolPriceModel = $product->defaultPrice()->first(); - $pricePerDay = $poolPriceModel?->getCurrentPrice($product->isOnSale()); - $regularPricePerDay = $poolPriceModel?->getCurrentPrice(false) ?? $pricePerDay; + if ($priceModel) { + // Use the price model from price_id relationship + $pricePerDay = $priceModel->getCurrentPrice($allocatedSingle->isOnSale() || $product->isOnSale()); + $regularPricePerDay = $priceModel->getCurrentPrice(false) ?? $pricePerDay; + $currency = $priceModel->currency; + } else { + // Fallback: Get price from the allocated single, with fallback to pool price + $priceModel = $allocatedSingle->defaultPrice()->first(); + $pricePerDay = $priceModel?->getCurrentPrice($allocatedSingle->isOnSale()); + $regularPricePerDay = $priceModel?->getCurrentPrice(false) ?? $pricePerDay; + $currency = $priceModel?->currency; + + // Fallback to pool price if single has no price + if ($pricePerDay === null && $product->hasPrice()) { + $poolPriceModel = $product->defaultPrice()->first(); + $pricePerDay = $poolPriceModel?->getCurrentPrice($product->isOnSale()); + $regularPricePerDay = $poolPriceModel?->getCurrentPrice(false) ?? $pricePerDay; + $currency = $poolPriceModel?->currency; + } } } else { // Allocated single not found - this is an error state, mark as unavailable @@ -538,6 +555,10 @@ class CartItem extends Model // Pass cart item ID to exclude this item from usage calculation $pricePerDay = $product->getCurrentPrice(null, $this->cart, $from, $until, $this->id); $regularPricePerDay = $product->getCurrentPrice(false, $this->cart, $from, $until, $this->id) ?? $pricePerDay; + + // Get currency from the price model + $priceModel = $product->defaultPrice()->first(); + $currency = $priceModel?->currency; } // If no price found, mark as unavailable @@ -563,6 +584,7 @@ class CartItem extends Model $this->update([ 'from' => $from, 'until' => $until, + 'currency' => $currency, 'price' => $pricePerUnit, 'regular_price' => $regularPricePerUnit, 'unit_amount' => $unitAmount, diff --git a/src/Traits/MayBePoolProduct.php b/src/Traits/MayBePoolProduct.php index f948258..fb3b2ff 100644 --- a/src/Traits/MayBePoolProduct.php +++ b/src/Traits/MayBePoolProduct.php @@ -744,7 +744,7 @@ trait MayBePoolProduct * @param \DateTimeInterface|null $from Start date for availability check * @param \DateTimeInterface|null $until End date for availability check * @param string|int|null $excludeCartItemId Cart item ID to exclude from usage calculation (for date updates) - * @return array|null ['price' => float, 'item' => Product, 'price_id' => string|null] + * @return array|null ['price' => float, 'item' => Product, 'price_id' => string|null, 'currency' => string|null] */ public function getNextAvailablePoolItemWithPrice( \Blax\Shop\Models\Cart $cart, @@ -856,6 +856,7 @@ trait MayBePoolProduct 'quantity' => $availableFromThisItem, 'item' => $item, 'price_id' => $priceModel?->id, + 'currency' => $priceModel?->currency, ]; } } @@ -895,6 +896,7 @@ trait MayBePoolProduct 'price' => $averagePrice, 'item' => $availableItems[0]['item'], 'price_id' => $availableItems[0]['price_id'], + 'currency' => $availableItems[0]['currency'], ]; } @@ -912,6 +914,7 @@ trait MayBePoolProduct 'price' => $availableItems[0]['price'], 'item' => $availableItems[0]['item'], 'price_id' => $availableItems[0]['price_id'], + 'currency' => $availableItems[0]['currency'], ]; } diff --git a/tests/Feature/Cart/CartItemAvailabilityValidationTest.php b/tests/Feature/Cart/CartItemAvailabilityValidationTest.php index ca6db9d..a49219b 100644 --- a/tests/Feature/Cart/CartItemAvailabilityValidationTest.php +++ b/tests/Feature/Cart/CartItemAvailabilityValidationTest.php @@ -129,11 +129,8 @@ class CartItemAvailabilityValidationTest extends TestCase // - Remove allocation (product_id = null) // - Set price to null (the real indicator of unavailability) $item = $this->cart->items()->first(); - $meta = $item->getMeta(); - unset($meta->allocated_single_item_name); $item->update([ 'product_id' => null, - 'meta' => json_encode($meta), 'price' => null, 'subtotal' => null, ]); diff --git a/tests/Feature/Pool/PoolProductPriceIdTest.php b/tests/Feature/Pool/PoolProductPriceIdTest.php index 9a506b3..75103dc 100644 --- a/tests/Feature/Pool/PoolProductPriceIdTest.php +++ b/tests/Feature/Pool/PoolProductPriceIdTest.php @@ -140,9 +140,10 @@ class PoolProductPriceIdTest extends TestCase $this->assertNotNull($cartItem->product_id); $this->assertEquals($this->singleItem1->id, $cartItem->product_id); - // Meta should still have the name for display purposes - $meta = $cartItem->getMeta(); - $this->assertEquals($this->singleItem1->name, $meta->allocated_single_item_name); + // Verify we can get the actual product through the relationship + $allocatedProduct = $cartItem->product; + $this->assertNotNull($allocatedProduct); + $this->assertEquals($this->singleItem1->name, $allocatedProduct->name); } #[Test] diff --git a/tests/Feature/Pool/PoolProductionBugTest.php b/tests/Feature/Pool/PoolProductionBugTest.php index 67e2cde..2be3578 100644 --- a/tests/Feature/Pool/PoolProductionBugTest.php +++ b/tests/Feature/Pool/PoolProductionBugTest.php @@ -267,9 +267,9 @@ class PoolProductionBugTest extends TestCase // The 3x 5000 items might be merged since they have the same price_id (pool price) // But different single items should NOT be merged - // Get all allocated single item names + // Get all allocated single item names via product relationship $allocatedNames = $items->map(fn($item) => [ - 'name' => $item->getMeta()->allocated_single_item_name ?? 'unknown', + 'name' => $item->product?->name ?? 'unknown', 'price' => $item->price, 'quantity' => $item->quantity, ])->toArray(); @@ -833,13 +833,12 @@ class PoolProductionBugTest extends TestCase $items = []; for ($i = 1; $i <= 4; $i++) { $item = $cart->addToCart($pool, 1); - $meta = $item->getMeta(); $items[$i] = [ 'id' => $item->id, 'quantity' => $item->quantity, 'price' => $item->price, 'allocated_id' => $item->product_id, - 'allocated_name' => $meta->allocated_single_item_name ?? 'none', + 'allocated_name' => $item->product?->name ?? 'none', ]; } @@ -850,13 +849,12 @@ class PoolProductionBugTest extends TestCase $cartItemDetails = []; $totalQuantity = 0; foreach ($cartItems as $item) { - $meta = $item->getMeta(); $cartItemDetails[] = [ 'id' => $item->id, 'quantity' => $item->quantity, 'price' => $item->price, 'allocated_id' => $item->product_id, - 'allocated_name' => $meta->allocated_single_item_name ?? 'none', + 'allocated_name' => $item->product?->name ?? 'none', ]; $totalQuantity += $item->quantity; } diff --git a/tests/Feature/Pool/PoolSmartAllocationTest.php b/tests/Feature/Pool/PoolSmartAllocationTest.php index 5e5366b..87f7102 100644 --- a/tests/Feature/Pool/PoolSmartAllocationTest.php +++ b/tests/Feature/Pool/PoolSmartAllocationTest.php @@ -343,7 +343,8 @@ class PoolSmartAllocationTest extends TestCase $cart->addToCart($this->pool, 3, [], $claimFrom, $claimUntil); $initialItems = $cart->fresh()->items->sortBy('price')->values(); - $initialAllocations = $initialItems->map(fn($i) => $i->getMeta()->allocated_single_item_name)->toArray(); + // Use product relationship to get allocated single item names + $initialAllocations = $initialItems->map(fn($i) => $i->product?->name)->toArray(); // Should have expensive items allocated $this->assertContains('Spot 4 - 40000', $initialAllocations); @@ -355,7 +356,8 @@ class PoolSmartAllocationTest extends TestCase $cart->setDates($newFrom, $newUntil); $newItems = $cart->fresh()->items->sortBy('price')->values(); - $newAllocations = $newItems->map(fn($i) => $i->getMeta()->allocated_single_item_name)->toArray(); + // Use product relationship to get allocated single item names + $newAllocations = $newItems->map(fn($i) => $i->product?->name)->toArray(); // Should now have cheap items allocated $this->assertContains('Spot 1 - 10000', $newAllocations);