From 3626c368d9616b7778cc99268d13ecd9248d0c82 Mon Sep 17 00:00:00 2001 From: Mohammad Mortazavi Date: Tue, 12 Dec 2023 11:11:26 +0330 Subject: [PATCH 1/6] Update pluralizing logic in HybridRelations; --- src/Eloquent/HybridRelations.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Eloquent/HybridRelations.php b/src/Eloquent/HybridRelations.php index 9551a6c43..1ee54124e 100644 --- a/src/Eloquent/HybridRelations.php +++ b/src/Eloquent/HybridRelations.php @@ -303,11 +303,11 @@ public function belongsToMany( // First, we'll need to determine the foreign key and "other key" for the // relationship. Once we have determined the keys we'll make the query // instances as well as the relationship instances we need for this. - $foreignPivotKey = $foreignPivotKey ?: $this->getForeignKey() . 's'; + $foreignPivotKey = $foreignPivotKey ?: Str::plural($this->getForeignKey()); $instance = new $related(); - $relatedPivotKey = $relatedPivotKey ?: $instance->getForeignKey() . 's'; + $relatedPivotKey = $relatedPivotKey ?: Str::plural($instance->getForeignKey()); // If no table name was provided, we can guess it by concatenating the two // models using underscores in alphabetical order. The two model names From 9c9e6d828af300e6593fd17d42bb0e9f5e89b33b Mon Sep 17 00:00:00 2001 From: Mohammad Mortazavi Date: Tue, 12 Dec 2023 11:12:45 +0330 Subject: [PATCH 2/6] Update BelongsToMany.php --- src/Relations/BelongsToMany.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Relations/BelongsToMany.php b/src/Relations/BelongsToMany.php index 082f95e06..8ff311f3f 100644 --- a/src/Relations/BelongsToMany.php +++ b/src/Relations/BelongsToMany.php @@ -183,13 +183,13 @@ public function attach($id, array $attributes = [], $touch = true) if ($id instanceof Model) { $model = $id; - $id = $model->getKey(); + $id = $this->parseId($model); // Attach the new parent id to the related model. - $model->push($this->foreignPivotKey, $this->parent->getKey(), true); + $model->push($this->foreignPivotKey, $this->parent->{$this->parentKey}, true); } else { if ($id instanceof Collection) { - $id = $id->modelKeys(); + $id = $this->parseIds($id); } $query = $this->newRelatedQuery(); @@ -221,7 +221,7 @@ public function attach($id, array $attributes = [], $touch = true) public function detach($ids = [], $touch = true) { if ($ids instanceof Model) { - $ids = (array) $ids->getKey(); + $ids = $this->parseIds($ids); } $query = $this->newRelatedQuery(); @@ -242,13 +242,13 @@ public function detach($ids = [], $touch = true) // Prepare the query to select all related objects. if (count($ids) > 0) { - $query->whereIn($this->related->getKeyName(), $ids); + $query->whereIn($this->relatedKey, $ids); } // Remove the relation to the parent. assert($this->parent instanceof Model); assert($query instanceof \MongoDB\Laravel\Eloquent\Builder); - $query->pull($this->foreignPivotKey, $this->parent->getKey()); + $query->pull($this->foreignPivotKey, $this->parent->{$this->parentKey}); if ($touch) { $this->touchIfTouching(); From 826d9454c9eeffcdf96d52693242c6fe25f72a4f Mon Sep 17 00:00:00 2001 From: Mohammad Mortazavi Date: Tue, 12 Dec 2023 11:13:41 +0330 Subject: [PATCH 3/6] Add tests for sync, attach and detach actions with custom keys; --- tests/Models/Client.php | 11 ++ tests/Models/Experience.php | 26 ---- tests/RelationsTest.php | 228 ++++++++++++++++++++++++++++++++---- 3 files changed, 213 insertions(+), 52 deletions(-) delete mode 100644 tests/Models/Experience.php diff --git a/tests/Models/Client.php b/tests/Models/Client.php index 2ab4f5e33..36dd202ad 100644 --- a/tests/Models/Client.php +++ b/tests/Models/Client.php @@ -20,6 +20,17 @@ public function users(): BelongsToMany return $this->belongsToMany(User::class); } + public function skillsWithCustomKeys() + { + return $this->belongsToMany( + Skill::class, + foreignPivotKey: 'cclient_ids', + relatedPivotKey: 'cskill_ids', + parentKey: 'cclient_id', + relatedKey: 'cskill_id' + ); + } + public function photo(): MorphOne { return $this->morphOne(Photo::class, 'has_image'); diff --git a/tests/Models/Experience.php b/tests/Models/Experience.php deleted file mode 100644 index 617073c79..000000000 --- a/tests/Models/Experience.php +++ /dev/null @@ -1,26 +0,0 @@ - 'int']; - - public function skillsWithCustomRelatedKey() - { - 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 652f3d7bf..8c0a7a4a7 100644 --- a/tests/RelationsTest.php +++ b/tests/RelationsTest.php @@ -10,7 +10,6 @@ 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\Label; @@ -36,7 +35,6 @@ public function tearDown(): void Photo::truncate(); Label::truncate(); Skill::truncate(); - Experience::truncate(); } public function testHasMany(): void @@ -350,48 +348,226 @@ public function testBelongsToManyAttachEloquentCollection(): void $this->assertCount(2, $user->clients); } - public function testBelongsToManySyncEloquentCollectionWithCustomRelatedKey(): void + public function testBelongsToManySyncWithCustomKeys(): void { - $experience = Experience::create(['years' => '5']); + $client = Client::create(['cclient_id' => (string) (new ObjectId()), 'years' => '5']); + $skill1 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'PHP']); + $skill2 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'Laravel']); + + $client = Client::query()->find($client->_id); + $client->skillsWithCustomKeys()->sync([$skill1->cskill_id, $skill2->cskill_id]); + $this->assertCount(2, $client->skillsWithCustomKeys); + + self::assertIsString($skill1->cskill_id); + self::assertContains($skill1->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($skill1->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + + self::assertIsString($skill2->cskill_id); + self::assertContains($skill2->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($skill2->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + + $check = Skill::query()->find($skill1->_id); + self::assertIsString($check->cskill_id); + self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + + $check = Skill::query()->find($skill2->_id); + self::assertIsString($check->cskill_id); + self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + } + + public function testBelongsToManySyncModelWithCustomKeys(): void + { + $client = Client::create(['cclient_id' => (string) (new ObjectId()), 'years' => '5']); + $skill1 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'PHP']); + + $client = Client::query()->find($client->_id); + $client->skillsWithCustomKeys()->sync($skill1); + $this->assertCount(1, $client->skillsWithCustomKeys); + + self::assertIsString($skill1->cskill_id); + self::assertContains($skill1->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($skill1->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + + $check = Skill::query()->find($skill1->_id); + self::assertIsString($check->_id); + self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + } + + public function testBelongsToManySyncEloquentCollectionWithCustomKeys(): void + { + $client = Client::create(['cclient_id' => (string) (new ObjectId()), '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); + $client = Client::query()->find($client->_id); + $client->skillsWithCustomKeys()->sync($collection); + $this->assertCount(2, $client->skillsWithCustomKeys); self::assertIsString($skill1->cskill_id); - self::assertContains($skill1->cskill_id, $experience->skillsWithCustomRelatedKey->pluck('cskill_id')); + self::assertContains($skill1->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($skill1->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); self::assertIsString($skill2->cskill_id); - self::assertContains($skill2->cskill_id, $experience->skillsWithCustomRelatedKey->pluck('cskill_id')); + self::assertContains($skill2->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($skill2->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + + $check = Skill::query()->find($skill1->_id); + self::assertIsString($check->cskill_id); + self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + + $check = Skill::query()->find($skill2->_id); + self::assertIsString($check->cskill_id); + self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + } + + public function testBelongsToManyAttachWithCustomKeys(): void + { + $client = Client::create(['cclient_id' => (string) (new ObjectId()), 'years' => '5']); + $skill1 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'PHP']); + $skill2 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'Laravel']); + + $client = Client::query()->find($client->_id); + $client->skillsWithCustomKeys()->attach([$skill1->cskill_id, $skill2->cskill_id]); + $this->assertCount(2, $client->skillsWithCustomKeys); + + self::assertIsString($skill1->cskill_id); + self::assertContains($skill1->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($skill1->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + + self::assertIsString($skill2->cskill_id); + self::assertContains($skill2->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($skill2->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + + $check = Skill::query()->find($skill1->_id); + self::assertIsString($check->cskill_id); + self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + + $check = Skill::query()->find($skill2->_id); + self::assertIsString($check->cskill_id); + self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + } + + public function testBelongsToManyAttachModelWithCustomKeys(): void + { + $client = Client::create(['cclient_id' => (string) (new ObjectId()), 'years' => '5']); + $skill1 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'PHP']); - $skill1->refresh(); - self::assertIsString($skill1->_id); - self::assertNotContains($skill1->_id, $experience->skillsWithCustomRelatedKey->pluck('cskill_id')); + $client = Client::query()->find($client->_id); + $client->skillsWithCustomKeys()->attach($skill1); + $this->assertCount(1, $client->skillsWithCustomKeys); + + self::assertIsString($skill1->cskill_id); + self::assertContains($skill1->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($skill1->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); - $skill2->refresh(); - self::assertIsString($skill2->_id); - self::assertNotContains($skill2->_id, $experience->skillsWithCustomRelatedKey->pluck('cskill_id')); + $check = Skill::query()->find($skill1->_id); + self::assertIsString($check->_id); + self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); } - public function testBelongsToManySyncEloquentCollectionWithCustomParentKey(): void + public function testBelongsToManyAttachEloquentCollectionWithCustomKeys(): void { - $experience = Experience::create(['cexperience_id' => (string) (new ObjectId()), 'years' => '5']); - $skill1 = Skill::create(['name' => 'PHP']); - $skill2 = Skill::create(['name' => 'Laravel']); + $client = Client::create(['cclient_id' => (string) (new ObjectId()), '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->skillsWithCustomParentKey()->sync($collection); - $this->assertCount(2, $experience->skillsWithCustomParentKey); + $client = Client::query()->find($client->_id); + $client->skillsWithCustomKeys()->attach($collection); + $this->assertCount(2, $client->skillsWithCustomKeys); + + self::assertIsString($skill1->cskill_id); + self::assertContains($skill1->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($skill1->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + + self::assertIsString($skill2->cskill_id); + self::assertContains($skill2->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($skill2->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + + $check = Skill::query()->find($skill1->_id); + self::assertIsString($check->cskill_id); + self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + + $check = Skill::query()->find($skill2->_id); + self::assertIsString($check->cskill_id); + self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + } + + public function testBelongsToManyDetachWithCustomKeys(): void + { + $client = Client::create(['cclient_id' => (string) (new ObjectId()), 'years' => '5']); + $skill1 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'PHP']); + $skill2 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'Laravel']); + + $client = Client::query()->find($client->_id); + $client->skillsWithCustomKeys()->sync([$skill1->cskill_id, $skill2->cskill_id]); + $this->assertCount(2, $client->skillsWithCustomKeys); + + $client->skillsWithCustomKeys()->detach($skill1->cskill_id); + $client->load('skillsWithCustomKeys'); // Reload the relationship based on the latest pivot column's data + $this->assertCount(1, $client->skillsWithCustomKeys); + + self::assertIsString($skill1->cskill_id); + self::assertNotContains($skill1->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($skill1->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); - self::assertIsString($skill1->_id); - self::assertContains($skill1->_id, $experience->skillsWithCustomParentKey->pluck('_id')); + self::assertIsString($skill2->cskill_id); + self::assertContains($skill2->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($skill2->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + + $check = Skill::query()->find($skill1->_id); + self::assertIsString($check->cskill_id); + self::assertNotContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + + $check = Skill::query()->find($skill2->_id); + self::assertIsString($check->cskill_id); + self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + } - self::assertIsString($skill2->_id); - self::assertContains($skill2->_id, $experience->skillsWithCustomParentKey->pluck('_id')); + public function testBelongsToManyDetachModelWithCustomKeys(): void + { + $client = Client::create(['cclient_id' => (string) (new ObjectId()), 'years' => '5']); + $skill1 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'PHP']); + $skill2 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'Laravel']); + + $client = Client::query()->find($client->_id); + $client->skillsWithCustomKeys()->sync([$skill1->cskill_id, $skill2->cskill_id]); + $this->assertCount(2, $client->skillsWithCustomKeys); + + $client->skillsWithCustomKeys()->detach($skill1); + $client->load('skillsWithCustomKeys'); // Reload the relationship based on the latest pivot column's data + $this->assertCount(1, $client->skillsWithCustomKeys); + + self::assertIsString($skill1->cskill_id); + self::assertNotContains($skill1->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($skill1->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + + self::assertIsString($skill2->cskill_id); + self::assertContains($skill2->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($skill2->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + + $check = Skill::query()->find($skill1->_id); + self::assertIsString($check->cskill_id); + self::assertNotContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + + $check = Skill::query()->find($skill2->_id); + self::assertIsString($check->cskill_id); + self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id')); + self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id')); } public function testBelongsToManySyncAlreadyPresent(): void From e54b822c3d26d56ffdf90441f4a701a9d6476b5f Mon Sep 17 00:00:00 2001 From: hans-thomas Date: Tue, 12 Dec 2023 07:44:24 +0000 Subject: [PATCH 4/6] apply phpcbf formatting --- tests/Models/Client.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Models/Client.php b/tests/Models/Client.php index 36dd202ad..4e7e7ecc9 100644 --- a/tests/Models/Client.php +++ b/tests/Models/Client.php @@ -27,7 +27,7 @@ public function skillsWithCustomKeys() foreignPivotKey: 'cclient_ids', relatedPivotKey: 'cskill_ids', parentKey: 'cclient_id', - relatedKey: 'cskill_id' + relatedKey: 'cskill_id', ); } From cfe4056c200d549ffc5d67df6a356d38ca75fd2b Mon Sep 17 00:00:00 2001 From: Mohammad Mortazavi Date: Tue, 12 Dec 2023 14:55:50 +0330 Subject: [PATCH 5/6] WIP --- tests/Models/Experience.php | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/Models/Experience.php b/tests/Models/Experience.php index 4c2869d9e..2852ece5f 100644 --- a/tests/Models/Experience.php +++ b/tests/Models/Experience.php @@ -15,16 +15,6 @@ class Experience extends Eloquent 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'); - } - public function sqlUsers(): MorphToMany { return $this->morphToMany(SqlUser::class, 'experienced'); From 4f1eecb6837d85dfb34c6dacbd80a6aa4204b65c Mon Sep 17 00:00:00 2001 From: Mohammad Mortazavi Date: Tue, 12 Dec 2023 17:51:05 +0330 Subject: [PATCH 6/6] Revert "Update pluralizing logic in HybridRelations;" This reverts commit 3626c368d9616b7778cc99268d13ecd9248d0c82. --- src/Eloquent/HybridRelations.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Eloquent/HybridRelations.php b/src/Eloquent/HybridRelations.php index 1c95c8501..5c058f50f 100644 --- a/src/Eloquent/HybridRelations.php +++ b/src/Eloquent/HybridRelations.php @@ -303,11 +303,11 @@ public function belongsToMany( // First, we'll need to determine the foreign key and "other key" for the // relationship. Once we have determined the keys we'll make the query // instances as well as the relationship instances we need for this. - $foreignPivotKey = $foreignPivotKey ?: Str::plural($this->getForeignKey()); + $foreignPivotKey = $foreignPivotKey ?: $this->getForeignKey() . 's'; $instance = new $related(); - $relatedPivotKey = $relatedPivotKey ?: Str::plural($instance->getForeignKey()); + $relatedPivotKey = $relatedPivotKey ?: $instance->getForeignKey() . 's'; // If no table name was provided, we can guess it by concatenating the two // models using underscores in alphabetical order. The two model names