Merge pull request #975 from drbyte/fix-sync-saved-callback-bug

Add fix for broken sync saved event
spatie · Dec 16, 2018 · 8ca8230 · 8ca8230
2 parents 297af85 + 658fd75
commit 8ca8230
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 5 deletions.
diff --git a/src/Traits/HasPermissions.php b/src/Traits/HasPermissions.php
@@ -419,9 +419,14 @@ public function givePermissionTo(...$permissions)
             $class = \get_class($model);
 
             $class::saved(
-                function ($model) use ($permissions) {
-                    $model->permissions()->sync($permissions, false);
-                    $model->load('permissions');
+                function ($object) use ($permissions, $model) {
+                    static $modelLastFiredOn;
+                    if ($modelLastFiredOn !== null && $modelLastFiredOn === $model) {
+                        return;
+                    }
+                    $object->permissions()->sync($permissions, false);
+                    $object->load('permissions');
+                    $modelLastFiredOn = $object;
                 });
         }
 

diff --git a/src/Traits/HasRoles.php b/src/Traits/HasRoles.php
@@ -121,8 +121,14 @@ public function assignRole(...$roles)
             $class = \get_class($model);
 
             $class::saved(
-                function ($model) use ($roles) {
-                    $model->roles()->sync($roles, false);
+                function ($object) use ($roles, $model) {
+                    static $modelLastFiredOn;
+                    if ($modelLastFiredOn !== null && $modelLastFiredOn === $model) {
+                        return;
+                    }
+                    $object->roles()->sync($roles, false);
+                    $object->load('roles');
+                    $modelLastFiredOn = $object;
                 });
         }
 

diff --git a/tests/HasPermissionsTest.php b/tests/HasPermissionsTest.php
@@ -449,4 +449,34 @@ public function it_can_sync_permissions_to_a_model_that_is_not_persisted()
         $this->assertTrue($user->hasPermissionTo('edit-articles'));
         $this->assertTrue($user->fresh()->hasPermissionTo('edit-articles'));
     }
+
+    /** @test */
+    public function calling_givePermissionTo_before_saving_object_doesnt_interfere_with_other_objects()
+    {
+        $user = new User(['email' => '[email protected]']);
+        $user->givePermissionTo('edit-news');
+        $user->save();
+
+        $user2 = new User(['email' => '[email protected]']);
+        $user2->givePermissionTo('edit-articles');
+        $user2->save();
+
+        $this->assertTrue($user2->fresh()->hasPermissionTo('edit-articles'));
+        $this->assertFalse($user2->fresh()->hasPermissionTo('edit-news'));
+    }
+
+    /** @test */
+    public function calling_syncPermissions_before_saving_object_doesnt_interfere_with_other_objects()
+    {
+        $user = new User(['email' => '[email protected]']);
+        $user->syncPermissions('edit-news');
+        $user->save();
+
+        $user2 = new User(['email' => '[email protected]']);
+        $user2->syncPermissions('edit-articles');
+        $user2->save();
+
+        $this->assertTrue($user2->fresh()->hasPermissionTo('edit-articles'));
+        $this->assertFalse($user2->fresh()->hasPermissionTo('edit-news'));
+    }
 }
diff --git a/tests/HasRolesTest.php b/tests/HasRolesTest.php
@@ -200,6 +200,36 @@ public function it_will_sync_roles_to_a_model_that_is_not_persisted()
         $this->assertTrue($user->hasRole($this->testUserRole));
     }
 
+    /** @test */
+    public function calling_syncRoles_before_saving_object_doesnt_interfere_with_other_objects()
+    {
+        $user = new User(['email' => '[email protected]']);
+        $user->syncRoles('testRole');
+        $user->save();
+
+        $user2 = new User(['email' => '[email protected]']);
+        $user2->syncRoles('testRole2');
+        $user2->save();
+
+        $this->assertTrue($user2->fresh()->hasRole('testRole2'));
+        $this->assertFalse($user2->fresh()->hasRole('testRole'));
+    }
+
+    /** @test */
+    public function calling_assignRole_before_saving_object_doesnt_interfere_with_other_objects()
+    {
+        $user = new User(['email' => '[email protected]']);
+        $user->assignRole('testRole');
+        $user->save();
+
+        $admin_user = new User(['email' => '[email protected]']);
+        $admin_user->assignRole('testRole2');
+        $admin_user->save();
+
+        $this->assertTrue($admin_user->fresh()->hasRole('testRole2'));
+        $this->assertFalse($admin_user->fresh()->hasRole('testRole'));
+    }
+
     /** @test */
     public function it_throws_an_exception_when_syncing_a_role_from_another_guard()
     {