Skip to content

Commit 99e1028

Browse files
hans-thomashans-thomas
authored
Fix BelongsToMany relationship bugs when using custom keys (#2695)
1 parent f0eed1a commit 99e1028

File tree

4 files changed

+219
-42
lines changed

4 files changed

+219
-42
lines changed

‎src/Relations/BelongsToMany.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -183,13 +183,13 @@ public function attach($id, array $attributes = [], $touch = true)
183183
if ($id instanceof Model) {
184184
$model = $id;
185185

186-
$id = $model->getKey();
186+
$id = $this->parseId($model);
187187

188188
// Attach the new parent id to the related model.
189-
$model->push($this->foreignPivotKey, $this->parent->getKey(), true);
189+
$model->push($this->foreignPivotKey, $this->parent->{$this->parentKey}, true);
190190
} else {
191191
if ($id instanceof Collection) {
192-
$id = $id->modelKeys();
192+
$id = $this->parseIds($id);
193193
}
194194

195195
$query = $this->newRelatedQuery();
@@ -221,7 +221,7 @@ public function attach($id, array $attributes = [], $touch = true)
221221
public function detach($ids = [], $touch = true)
222222
{
223223
if ($ids instanceof Model) {
224-
$ids = (array) $ids->getKey();
224+
$ids = $this->parseIds($ids);
225225
}
226226

227227
$query = $this->newRelatedQuery();
@@ -242,13 +242,13 @@ public function detach($ids = [], $touch = true)
242242

243243
// Prepare the query to select all related objects.
244244
if (count($ids) > 0) {
245-
$query->whereIn($this->related->getKeyName(), $ids);
245+
$query->whereIn($this->relatedKey, $ids);
246246
}
247247

248248
// Remove the relation to the parent.
249249
assert($this->parent instanceof Model);
250250
assert($query instanceof \MongoDB\Laravel\Eloquent\Builder);
251-
$query->pull($this->foreignPivotKey, $this->parent->getKey());
251+
$query->pull($this->foreignPivotKey, $this->parent->{$this->parentKey});
252252

253253
if ($touch) {
254254
$this->touchIfTouching();

‎tests/Models/Client.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,17 @@ public function users(): BelongsToMany
2020
return $this->belongsToMany(User::class);
2121
}
2222

23+
public function skillsWithCustomKeys()
24+
{
25+
return $this->belongsToMany(
26+
Skill::class,
27+
foreignPivotKey: 'cclient_ids',
28+
relatedPivotKey: 'cskill_ids',
29+
parentKey: 'cclient_id',
30+
relatedKey: 'cskill_id',
31+
);
32+
}
33+
2334
public function photo(): MorphOne
2435
{
2536
return $this->morphOne(Photo::class, 'has_image');

‎tests/Models/Experience.php

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,6 @@ class Experience extends Eloquent
1515

1616
protected $casts = ['years' => 'int'];
1717

18-
public function skillsWithCustomRelatedKey()
19-
{
20-
return $this->belongsToMany(Skill::class, relatedKey: 'cskill_id');
21-
}
22-
23-
public function skillsWithCustomParentKey()
24-
{
25-
return $this->belongsToMany(Skill::class, parentKey: 'cexperience_id');
26-
}
27-
2818
public function sqlUsers(): MorphToMany
2919
{
3020
return $this->morphToMany(SqlUser::class, 'experienced');

‎tests/RelationsTest.php

Lines changed: 202 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
use MongoDB\Laravel\Tests\Models\Address;
1111
use MongoDB\Laravel\Tests\Models\Book;
1212
use MongoDB\Laravel\Tests\Models\Client;
13-
use MongoDB\Laravel\Tests\Models\Experience;
1413
use MongoDB\Laravel\Tests\Models\Group;
1514
use MongoDB\Laravel\Tests\Models\Item;
1615
use MongoDB\Laravel\Tests\Models\Label;
@@ -36,7 +35,6 @@ public function tearDown(): void
3635
Photo::truncate();
3736
Label::truncate();
3837
Skill::truncate();
39-
Experience::truncate();
4038
}
4139

4240
public function testHasMany(): void
@@ -350,48 +348,226 @@ public function testBelongsToManyAttachEloquentCollection(): void
350348
$this->assertCount(2, $user->clients);
351349
}
352350

353-
public function testBelongsToManySyncEloquentCollectionWithCustomRelatedKey(): void
351+
public function testBelongsToManySyncWithCustomKeys(): void
354352
{
355-
$experience = Experience::create(['years' => '5']);
353+
$client = Client::create(['cclient_id' => (string) (new ObjectId()), 'years' => '5']);
354+
$skill1 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'PHP']);
355+
$skill2 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'Laravel']);
356+
357+
$client = Client::query()->find($client->_id);
358+
$client->skillsWithCustomKeys()->sync([$skill1->cskill_id, $skill2->cskill_id]);
359+
$this->assertCount(2, $client->skillsWithCustomKeys);
360+
361+
self::assertIsString($skill1->cskill_id);
362+
self::assertContains($skill1->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
363+
self::assertNotContains($skill1->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
364+
365+
self::assertIsString($skill2->cskill_id);
366+
self::assertContains($skill2->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
367+
self::assertNotContains($skill2->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
368+
369+
$check = Skill::query()->find($skill1->_id);
370+
self::assertIsString($check->cskill_id);
371+
self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
372+
self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
373+
374+
$check = Skill::query()->find($skill2->_id);
375+
self::assertIsString($check->cskill_id);
376+
self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
377+
self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
378+
}
379+
380+
public function testBelongsToManySyncModelWithCustomKeys(): void
381+
{
382+
$client = Client::create(['cclient_id' => (string) (new ObjectId()), 'years' => '5']);
383+
$skill1 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'PHP']);
384+
385+
$client = Client::query()->find($client->_id);
386+
$client->skillsWithCustomKeys()->sync($skill1);
387+
$this->assertCount(1, $client->skillsWithCustomKeys);
388+
389+
self::assertIsString($skill1->cskill_id);
390+
self::assertContains($skill1->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
391+
self::assertNotContains($skill1->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
392+
393+
$check = Skill::query()->find($skill1->_id);
394+
self::assertIsString($check->_id);
395+
self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
396+
self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
397+
}
398+
399+
public function testBelongsToManySyncEloquentCollectionWithCustomKeys(): void
400+
{
401+
$client = Client::create(['cclient_id' => (string) (new ObjectId()), 'years' => '5']);
356402
$skill1 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'PHP']);
357403
$skill2 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'Laravel']);
358404
$collection = new Collection([$skill1, $skill2]);
359405

