Remove price failed_at data if success (#19)

* Remove price failed_at data if success * PR Feedback * Update README for batch jobs * Add test for then callback resetting failure data * Add name to job batch --------- Co-authored-by: Finn Paes <[email protected]>
justbetter · Jan 17, 2024 · 8c6d467 · 8c6d467
1 parent fe28566
commit 8c6d467
Show file tree

Hide file tree

Showing 10 changed files with 154 additions and 43 deletions.
diff --git a/README.md b/README.md
@@ -29,7 +29,7 @@ This package can:
 
 Require this package: `composer require justbetter/laravel-magento-prices`
 
-Publish the config
+Publish the config:
 ```
 php artisan vendor:publish --provider="JustBetter\MagentoPrices\ServiceProvider" --tag="config"
 ```
@@ -39,6 +39,11 @@ Publish the activity log's migrations:
 php artisan vendor:publish --provider="Spatie\Activitylog\ActivitylogServiceProvider" --tag="activitylog-migrations"
 ```
 
+Publish the batches table migration:
+```
+php artisan queue:batches-table
+```
+
 Run migrations.
 
 

diff --git a/src/Actions/SyncPrices.php b/src/Actions/SyncPrices.php
@@ -9,7 +9,7 @@
 
 class SyncPrices implements SyncsPrices
 {
-    public function sync(int $retrieveLimit = null, int $updateLimit = null): void
+    public function sync(?int $retrieveLimit = null, ?int $updateLimit = null): void
     {
         $this->resetDoubleStatus();
 

diff --git a/src/Contracts/SyncsPrices.php b/src/Contracts/SyncsPrices.php
@@ -4,5 +4,5 @@
 
 interface SyncsPrices
 {
-    public function sync(int $retrieveLimit = null, int $updateLimit = null): void;
+    public function sync(?int $retrieveLimit = null, ?int $updateLimit = null): void;
 }
diff --git a/src/Data/PriceData.php b/src/Data/PriceData.php
@@ -24,8 +24,8 @@ class PriceData implements Arrayable
     public function __construct(
         string $sku,
         Collection $basePrices,
-        Collection $tierPrices = null,
-        Collection $specialPrices = null
+        ?Collection $tierPrices = null,
+        ?Collection $specialPrices = null
     ) {
         $this->sku = $sku;
         $this->basePrices = $basePrices;

diff --git a/src/Data/SpecialPriceData.php b/src/Data/SpecialPriceData.php
@@ -17,7 +17,7 @@ class SpecialPriceData implements Arrayable
 
     public Carbon $to;
 
-    public function __construct(Money $price, int $storeId = 0, Carbon $from = null, Carbon $to = null)
+    public function __construct(Money $price, int $storeId = 0, ?Carbon $from = null, ?Carbon $to = null)
     {
         $this->price = $price;
         $this->storeId = $storeId;

diff --git a/src/Jobs/UpdateMagentoBasePricesJob.php b/src/Jobs/UpdateMagentoBasePricesJob.php
@@ -2,6 +2,7 @@
 
 namespace JustBetter\MagentoPrices\Jobs;
 
+use Illuminate\Bus\Batchable;
 use Illuminate\Bus\Queueable;
 use Illuminate\Contracts\Queue\ShouldBeUnique;
 use Illuminate\Contracts\Queue\ShouldQueue;
@@ -15,6 +16,7 @@
 
 class UpdateMagentoBasePricesJob implements ShouldBeUnique, ShouldQueue
 {
+    use Batchable;
     use Dispatchable;
     use InteractsWithQueue;
     use Queueable;

diff --git a/src/Jobs/UpdatePriceJob.php b/src/Jobs/UpdatePriceJob.php
@@ -8,6 +8,7 @@
 use Illuminate\Foundation\Bus\Dispatchable;
 use Illuminate\Queue\InteractsWithQueue;
 use Illuminate\Queue\SerializesModels;
+use Illuminate\Support\Facades\Bus;
 use JustBetter\MagentoPrices\Data\PriceData;
 use JustBetter\MagentoPrices\Events\UpdatedPriceEvent;
 use JustBetter\MagentoPrices\Models\MagentoPrice;
@@ -46,9 +47,20 @@ public function handle(ChecksMagentoExistence $checksMagentoExistence): void
 
         $data = $model->getData();
 
-        $this->handleBasePrices($data);
-        $this->handleTierPrices($data);
-        $this->handleSpecialPrices($data);
+        Bus::batch(array_filter([
+            $this->handleBasePrices($data),
+            $this->handleTierPrices($data),
+            $this->handleSpecialPrices($data),
+        ]))
+            ->name("Magento price update $this->sku")
+            ->onQueue(config('magento-prices.queue'))
+            ->then(function () use ($model): void {
+                $model->update([
+                    'fail_count' => 0,
+                    'last_failed' => null,
+                ]);
+            })
+            ->dispatch();
 
         $model->update([
             'update' => false,
@@ -57,55 +69,57 @@ public function handle(ChecksMagentoExistence $checksMagentoExistence): void
         UpdatedPriceEvent::dispatch($this->sku);
     }
 
-    protected function handleBasePrices(PriceData $data): void
+    protected function handleBasePrices(PriceData $data): ?UpdateMagentoBasePricesJob
     {
         if ($this->shouldUpdateType('base') && $data->basePrices->isNotEmpty()) {
-            UpdateMagentoBasePricesJob::dispatch($data);
+            return new UpdateMagentoBasePricesJob($data);
         }
+
+        return null;
     }
 
-    protected function handleTierPrices(PriceData $data): void
+    protected function handleTierPrices(PriceData $data): ?UpdateMagentoTierPricesJob
     {
         if ($this->shouldUpdateType('tier') && $data->tierPrices->isNotEmpty()) {
-            UpdateMagentoTierPricesJob::dispatch($data);
-
             $data->getModel()->update([
                 'has_tier' => true,
             ]);
 
-            return;
+            return new UpdateMagentoTierPricesJob($data);
         }
 
         // Delete the tier price
         if ($data->getModel()->has_tier) {
-            UpdateMagentoTierPricesJob::dispatch($data);
-
             $data->getModel()->update([
                 'has_tier' => false,
             ]);
+
+            return new UpdateMagentoTierPricesJob($data);
         }
+
+        return null;
     }
 
-    protected function handleSpecialPrices(PriceData $data): void
+    protected function handleSpecialPrices(PriceData $data): ?UpdateMagentoSpecialPricesJob
     {
         if ($this->shouldUpdateType('special') && $data->specialPrices->isNotEmpty()) {
-            UpdateMagentoSpecialPricesJob::dispatch($data);
-
             $data->getModel()->update([
                 'has_special' => true,
             ]);
 
-            return;
+            return new UpdateMagentoSpecialPricesJob($data);
         }
 
         // Delete the special price
         if ($data->getModel()->has_special) {
-            UpdateMagentoSpecialPricesJob::dispatch($data);
-
             $data->getModel()->update([
                 'has_special' => false,
             ]);
+
+            return new UpdateMagentoSpecialPricesJob($data);
         }
+
+        return null;
     }
 
     protected function shouldUpdateType(string $type): bool

diff --git a/tests/Feature/UpdatePricesJobTest.php b/tests/Feature/UpdatePricesJobTest.php
@@ -2,6 +2,7 @@
 
 namespace JustBetter\MagentoPrices\Tests\Feature;
 
+use Illuminate\Bus\PendingBatch;
 use Illuminate\Support\Facades\Bus;
 use JustBetter\MagentoPrices\Commands\RetrievePricesCommand;
 use JustBetter\MagentoPrices\Commands\UpdatePriceCommand;
@@ -30,6 +31,14 @@ public function test_it_updates(): void
 
         $this->artisan(UpdatePriceCommand::class, ['sku' => '123']);
 
-        Bus::assertDispatched(UpdateMagentoBasePricesJob::class);
+        Bus::assertBatched(function (PendingBatch $batch) {
+            foreach ($batch->jobs as $job) {
+                if ($job instanceof UpdateMagentoBasePricesJob) {
+                    return true;
+                }
+            }
+
+            return false;
+        });
     }
 }
diff --git a/tests/Helpers/MoneyHelperTest.php b/tests/Helpers/MoneyHelperTest.php
@@ -9,7 +9,7 @@
 class MoneyHelperTest extends TestCase
 {
     /** @dataProvider provider */
-    public function test_it_creates_money(float $amount, string $method, float $expectedAmount = null): void
+    public function test_it_creates_money(float $amount, string $method, ?float $expectedAmount = null): void
     {
         config()->set('laravel-magento-prices.currency', 'EUR');
         config()->set('laravel-magento-prices.precision', 4);

diff --git a/tests/Jobs/UpdatePriceJobTest.php b/tests/Jobs/UpdatePriceJobTest.php
@@ -2,6 +2,8 @@
 
 namespace JustBetter\MagentoPrices\Tests\Jobs;
 
+use Illuminate\Bus\PendingBatch;
+use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Bus;
 use Illuminate\Support\Facades\Event;
 use JustBetter\MagentoPrices\Events\UpdatedPriceEvent;
@@ -65,11 +67,21 @@ public function test_it_dispatches_update_jobs(): void
     {
         UpdatePriceJob::dispatchSync('::sku::');
 
-        Bus::assertDispatched(UpdateMagentoBasePricesJob::class);
-        Bus::assertDispatched(UpdateMagentoTierPricesJob::class);
-        Bus::assertDispatched(UpdateMagentoSpecialPricesJob::class);
+        Bus::assertBatched(function (PendingBatch $batch): bool {
+            /** @var Collection $jobs */
+            $jobs = $batch->jobs->map(function (mixed $job): string {
+                return get_class($job);
+            });
+
+            if ($jobs->contains('JustBetter\MagentoPrices\Jobs\UpdateMagentoBasePricesJob') && $jobs->contains('JustBetter\MagentoPrices\Jobs\UpdateMagentoTierPricesJob') && $jobs->contains('JustBetter\MagentoPrices\Jobs\UpdateMagentoSpecialPricesJob')) {
+                return true;
+            }
+
+            return false;
+        });
 
         $model = MagentoPrice::findBySku('::sku::');
+
         $this->assertFalse($model->update);
     }
 
@@ -85,11 +97,21 @@ public function test_it_dispatches_update_jobs_by_type(string $type, string $dis
     {
         UpdatePriceJob::dispatchSync('::sku::', $type);
 
-        Bus::assertDispatched($dispatch);
+        Bus::assertBatched(function (PendingBatch $batch) use ($dispatch): bool {
+            return $batch->jobs->first() instanceof $dispatch;
+        });
+
+        Bus::assertBatched(function (PendingBatch $batch) use ($notDispatch): bool {
+            foreach ($notDispatch as $notDispatchJobClass) {
+                foreach ($batch->jobs as $job) {
+                    if ($job instanceof $notDispatchJobClass) {
+                        return false;
+                    }
+                }
+            }
 
-        foreach ($notDispatch as $jobClass) {
-            Bus::assertNotDispatched($jobClass);
-        }
+            return true;
+        });
     }
 
     public static function jobTypes(): array
@@ -120,9 +142,15 @@ public function test_it_does_not_dispatch_empty_base(): void
 
         UpdatePriceJob::dispatchSync('::sku::');
 
-        Bus::assertNotDispatched(UpdateMagentoBasePricesJob::class);
-        Bus::assertDispatched(UpdateMagentoTierPricesJob::class);
-        Bus::assertDispatched(UpdateMagentoSpecialPricesJob::class);
+        Bus::assertBatched(function (PendingBatch $batch): bool {
+            foreach ($batch->jobs as $job) {
+                if ($job instanceof UpdateMagentoBasePricesJob) {
+                    return false;
+                }
+            }
+
+            return true;
+        });
     }
 
     public function test_it_does_not_dispatch_empty_tier(): void
@@ -132,9 +160,15 @@ public function test_it_does_not_dispatch_empty_tier(): void
 
         UpdatePriceJob::dispatchSync('::sku::');
 
-        Bus::assertDispatched(UpdateMagentoBasePricesJob::class);
-        Bus::assertNotDispatched(UpdateMagentoTierPricesJob::class);
-        Bus::assertDispatched(UpdateMagentoSpecialPricesJob::class);
+        Bus::assertBatched(function (PendingBatch $batch): bool {
+            foreach ($batch->jobs as $job) {
+                if ($job instanceof UpdateMagentoTierPricesJob) {
+                    return false;
+                }
+            }
+
+            return true;
+        });
     }
 
     public function test_it_does_not_dispatch_empty_special(): void
@@ -144,9 +178,15 @@ public function test_it_does_not_dispatch_empty_special(): void
 
         UpdatePriceJob::dispatchSync('::sku::');
 
-        Bus::assertDispatched(UpdateMagentoBasePricesJob::class);
-        Bus::assertDispatched(UpdateMagentoTierPricesJob::class);
-        Bus::assertNotDispatched(UpdateMagentoSpecialPricesJob::class);
+        Bus::assertBatched(function (PendingBatch $batch): bool {
+            foreach ($batch->jobs as $job) {
+                if ($job instanceof UpdateMagentoSpecialPricesJob) {
+                    return false;
+                }
+            }
+
+            return true;
+        });
     }
 
     public function test_it_does_dispatch_empty_tier_to_remove_tier_prices(): void
@@ -156,7 +196,15 @@ public function test_it_does_dispatch_empty_tier_to_remove_tier_prices(): void
 
         UpdatePriceJob::dispatchSync('::sku::');
 
-        Bus::assertDispatched(UpdateMagentoTierPricesJob::class);
+        Bus::assertBatched(function (PendingBatch $batch): bool {
+            foreach ($batch->jobs as $job) {
+                if ($job instanceof UpdateMagentoTierPricesJob) {
+                    return true;
+                }
+            }
+
+            return false;
+        });
 
         $model = MagentoPrice::findBySku('::sku::');
         $this->assertFalse($model->has_tier);
@@ -169,7 +217,15 @@ public function test_it_does_dispatch_empty_special_to_remove_special_prices():
 
         UpdatePriceJob::dispatchSync('::sku::');
 
-        Bus::assertDispatched(UpdateMagentoSpecialPricesJob::class);
+        Bus::assertBatched(function (PendingBatch $batch): bool {
+            foreach ($batch->jobs as $job) {
+                if ($job instanceof UpdateMagentoSpecialPricesJob) {
+                    return true;
+                }
+            }
+
+            return false;
+        });
 
         $model = MagentoPrice::findBySku('::sku::');
         $this->assertFalse($model->has_special);
@@ -194,4 +250,29 @@ public function test_it_does_nothing(): void
 
         UpdatePriceJob::dispatch('::non_existent::');
     }
+
+    public function test_it_clears_failure_data_if_success(): void
+    {
+        MagentoPrice::findBySku('::sku::')
+            ->update([
+                'fail_count' => 5,
+                'last_failed' => now(),
+            ]);
+
+        UpdatePriceJob::dispatchSync('::sku::');
+
+        Bus::assertBatched(function (PendingBatch $batch) {
+            /* @var \Illuminate\Queue\SerializableClosure $thenCallback */
+            [$thenCallback] = $batch->thenCallbacks();
+
+            $thenCallback->getClosure()->call($this);
+
+            $model = MagentoPrice::findBySku('::sku::');
+
+            $this->assertEquals(0, $model->fail_count);
+            $this->assertNull($model->last_failed);
+
+            return true;
+        });
+    }
 }