custom collection with pk index broke many_many with eager relation

spira · Nov 19, 2015 · 400c4c7 · 400c4c7
1 parent 0e2be67
commit 400c4c7
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 40 deletions.
diff --git a/api/app/Extensions/Controller/RequestValidationTrait.php b/api/app/Extensions/Controller/RequestValidationTrait.php
@@ -67,6 +67,7 @@ protected function notFoundException($keyName = '')
     protected function notFoundManyException($ids, $models, $keyName = '')
     {
         $errors = [];
+        $models->keyBy($keyName);
         foreach ($ids as $id) {
             if ($models->get($id)) {
                 $errors[] = null;

diff --git a/api/src/Model/Collection/Collection.php b/api/src/Model/Collection/Collection.php
@@ -10,8 +10,6 @@
 
 namespace Spira\Model\Collection;
 
-use Spira\Model\Model\BaseModel;
-
 class Collection extends \Illuminate\Database\Eloquent\Collection
 {
     /**
@@ -27,10 +25,7 @@ class Collection extends \Illuminate\Database\Eloquent\Collection
      */
     public function __construct($items = [], $className = null)
     {
-        $items = is_array($items) ? $items : $this->getArrayableItems($items);
-        foreach ($items as $item) {
-            $this->add($item);
-        }
+        parent::__construct($items);
         $this->className = $className;
     }
 
@@ -64,11 +59,7 @@ public function keyBy($keyBy)
     public function add($item)
     {
         $this->checkItem($item);
-        if ($item instanceof BaseModel) {
-            $this->items[$this->getItemKey($item)] = $item;
-        } else {
-            $this->items[] = $item;
-        }
+        parent::add($item);
 
         return $this;
     }
@@ -85,31 +76,6 @@ protected function checkItem($item)
         }
     }
 
-    /**
-     * @param BaseModel $model
-     * @return string
-     */
-    protected function getItemHash(BaseModel $model)
-    {
-        return spl_object_hash($model);
-    }
-
-    /**
-     * @param BaseModel $model
-     * @return mixed|string
-     */
-    protected function getItemKey(BaseModel $model)
-    {
-        if ($model->exists) {
-            // In the case of a composite key, the primary key is an array of values. In this case just use
-            // the item hash instead.
-            if (! is_array($model->getKeyName())) {
-                return $model->getKey();
-            }
-        }
-
-        return $this->getItemHash($model);
-    }
 
     /**
      * @return string Name of the Model collection consist of

diff --git a/api/tests/integration/ArticleTagTest.php b/api/tests/integration/ArticleTagTest.php
@@ -131,12 +131,20 @@ public function testPutTags()
         $updatedArticle = Article::find($entity->article_id);
         $updatedTags = $updatedArticle->tags->toArray();
 
-        $this->assertArrayHasKey($existingTagWillStay['tagId'], $updatedTags);
+        $existingTagId = $existingTagWillStay['tagId'];
+        $this->assertCount(1, array_filter($updatedTags, function($piece) use ($existingTagId){
+            return $piece['tag_id'] == $existingTagId;
+        }));
+
         foreach ($previousTagsWillBeRemoved as $removedTag) {
             if ($removedTag->tag_id == $existingTagWillStay['tagId']) {
                 continue;
             }
-            $this->assertArrayNotHasKey($removedTag->tag_id, $updatedTags);
+
+            $removedTagId = $removedTag->tag_id;
+            $this->assertCount(0, array_filter($updatedTags, function($piece) use ($removedTagId){
+                return $piece['tag_id'] == $removedTagId;
+            }));
         }
 
         $this->assertEquals(5, count($updatedTags));

diff --git a/api/tests/integration/TagTest.php b/api/tests/integration/TagTest.php
@@ -193,12 +193,20 @@ public function testPutTags()
         $updatedTag = Tag::find($entity->tag_id);
         $updatedTags = $updatedTag->childTags->toArray();
 
-        $this->assertArrayHasKey($existingTagWillStay['tagId'], $updatedTags);
+        $existingTagId = $existingTagWillStay['tagId'];
+        $this->assertCount(1, array_filter($updatedTags, function($piece) use ($existingTagId){
+            return $piece['tag_id'] == $existingTagId;
+        }));
+
         foreach ($previousTagsWillBeRemoved as $removedTag) {
             if ($removedTag->tag_id == $existingTagWillStay['tagId']) {
                 continue;
             }
-            $this->assertArrayNotHasKey($removedTag->tag_id, $updatedTags);
+
+            $removedTagId = $removedTag->tag_id;
+            $this->assertCount(0, array_filter($updatedTags, function($piece) use ($removedTagId){
+                return $piece['tag_id'] == $removedTagId;
+            }));
         }
 
         $this->assertEquals(5, count($updatedTags));