fixed DisposalBuilder and ReversionFinder

domain/src/Aggregates/SharePoolingAsset/Entities/SharePoolingAssetAcquisition.php

@@ -74,8 +74,12 @@ public function increaseSameDayQuantity(Quantity $quantity): self
         return $this;
     }
 
-    /** Increase the same-day quantity and adjust the 30-day quantity accordingly. */
-    public function increaseSameDayQuantityUpToAvailableQuantity(Quantity $quantity): self
+    /**
+     * Increase the same-day quantity and adjust the 30-day quantity accordingly.
+     *
+     * @return Quantity the added quantity
+     */
+    public function increaseSameDayQuantityUpToAvailableQuantity(Quantity $quantity): Quantity
     {
         // Adjust same-day quantity
         $quantityToAdd = Quantity::minimum($quantity, $this->availableSameDayQuantity());
@@ -85,7 +89,7 @@ public function increaseSameDayQuantityUpToAvailableQuantity(Quantity $quantity)
         $quantityToDeduct = Quantity::minimum($quantityToAdd, $this->thirtyDayQuantity);
         $this->thirtyDayQuantity = $this->thirtyDayQuantity->minus($quantityToDeduct);
 
-        return $this;
+        return $quantityToAdd;
     }
 
     /** @throws SharePoolingAssetAcquisitionException */
@@ -111,12 +115,13 @@ public function increaseThirtyDayQuantity(Quantity $quantity): self
         return $this;
     }
 
-    public function increaseThirtyDayQuantityUpToAvailableQuantity(Quantity $quantity): self
+    /** @return Quantity the added quantity */
+    public function increaseThirtyDayQuantityUpToAvailableQuantity(Quantity $quantity): Quantity
     {
         $quantityToAdd = Quantity::minimum($quantity, $this->availableThirtyDayQuantity());
         $this->thirtyDayQuantity = $this->thirtyDayQuantity->plus($quantityToAdd);
 
-        return $this;
+        return $quantityToAdd;
     }
 
     /** @throws SharePoolingAssetAcquisitionException */

domain/src/Aggregates/SharePoolingAsset/Entities/SharePoolingAssetDisposals.php

@@ -65,16 +65,6 @@ public function reverse(): SharePoolingAssetDisposals
         return new self(array_reverse($this->disposals));
     }
 
