Skip to content

Fix BelongsToMany relationship bugs when using custom keys #2695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Dec 13, 2023
Merged

Fix BelongsToMany relationship bugs when using custom keys #2695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3626c36
Update pluralizing logic in HybridRelations;
hans-thomas Dec 12, 2023
9c9e6d8
Update BelongsToMany.php
hans-thomas Dec 12, 2023
826d945
Add tests for sync, attach and detach actions with custom keys;
hans-thomas Dec 12, 2023
e54b822
apply phpcbf formatting
hans-thomas Dec 12, 2023
3e6913b
Merge branch '4.1' into belongs-to-many-relation-with-custom-keys
hans-thomas Dec 12, 2023
cfe4056
WIP
hans-thomas Dec 12, 2023
4f1eecb
Revert "Update pluralizing logic in HybridRelations;"
hans-thomas Dec 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/Relations/BelongsToMany.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down
11 changes: 11 additions & 0 deletions tests/Models/Client.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
10 changes: 0 additions & 10 deletions tests/Models/Experience.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
228 changes: 202 additions & 26 deletions tests/RelationsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -36,7 +35,6 @@ public function tearDown(): void
Photo::truncate();
Label::truncate();
Skill::truncate();
Experience::truncate();
}

public function testHasMany(): void
Expand Down Expand Up @@ -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
Expand Down