Return $this instead of throwing "already assigned" exception in `M…

…anager::assign()` (#253)
yiisoft · Feb 14, 2024 · 98cf8d7 · 98cf8d7
1 parent d99efda
commit 98cf8d7
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -56,6 +56,7 @@
 - Bug #237: Handle not found base item in access tree (@arogachev)
 - Enh #245: Handle same names during renaming item in `AssignmentsStorage` (@arogachev)
 - Chg #208: Rename `getAccessTree()` to `getHierarchy()` in `ItemsStorageInterface` (@arogachev)
+- Enh #252: Return `$this` instead of throwing "already assigned" exception in `Manager::assign()` (@arogachev)
 
 ## 1.0.2 April 20, 2023
 

diff --git a/src/Manager.php b/src/Manager.php
@@ -153,9 +153,7 @@ public function assign(string $itemName, int|Stringable|string $userId, ?int $cr
         }
 
         if ($this->assignmentsStorage->exists($itemName, $userId)) {
-            throw new InvalidArgumentException(
-                "\"$itemName\" {$item->getType()} has already been assigned to user $userId.",
-            );
+            return $this;
         }
 
         $assignment = new Assignment($userId, $itemName, $createdAt ?? time());

diff --git a/tests/Common/ManagerLogicTestTrait.php b/tests/Common/ManagerLogicTestTrait.php
@@ -485,23 +485,13 @@ public function testAssignUnknownItem(): void
         $this->expectException(InvalidArgumentException::class);
         $this->expectExceptionMessage('There is no item named "nonExistRole".');
 
-        $manager->assign(
-            'nonExistRole',
-            'reader'
-        );
+        $manager->assign(itemName: 'nonExistRole', userId: 'reader');
     }
 
     public function testAssignAlreadyAssignedItem(): void
     {
         $manager = $this->createFilledManager();
-
-        $this->expectException(InvalidArgumentException::class);
-        $this->expectExceptionMessage('"reader" role has already been assigned to user reader A.');
-
-        $manager->assign(
-            'reader',
-            'reader A'
-        );
+        $this->assertSame($manager, $manager->assign(itemName: 'reader', userId: 'reader A'));
     }
 
     public function testAssignPermissionDirectlyWhenItIsDisabled(): void
@@ -511,7 +501,7 @@ public function testAssignPermissionDirectlyWhenItIsDisabled(): void
 
         $this->expectException(InvalidArgumentException::class);
         $this->expectExceptionMessage('Assigning permissions directly is disabled. Prefer assigning roles only.');
-        $manager->assign('readPost', 'id7');
+        $manager->assign(itemName: 'readPost', userId: 'id7');
     }
 
     public function testAssignPermissionDirectlyWhenEnabled(): void
@@ -1021,8 +1011,8 @@ public function testDataPersistency(): void
             ->addRole((new Role('role1'))->withCreatedAt(1_694_502_936)->withUpdatedAt(1_694_502_936))
             ->addRole((new Role('role2'))->withCreatedAt(1_694_502_976)->withUpdatedAt(1_694_502_976))
             ->addChild('role1', 'role2');
-        $manager->assign('role1', 1);
-        $manager->assign('role2', 2);
+        $manager->assign(itemName: 'role1', userId: 1);
+        $manager->assign(itemName: 'role2', userId: 2);
 
         $this->assertEquals(
             [