Skip to content

Commit bd6aef6

Browse files
committed
Make role model sync atomic
1 parent afd2401 commit bd6aef6

2 files changed

Lines changed: 23 additions & 9 deletions

File tree

src/Traits/HasAssignedModels.php

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -63,19 +63,21 @@ public function removeFromModels(array|Collection|Model|int|string $models, ?str
6363
*/
6464
public function syncModels(array|Collection|Model|int|string $models, ?string $modelClass = null): static
6565
{
66-
if ($this->exists) {
67-
$this->newPivotQueryForRole()->delete();
68-
}
66+
return $this->getConnection()->transaction(function () use ($models, $modelClass) {
67+
if ($this->exists) {
68+
$this->newPivotQueryForRole()->delete();
69+
}
6970

70-
$teamPivot = $this->teamPivot();
71+
$teamPivot = $this->teamPivot();
7172

72-
foreach ($this->groupModelsByMorphClass($models, $modelClass) as $morphClass => $ids) {
73-
$this->relationForModel($morphClass)->attach($ids, $teamPivot);
74-
}
73+
foreach ($this->groupModelsByMorphClass($models, $modelClass) as $morphClass => $ids) {
74+
$this->relationForModel($morphClass)->attach($ids, $teamPivot);
75+
}
7576

76-
$this->unsetRelation('users');
77+
$this->unsetRelation('users');
7778

78-
return $this;
79+
return $this;
80+
});
7981
}
8082

8183
/**

tests/Traits/HasAssignedModelsTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<?php
22

33
use Illuminate\Support\Facades\DB;
4+
use Illuminate\Database\QueryException;
45
use Spatie\Permission\Models\Role;
56
use Spatie\Permission\PermissionRegistrar;
67
use Spatie\Permission\Support\Config;
@@ -62,6 +63,17 @@
6263
expect($user1->fresh()->hasRole($this->testUserRole))->toBeTrue();
6364
});
6465

66+
it('does not detach models when syncing an unsaved model', function () {
67+
$user = User::create(['email' => 'user@test.com']);
68+
$this->testUserRole->syncModels($user);
69+
70+
$unsavedUser = new User(['email' => 'unsaved-user@test.com']);
71+
72+
expect(fn () => $this->testUserRole->syncModels($unsavedUser))->toThrow(QueryException::class);
73+
74+
expect($user->fresh()->hasRole($this->testUserRole))->toBeTrue();
75+
});
76+
6577
it('can sync models using IDs', function () {
6678
$user1 = User::create(['email' => 'user1@test.com']);
6779
$user2 = User::create(['email' => 'user2@test.com']);

0 commit comments

Comments
 (0)