Skip to content

Commit

Permalink
[Fix] BelongsToMany sync does't use configured keys (#2667)
Browse files Browse the repository at this point in the history
* Merge testSyncBelongsToMany into testBelongsToManySync;
* modelKeys replaced with parseIds in sync method;
* Add test for handling a collection in sync method;
  • Loading branch information
hans-thomas committed Nov 6, 2023
1 parent 0cdba6a commit 5387d54
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 42 deletions.
8 changes: 4 additions & 4 deletions src/Relations/BelongsToMany.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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.
Expand Down
26 changes: 26 additions & 0 deletions tests/Models/Experience.php
Original file line number Diff line number Diff line change
@@ -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');
}
}
14 changes: 14 additions & 0 deletions tests/Models/Skill.php
Original file line number Diff line number Diff line change
@@ -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;
}
102 changes: 64 additions & 38 deletions tests/RelationsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -30,6 +33,8 @@ public function tearDown(): void
Role::truncate();
Group::truncate();
Photo::truncate();
Skill::truncate();
Experience::truncate();
}

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

0 comments on commit 5387d54

Please sign in to comment.