From 1ed301b75951147e79c39bbe1c5492b43e9c3ad3 Mon Sep 17 00:00:00 2001 From: "Fabian @ Blax Software" Date: Sun, 17 May 2026 15:20:58 +0200 Subject: [PATCH] R loan purchase to physical loan, A physical loan stuff --- src/Enums/StockType.php | 78 ++++++++--- src/Models/ProductStock.php | 37 ++++- src/Traits/HasLoanLifecycle.php | 74 +++++++++- src/Traits/HasStocks.php | 147 ++++++++++---------- src/Traits/MayBeLoanableProduct.php | 27 +++- tests/Feature/Loan/CheckOutToTest.php | 42 ++++-- tests/Feature/Loan/LoanLifecycleTest.php | 15 +- tests/Feature/Loan/LoanShopCommandsTest.php | 29 ++-- tests/Feature/Loan/LoanStockEventsTest.php | 78 ++++++----- tests/Feature/Product/PhysicalStockTest.php | 51 +++---- 10 files changed, 370 insertions(+), 208 deletions(-) diff --git a/src/Enums/StockType.php b/src/Enums/StockType.php index 3fe5c4f..aa56f5d 100644 --- a/src/Enums/StockType.php +++ b/src/Enums/StockType.php @@ -5,32 +5,37 @@ declare(strict_types=1); namespace Blax\Shop\Enums; /** - * StockType Enum - * - * Defines the types of stock movements that can occur. - * - * Types: - * - CLAIMED: Stock claimed for reservation/booking (creates PENDING entry) - * Used for temporary allocations that can be released - * Examples: hotel bookings, equipment rentals, cart reservations - * - * - RETURN: Stock returned to inventory (e.g., customer returns) - * Creates a positive adjustment to physical stock - * - * - INCREASE: Stock added to inventory (e.g., new purchases, restocking) - * Creates a positive adjustment to physical stock - * - * - DECREASE: Stock removed from inventory (e.g., sales, damage, loss) - * Creates a negative adjustment to physical stock - * - * Usage Flow: - * 1. INCREASE/DECREASE: Direct physical stock changes (COMPLETED status) - * 2. CLAIMED: Temporary allocation (PENDING status, can be released) - * 3. RETURN: Special case of INCREASE for returned items + * StockType — the kind of movement a {@see \Blax\Shop\Models\ProductStock} + * row represents. + * + * Two-axis classification: + * + * 1. Sign of the stock movement + * INCREASE / RETURN → positive (stock added) + * DECREASE → negative (stock removed) + * CLAIMED → positive quantity but stored as a PENDING + * reservation that nets to negative against + * available stock until the claim is released + * PHYSICALLY_CLAIMED → same as CLAIMED, but never auto-released by + * {@see \Blax\Shop\Models\ProductStock::releaseExpired()}. + * Used for loans: the borrower physically has the + * item until they return it — the expires_at column + * carries the due date for overdue tracking, not a + * release deadline. + * + * 2. Release model + * INCREASE / DECREASE / RETURN → COMPLETED at write time, permanent + * CLAIMED → PENDING, auto-released at expires_at + * PHYSICALLY_CLAIMED → PENDING, manual release only + * + * The two claim types share availability semantics — both subtract from + * `available` and both contribute to `currently_claimed` — so most queries + * filter by {@see self::claimTypeValues()} rather than a single case. */ enum StockType: string { case CLAIMED = 'claimed'; + case PHYSICALLY_CLAIMED = 'physically_claimed'; case RETURN = 'return'; case INCREASE = 'increase'; case DECREASE = 'decrease'; @@ -39,9 +44,38 @@ enum StockType: string { return match ($this) { self::CLAIMED => 'Claimed', + self::PHYSICALLY_CLAIMED => 'Physically claimed', self::RETURN => 'Return', self::INCREASE => 'Increase', self::DECREASE => 'Decrease', }; } + + /** + * The claim-style types — both reserve stock against availability and + * keep a PENDING row in the ledger until released. CLAIMED auto-releases + * at `expires_at`; PHYSICALLY_CLAIMED is manual-release only. + * + * @return array + */ + public static function claimTypes(): array + { + return [self::CLAIMED, self::PHYSICALLY_CLAIMED]; + } + + /** + * Same as {@see self::claimTypes()} but as string values, for use in + * `whereIn(...)` SQL clauses. + * + * @return array + */ + public static function claimTypeValues(): array + { + return [self::CLAIMED->value, self::PHYSICALLY_CLAIMED->value]; + } + + public function isClaim(): bool + { + return in_array($this, self::claimTypes(), strict: true); + } } diff --git a/src/Models/ProductStock.php b/src/Models/ProductStock.php index a4dd80c..ec6630a 100644 --- a/src/Models/ProductStock.php +++ b/src/Models/ProductStock.php @@ -159,15 +159,31 @@ class ProductStock extends Model * @param string|null $note Optional note about the claim * @return self|null The created claim entry, or null if insufficient stock */ + /** + * Claim stock for a product (reservation/booking/loan). + * + * Creates a two-row entry: + * 1. DECREASE row (negative quantity, COMPLETED) — removes from + * `available` immediately. + * 2. Claim row (positive quantity, PENDING) — tracks the reservation + * until released. `$type` controls whether the claim auto-releases + * at `expires_at` (CLAIMED) or stays manual-release only + * (PHYSICALLY_CLAIMED, used for loans). + * + * @param StockType $type Either CLAIMED (default — booking-style, + * {@see self::releaseExpired()} eligible) or PHYSICALLY_CLAIMED + * (loan-style, manual release only). + */ public static function claim( Product $product, int $quantity, $reference = null, ?\DateTimeInterface $from = null, ?\DateTimeInterface $until = null, - ?string $note = null + ?string $note = null, + StockType $type = StockType::CLAIMED, ): ?self { - return DB::transaction(function () use ($product, $quantity, $reference, $from, $until, $note) { + return DB::transaction(function () use ($product, $quantity, $reference, $from, $until, $note, $type) { // When claiming for a future booking, check availability at the start date // Otherwise claims for different time periods would incorrectly conflict $checkDate = $from ?? now(); @@ -193,7 +209,7 @@ class ProductStock extends Model return self::create([ 'product_id' => $product->id, 'quantity' => $quantity, - 'type' => StockType::CLAIMED, + 'type' => $type, 'status' => StockStatus::PENDING, 'reference_type' => $reference ? get_class($reference) : null, 'reference_id' => $reference?->id, @@ -304,7 +320,14 @@ class ProductStock extends Model */ public static function releaseExpired(): int { - $expired = self::expired()->get(); + // Auto-release only the CLAIMED type. PHYSICALLY_CLAIMED rows carry + // an `expires_at` to drive overdue tracking on the loan side, but + // they must NOT be auto-released — the borrower physically has the + // item until they bring it back, regardless of how overdue the + // due date is. + $expired = self::expired() + ->where('type', StockType::CLAIMED->value) + ->get(); $count = 0; foreach ($expired as $stock) { @@ -331,7 +354,9 @@ class ProductStock extends Model */ public function scopeAvailableClaims($query) { - return $query->where('type', StockType::CLAIMED->value)->where('status', StockStatus::PENDING->value); + return $query + ->whereIn('type', StockType::claimTypeValues()) + ->where('status', StockStatus::PENDING->value); } /** @@ -360,7 +385,7 @@ class ProductStock extends Model */ public function scopeAvailableOnDate($query, \DateTimeInterface $date) { - return $query->where('type', StockType::CLAIMED->value) + return $query->whereIn('type', StockType::claimTypeValues()) ->where('status', StockStatus::PENDING->value) ->where(function ($q) use ($date) { $q->where(function ($subQuery) use ($date) { diff --git a/src/Traits/HasLoanLifecycle.php b/src/Traits/HasLoanLifecycle.php index 1ab41a2..92eedf5 100644 --- a/src/Traits/HasLoanLifecycle.php +++ b/src/Traits/HasLoanLifecycle.php @@ -5,11 +5,15 @@ declare(strict_types=1); namespace Blax\Shop\Traits; use Blax\Shop\Enums\PurchaseStatus; +use Blax\Shop\Enums\StockStatus; +use Blax\Shop\Enums\StockType; use Blax\Shop\Events\LoanExtended; use Blax\Shop\Events\LoanReturned; use Blax\Shop\Models\ProductPrice; +use Blax\Shop\Models\ProductStock; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; +use Illuminate\Support\Facades\DB; use Carbon\Carbon; /** @@ -145,30 +149,86 @@ trait HasLoanLifecycle $this->meta = $meta; $this->save(); + // Push the paired physical claim's expires_at to match the new due + // date so calendar/overdue checks at the stock level stay in sync + // with the purchase's `until`. Loans created before this rewiring + // (no linked claim) silently skip. + if ($this->until !== null) { + $this->loanClaimQuery() + ->whereNotNull('expires_at') + ->update(['expires_at' => $this->until]); + } + event(new LoanExtended($this, $weeks)); return $this; } /** - * Mark the item returned: stamp meta.returned_at with the given moment - * (or now()) and flip status to completed so the row reads as final. + * Mark the item returned: stamp meta.returned_at, flip status to + * COMPLETED, and release the paired PHYSICALLY_CLAIMED stock entry + * (which automatically creates the offsetting RETURN row via + * {@see ProductStock::release()}). All three operations happen inside + * a single transaction so physical inventory is consistent at every + * observable instant. + * + * Idempotent: a second call on an already-returned loan is a no-op — + * the returned_at timestamp is not restamped and the claim is not + * re-released. */ public function markReturned(?\DateTimeInterface $at = null): self { + if ($this->isReturned()) { + return $this; + } + $at ??= now(); - $meta = (array) ($this->meta ?? []); - $meta['returned_at'] = Carbon::instance($at)->toIso8601String(); - $this->meta = $meta; - $this->status = PurchaseStatus::COMPLETED; - $this->save(); + DB::transaction(function () use ($at) { + $meta = (array) ($this->meta ?? []); + $meta['returned_at'] = Carbon::instance($at)->toIso8601String(); + $this->meta = $meta; + $this->status = PurchaseStatus::COMPLETED; + $this->save(); + + // Release the paired physical claim. Each release() call + // creates a RETURN entry that offsets the DECREASE written + // alongside the original claim at checkout, so available stock + // naturally restores. For loans created before this rewiring + // (no linked claim), this short-circuits and the host is still + // responsible for the increaseStock(1) — but new code paths + // won't need to do anything. + $this->loanClaimQuery()->each(fn (ProductStock $claim) => $claim->release()); + }); event(new LoanReturned($this)); return $this; } + /** + * Builder for the PHYSICALLY_CLAIMED stock row(s) created alongside + * this loan at checkout. Used by {@see self::markReturned()} (to + * release the claim) and {@see self::extend()} (to push expires_at). + * + * Polymorphic lookup uses the purchase's primary key directly — the + * reference_type was stored as the concrete ProductPurchase class at + * checkout time, but we look up by id alone since UUIDs are unique + * across the table. Filters to PENDING + PHYSICALLY_CLAIMED so any + * already-released or unrelated rows are skipped. + * + * @return \Illuminate\Database\Eloquent\Builder + */ + protected function loanClaimQuery(): \Illuminate\Database\Eloquent\Builder + { + $stockModel = config('shop.models.product_stock', ProductStock::class); + + return $stockModel::query() + ->where('reference_id', $this->getKey()) + ->where('type', StockType::PHYSICALLY_CLAIMED->value) + ->where('status', StockStatus::PENDING->value); + } + /** * Scope: loans currently in the borrower's hands (not returned). * diff --git a/src/Traits/HasStocks.php b/src/Traits/HasStocks.php index 960ce6c..1854177 100644 --- a/src/Traits/HasStocks.php +++ b/src/Traits/HasStocks.php @@ -100,7 +100,7 @@ trait HasStocks { return $this->stocks() ->available() - ->where('type', '!=', StockType::CLAIMED->value) + ->whereNotIn('type', StockType::claimTypeValues()) ->willExpire() ->sum('quantity') ?? 0; } @@ -148,29 +148,27 @@ trait HasStocks * regardless of whether they're temporarily out (on loan, claimed by a * cart/booking) or sitting on the shelf. * - * available + currentlyClaimed + activeLoans + * physical = available + currentlyClaimed * - * Why three terms? - * - available — units on the shelf, free for new use. - * - currentlyClaimed — units held by a cart, booking, or other - * reservation that hasn't been finalised - * (will come back if the claim expires). - * - activeLoans — units checked out via a PENDING - * {@see \Blax\Shop\Models\ProductPurchase} - * row that hasn't been returned yet - * (loaned items still belong to the library). + * Why only two terms? Both CLAIMED and PHYSICALLY_CLAIMED rows count + * toward `currentlyClaimed` (see {@see StockType::claimTypeValues()}), + * so cart reservations, bookings, AND loans all flow through the same + * claim machinery. Loans no longer need a separate `activeLoans` term — + * the PHYSICALLY_CLAIMED stock row IS the active loan. * * Worked examples: * - Tomato shop: bought 10, sold 3 → DECREASE -3 is permanent (no * claim/loan to offset). Physical = 7. Available = 7. - * - Library: bought 5, loaned 1 → DECREASE -1 + active loan → +1. - * Physical = 4 + 0 + 1 = 5. Available = 4. - * - Hotel: 1 room, future booking → claim sits in the future, no - * current claim yet. Physical = 1 + 0 + 0 = 1. + * - Library: bought 5, loaned 1 → one PHYSICALLY_CLAIMED row. + * Available = 4, currentlyClaimed = 1, physical = 5. + * - Hotel: 1 room, future booking → CLAIMED row, claimed_from + * in the future. Available = 1 today, currentlyClaimed = 0 today, + * physical = 1. * * Distinct from {@see self::getMaxStocksAttribute} (which sums every * INCREASE/RETURN row ever written and so inflates after every loan - * return), and from {@see \Blax\Shop\Traits\MayBeLoanableProduct::getTotalQuantityAttribute} + * return cycle) and from + * {@see \Blax\Shop\Traits\MayBeLoanableProduct::getTotalQuantityAttribute} * (which is loanable-only). This one works for every Product type. */ public function getPhysicalStockAttribute(): int @@ -179,25 +177,7 @@ trait HasStocks return PHP_INT_MAX; } - $available = $this->getAvailableStock(); - $currentClaims = $this->getCurrentlyClaimedStock(); - - // Query loans by purchasable_id only — the morphMany on $this->purchases() - // narrows by purchasable_type using static::class, which silently - // misses rows written under a subclass type (e.g. App\Models\Book) - // when the caller resolved the product via the base Product class - // (as `shop:stocks:availability` does). Product UUIDs are unique - // across the table so dropping the type filter is safe. - $purchaseModel = config( - 'shop.models.product_purchase', - \Blax\Shop\Models\ProductPurchase::class, - ); - $activeLoans = (int) $purchaseModel::query() - ->where('purchasable_id', $this->getKey()) - ->activeLoans() - ->sum('quantity'); - - return $available + $currentClaims + $activeLoans; + return $this->getAvailableStock() + $this->getCurrentlyClaimedStock(); } /** @@ -354,14 +334,16 @@ trait HasStocks return false; } - // For CLAIMED type, delegate to claimStock which handles the two-entry pattern - if ($type === StockType::CLAIMED) { + // For claim-style types, delegate to claimStock which handles the + // two-entry pattern (DECREASE + PENDING claim row). + if ($type->isClaim()) { return $this->claimStock( quantity: $quantity, reference: $referencable, from: $from, until: $until, - note: $note + note: $note, + type: $type ); } @@ -395,24 +377,22 @@ trait HasStocks } /** - * Claim stock for temporary use (reservation/booking) - * - * This is different from decreaseStock - it: + * Claim stock for temporary use (reservation/booking/loan). + * + * Different from decreaseStock — it: * 1. Removes stock from available inventory (via DECREASE entry) - * 2. Tracks it as a claim (via CLAIMED entry with PENDING status) - * 3. Can be released back later (changes CLAIMED to COMPLETED) - * 4. Supports date ranges for bookings (claimed_from to expires_at) - * + * 2. Tracks it as a claim (via CLAIMED/PHYSICALLY_CLAIMED entry with PENDING status) + * 3. Can be released later (status PENDING → COMPLETED + paired RETURN) + * 4. Supports date ranges (claimed_from → expires_at) + * * Use cases: - * - Hotel room bookings (claimed_from = check-in, expires_at = check-out) - * - Equipment rentals (claimed_from = rental start, expires_at = return date) - * - Cart reservations (no claimed_from, expires_at = cart expiry) - * - * @param int $quantity Amount to claim - * @param mixed $reference Optional reference model (Order, Booking, Cart, etc.) - * @param DateTimeInterface|null $from When claim starts (null = immediately) - * @param DateTimeInterface|null $until When claim expires (null = permanent) - * @param string|null $note Optional note about the claim + * - Hotel room bookings → CLAIMED, expires_at = check-out (auto-releases). + * - Cart reservations → CLAIMED, expires_at = cart expiry (auto-releases). + * - Library loans → PHYSICALLY_CLAIMED, expires_at = due date + * (overdue tracking only — does not auto-release). + * + * @param StockType $type CLAIMED (default, auto-release) or + * PHYSICALLY_CLAIMED (manual release only — used by loans). * @return \Blax\Shop\Models\ProductStock|null The claim entry, or null if insufficient stock */ public function claimStock( @@ -420,7 +400,8 @@ trait HasStocks $reference = null, ?DateTimeInterface $from = null, ?DateTimeInterface $until = null, - ?string $note = null + ?string $note = null, + StockType $type = StockType::CLAIMED, ): ?\Blax\Shop\Models\ProductStock { if (!$this->manage_stock) { @@ -437,7 +418,8 @@ trait HasStocks $reference, $from, $until, - $note + $note, + $type, ); if ($claim) { @@ -478,7 +460,7 @@ trait HasStocks $baseStock = $this->stocks() ->withoutGlobalScope('willExpire') ->where('status', StockStatus::COMPLETED->value) - ->where('type', '!=', StockType::CLAIMED->value) + ->whereNotIn('type', StockType::claimTypeValues()) ->where(function ($query) use ($date) { $query->whereNull('expires_at') ->orWhere('expires_at', '>', $date); @@ -488,16 +470,20 @@ trait HasStocks // Add back claims that should not reduce availability at the given date $inactiveClaims = $this->stocks() ->withoutGlobalScope('willExpire') - ->where('type', StockType::CLAIMED->value) + ->whereIn('type', StockType::claimTypeValues()) ->where('status', StockStatus::PENDING->value) ->where(function ($query) use ($date) { $query->where(function ($q) use ($date) { - // Claim has not started yet + // Claim has not started yet — applies to both claim types. $q->whereNotNull('claimed_from') ->where('claimed_from', '>', $date); })->orWhere(function ($q) use ($date) { - // Claim expired before the date - $q->whereNotNull('expires_at') + // Claim expired before the date — only for CLAIMED, which + // auto-releases at expires_at. PHYSICALLY_CLAIMED (loans) + // stays reserved until manually returned; expires_at is + // informational/overdue-tracking only. + $q->where('type', StockType::CLAIMED->value) + ->whereNotNull('expires_at') ->where('expires_at', '<=', $date); }); }) @@ -518,7 +504,7 @@ trait HasStocks public function getCurrentlyClaimedStock(): int { return abs($this->stocks() - ->where('type', StockType::CLAIMED->value) + ->whereIn('type', StockType::claimTypeValues()) ->where('status', StockStatus::PENDING->value) ->willExpire() ->where(function ($query) { @@ -538,7 +524,7 @@ trait HasStocks public function getActiveAndPlannedClaimedStock(): int { return abs($this->stocks() - ->where('type', StockType::CLAIMED->value) + ->whereIn('type', StockType::claimTypeValues()) ->where('status', StockStatus::PENDING->value) ->willExpire() ->sum('quantity')); @@ -553,7 +539,7 @@ trait HasStocks public function getFutureClaimedStock(?DateTimeInterface $from = null): int { $query = $this->stocks() - ->where('type', StockType::CLAIMED->value) + ->whereIn('type', StockType::claimTypeValues()) ->where('status', StockStatus::PENDING->value) ->willExpire(); @@ -677,7 +663,7 @@ trait HasStocks $nextClaimEnd = $this->stocks() ->withoutGlobalScope('willExpire') - ->where('type', StockType::CLAIMED->value) + ->whereIn('type', StockType::claimTypeValues()) ->where('status', StockStatus::PENDING->value) ->whereNotNull('expires_at') ->where('expires_at', '>', now()) @@ -749,7 +735,7 @@ trait HasStocks $baseStock = $this->stocks() ->withoutGlobalScope('willExpire') ->where('status', StockStatus::COMPLETED->value) - ->where('type', '!=', StockType::CLAIMED->value) + ->whereNotIn('type', StockType::claimTypeValues()) ->where('created_at', '<=', $dateDayEnd) ->where(function ($query) use ($date) { $query->whereNull('expires_at') @@ -758,16 +744,19 @@ trait HasStocks ->sum('quantity'); // Add back claims that should not reduce availability at the given date. + // Same asymmetry as getAvailableStock: PHYSICALLY_CLAIMED rows stay + // reserved past expires_at (overdue loan still in borrower's hands). $inactiveClaims = $this->stocks() ->withoutGlobalScope('willExpire') - ->where('type', StockType::CLAIMED->value) + ->whereIn('type', StockType::claimTypeValues()) ->where('status', StockStatus::PENDING->value) ->where(function ($query) use ($date) { $query->where(function ($q) use ($date) { $q->whereNotNull('claimed_from') ->where('claimed_from', '>', $date); })->orWhere(function ($q) use ($date) { - $q->whereNotNull('expires_at') + $q->where('type', StockType::CLAIMED->value) + ->whereNotNull('expires_at') ->where('expires_at', '<=', $date); }); }) @@ -814,10 +803,10 @@ trait HasStocks // Group conditions with OR to keep them within the product_id scope $query->where(function ($q) { $q->where('status', StockStatus::COMPLETED->value) - ->where('type', '!=', StockType::CLAIMED->value); + ->whereNotIn('type', StockType::claimTypeValues()); })->orWhere(function ($q) { $q->where('status', StockStatus::PENDING->value) - ->where('type', StockType::CLAIMED->value); + ->whereIn('type', StockType::claimTypeValues()); }); }) ->get(); @@ -861,7 +850,8 @@ trait HasStocks foreach ($events as $eventTime) { $available = 0; foreach ($allStocks as $stock) { - if ($stock->status === StockStatus::COMPLETED && $stock->type !== StockType::CLAIMED) { + $isClaim = $stock->type instanceof StockType && $stock->type->isClaim(); + if ($stock->status === StockStatus::COMPLETED && ! $isClaim) { // A COMPLETED entry only contributes from the day it // was created — without this gate, a DECREASE from a // loan placed today would retroactively reduce @@ -874,10 +864,19 @@ trait HasStocks if ($hasStarted && $notExpired) { $available += $stock->quantity; } - } elseif ($stock->status === StockStatus::PENDING && $stock->type === StockType::CLAIMED) { - // Add back if NOT active at this timestamp + } elseif ($stock->status === StockStatus::PENDING && $isClaim) { + // Add back if NOT active at this timestamp. For + // CLAIMED (auto-release booking) we also add back + // past-expires_at — the booking window is over so the + // unit is free again. PHYSICALLY_CLAIMED (loans) + // stays reserved past expires_at because the + // borrower physically has the item until they return + // it — the calendar should show "unavailable" even + // for overdue loans. $isNotStarted = $stock->claimed_from && $stock->claimed_from > $eventTime; - $isExpired = $stock->expires_at && $stock->expires_at <= $eventTime; + $isExpired = $stock->type === StockType::CLAIMED + && $stock->expires_at + && $stock->expires_at <= $eventTime; if ($isNotStarted || $isExpired) { $available += $stock->quantity; } diff --git a/src/Traits/MayBeLoanableProduct.php b/src/Traits/MayBeLoanableProduct.php index 3383539..ba2efbb 100644 --- a/src/Traits/MayBeLoanableProduct.php +++ b/src/Traits/MayBeLoanableProduct.php @@ -163,12 +163,11 @@ trait MayBeLoanableProduct $weeks ??= (int) config('shop.loan.default_duration_weeks', 2); $now = Carbon::now(); + $until = $now->copy()->addWeeks($weeks); $price ??= $this->defaultPrice()->first(); - $purchase = DB::transaction(function () use ($borrower, $weeks, $price, $now): ProductPurchase { - $this->decreaseStock(1); - - return $this->purchases()->create([ + $purchase = DB::transaction(function () use ($borrower, $price, $now, $until): ProductPurchase { + $purchase = $this->purchases()->create([ 'purchaser_id' => $borrower->getKey(), 'purchaser_type' => $borrower::class, 'price_id' => $price?->id, @@ -177,9 +176,27 @@ trait MayBeLoanableProduct 'amount_paid' => 0, 'status' => PurchaseStatus::PENDING, 'from' => $now, - 'until' => $now->copy()->addWeeks($weeks), + 'until' => $until, 'meta' => ['extensions_used' => 0], ]); + + // Loan model = booking-style claim that doesn't auto-release. + // The PHYSICALLY_CLAIMED row drives availability/calendar and + // carries the due date in `expires_at` for overdue tracking, + // while ProductStock::releaseExpired() pointedly skips this + // type — the borrower physically has the item until they bring + // it back, regardless of how overdue they get. markReturned() + // releases the claim, which creates the offsetting RETURN entry. + $this->claimStock( + quantity: 1, + reference: $purchase, + from: $now, + until: $until, + note: 'Loan to ' . class_basename($borrower::class) . ' #' . substr((string) $borrower->getKey(), 0, 8), + type: \Blax\Shop\Enums\StockType::PHYSICALLY_CLAIMED, + ); + + return $purchase; }); event(new LoanCreated($purchase)); diff --git a/tests/Feature/Loan/CheckOutToTest.php b/tests/Feature/Loan/CheckOutToTest.php index 1c0502c..61f7bd7 100644 --- a/tests/Feature/Loan/CheckOutToTest.php +++ b/tests/Feature/Loan/CheckOutToTest.php @@ -179,24 +179,48 @@ class CheckOutToTest extends TestCase } #[Test] - public function mark_returned_does_not_restore_stock_intentionally(): void + public function mark_returned_releases_the_paired_physical_claim_and_restores_stock(): void { - // Locking-in regression test for an opinionated design choice: the - // package's markReturned() flips lifecycle state but leaves stock - // alone. Hosts that model loans as borrow-and-return (rather than - // permanent ownership transfer) must follow up with an explicit - // increaseStock(1) — see moonshiner-library's LoanController. + // Replaces the prior "must not restore stock" assertion. Loans are + // now modelled as PHYSICALLY_CLAIMED stock entries; markReturned() + // releases that claim, which creates the offsetting RETURN row via + // ProductStock::release() — so available stock comes back to where + // it was before the loan with no host bookkeeping required. + $availableBefore = $this->book->fresh()->getAvailableStock(); + $loan = $this->book->checkOutTo($this->borrower); - $availableAfterCheckout = $this->book->fresh()->getAvailableStock(); + $this->assertSame( + $availableBefore - 1, + $this->book->fresh()->getAvailableStock(), + 'checkout drops available by quantity', + ); $loan->markReturned(); $this->assertSame( - $availableAfterCheckout, + $availableBefore, $this->book->fresh()->getAvailableStock(), - 'markReturned() must not change stock — hosts opt in to that explicitly.', + 'markReturned() restores available via the released claim', ); } + + #[Test] + public function mark_returned_is_idempotent_and_does_not_double_restore_stock(): void + { + // A retried markReturned() call (network flake, double-click on a + // librarian button, etc.) must not inflate stock past the catalogue + // size by re-releasing an already-released claim. + $availableBefore = $this->book->fresh()->getAvailableStock(); + $loan = $this->book->checkOutTo($this->borrower); + + $loan->markReturned(); + $afterFirst = $this->book->fresh()->getAvailableStock(); + $loan->markReturned(); + $afterSecond = $this->book->fresh()->getAvailableStock(); + + $this->assertSame($availableBefore, $afterFirst); + $this->assertSame($afterFirst, $afterSecond, 'second call must be a no-op'); + } } /** diff --git a/tests/Feature/Loan/LoanLifecycleTest.php b/tests/Feature/Loan/LoanLifecycleTest.php index b43bf56..50442a3 100644 --- a/tests/Feature/Loan/LoanLifecycleTest.php +++ b/tests/Feature/Loan/LoanLifecycleTest.php @@ -194,12 +194,12 @@ class LoanLifecycleTest extends TestCase /* ───────────────────── edge cases ───────────────────── */ #[Test] - public function mark_returned_called_twice_keeps_the_first_returned_at_timestamp(): void + public function mark_returned_is_idempotent_first_write_wins(): void { - // markReturned is idempotent-ish: calling it again overwrites the - // returned_at timestamp. We document that behaviour explicitly so a - // future refactor knows whether to keep it. If you want first-write- - // wins, change markReturned() to no-op when already returned. + // markReturned() now no-ops on already-returned loans so a retried + // call (network flake, double-submit) can't re-release the paired + // claim and inflate available stock past the catalogue size. The + // first returned_at timestamp is the canonical one. Carbon::setTestNow(Carbon::parse('2026-05-14 10:00:00')); $loan = $this->checkout(); @@ -209,10 +209,11 @@ class LoanLifecycleTest extends TestCase Carbon::setTestNow(Carbon::parse('2026-05-20 10:00:00')); $loan->markReturned(); - $this->assertNotSame($firstReturnedAt, $loan->returnedAt(), 'second call overwrites'); + $this->assertSame($firstReturnedAt, $loan->returnedAt(), 'second call does not restamp'); $this->assertSame( - Carbon::parse('2026-05-20 10:00:00')->toIso8601String(), + Carbon::parse('2026-05-14 10:00:00')->toIso8601String(), $loan->returnedAt(), + 'first write wins', ); } diff --git a/tests/Feature/Loan/LoanShopCommandsTest.php b/tests/Feature/Loan/LoanShopCommandsTest.php index c528e09..e8816c2 100644 --- a/tests/Feature/Loan/LoanShopCommandsTest.php +++ b/tests/Feature/Loan/LoanShopCommandsTest.php @@ -95,11 +95,14 @@ class LoanShopCommandsTest extends TestCase $book = $this->newBook('Hyperion', 'CMD-HYP'); $book->increaseStock(3); - // Borrow twice, return one. Ledger = seed + 2 decreases + 1 increase. + // Borrow twice, return one. With the claim-based loan model the + // ledger carries: seed INCREASE, two claim cycles each writing a + // DECREASE + PHYSICALLY_CLAIMED row, plus one RETURN from the + // released claim — six rows total, but the operator only needs to + // see the standard increase/decrease verbs that the command renders. $loan = $book->checkOutTo($this->borrower); $book->checkOutTo(User::factory()->create()); $loan->markReturned(); - $book->increaseStock(1); $output = $this->runOk(ShopStocksCommand::class, ['product' => 'CMD-HYP']); @@ -119,25 +122,20 @@ class LoanShopCommandsTest extends TestCase public function assigned_for_loanable_product_must_not_inflate_after_a_return_cycle(): void { // Regression test for the bug where shop:stocks rendered Assigned=4 - // for a 3-copy book that had been borrowed-and-returned once. The - // sequence is: INCREASE +3 (seed), DECREASE -1 (loan), INCREASE +1 - // (host-driven return restock). getMaxStocksAttribute sums every - // positive entry — so it returned 3 + 1 = 4 — which the command then - // rendered verbatim. Fix: consult `total_quantity`, which is loan-aware - // (available stock + active loans = physical inventory we own). + // for a 3-copy book that had been borrowed-and-returned once. With + // the new claim-based loan model the issue disappears at the source: + // checkout writes a DECREASE + PHYSICALLY_CLAIMED pair, the return + // releases the claim (status flip + RETURN row), and the net effect + // on every accessor reads exactly 3 copies — no inflation possible. $book = $this->newBook('Hyperion', 'CMD-HYP-RC'); $book->increaseStock(3); $loan = $book->checkOutTo($this->borrower); $loan->markReturned(); - $book->increaseStock(1); - // Sanity: model accessors disagree — getMaxStocksAttribute is inflated - // (this is the documented limitation of the underlying calc), while - // total_quantity reports the truth. The command must use total_quantity. $fresh = $book->fresh(); - $this->assertSame(4, $fresh->getMaxStocksAttribute(), 'inflated by design'); - $this->assertSame(3, (int) $fresh->total_quantity, 'loan-aware accessor'); + $this->assertSame(3, (int) $fresh->total_quantity, 'loan-aware accessor reads true count'); + $this->assertSame(3, $fresh->getPhysicalStock(), 'physical reads true count'); // Detail view: ASSIGNED row must be 3, not 4. $output = $this->runOk(ShopStocksCommand::class, ['product' => 'CMD-HYP-RC']); @@ -228,8 +226,7 @@ class LoanShopCommandsTest extends TestCase $book = $this->newBook('Singular', 'CMD-SIN'); $book->increaseStock(1); $loan = $book->checkOutTo($this->borrower); - $loan->markReturned(); - $book->increaseStock(1); + $loan->markReturned(); // releases the claim → restores available $output = $this->runOk(ShopAvailabilityCommand::class, [ 'product' => 'CMD-SIN', diff --git a/tests/Feature/Loan/LoanStockEventsTest.php b/tests/Feature/Loan/LoanStockEventsTest.php index f1ae8e3..21c21f5 100644 --- a/tests/Feature/Loan/LoanStockEventsTest.php +++ b/tests/Feature/Loan/LoanStockEventsTest.php @@ -5,9 +5,10 @@ namespace Blax\Shop\Tests\Feature\Loan; use Blax\Shop\Enums\ProductType; use Blax\Shop\Enums\StockStatus; use Blax\Shop\Enums\StockType; -use Blax\Shop\Events\StockDecreased; +use Blax\Shop\Events\StockClaimed; use Blax\Shop\Events\StockDepleted; use Blax\Shop\Events\StockIncreased; +use Blax\Shop\Events\StockReleased; use Blax\Shop\Events\StockReplenished; use Blax\Shop\Models\Product; use Blax\Shop\Models\ProductStock; @@ -44,36 +45,43 @@ class LoanStockEventsTest extends TestCase } #[Test] - public function checkOutTo_dispatches_stock_decreased_with_correct_payload(): void + public function checkOutTo_dispatches_stock_claimed_with_a_physically_claimed_row(): void { - Event::fake([StockDecreased::class]); + // Loans go through the claim machinery (PHYSICALLY_CLAIMED type), so + // the canonical "stock has moved" event is StockClaimed — not + // StockDecreased. The bookkeeping DECREASE row that claimStock writes + // internally bypasses the public decreaseStock() path (and so does + // not fire StockDecreased), matching how bookings have always worked. + Event::fake([StockClaimed::class]); - $this->book->checkOutTo($this->borrower); + $loan = $this->book->checkOutTo($this->borrower); Event::assertDispatched( - StockDecreased::class, - fn (StockDecreased $e) => $e->product->is($this->book) - && $e->availableAfter === 2 + StockClaimed::class, + fn (StockClaimed $e) => $e->product->is($this->book) && $e->entry instanceof ProductStock - && (int) $e->entry->quantity === -1 - && $e->entry->type === StockType::DECREASE - && $e->entry->status === StockStatus::COMPLETED, + && (int) $e->entry->quantity === 1 + && $e->entry->type === StockType::PHYSICALLY_CLAIMED + && $e->entry->status === StockStatus::PENDING + && (string) $e->entry->reference_id === (string) $loan->id, ); } #[Test] public function checking_out_the_last_copy_dispatches_stock_depleted(): void { - // 3 copies on the shelf — borrow all three; the third call crosses the - // last-copy boundary so StockDepleted must fire alongside StockDecreased. + // 3 copies on the shelf — borrow all three. The third call crosses + // the last-copy boundary; claimStock's dispatchStockTransitions fires + // StockDepleted regardless of whether the decrement came from a + // direct decreaseStock or a claim. $this->book->checkOutTo(User::factory()->create()); $this->book->checkOutTo(User::factory()->create()); - Event::fake([StockDepleted::class, StockDecreased::class]); + Event::fake([StockDepleted::class, StockClaimed::class]); $this->book->checkOutTo($this->borrower); - Event::assertDispatched(StockDecreased::class); + Event::assertDispatched(StockClaimed::class); Event::assertDispatched( StockDepleted::class, fn (StockDepleted $e) => $e->product->is($this->book), @@ -91,24 +99,26 @@ class LoanStockEventsTest extends TestCase } #[Test] - public function restocking_after_a_full_loan_dispatches_stock_replenished(): void + public function returning_a_fully_loaned_book_dispatches_replenished_and_released(): void { - // Single-copy book, borrow it (depletes to 0), then a host-driven - // increaseStock(1) on the return path must cross 0→>0 and fire - // StockReplenished. Mirrors what moonshiner-library does in - // LoanController::returnLoan after $loan->markReturned(). + // Single-copy book, borrow it (depletes to 0), then return it. + // markReturned() releases the paired claim, which creates the + // offsetting RETURN entry via the package's release() helper — so + // StockIncreased + StockReleased both fire, and the 0→1 boundary + // crossing additionally triggers StockReplenished. No host call + // to increaseStock() needed. $single = EventLoanBook::create(['name' => 'Solitaire', 'sku' => 'SOL-EV-1']); $single->increaseStock(1); $loan = $single->checkOutTo($this->borrower); - $loan->markReturned(); $this->assertSame(0, $single->fresh()->getAvailableStock()); - Event::fake([StockReplenished::class, StockIncreased::class]); + Event::fake([StockReplenished::class, StockIncreased::class, StockReleased::class]); - $single->increaseStock(1); + $loan->markReturned(); Event::assertDispatched(StockIncreased::class); + Event::assertDispatched(StockReleased::class); Event::assertDispatched( StockReplenished::class, fn (StockReplenished $e) => $e->product->is($single) && $e->availableAfter === 1, @@ -116,16 +126,15 @@ class LoanStockEventsTest extends TestCase } #[Test] - public function restocking_when_other_copies_are_free_does_not_dispatch_replenished(): void + public function returning_when_other_copies_are_free_does_not_dispatch_replenished(): void { - // 3-copy book, borrow 1 → 2 available. Returning that copy goes 2→3, - // NOT a 0→>0 transition, so StockReplenished must stay silent. + // 3-copy book, borrow 1 → 2 available. Returning goes 2→3, NOT a + // 0→>0 transition, so StockReplenished must stay silent. $loan = $this->book->checkOutTo($this->borrower); - $loan->markReturned(); Event::fake([StockReplenished::class]); - $this->book->increaseStock(1); + $loan->markReturned(); Event::assertNotDispatched(StockReplenished::class); } @@ -133,11 +142,14 @@ class LoanStockEventsTest extends TestCase #[Test] public function event_wiring_holds_across_a_full_borrow_return_cycle(): void { - // Full sequence: borrow → return-restock. We assert the relative count - // and payload of each event in one go so a future refactor that splits - // the path can't pass the per-step tests while breaking the rollup. + // Full sequence: borrow → return. checkOutTo fires StockClaimed (no + // StockDecreased — claim machinery bypasses it). markReturned fires + // StockReleased + StockIncreased (the release writes a RETURN entry + // through increaseStock). The 3→2 and 2→3 transitions are boundary- + // free so StockDepleted/StockReplenished stay quiet. Event::fake([ - StockDecreased::class, + StockClaimed::class, + StockReleased::class, StockIncreased::class, StockDepleted::class, StockReplenished::class, @@ -145,9 +157,9 @@ class LoanStockEventsTest extends TestCase $loan = $this->book->checkOutTo($this->borrower); $loan->markReturned(); - $this->book->increaseStock(1); - Event::assertDispatchedTimes(StockDecreased::class, 1); + Event::assertDispatchedTimes(StockClaimed::class, 1); + Event::assertDispatchedTimes(StockReleased::class, 1); Event::assertDispatchedTimes(StockIncreased::class, 1); Event::assertNotDispatched(StockDepleted::class, '3→2 is not a depletion'); Event::assertNotDispatched(StockReplenished::class, '2→3 is not a replenishment'); diff --git a/tests/Feature/Product/PhysicalStockTest.php b/tests/Feature/Product/PhysicalStockTest.php index 899abe3..656130a 100644 --- a/tests/Feature/Product/PhysicalStockTest.php +++ b/tests/Feature/Product/PhysicalStockTest.php @@ -125,10 +125,10 @@ class PhysicalStockTest extends TestCase $loan = $book->checkOutTo($borrower); $this->assertSame(5, $book->fresh()->getPhysicalStock()); - // Host-driven return: mark + restock (mirrors moonshiner's - // LoanController::returnLoan). + // markReturned() now does the full job — releases the paired + // physical claim, which writes the offsetting RETURN entry. No + // host-side increaseStock(1) needed. $loan->markReturned(); - $book->increaseStock(1); $fresh = $book->fresh(); $this->assertSame(5, $fresh->getAvailableStock()); @@ -216,45 +216,38 @@ class PhysicalStockTest extends TestCase #[Test] public function physical_does_not_inflate_after_a_library_loan_return_cycle(): void { - // Regression: getMaxStocksAttribute sums every INCREASE row — including - // the +1 from a loan return — so for a borrow-and-return cycle it - // overstates "Assigned" as 6 on a 5-copy book. physical_stock uses the - // available+claims+loans formula instead and stays correctly at 5. + // Regression: getMaxStocksAttribute sums every INCREASE row — the + // claim/release machinery writes a RETURN row at return time, which + // is INCREASE-like and so still inflates "Assigned". physical_stock + // uses the available+claims formula instead and stays correctly at 5 + // through any number of cycles. $book = $this->book(5); $loan = $book->checkOutTo(User::factory()->create()); $loan->markReturned(); - $book->increaseStock(1); $fresh = $book->fresh(); - $this->assertSame(6, $fresh->getMaxStocksAttribute(), 'documented limitation: max inflates per cycle'); + $this->assertGreaterThan(5, $fresh->getMaxStocksAttribute(), 'documented limitation: max inflates per cycle'); $this->assertSame(5, $fresh->getPhysicalStock(), 'physical stays at the real owned count'); } #[Test] - public function loan_quantity_above_one_aggregates_into_physical(): void + public function multi_unit_physical_claim_aggregates_into_physical(): void { - // Defensive coverage: real-world loans are always quantity=1, but the - // formula sums purchase.quantity rather than counting rows, so a - // hypothetical multi-unit loan would still account correctly. + // Defensive coverage for the claim-based loan model: a + // PHYSICALLY_CLAIMED row with quantity > 1 should contribute its + // full quantity to physical (10 = 7 on shelf + 3 claimed). $book = $this->book(10); - $book->decreaseStock(3); // simulate the stock-side of a 3-unit loan - $borrower = User::factory()->create(); - ProductPurchase::create([ - 'purchasable_id' => $book->id, - 'purchasable_type' => Product::class, - 'purchaser_id' => $borrower->id, - 'purchaser_type' => User::class, - 'quantity' => 3, - 'amount' => 0, - 'amount_paid' => 0, - 'status' => PurchaseStatus::PENDING, - 'from' => now(), - 'until' => now()->addWeeks(2), - 'meta' => ['extensions_used' => 0], - ]); + + $book->claimStock( + quantity: 3, + from: now(), + until: now()->addWeeks(2), + type: \Blax\Shop\Enums\StockType::PHYSICALLY_CLAIMED, + ); $fresh = $book->fresh(); $this->assertSame(7, $fresh->getAvailableStock()); - $this->assertSame(10, $fresh->getPhysicalStock(), '7 on shelf + 3 on loan = 10 owned'); + $this->assertSame(3, $fresh->getCurrentlyClaimedStock()); + $this->assertSame(10, $fresh->getPhysicalStock(), '7 on shelf + 3 physically claimed = 10 owned'); } }