360-
$experience = Experience::query()->find($experience->id);
361-
$experience->skillsWithCustomRelatedKey()->sync($collection);
362-
$this->assertCount(2, $experience->skillsWithCustomRelatedKey);
406+
$client = Client::query()->find($client->_id);
407+
$client->skillsWithCustomKeys()->sync($collection);
408+
$this->assertCount(2, $client->skillsWithCustomKeys);
363409

364410
self::assertIsString($skill1->cskill_id);
365-
self::assertContains($skill1->cskill_id, $experience->skillsWithCustomRelatedKey->pluck('cskill_id'));
411+
self::assertContains($skill1->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
412+
self::assertNotContains($skill1->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
366413

367414
self::assertIsString($skill2->cskill_id);
368-
self::assertContains($skill2->cskill_id, $experience->skillsWithCustomRelatedKey->pluck('cskill_id'));
415+
self::assertContains($skill2->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
416+
self::assertNotContains($skill2->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
417+
418+
$check = Skill::query()->find($skill1->_id);
419+
self::assertIsString($check->cskill_id);
420+
self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
421+
self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
422+
423+
$check = Skill::query()->find($skill2->_id);
424+
self::assertIsString($check->cskill_id);
425+
self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
426+
self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
427+
}
428+
429+
public function testBelongsToManyAttachWithCustomKeys(): void
430+
{
431+
$client = Client::create(['cclient_id' => (string) (new ObjectId()), 'years' => '5']);
432+
$skill1 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'PHP']);
433+
$skill2 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'Laravel']);
434+
435+
$client = Client::query()->find($client->_id);
436+
$client->skillsWithCustomKeys()->attach([$skill1->cskill_id, $skill2->cskill_id]);
437+
$this->assertCount(2, $client->skillsWithCustomKeys);
438+
439+
self::assertIsString($skill1->cskill_id);
440+
self::assertContains($skill1->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
441+
self::assertNotContains($skill1->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
442+
443+
self::assertIsString($skill2->cskill_id);
444+
self::assertContains($skill2->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
445+
self::assertNotContains($skill2->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
446+
447+
$check = Skill::query()->find($skill1->_id);
448+
self::assertIsString($check->cskill_id);
449+
self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
450+
self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
451+
452+
$check = Skill::query()->find($skill2->_id);
453+
self::assertIsString($check->cskill_id);
454+
self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
455+
self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
456+
}
457+
458+
public function testBelongsToManyAttachModelWithCustomKeys(): void
459+
{
460+
$client = Client::create(['cclient_id' => (string) (new ObjectId()), 'years' => '5']);
461+
$skill1 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'PHP']);
369462

370-
$skill1->refresh();
371-
self::assertIsString($skill1->_id);
372-
self::assertNotContains($skill1->_id, $experience->skillsWithCustomRelatedKey->pluck('cskill_id'));
463+
$client = Client::query()->find($client->_id);
464+
$client->skillsWithCustomKeys()->attach($skill1);
465+
$this->assertCount(1, $client->skillsWithCustomKeys);
466+
467+
self::assertIsString($skill1->cskill_id);
468+
self::assertContains($skill1->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
469+
self::assertNotContains($skill1->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
373470

374-
$skill2->refresh();
375-
self::assertIsString($skill2->_id);
376-
self::assertNotContains($skill2->_id, $experience->skillsWithCustomRelatedKey->pluck('cskill_id'));
471+
$check = Skill::query()->find($skill1->_id);
472+
self::assertIsString($check->_id);
473+
self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
474+
self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
377475
}
378476

379-
public function testBelongsToManySyncEloquentCollectionWithCustomParentKey(): void
477+
public function testBelongsToManyAttachEloquentCollectionWithCustomKeys(): void
380478
{
381-
$experience = Experience::create(['cexperience_id' => (string) (new ObjectId()), 'years' => '5']);
382-
$skill1 = Skill::create(['name' => 'PHP']);
383-
$skill2 = Skill::create(['name' => 'Laravel']);
479+
$client = Client::create(['cclient_id' => (string) (new ObjectId()), 'years' => '5']);
480+
$skill1 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'PHP']);
481+
$skill2 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'Laravel']);
384482
$collection = new Collection([$skill1, $skill2]);
385483

