[Fix] BelongsToMany sync does't use configured keys (#2667)

* Merge testSyncBelongsToMany into testBelongsToManySync;
* modelKeys replaced with parseIds in sync method;
* Add test for handling a collection in sync method;

Author: hans-thomas

src/Relations/BelongsToMany.php

@@ -76,7 +76,7 @@ protected function setWhere()
     {
         $foreign = $this->getForeignKey();
 
-        $this->query->where($foreign, '=', $this->parent->getKey());
+        $this->query->where($foreign, '=', $this->parent->{$this->parentKey});
 
         return $this;
     }
@@ -116,7 +116,7 @@ public function sync($ids, $detaching = true)
         ];
 
         if ($ids instanceof Collection) {
-            $ids = $ids->modelKeys();
+            $ids = $this->parseIds($ids);
         } elseif ($ids instanceof Model) {
             $ids = $this->parseIds($ids);
         }
@@ -190,10 +190,10 @@ public function attach($id, array $attributes = [], $touch = true)
 
             $query = $this->newRelatedQuery();
 
-            $query->whereIn($this->related->getKeyName(), (array) $id);
+            $query->whereIn($this->relatedKey, (array) $id);
 
             // Attach the new parent id to the related model.
-            $query->push($this->foreignPivotKey, $this->parent->getKey(), true);
+            $query->push($this->foreignPivotKey, $this->parent->{$this->parentKey}, true);
         }
 
         // Attach the new ids to the parent model.

tests/Models/Experience.php

@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace MongoDB\Laravel\Tests\Models;
+
+use MongoDB\Laravel\Eloquent\Model as Eloquent;
+
+class Experience extends Eloquent
+{
+    protected $connection       = 'mongodb';
+    protected $collection       = 'experiences';
+    protected static $unguarded = true;
+
+    protected $casts = ['years' => 'int'];
+
+    public function skillsWithCustomRelatedKey()
+    {
+        return $this->belongsToMany(Skill::class, relatedKey: 'cskill_id');
+    }
+
+    public function skillsWithCustomParentKey()
+    {
+        return $this->belongsToMany(Skill::class, parentKey: 'cexperience_id');
+    }
+}

tests/Models/Skill.php

@@ -0,0 +1,14 @@
+<?php
+
+declare(strict_types=1);
+
+namespace MongoDB\Laravel\Tests\Models;
+
+use MongoDB\Laravel\Eloquent\Model as Eloquent;
+
+class Skill extends Eloquent
+{
+    protected $connection       = 'mongodb';
+    protected $collection       = 'skills';
+    protected static $unguarded = true;
+}

tests/RelationsTest.php

@@ -6,13 +6,16 @@
 
 use Illuminate\Database\Eloquent\Collection;
 use Mockery;
+use MongoDB\BSON\ObjectId;
 use MongoDB\Laravel\Tests\Models\Address;
 use MongoDB\Laravel\Tests\Models\Book;
 use MongoDB\Laravel\Tests\Models\Client;
+use MongoDB\Laravel\Tests\Models\Experience;
 use MongoDB\Laravel\Tests\Models\Group;
 use MongoDB\Laravel\Tests\Models\Item;
 use MongoDB\Laravel\Tests\Models\Photo;
 use MongoDB\Laravel\Tests\Models\Role;
+use MongoDB\Laravel\Tests\Models\Skill;
 use MongoDB\Laravel\Tests\Models\Soft;
 use MongoDB\Laravel\Tests\Models\User;
 
@@ -30,6 +33,8 @@ public function tearDown(): void
         Role::truncate();
         Group::truncate();
         Photo::truncate();
+        Skill::truncate();
+        Experience::truncate();
     }
 
     public function testHasMany(): void
@@ -255,36 +260,6 @@ public function testBelongsToMany(): void
         $this->assertCount(1, $client->users);
     }
 
