From 98fa7d1811f140c40d7c9554bbf846ebd6dc862c Mon Sep 17 00:00:00 2001 From: Mohammad Mortazavi Date: Sat, 4 Nov 2023 17:00:49 +0330 Subject: [PATCH 01/11] Merge testSyncBelongsToMany into testBelongsToManySync; --- tests/RelationsTest.php | 53 ++++++++++++----------------------------- 1 file changed, 15 insertions(+), 38 deletions(-) diff --git a/tests/RelationsTest.php b/tests/RelationsTest.php index b087ca481..16f491aa9 100644 --- a/tests/RelationsTest.php +++ b/tests/RelationsTest.php @@ -255,36 +255,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 +297,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 From 6014ae59830a43a30499a09101521a01b483d4de Mon Sep 17 00:00:00 2001 From: Mohammad Mortazavi Date: Sat, 4 Nov 2023 21:35:40 +0330 Subject: [PATCH 02/11] modelKeys replaced with parseIds in sync method; --- src/Relations/BelongsToMany.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Relations/BelongsToMany.php b/src/Relations/BelongsToMany.php index 08e7ac984..7f57bd2b1 100644 --- a/src/Relations/BelongsToMany.php +++ b/src/Relations/BelongsToMany.php @@ -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); } From 538ed8967f386f864e4b021c432712805e85ad84 Mon Sep 17 00:00:00 2001 From: Mohammad Mortazavi Date: Sat, 4 Nov 2023 21:36:39 +0330 Subject: [PATCH 03/11] Add test for handling a collection in sync method; --- tests/Models/Skill.php | 34 ++++++++++++++++++++++++++++++++++ tests/Models/User.php | 5 +++++ tests/RelationsTest.php | 29 +++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+) create mode 100644 tests/Models/Skill.php diff --git a/tests/Models/Skill.php b/tests/Models/Skill.php new file mode 100644 index 000000000..fb20749bf --- /dev/null +++ b/tests/Models/Skill.php @@ -0,0 +1,34 @@ + $model->custom_id = (string)(new ObjectId())); + } + + public function users(): BelongsToMany + { + return $this->belongsToMany(User::class); + } +} diff --git a/tests/Models/User.php b/tests/Models/User.php index 4e0d7294c..80060fe39 100644 --- a/tests/Models/User.php +++ b/tests/Models/User.php @@ -86,6 +86,11 @@ public function clients() return $this->belongsToMany(Client::class); } + public function skills() + { + return $this->belongsToMany(Skill::class); + } + public function groups() { return $this->belongsToMany(Group::class, 'groups', 'users', 'groups', '_id', '_id', 'groups'); diff --git a/tests/RelationsTest.php b/tests/RelationsTest.php index 16f491aa9..a447d5ae7 100644 --- a/tests/RelationsTest.php +++ b/tests/RelationsTest.php @@ -6,6 +6,7 @@ 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; @@ -13,6 +14,7 @@ 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 +32,7 @@ public function tearDown(): void Role::truncate(); Group::truncate(); Photo::truncate(); + Skill::truncate(); } public function testHasMany(): void @@ -343,6 +346,32 @@ public function testBelongsToManyAttachEloquentCollection(): void $this->assertCount(2, $user->clients); } + public function testBelongsToManySyncEloquentCollection(): void + { + $user = User::create(['name' => 'Hans Thomas']); + $skill1 = Skill::create(['name' => 'PHP 1']); + $skill2 = Skill::create(['name' => 'Laravel 2']); + $collection = new Collection([$skill1, $skill2]); + + $user = User::query()->find($user->_id); + $user->skills()->sync($collection); + $this->assertCount(2, $user->skills); + + self::assertIsString($skill1->custom_id); + self::assertContains($skill1->custom_id,$user->custom_skill_ids); + + self::assertIsString($skill2->custom_id); + self::assertContains($skill2->custom_id,$user->custom_skill_ids); + + $skill1->refresh(); + self::assertIsString($skill1->_id); + self::assertNotContains($skill1->_id,$user->custom_skill_ids); + + $skill2->refresh(); + self::assertIsString($skill2->_id); + self::assertNotContains($skill2->_id,$user->custom_skill_ids); + } + public function testBelongsToManySyncAlreadyPresent(): void { $user = User::create(['name' => 'John Doe']); From 79ad8a32945a08491241271993c9985d677ccf70 Mon Sep 17 00:00:00 2001 From: Mohammad Mortazavi Date: Sat, 4 Nov 2023 21:46:23 +0330 Subject: [PATCH 04/11] Fix phpcs; --- tests/Models/Skill.php | 4 +--- tests/RelationsTest.php | 9 ++++----- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/tests/Models/Skill.php b/tests/Models/Skill.php index fb20749bf..1437780dc 100644 --- a/tests/Models/Skill.php +++ b/tests/Models/Skill.php @@ -5,8 +5,6 @@ namespace MongoDB\Laravel\Tests\Models; use Illuminate\Database\Eloquent\Relations\BelongsToMany; -use Illuminate\Database\Eloquent\Relations\HasMany; -use Illuminate\Database\Eloquent\Relations\MorphOne; use MongoDB\BSON\ObjectId; use MongoDB\Laravel\Eloquent\Model as Eloquent; @@ -24,7 +22,7 @@ public function getForeignKey() protected static function booted() { - static::creating(fn (self $model) => $model->custom_id = (string)(new ObjectId())); + static::creating(fn (self $model) => $model->custom_id = (string) (new ObjectId())); } public function users(): BelongsToMany diff --git a/tests/RelationsTest.php b/tests/RelationsTest.php index a447d5ae7..c038c9628 100644 --- a/tests/RelationsTest.php +++ b/tests/RelationsTest.php @@ -6,7 +6,6 @@ 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; @@ -358,18 +357,18 @@ public function testBelongsToManySyncEloquentCollection(): void $this->assertCount(2, $user->skills); self::assertIsString($skill1->custom_id); - self::assertContains($skill1->custom_id,$user->custom_skill_ids); + self::assertContains($skill1->custom_id, $user->custom_skill_ids); self::assertIsString($skill2->custom_id); - self::assertContains($skill2->custom_id,$user->custom_skill_ids); + self::assertContains($skill2->custom_id, $user->custom_skill_ids); $skill1->refresh(); self::assertIsString($skill1->_id); - self::assertNotContains($skill1->_id,$user->custom_skill_ids); + self::assertNotContains($skill1->_id, $user->custom_skill_ids); $skill2->refresh(); self::assertIsString($skill2->_id); - self::assertNotContains($skill2->_id,$user->custom_skill_ids); + self::assertNotContains($skill2->_id, $user->custom_skill_ids); } public function testBelongsToManySyncAlreadyPresent(): void From baa7b6ee7225780ab95f2981412f7d77d4640137 Mon Sep 17 00:00:00 2001 From: Mohammad Mortazavi Date: Sun, 5 Nov 2023 16:13:45 +0330 Subject: [PATCH 05/11] Use relatedKey instead of key; --- src/Relations/BelongsToMany.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Relations/BelongsToMany.php b/src/Relations/BelongsToMany.php index 7f57bd2b1..b8c3f5371 100644 --- a/src/Relations/BelongsToMany.php +++ b/src/Relations/BelongsToMany.php @@ -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. From 32f8d7719cb580eff0855fbe2e859e67a8085955 Mon Sep 17 00:00:00 2001 From: Mohammad Mortazavi Date: Sun, 5 Nov 2023 16:14:13 +0330 Subject: [PATCH 06/11] Remove skills relationship from User model; --- tests/Models/User.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/Models/User.php b/tests/Models/User.php index 80060fe39..4e0d7294c 100644 --- a/tests/Models/User.php +++ b/tests/Models/User.php @@ -86,11 +86,6 @@ public function clients() return $this->belongsToMany(Client::class); } - public function skills() - { - return $this->belongsToMany(Skill::class); - } - public function groups() { return $this->belongsToMany(Group::class, 'groups', 'users', 'groups', '_id', '_id', 'groups'); From a6e11aad4da1064683d00a60d5e2ad9f83945cf6 Mon Sep 17 00:00:00 2001 From: Mohammad Mortazavi Date: Sun, 5 Nov 2023 16:15:35 +0330 Subject: [PATCH 07/11] Test updated; --- tests/Models/Experience.php | 21 +++++++++++++++++++++ tests/Models/Skill.php | 18 ------------------ tests/RelationsTest.php | 29 ++++++++++++++++------------- 3 files changed, 37 insertions(+), 31 deletions(-) create mode 100644 tests/Models/Experience.php diff --git a/tests/Models/Experience.php b/tests/Models/Experience.php new file mode 100644 index 000000000..936f4b46a --- /dev/null +++ b/tests/Models/Experience.php @@ -0,0 +1,21 @@ + 'int']; + + public function skills() + { + return $this->belongsToMany(Skill::class, relatedKey: 'cskill_id'); + } +} diff --git a/tests/Models/Skill.php b/tests/Models/Skill.php index 1437780dc..c4c1dbd0a 100644 --- a/tests/Models/Skill.php +++ b/tests/Models/Skill.php @@ -4,8 +4,6 @@ namespace MongoDB\Laravel\Tests\Models; -use Illuminate\Database\Eloquent\Relations\BelongsToMany; -use MongoDB\BSON\ObjectId; use MongoDB\Laravel\Eloquent\Model as Eloquent; class Skill extends Eloquent @@ -13,20 +11,4 @@ class Skill extends Eloquent protected $connection = 'mongodb'; protected $collection = 'skills'; protected static $unguarded = true; - protected $primaryKey = 'custom_id'; - - public function getForeignKey() - { - return 'custom_skill_id'; - } - - protected static function booted() - { - static::creating(fn (self $model) => $model->custom_id = (string) (new ObjectId())); - } - - public function users(): BelongsToMany - { - return $this->belongsToMany(User::class); - } } diff --git a/tests/RelationsTest.php b/tests/RelationsTest.php index c038c9628..fa61e9c86 100644 --- a/tests/RelationsTest.php +++ b/tests/RelationsTest.php @@ -6,9 +6,11 @@ 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; @@ -32,6 +34,7 @@ public function tearDown(): void Group::truncate(); Photo::truncate(); Skill::truncate(); + Experience::truncate(); } public function testHasMany(): void @@ -345,30 +348,30 @@ public function testBelongsToManyAttachEloquentCollection(): void $this->assertCount(2, $user->clients); } - public function testBelongsToManySyncEloquentCollection(): void + public function testBelongsToManySyncEloquentCollectionWithCustomRelatedKey(): void { - $user = User::create(['name' => 'Hans Thomas']); - $skill1 = Skill::create(['name' => 'PHP 1']); - $skill2 = Skill::create(['name' => 'Laravel 2']); + $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]); - $user = User::query()->find($user->_id); - $user->skills()->sync($collection); - $this->assertCount(2, $user->skills); + $experience = Experience::query()->find($experience->id); + $experience->skills()->sync($collection); + $this->assertCount(2, $experience->skills); - self::assertIsString($skill1->custom_id); - self::assertContains($skill1->custom_id, $user->custom_skill_ids); + self::assertIsString($skill1->cskill_id); + self::assertContains($skill1->cskill_id, $experience->skill_ids); - self::assertIsString($skill2->custom_id); - self::assertContains($skill2->custom_id, $user->custom_skill_ids); + self::assertIsString($skill2->cskill_id); + self::assertContains($skill2->cskill_id, $experience->skill_ids); $skill1->refresh(); self::assertIsString($skill1->_id); - self::assertNotContains($skill1->_id, $user->custom_skill_ids); + self::assertNotContains($skill1->_id, $experience->skill_ids); $skill2->refresh(); self::assertIsString($skill2->_id); - self::assertNotContains($skill2->_id, $user->custom_skill_ids); + self::assertNotContains($skill2->_id, $experience->skill_ids); } public function testBelongsToManySyncAlreadyPresent(): void From fe280095df1e7ed1b441d2b280f5cac8144c8815 Mon Sep 17 00:00:00 2001 From: Mohammad Mortazavi Date: Sun, 5 Nov 2023 16:37:00 +0330 Subject: [PATCH 08/11] Apply parentKey instead of getKey in setWhere method; --- src/Relations/BelongsToMany.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Relations/BelongsToMany.php b/src/Relations/BelongsToMany.php index b8c3f5371..a1b028c9f 100644 --- a/src/Relations/BelongsToMany.php +++ b/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; } From 4273d2079f1725c6eab26185a1ad3c85b20f7cf6 Mon Sep 17 00:00:00 2001 From: Mohammad Mortazavi Date: Sun, 5 Nov 2023 16:37:32 +0330 Subject: [PATCH 09/11] Add test for custom parentKey; --- tests/Models/Experience.php | 5 +++++ tests/RelationsTest.php | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/tests/Models/Experience.php b/tests/Models/Experience.php index 936f4b46a..0446751aa 100644 --- a/tests/Models/Experience.php +++ b/tests/Models/Experience.php @@ -18,4 +18,9 @@ public function skills() { return $this->belongsToMany(Skill::class, relatedKey: 'cskill_id'); } + + public function skillsWithCustomParentKey() + { + return $this->belongsToMany(Skill::class, parentKey: 'cexperience_id'); + } } diff --git a/tests/RelationsTest.php b/tests/RelationsTest.php index fa61e9c86..0a400ecb1 100644 --- a/tests/RelationsTest.php +++ b/tests/RelationsTest.php @@ -5,6 +5,7 @@ namespace MongoDB\Laravel\Tests; use Illuminate\Database\Eloquent\Collection; +use Illuminate\Support\Facades\DB; use Mockery; use MongoDB\BSON\ObjectId; use MongoDB\Laravel\Tests\Models\Address; @@ -374,6 +375,24 @@ public function testBelongsToManySyncEloquentCollectionWithCustomRelatedKey(): v self::assertNotContains($skill2->_id, $experience->skill_ids); } + 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']); From b875121270429a36b75787ea7748e200f138770d Mon Sep 17 00:00:00 2001 From: Mohammad Mortazavi Date: Sun, 5 Nov 2023 17:01:18 +0330 Subject: [PATCH 10/11] Fix phpcs; --- tests/RelationsTest.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/RelationsTest.php b/tests/RelationsTest.php index 0a400ecb1..fb43fd967 100644 --- a/tests/RelationsTest.php +++ b/tests/RelationsTest.php @@ -5,7 +5,6 @@ namespace MongoDB\Laravel\Tests; use Illuminate\Database\Eloquent\Collection; -use Illuminate\Support\Facades\DB; use Mockery; use MongoDB\BSON\ObjectId; use MongoDB\Laravel\Tests\Models\Address; @@ -352,8 +351,8 @@ public function testBelongsToManyAttachEloquentCollection(): void 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']); + $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); @@ -377,7 +376,7 @@ public function testBelongsToManySyncEloquentCollectionWithCustomRelatedKey(): v public function testBelongsToManySyncEloquentCollectionWithCustomParentKey(): void { - $experience = Experience::create(['cexperience_id' => (string) (new ObjectId()),'years' => '5']); + $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]); From 19f51c79d9d45973f295916e30d11baee84a0e11 Mon Sep 17 00:00:00 2001 From: Mohammad Mortazavi Date: Sun, 5 Nov 2023 17:27:43 +0330 Subject: [PATCH 11/11] Improve DX; Rename skills relationship name to skillsWithCustomRelatedKey. --- tests/Models/Experience.php | 2 +- tests/RelationsTest.php | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/Models/Experience.php b/tests/Models/Experience.php index 0446751aa..617073c79 100644 --- a/tests/Models/Experience.php +++ b/tests/Models/Experience.php @@ -14,7 +14,7 @@ class Experience extends Eloquent protected $casts = ['years' => 'int']; - public function skills() + public function skillsWithCustomRelatedKey() { return $this->belongsToMany(Skill::class, relatedKey: 'cskill_id'); } diff --git a/tests/RelationsTest.php b/tests/RelationsTest.php index fb43fd967..6b2e2539f 100644 --- a/tests/RelationsTest.php +++ b/tests/RelationsTest.php @@ -356,22 +356,22 @@ public function testBelongsToManySyncEloquentCollectionWithCustomRelatedKey(): v $collection = new Collection([$skill1, $skill2]); $experience = Experience::query()->find($experience->id); - $experience->skills()->sync($collection); - $this->assertCount(2, $experience->skills); + $experience->skillsWithCustomRelatedKey()->sync($collection); + $this->assertCount(2, $experience->skillsWithCustomRelatedKey); self::assertIsString($skill1->cskill_id); - self::assertContains($skill1->cskill_id, $experience->skill_ids); + self::assertContains($skill1->cskill_id, $experience->skillsWithCustomRelatedKey->pluck('cskill_id')); self::assertIsString($skill2->cskill_id); - self::assertContains($skill2->cskill_id, $experience->skill_ids); + self::assertContains($skill2->cskill_id, $experience->skillsWithCustomRelatedKey->pluck('cskill_id')); $skill1->refresh(); self::assertIsString($skill1->_id); - self::assertNotContains($skill1->_id, $experience->skill_ids); + self::assertNotContains($skill1->_id, $experience->skillsWithCustomRelatedKey->pluck('cskill_id')); $skill2->refresh(); self::assertIsString($skill2->_id); - self::assertNotContains($skill2->_id, $experience->skill_ids); + self::assertNotContains($skill2->_id, $experience->skillsWithCustomRelatedKey->pluck('cskill_id')); } public function testBelongsToManySyncEloquentCollectionWithCustomParentKey(): void