386-
$experience = Experience::query()->find($experience->id);
387-
$experience->skillsWithCustomParentKey()->sync($collection);
388-
$this->assertCount(2, $experience->skillsWithCustomParentKey);
484+
$client = Client::query()->find($client->_id);
485+
$client->skillsWithCustomKeys()->attach($collection);
486+
$this->assertCount(2, $client->skillsWithCustomKeys);
487+
488+
self::assertIsString($skill1->cskill_id);
489+
self::assertContains($skill1->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
490+
self::assertNotContains($skill1->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
491+
492+
self::assertIsString($skill2->cskill_id);
493+
self::assertContains($skill2->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
494+
self::assertNotContains($skill2->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
495+
496+
$check = Skill::query()->find($skill1->_id);
497+
self::assertIsString($check->cskill_id);
498+
self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
499+
self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
500+
501+
$check = Skill::query()->find($skill2->_id);
502+
self::assertIsString($check->cskill_id);
503+
self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
504+
self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
505+
}
506+
507+
public function testBelongsToManyDetachWithCustomKeys(): void
508+
{
509+
$client = Client::create(['cclient_id' => (string) (new ObjectId()), 'years' => '5']);
510+
$skill1 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'PHP']);
511+
$skill2 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'Laravel']);
512+
513+
$client = Client::query()->find($client->_id);
514+
$client->skillsWithCustomKeys()->sync([$skill1->cskill_id, $skill2->cskill_id]);
515+
$this->assertCount(2, $client->skillsWithCustomKeys);
516+
517+
$client->skillsWithCustomKeys()->detach($skill1->cskill_id);
518+
$client->load('skillsWithCustomKeys'); // Reload the relationship based on the latest pivot column's data
519+
$this->assertCount(1, $client->skillsWithCustomKeys);
520+
521+
self::assertIsString($skill1->cskill_id);
522+
self::assertNotContains($skill1->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
523+
self::assertNotContains($skill1->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
389524

390-
self::assertIsString($skill1->_id);
391-
self::assertContains($skill1->_id, $experience->skillsWithCustomParentKey->pluck('_id'));
525+
self::assertIsString($skill2->cskill_id);
526+
self::assertContains($skill2->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
527+
self::assertNotContains($skill2->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
528+
529+
$check = Skill::query()->find($skill1->_id);
530+
self::assertIsString($check->cskill_id);
531+
self::assertNotContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
532+
self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
533+
534+
$check = Skill::query()->find($skill2->_id);
535+
self::assertIsString($check->cskill_id);
536+
self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
537+
self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
538+
}
392539

393-
self::assertIsString($skill2->_id);
394-
self::assertContains($skill2->_id, $experience->skillsWithCustomParentKey->pluck('_id'));
540+
public function testBelongsToManyDetachModelWithCustomKeys(): void
541+
{
542+
$client = Client::create(['cclient_id' => (string) (new ObjectId()), 'years' => '5']);
543+
$skill1 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'PHP']);
544+
$skill2 = Skill::create(['cskill_id' => (string) (new ObjectId()), 'name' => 'Laravel']);
545+
546+
$client = Client::query()->find($client->_id);
547+
$client->skillsWithCustomKeys()->sync([$skill1->cskill_id, $skill2->cskill_id]);
548+
$this->assertCount(2, $client->skillsWithCustomKeys);
549+
550+
$client->skillsWithCustomKeys()->detach($skill1);
551+
$client->load('skillsWithCustomKeys'); // Reload the relationship based on the latest pivot column's data
552+
$this->assertCount(1, $client->skillsWithCustomKeys);
553+
554+
self::assertIsString($skill1->cskill_id);
555+
self::assertNotContains($skill1->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
556+
self::assertNotContains($skill1->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
557+
558+
self::assertIsString($skill2->cskill_id);
559+
self::assertContains($skill2->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
560+
self::assertNotContains($skill2->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
561+
562+
$check = Skill::query()->find($skill1->_id);
563+
self::assertIsString($check->cskill_id);
564+
self::assertNotContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
565+
self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
566+
567+
$check = Skill::query()->find($skill2->_id);
568+
self::assertIsString($check->cskill_id);
569+
self::assertContains($check->cskill_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
570+
self::assertNotContains($check->_id, $client->skillsWithCustomKeys->pluck('cskill_id'));
395571
}
396572

397573
public function testBelongsToManySyncAlreadyPresent(): void

0 commit comments

Comments
 (0)