-    public function testSyncBelongsToMany()
-    {
-        $user = User::create(['name' => 'John Doe']);
-
-        $first  = Client::query()->create(['name' => 'Hans']);
-        $second = Client::query()->create(['name' => 'Thomas']);
-
-        $user->load('clients');
-        self::assertEmpty($user->clients);
-
-        $user->clients()->sync($first);
-
-        $user->load('clients');
-        self::assertCount(1, $user->clients);
-        self::assertTrue($user->clients->first()->is($first));
-
-        $user->clients()->sync($second);
-
-        $user->load('clients');
-        self::assertCount(1, $user->clients);
-        self::assertTrue($user->clients->first()->is($second));
-
-        $user->clients()->syncWithoutDetaching($first);
-
-        $user->load('clients');
-        self::assertCount(2, $user->clients);
-        self::assertTrue($user->clients->first()->is($first));
-        self::assertTrue($user->clients->last()->is($second));
-    }
-
     public function testBelongsToManyAttachesExistingModels(): void
     {
         $user = User::create(['name' => 'John Doe', 'client_ids' => ['1234523']]);
@@ -327,20 +302,27 @@ public function testBelongsToManyAttachesExistingModels(): void
     public function testBelongsToManySync(): void
     {
         // create test instances
-        $user    = User::create(['name' => 'John Doe']);
-        $client1 = Client::create(['name' => 'Pork Pies Ltd.'])->_id;
-        $client2 = Client::create(['name' => 'Buffet Bar Inc.'])->_id;
+        $user    = User::create(['name' => 'Hans Thomas']);
+        $client1 = Client::create(['name' => 'Pork Pies Ltd.']);
+        $client2 = Client::create(['name' => 'Buffet Bar Inc.']);
 
         // Sync multiple
-        $user->clients()->sync([$client1, $client2]);
+        $user->clients()->sync([$client1->_id, $client2->_id]);
         $this->assertCount(2, $user->clients);
 
-        // Refresh user
-        $user = User::where('name', '=', 'John Doe')->first();
+        // Sync single wrapped by an array
+        $user->clients()->sync([$client1->_id]);
+        $user->load('clients');
+
+        $this->assertCount(1, $user->clients);
+        self::assertTrue($user->clients->first()->is($client1));
+
+        // Sync single model
+        $user->clients()->sync($client2);
+        $user->load('clients');
 
-        // Sync single
-        $user->clients()->sync([$client1]);
         $this->assertCount(1, $user->clients);
+        self::assertTrue($user->clients->first()->is($client2));
     }
 
     public function testBelongsToManyAttachArray(): void
@@ -366,6 +348,50 @@ public function testBelongsToManyAttachEloquentCollection(): void
         $this->assertCount(2, $user->clients);
     }
 
+    public function testBelongsToManySyncEloquentCollectionWithCustomRelatedKey(): void
+    {
+        $experience = Experience::create(['years' => '5']);
+        $skill1    = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'PHP']);
+        $skill2    = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'Laravel']);
+        $collection = new Collection([$skill1, $skill2]);
+
+        $experience = Experience::query()->find($experience->id);
+        $experience->skillsWithCustomRelatedKey()->sync($collection);
+        $this->assertCount(2, $experience->skillsWithCustomRelatedKey);
+
+        self::assertIsString($skill1->cskill_id);
+        self::assertContains($skill1->cskill_id, $experience->skillsWithCustomRelatedKey->pluck('cskill_id'));
+
+        self::assertIsString($skill2->cskill_id);
+        self::assertContains($skill2->cskill_id, $experience->skillsWithCustomRelatedKey->pluck('cskill_id'));
+
+        $skill1->refresh();
+        self::assertIsString($skill1->_id);
+        self::assertNotContains($skill1->_id, $experience->skillsWithCustomRelatedKey->pluck('cskill_id'));
+
+        $skill2->refresh();
+        self::assertIsString($skill2->_id);
+        self::assertNotContains($skill2->_id, $experience->skillsWithCustomRelatedKey->pluck('cskill_id'));
+    }
+
+    public function testBelongsToManySyncEloquentCollectionWithCustomParentKey(): void
+    {
+        $experience = Experience::create(['cexperience_id' => (string) (new ObjectId()), 'years' => '5']);
+        $skill1    = Skill::create(['name' => 'PHP']);
+        $skill2    = Skill::create(['name' => 'Laravel']);
+        $collection = new Collection([$skill1, $skill2]);
+
+        $experience = Experience::query()->find($experience->id);
+        $experience->skillsWithCustomParentKey()->sync($collection);
+        $this->assertCount(2, $experience->skillsWithCustomParentKey);
+
+        self::assertIsString($skill1->_id);
+        self::assertContains($skill1->_id, $experience->skillsWithCustomParentKey->pluck('_id'));
+
+        self::assertIsString($skill2->_id);
+        self::assertContains($skill2->_id, $experience->skillsWithCustomParentKey->pluck('_id'));
+    }
+
     public function testBelongsToManySyncAlreadyPresent(): void
     {
         $user    = User::create(['name' => 'John Doe']);