Merge pull request #220 from plank/fix-collection-delete

Fix MediableCollection::delete() bugs

Author: frasmage

src/MediableCollection.php

@@ -4,7 +4,7 @@
 namespace Plank\Mediable;
 
 use Closure;
-use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Query\Builder;
 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Relations\MorphToMany;
@@ -116,24 +116,33 @@ public function delete(): void
         $query = $relation->newPivotStatement();
         $classes = [];
 
-        $this->each(function (Model $item) use ($query, $relation, $classes) {
-            // collect list of ids of each class in case not all
-            // items belong to the same class
-            $classes[get_class($item)][] = $item->getKey();
+        $this->each(
+            function (Model $item) use ($query, $relation, &$classes) {
+                // collect list of ids of each class in case not all
+                // items belong to the same class
+                $classes[get_class($item)][] = $item->getKey();
+            }
+        );
 
-            // select pivots matching each item for deletion
-            $query->orWhere(function (Builder $q) use ($item, $relation) {
-                $q->where($relation->getMorphType(), get_class($item));
-                $q->where($relation->getQualifiedForeignPivotKeyName(), $item->getKey());
-            });
-        });
+        // delete each item by class
+        collect($classes)->each(
+            function (array $ids, string $class) use ($query, $relation) {
+                // select pivots matching each item for deletion
+                $query->orWhere(
+                    function (Builder $q) use ($class, $ids, $relation) {
+                        $q->where($relation->getMorphType(), $class);
+                        $q->whereIn(
+                            $relation->getQualifiedForeignPivotKeyName(),
+                            $ids
+                        );
+                    }
+                );
+
+                $class::whereIn((new $class)->getKeyName(), $ids)->delete();
+            }
+        );
 
         // delete pivots
         $query->delete();
-
-        // delete each item by class
-        collect($classes)->each(function (array $ids, string $class) {
-            $class::whereIn((new $class)->getKeyName(), $ids)->delete();
-        });
     }
 }

tests/Integration/MediableCollectionTest.php

@@ -2,6 +2,7 @@
 
 namespace Plank\Mediable\Tests\Integration;
 
+use Illuminate\Support\Facades\DB;
 use Plank\Mediable\Media;
 use Plank\Mediable\MediableCollection;
 use Plank\Mediable\Tests\Mocks\SampleMediable;
@@ -146,4 +147,30 @@ public function test_it_can_lazy_eager_load_media_with_relations_by_tag_match_al
         $this->assertTrue($collection[0]->media[0]->relationLoaded('originalMedia'));
         $this->assertTrue($collection[0]->media[0]->relationLoaded('variants'));
     }
+
+    public function testDelete()
+    {
+        $mediable1 = factory(SampleMediable::class)->create(['id' => 1]);
+        $mediable2 = factory(SampleMediable::class)->create(['id' => 2]);
+        $mediable3 = factory(SampleMediable::class)->create(['id' => 3]);
+        $media1 = factory(Media::class)->create(['id' => 1]);
+        $media2 = factory(Media::class)->create(['id' => 2]);
+        $mediable1->attachMedia($media1, 'foo');
+        $mediable1->attachMedia($media2, ['foo', 'bar', 'baz']);
+        $mediable2->attachMedia($media2, ['foo']);
+        $mediable3->attachMedia($media2, ['foo']);
+        $collection = new MediableCollection([$mediable1, $mediable2]);
+
+        $collection->delete();
+
+        $mediableResults = SampleMediable::all();
+        $this->assertEquals([3], $mediableResults->modelKeys());
+
+        $query = $mediable1->media()->newPivotStatement();
+        $pivots = $query->get();
+        $this->assertCount(1, $pivots);
+        $this->assertEquals("2", $pivots[0]->media_id);
+        $this->assertEquals("3", $pivots[0]->mediable_id);
+        $this->assertEquals("foo", $pivots[0]->tag);
+    }
 }