Fully support pivot records without foreign key

staudenmeir · May 16, 2020 · 2ebb513 · 2ebb513
1 parent a738f16
commit 2ebb513
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 46 deletions.
diff --git a/src/Relations/BelongsToJson.php b/src/Relations/BelongsToJson.php
@@ -49,7 +49,7 @@ protected function getEagerModelKeys(array $models)
         $keys = [];
 
         foreach ($models as $model) {
-            $keys = array_merge($keys, (array) $this->getForeignKeys($model));
+            $keys = array_merge($keys, $this->getForeignKeys($model));
         }
 
         sort($keys);
@@ -72,7 +72,7 @@ public function match(array $models, Collection $results, $relation)
         foreach ($models as $model) {
             $matches = [];
 
-            foreach ((array) $this->getForeignKeys($model) as $id) {
+            foreach ($this->getForeignKeys($model) as $id) {
                 if (isset($dictionary[$id])) {
                     $matches[] = $dictionary[$id];
                 }
@@ -191,6 +191,7 @@ protected function pivotAttributes(Model $model, Model $parent)
     /**
      * Get the foreign key values.
      *
+     * @param \Illuminate\Database\Eloquent\Model|null $model
      * @return array
      */
     public function getForeignKeys(Model $model = null)
@@ -199,7 +200,7 @@ public function getForeignKeys(Model $model = null)
 
         $keys = (array) $model->{$this->foreignKey};
 
-        return array_filter($keys, function($key) {
+        return array_filter($keys, function ($key) {
             return $key !== null;
         });
     }

diff --git a/src/Relations/InteractsWithPivotRecords.php b/src/Relations/InteractsWithPivotRecords.php
@@ -17,11 +17,11 @@ trait InteractsWithPivotRecords
      */
     public function attach($ids)
     {
-        $records = $this->decodeRecords();
+        [$records, $others] = $this->decodeRecords();
 
         $records = $this->formatIds($this->parseIds($ids)) + $records;
 
-        $this->child->{$this->path} = $this->encodeRecords($records);
+        $this->child->{$this->path} = $this->encodeRecords($records, $others);
 
         return $this->child;
     }
@@ -34,18 +34,18 @@ public function attach($ids)
      */
     public function detach($ids = null)
     {
+        [$records, $others] = $this->decodeRecords();
+
         if (!is_null($ids)) {
             $records = array_diff_key(
-                $this->decodeRecords(),
+                $records,
                 array_flip($this->parseIds($ids))
             );
-
-            $records = $this->encodeRecords($records);
         } else {
             $records = [];
         }
 
-        $this->child->{$this->path} = $records;
+        $this->child->{$this->path} = $this->encodeRecords($records, $others);
 
         return $this->child;
     }
@@ -58,9 +58,11 @@ public function detach($ids = null)
      */
     public function sync($ids)
     {
+        [, $others] = $this->decodeRecords();
+
         $records = $this->formatIds($this->parseIds($ids));
 
-        $this->child->{$this->path} = $this->encodeRecords($records);
+        $this->child->{$this->path} = $this->encodeRecords($records, $others);
 
         return $this->child;
     }
@@ -73,16 +75,16 @@ public function sync($ids)
      */
     public function toggle($ids)
     {
-        $ids = $this->formatIds($this->parseIds($ids));
+        [$records, $others] = $this->decodeRecords();
 
-        $records = $this->decodeRecords();
+        $ids = $this->formatIds($this->parseIds($ids));
 
         $records = array_diff_key(
             $ids + $records,
             array_intersect_key($records, $ids)
         );
 
-        $this->child->{$this->path} = $this->encodeRecords($records);
+        $this->child->{$this->path} = $this->encodeRecords($records, $others);
 
         return $this->child;
     }
@@ -94,25 +96,38 @@ public function toggle($ids)
      */
     protected function decodeRecords()
     {
+        $records = [];
+        $others = [];
+
         $key = str_replace('->', '.', $this->key);
 
-        return collect((array) $this->child->{$this->path})
-            ->mapWithKeys(function ($record) use ($key) {
-                if (!is_array($record)) {
-                    return [$record => []];
-                }
+        foreach ((array) $this->child->{$this->path} as $record) {
+            if (!is_array($record)) {
+                $records[$record] = [];
 
-                return [Arr::get($record, $key) => $record];
-            })->all();
+                continue;
+            }
+
+            $foreignKey = Arr::get($record, $key);
+
+            if (!is_null($foreignKey)) {
+                $records[$foreignKey] = $record;
+            } else {
+                $others[] = $record;
+            }
+        }
+
+        return [$records, $others];
     }
 
     /**
      * Encode the records for the child model.
      *
      * @param array $records
+     * @param array $others
      * @return array
      */
-    protected function encodeRecords(array $records)
+    protected function encodeRecords(array $records, array $others)
     {
         if (!$this->key) {
             return array_keys($records);
@@ -124,7 +139,10 @@ protected function encodeRecords(array $records)
             Arr::set($attributes, $key, $id);
         }
 
-        return array_values($records);
+        return array_merge(
+            array_values($records),
+            $others
+        );
     }
 
     /**

diff --git a/tests/BelongsToJsonTest.php b/tests/BelongsToJsonTest.php
@@ -155,7 +155,14 @@ public function testAttach()
 
     public function testAttachWithObjects()
     {
-        $user = (new User)->roles2()->attach([
+        $user = (new User);
+        $user->options = [
+            'roles' => [
+                ['foo' => 'bar'],
+            ],
+        ];
+
+        $user->roles2()->attach([
             1 => ['role' => ['active' => true]],
             2 => ['role' => ['active' => false]],
         ]);
@@ -171,6 +178,7 @@ public function testAttachWithObjects()
         $roles = $user->load('roles2')->roles2->sortBy('id')->values();
         $this->assertEquals([1, 2, 3], $roles->pluck('id')->all());
         $this->assertEquals([true, true, false], $roles->pluck('pivot.role.active')->all());
+        $this->assertEquals(['foo' => 'bar'], $user->options['roles'][3]);
     }
 
     public function testAttachWithObjectsInColumn()
@@ -198,10 +206,12 @@ public function testDetachWithObjects()
 
         $this->assertEquals([1], $user->roles2->pluck('id')->all());
         $this->assertEquals([true], $user->roles2->pluck('pivot.role.active')->all());
+        $this->assertEquals(['foo' => 'bar'], $user->options['roles'][1]);
 
         $user->roles2()->detach();
 
         $this->assertEquals([], $user->roles2()->pluck('id')->all());
+        $this->assertEquals(['foo' => 'bar'], $user->options['roles'][0]);
     }
 
     public function testSync()
@@ -220,6 +230,7 @@ public function testSyncWithObjects()
 
         $this->assertEquals([2, 3], $user->roles2->pluck('id')->all());
         $this->assertEquals([true, false], $user->roles2->pluck('pivot.role.active')->all());
+        $this->assertEquals(['foo' => 'bar'], $user->options['roles'][2]);
     }
 
     public function testToggle()
@@ -238,21 +249,13 @@ public function testToggleWithObjects()
 
         $this->assertEquals([1, 3], $user->roles2->pluck('id')->all());
         $this->assertEquals([true, false], $user->roles2->pluck('pivot.role.active')->all());
+        $this->assertEquals(['foo' => 'bar'], $user->options['roles'][2]);
     }
 
-    public function testForeignKeysDoNotIncludeNullValues()
+    public function testForeignKeys()
     {
-        $keys = User::first()->postsOnly()->getForeignKeys();
+        $keys = User::first()->roles()->getForeignKeys();
 
         $this->assertEquals([1, 2], $keys);
     }
-
-    public function testForeignKeysDoNotIncludeNullValuesWhenEagerLoading()
-    {
-        DB::enableQueryLog();
-
-        User::with('postsOnly')->get();
-
-        $this->assertStringEndsWith('(1, 2)', DB::getQueryLog()[1]['query']);
-    }
 }
diff --git a/tests/Models/User.php b/tests/Models/User.php
@@ -28,11 +28,6 @@ public function roles3()
         return $this->belongsToJson(Role::class, 'options[]->role_id');
     }
 
-    public function postsOnly()
-    {
-        return $this->belongsToJson(Post::class, 'options->posts_and_comments[]->post->id');
-    }
-
     public function teamMate()
     {
         return $this->hasOneThrough(self::class, Team::class, 'options->owner_id', 'options->team_id');

diff --git a/tests/TestCase.php b/tests/TestCase.php
@@ -107,13 +107,7 @@ protected function seed()
                 'roles' => [
                     ['role' => ['id' => 1, 'active' => true]],
                     ['role' => ['id' => 2, 'active' => false]],
-                ],
-                'posts_and_comments' => [
-                    ['post' => ['id' => 1, 'active' => true]],
-                    ['post' => ['id' => 2, 'active' => true]],
-                    ['comment' => ['id' => 3, 'active' => false]],
-                    ['comment' => ['id' => 4, 'active' => false]],
-                    ['comment' => ['id' => 5, 'active' => false]],
+                    ['foo' => 'bar'],
                 ],
             ],
         ]);