-    public function unprocessed(): SharePoolingAssetDisposals
-    {
-        $disposals = array_filter(
-            $this->disposals,
-            fn (SharePoolingAssetDisposal $disposal) => ! $disposal->processed,
-        );
-
-        return self::make(...$disposals);
-    }
-
     public function withAvailableSameDayQuantity(): SharePoolingAssetDisposals
     {
         $disposals = array_filter(

domain/src/Aggregates/SharePoolingAsset/Services/DisposalBuilder/DisposalBuilder.php

@@ -18,7 +18,7 @@
  */
 final class DisposalBuilder
 {
-    public static function process(
+    public static function make(
         DisposeOfSharePoolingAsset $disposal,
         SharePoolingAssetTransactions $transactions,
     ): SharePoolingAssetDisposal {
@@ -107,13 +107,10 @@ private static function processSameDayAcquisitions(
         // Deduct the applied quantity from the same-day acquisitions
         $remainder = $availableSameDayQuantity;
         foreach ($sameDayAcquisitions as $acquisition) {
-            $quantityToAllocate = Quantity::minimum($remainder, $acquisition->availableSameDayQuantity());
+            $quantityToAllocate = $acquisition->increaseSameDayQuantityUpToAvailableQuantity($remainder);
             $sameDayQuantityAllocation->allocateQuantity($quantityToAllocate, $acquisition);
 
-            $acquisition->increaseSameDayQuantityUpToAvailableQuantity($remainder);
-
-            $remainder = $remainder->minus($quantityToAllocate);
-            if ($remainder->isZero()) {
+            if (($remainder = $remainder->minus($quantityToAllocate))->isZero()) {
                 break;
             }
         }
@@ -146,37 +143,13 @@ private static function processAcquisitionsWithinThirtyDays(
 
         foreach ($withinThirtyDaysAcquisitions as $acquisition) {
             // Apply the acquisition's cost basis to the disposed of asset up to the remaining quantity
-            $thirtyDayQuantityToApply = Quantity::minimum($acquisition->availableThirtyDayQuantity(), $remainingQuantity);
-
-            // Also deduct same-day disposals with available same-day quantity that haven't been processed yet
-            $sameDayDisposals = $transactions->disposalsMadeOn($acquisition->date)
-                ->unprocessed()
-                ->withAvailableSameDayQuantity();
-
-            foreach ($sameDayDisposals as $disposal) {
-                $sameDayQuantityToApply = Quantity::minimum($disposal->availableSameDayQuantity(), $thirtyDayQuantityToApply);
-                $disposal->sameDayQuantityAllocation->allocateQuantity($sameDayQuantityToApply, $acquisition);
-                $acquisition->increaseSameDayQuantityUpToAvailableQuantity($sameDayQuantityToApply);
-                $thirtyDayQuantityToApply = $thirtyDayQuantityToApply->minus($sameDayQuantityToApply);
-                if ($thirtyDayQuantityToApply->isZero()) {
-                    break;
-                }
-            }
-
-            if ($thirtyDayQuantityToApply->isZero()) {
-                continue;
-            }
-
+            $quantityToAllocate = $acquisition->increaseThirtyDayQuantityUpToAvailableQuantity($remainingQuantity);
             $averageCostBasisPerUnit = $acquisition->averageCostBasisPerUnit();
+            $costBasis = $costBasis->plus($averageCostBasisPerUnit->multipliedBy($quantityToAllocate));
+            $thirtyDayQuantityAllocation->allocateQuantity($quantityToAllocate, $acquisition);
 
-            $costBasis = $costBasis->plus($averageCostBasisPerUnit->multipliedBy($thirtyDayQuantityToApply));
-
-            $thirtyDayQuantityAllocation->allocateQuantity($thirtyDayQuantityToApply, $acquisition);
-            $acquisition->increaseThirtyDayQuantityUpToAvailableQuantity($thirtyDayQuantityToApply);
-            $remainingQuantity = $remainingQuantity->minus($thirtyDayQuantityToApply);
-
-            // Continue until there are no more transactions or we've covered all disposed tokens
-            if ($remainingQuantity->isZero()) {
+            // Continue until there are no more transactions or we've covered all disposed of tokens
+            if (($remainingQuantity = $remainingQuantity->minus($quantityToAllocate))->isZero()) {
                 break;
             }
         }

domain/src/Aggregates/SharePoolingAsset/Services/ReversionFinder/ReversionFinder.php

@@ -38,19 +38,20 @@ private static function addSameDayDisposalsToRevert(
             return $disposalsToRevert;
         }
 
-        // Get same-day disposals with part of their quantity not allocated to same-day acquisitions
-        $sameDayDisposals = $transactions->disposalsMadeOn($date)->withAvailableSameDayQuantity();
+        // Get all same-day disposals. As the average cost basis of same-day acquisitions is used to
+        // calculate the cost basis of the disposals, it's simpler to revert them all and start over
+        $sameDayDisposals = $transactions->disposalsMadeOn($date);
 
         if ($sameDayDisposals->isEmpty()) {
             return self::add30DayDisposalsToRevert($disposalsToRevert, $transactions, $date, $remainingQuantity);
         }
 
-        // As the average cost basis of same-day acquisitions is used to calculate the
-        // cost basis of the disposals, it's simpler to revert them all and start over
         $disposalsToRevert->add(...$sameDayDisposals);
 
-        // Deduct what's left (either the whole remaining quantity or the disposals' unallocated
-        // same-day quantity, whichever is smaller) from the remaining quantity to be allocated
+        // Deduct either the acquisition's remaining quantity or the disposals' unallocated same-day
+        // quantity (whichever is smaller) from the quantity to be allocated. The trick here is that,
+        // while we are going to revert and replay all same-day disposals, their same-day quantity
+        // already allocated to earlier same-day acquisitions must be taken into account, still
         $quantityToDeduct = Quantity::minimum($remainingQuantity, $sameDayDisposals->availableSameDayQuantity());
         $remainingQuantity = $remainingQuantity->minus($quantityToDeduct);
 
@@ -74,8 +75,8 @@ private static function add30DayDisposalsToRevert(
         foreach ($pastThirtyDaysDisposals as $disposal) {
             $disposalsToRevert->add($disposal);
 
-            // Deduct what's left (either the whole remaining quantity or the disposal's uncallocated
-            // 30-day quantity, whichever is smaller) from the remaining quantity to be allocated
+            // Deduct either the acquisition's remaining quantity or the disposal's uncallocated
+            // 30-day quantity (whichever is smaller) from the quantity to be allocated
             $quantityToDeduct = Quantity::minimum($remainingQuantity, $disposal->availableThirtyDayQuantity());
             $remainingQuantity = $remainingQuantity->minus($quantityToDeduct);
 
@@ -106,7 +107,7 @@ public static function disposalsToRevertOnDisposal(
             // Add disposals up to the disposal's quantity, starting with the most recent ones. That is
             // because older disposals get priority when allocating the 30-day quantity of acquisitions
             // made within 30 days of the disposal, so the last disposals in are the first out
-            $disposalsWithAllocatedThirtyDayQuantity = $transactions->processed()
+            $disposalsWithAllocatedThirtyDayQuantity = $transactions
                 ->disposalsWithThirtyDayQuantityAllocatedTo($acquisition)
                 ->reverse();
 

domain/src/Aggregates/SharePoolingAsset/SharePoolingAsset.php

@@ -66,7 +66,7 @@ public function acquire(AcquireSharePoolingAsset $action): void
 
         $disposalsToRevert = ReversionFinder::disposalsToRevertOnAcquisition(
             acquisition: $action,
-            transactions: $this->transactions->copy(),
+            transactions: $this->transactions,
         );
 
         $disposalsToRevert->isEmpty() || $this->revertDisposals($disposalsToRevert);
@@ -96,7 +96,7 @@ public function applySharePoolingAssetAcquired(SharePoolingAssetAcquired $event)
         // acquisition's same-day and 30-day quantities, these updates occur before the acquisition's event
         // is recorded, meaning the event is stored with the updated quantities. As a result, whenever the
         // aggregate is recreated from its events, the acquisition already has a same-day and/or 30-day
-        // quantity, but upon replaying the subsequent disposals, these quantities are updated *again*.
+        // quantity, but upon replaying the subsequent disposals, these quantities are updated *again*
         $this->transactions->add(clone $event->acquisition);
     }
 
@@ -111,12 +111,12 @@ public function disposeOf(DisposeOfSharePoolingAsset $action): void
 
         $disposalsToRevert = ReversionFinder::disposalsToRevertOnDisposal(
             disposal: $action,
-            transactions: $this->transactions->copy(),
+            transactions: $this->transactions,
         );
 
         $disposalsToRevert->isEmpty() || $this->revertDisposals($disposalsToRevert);
 
-        $sharePoolingAssetDisposal = DisposalBuilder::process(
+        $sharePoolingAssetDisposal = DisposalBuilder::make(
             disposal: $action,
             transactions: $this->transactions->copy(),
         );

domain/tests/Aggregates/SharePoolingAsset/Entities/SharePoolingAssetAcquisitionTest.php

@@ -109,23 +109,29 @@
     'scenario 2' => ['10', '40'],
 ]);
 
-it('can increase the same-day quantity up to the available quantity', function (string $increase, string $sameDayQuantity, string $thirtyDayQuantity) {
+it('can increase the same-day quantity up to the available quantity', function (
+    string $increase,
+    string $sameDayQuantity,
+    string $thirtyDayQuantity,
+    string $addedQuantity,
+) {
     /** @var SharePoolingAssetAcquisition */
     $acquisition = SharePoolingAssetAcquisition::factory()->make([
         'quantity' => new Quantity('100'),
         'sameDayQuantity' => new Quantity('30'),
         'thirtyDayQuantity' => new Quantity('60'),
     ]);
 
-    $acquisition->increaseSameDayQuantityUpToAvailableQuantity(new Quantity($increase));
+    $added = $acquisition->increaseSameDayQuantityUpToAvailableQuantity(new Quantity($increase));
 
     expect((string) $acquisition->sameDayQuantity())->toBe($sameDayQuantity);
     expect((string) $acquisition->thirtyDayQuantity())->toBe($thirtyDayQuantity);
+    expect((string) $added)->toBe($addedQuantity);
 })->with([
-    'scenario 1' => ['5', '35', '55'],
-    'scenario 2' => ['10', '40', '50'],
-    'scenario 3' => ['70', '100', '0'],
-    'scenario 4' => ['71', '100', '0'],
+    'scenario 1' => ['5', '35', '55', '5'],
+    'scenario 2' => ['10', '40', '50', '10'],
+    'scenario 3' => ['70', '100', '0', '70'],
+    'scenario 4' => ['71', '100', '0', '70'],
 ]);
 
 it('cannot decrease the same-day quantity because the quantity is too great', function () {
@@ -185,22 +191,28 @@
     'scenario 2' => ['10', '70'],
 ]);
 
-it('can increase the 30-day quantity up to the available quantity', function (string $increase, string $sameDayQuantity, string $thirtyDayQuantity) {
+it('can increase the 30-day quantity up to the available quantity', function (
+    string $increase,
+    string $sameDayQuantity,
+    string $thirtyDayQuantity,
+    string $addedQuantity,
+) {
     /** @var SharePoolingAssetAcquisition */
     $acquisition = SharePoolingAssetAcquisition::factory()->make([
         'quantity' => new Quantity('100'),
         'sameDayQuantity' => new Quantity('30'),
         'thirtyDayQuantity' => new Quantity('60'),
     ]);
 
-    $acquisition->increaseThirtyDayQuantityUpToAvailableQuantity(new Quantity($increase));
+    $added = $acquisition->increaseThirtyDayQuantityUpToAvailableQuantity(new Quantity($increase));
 
     expect((string) $acquisition->sameDayQuantity())->toBe($sameDayQuantity);
     expect((string) $acquisition->thirtyDayQuantity())->toBe($thirtyDayQuantity);
+    expect((string) $added)->toBe($addedQuantity);
 })->with([
-    'scenario 1' => ['5', '30', '65'],
-    'scenario 2' => ['10', '30', '70'],
-    'scenario 3' => ['15', '30', '70'],
+    'scenario 1' => ['5', '30', '65', '5'],
+    'scenario 2' => ['10', '30', '70', '10'],
+    'scenario 3' => ['15', '30', '70', '10'],
 ]);
 
 it('cannot decrease the 30-day quantity because the quantity is too great', function () {

domain/tests/Aggregates/SharePoolingAsset/Entities/SharePoolingAssetDisposalsTest.php

@@ -69,22 +69,6 @@
     expect($disposals->getIterator()[2])->toBe($disposal1);
 });
 
-it('can return the unprocessed disposals from a collection of disposals', function () {
-    /** @var list<SharePoolingAssetDisposal> */
-    $items = [
-        SharePoolingAssetDisposal::factory()->make(),
-        $unprocessed1 = SharePoolingAssetDisposal::factory()->unprocessed()->make(),
-        SharePoolingAssetDisposal::factory()->make(),
-        $unprocessed2 = SharePoolingAssetDisposal::factory()->unprocessed()->make(),
-    ];
-
-    $disposals = SharePoolingAssetDisposals::make(...$items)->unprocessed();
-
-    expect($disposals->count())->toBeInt()->toBe(2);
-    expect($disposals->getIterator()[0])->toBe($unprocessed1);
-    expect($disposals->getIterator()[1])->toBe($unprocessed2);
-});
-
 it('can return the disposals with available same-day quantity from a collection of disposals', function () {
     /** @var list<SharePoolingAssetDisposal> */
     $items = [

domain/tests/Aggregates/SharePoolingAsset/Services/DisposalBuilder/DisposalBuilderTest.php

@@ -20,7 +20,7 @@
         forFiat: false,
     );
 
-    $disposal = DisposalBuilder::process(
+    $disposal = DisposalBuilder::make(
         disposal: $action,
         transactions: SharePoolingAssetTransactions::make(SharePoolingAssetAcquisition::factory()->make()),
     );
@@ -40,7 +40,7 @@
 
     $acquisition = SharePoolingAssetAcquisition::factory()->make(['date' => LocalDate::parse('2021-10-22')]);
 
-    $disposal = DisposalBuilder::process($action, SharePoolingAssetTransactions::make($acquisition));
+    $disposal = DisposalBuilder::make($action, SharePoolingAssetTransactions::make($acquisition));
 
     expect($disposal->id)->toBe($id);
 });