fix: edge case in which hasPermission failed

uwla · Dec 11, 2024 · c88c812 · c88c812
1 parent 7113a7c
commit c88c812
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 9 deletions.
diff --git a/src/Models/Permission.php b/src/Models/Permission.php
@@ -69,8 +69,8 @@ public static function getByName($names, $modelType = null, $models = null): Col
             );
         }
 
-        $permissionCount = count($names);
-        if ($permissionCount == 0) {
+        $permission_count = count($names);
+        if ($permission_count == 0) {
             throw new InvalidArgumentException(
                 'No permission provided',
             );
@@ -95,7 +95,7 @@ public static function getByName($names, $modelType = null, $models = null): Col
             );
         }
 
-        if (count($models) != $permissionCount) {
+        if (count($models) != $permission_count) {
             throw new InvalidArgumentException(
                 'number of permissions and models must match'
             );
@@ -106,8 +106,8 @@ public static function getByName($names, $modelType = null, $models = null): Col
         }
 
         // each resource is identified by its model_id
-        $query->where(function ($q) use ($names, $models, $permissionCount) {
-            for ($i = 0; $i < $permissionCount; $i += 1) {
+        $query->where(function ($q) use ($names, $models, $permission_count) {
+            for ($i = 0; $i < $permission_count; $i += 1) {
                 $q->orWhere([
                     ['name', $names[$i]],
                     ['model_id', $models[$i]],
@@ -145,10 +145,11 @@ public static function createMany($names): Collection
         }
 
         $permissionsToCreate = [];
-        foreach ($names as $name);
-        $permissionsToCreate[] = ['name' => $name];
-        static::insert($permissionsToCreate);
+        foreach ($names as $name) {
+            $permissionsToCreate[] = ['name' => $name];
+        }
 
+        static::insert($permissionsToCreate);
         return static::whereIn('name', $names)->get();
     }
 }
diff --git a/src/Traits/HasPermission.php b/src/Traits/HasPermission.php
@@ -328,7 +328,8 @@ public function getPermissionNames()
      */
     public function hasPermission($permission, $resource = null, $id = null)
     {
-        return $this->hasPermissions([$permission], $resource, [$id]);
+        $ids = ($id == null) ? null : [$id];
+        return $this->hasPermissions([$permission], $resource, $ids);
     }
 
     /**

diff --git a/tests/Feature/HasPermissionPerUserTest.php b/tests/Feature/HasPermissionPerUserTest.php
@@ -98,11 +98,17 @@ public function test_has_permission()
     {
         $user = User::factory()->createOne();
         $permission = Permission::factory()->createOne();
+        $permission2 = Permission::factory()->createOne();
+
         $this->assertFalse($user->hasPermission($permission));
         $this->assertFalse($user->hasPermission($permission->name));
+
         $user->addPermission($permission);
+
         $this->assertTrue($user->hasPermission($permission));
         $this->assertTrue($user->hasPermission($permission->name));
+        $this->assertFalse($user->hasPermission($permission2));
+        $this->assertFalse($user->hasPermission($permission2->name));
     }
 
     /**
@@ -119,6 +125,7 @@ public function test_has_any_permission()
         $mixed = $permissions->merge($other_permissions);
         $mixed_names = $mixed->pluck('name')->toArray();
         $user->addPermissions($permissions);
+
         $this->assertFalse($user->hasPermissions($mixed));
         $this->assertFalse($user->hasPermissions($mixed_names));
         $this->assertTrue($user->hasAnyPermission($mixed));

diff --git a/tests/Feature/HasPermissionTest.php b/tests/Feature/HasPermissionTest.php
@@ -141,14 +141,23 @@ public function test_has_permission()
         $role = Role::factory()->createOne();
         $user = User::factory()->createOne();
         $permission = Permission::factory()->createOne();
+        $permission2 = Permission::factory()->createOne();
 
         $user->addRole($role);
         $this->assertFalse($role->hasPermission($permission));
         $this->assertFalse($user->hasPermission($permission));
 
         $role->addPermission($permission);
+
         $this->assertTrue($role->hasPermission($permission));
         $this->assertTrue($user->hasPermission($permission));
+        $this->assertTrue($role->hasPermission($permission->name));
+        $this->assertTrue($user->hasPermission($permission->name));
+
+        $this->assertFalse($role->hasPermission($permission2));
+        $this->assertFalse($user->hasPermission($permission2));
+        $this->assertFalse($role->hasPermission($permission2->name));
+        $this->assertFalse($user->hasPermission($permission2->name));
     }
 
     /**