Fixed bugs related to storing failed shipments

database/migrations/create_failed_shipments_table.php.stub

@@ -11,9 +11,8 @@ return new class extends Migration
         Schema::create('failed_shipments', function (Blueprint $table) {
             $table->id();
             $table->string('class_name');
-            $table->string('shipment');
             $table->string('subscriber');
-            $table->timestamp('last_retried_at');
+            $table->timestamp('last_retried_at')->nullable();
             $table->unsignedInteger('retries')->default(0);
 
             $table->timestamps();

src/DataShipper.php

@@ -122,7 +122,7 @@ public function pushModel(Model $model, array $only = [], string $mode = Package
     public function handleProblematicShipment(string $subscriber, string $shipment, array $packageIds): void {
         // We failed to push this dataset as an update for one or more subscribers.
         // Offload it to a table for retrying.
-        $packages = $this->repository->getPackagesByUuids($shipment, $packageIds);
+        $packages = $this->repository->getPackagesByUuids($packageIds, $shipment);
 
         $failedShipment = FailedShipment::create([
             'class_name' => $shipment,

src/Jobs/DispatchShipmentToSubscriber.php

@@ -37,7 +37,7 @@ public function handle()
             $packages = $repository->getPackagesByUuids($this->packageIds, $this->shipment);
             $subscriber->ship($packages);
         } catch (\Exception) {
-            DataShipper::handleProblematicShipment($this->shipment);
+            DataShipper::handleProblematicShipment($this->subscriber, $this->shipment, $this->packageIds);
         }
     }
 }

src/ShipmentRepository.php

@@ -57,7 +57,6 @@ public function getPackagesByUuids(array $ids, string $key, $startingIndex = 0):
             $pipe->get("{$key}-shipment-length");
         });
 
-        // Remove these jobs from the queue
         $count = array_pop($packages) - count($ids);
         if ($count > 0 && $count < $this->maxShipmentLength) {
             $this->connection()->pipeline(function ($pipe) use ($key) {

tests/DataShipperTest.php

@@ -186,7 +186,6 @@
 it('will not retry a shipment that has reached max retries', function() {
     \Autoklose\DataShipper\Models\FailedShipment::create([
         'class_name' => 'Test',
-        'shipment' => 'TestKey',
         'subscriber' => 'elasticsearch',
         'last_retried_at' => now()->subHour(),
         'retries' => config('data-shipper.shipments.max_retries')
@@ -203,7 +202,6 @@
 it('will retry shipments if they have not already been retried too many times', function() {
     \Autoklose\DataShipper\Models\FailedShipment::create([
         'class_name' => 'Test',
-        'shipment' => 'TestKey',
         'subscriber' => 'elasticsearch',
         'last_retried_at' => now()->subHour(),
     ]);
@@ -219,7 +217,6 @@
 it('will resubmit packages to be retried', function($changes) {
     $failedShipment = \Autoklose\DataShipper\Models\FailedShipment::create([
         'class_name' => 'Test',
-        'shipment' => 'TestKey',
         'subscriber' => 'elasticsearch',
         'last_retried_at' => now()->subHour(),
     ]);
@@ -239,6 +236,27 @@
     \Pest\Laravel\assertDatabaseMissing('failed_shipments', ['id' => $failedShipment->id]);
 })->with('bulk-changes');
 
+it('can store failed shipments', function($changes) {
+    \Autoklose\DataShipper\Models\FailedShipment::query()->delete();
+
+    \Pest\Laravel\partialMock(\Autoklose\DataShipper\Subscribers\ElasticsearchSubscriber::class, function(\Mockery\MockInterface $mock) {
+        $mock->expects('ship')->andReturnUsing(fn(...$args) => throw new Exception("Fear not. This is just a test."));
+    });
+
+    $key = \Autoklose\DataShipper\Tests\Models\TestModel::class;
+    DataShipper::pushMany($key, $changes, 'id');
+
+    $command = new \Autoklose\DataShipper\Commands\ShipIt();
+    $command->handle();
+
+    /** @var \Autoklose\DataShipper\ShipmentRepository $repository */
+    $repository = app()->make(\Autoklose\DataShipper\ShipmentRepository::class);
+    expect($repository->getShipmentLength($key))->toEqual(0);
+
+    \Pest\Laravel\assertDatabaseCount('failed_shipments', 1);
+    \Pest\Laravel\assertDatabaseCount('failed_packages', 10);
+})->with('bulk-changes');
+
 dataset('test-models', [
     [fn() => \Autoklose\DataShipper\Tests\Models\TestModel::create([
         'string_field' => 'some string',