Skip to content

Reset relations when permissions are updated or deleted#29

Open
pktharindu wants to merge 1 commit into
Silvanite:masterfrom
pktharindu:pktharindu-patch-1
Open

Reset relations when permissions are updated or deleted#29
pktharindu wants to merge 1 commit into
Silvanite:masterfrom
pktharindu:pktharindu-patch-1

Conversation

@pktharindu
Copy link
Copy Markdown

In my feature and unit tests I am creating users with specific permissions for certain scenarios, this means that I am calling setPermissions multiple times in a test and during this process I have encountered the below issues which this PR fixes.

Scenario A
When you call the setPermissions method the $role->getPermissions relation returns the previous set of permissions as the model is not refreshed. Changed it so that it calls the setRelations method in setPermissions to reset the relation.

Code example to replicate issue

Code

$role = \Pktharindu\NovaPermissions\Role::create(['slug' => 'admin']);

$role->setPermissions(['view clients']);

dump([
    'not refreshed is empty' => $role->getPermissions->pluck('permission_slug')->toArray(),
    'refreshed' => $role->refresh()->getPermissions->pluck('permission_slug')->toArray(),
]);

Output

array:2 [
  "not refreshed is empty" => []
  "refreshed" => array:1 [
    0 => "view clients"
  ]
]

Scenario B
If you call setPermissions multiple times on the same model instance where some permissions already exist they get deleted by the revokeAll method. Then the hasPermission method is called which is using a non-refreshed getPermissions relation. Added calls to setRelations to the revoke methods.

Code example to replicate issue

Code

$role = \Pktharindu\NovaPermissions\Role::create(['slug' => 'admin']);

$role->setPermissions(['view clients']);

dump([
    'permission created ok' => $role->refresh()->getPermissions->pluck('permission_slug')->toArray(),
]);

$role->setPermissions(['view clients', 'manage clients']);

dump([
    'updated permissions' => $role->refresh()->getPermissions->pluck('permission_slug')->toArray(),
]);

Current Output

array:1 [
  "permission created ok" => array:1 [
    0 => "view clients"
  ]
]
array:1 [
  "updated permissions" => array:1 [
    0 => "manage clients" // only new permission exists, 'view clients' was deleted but not recreated :(
  ]
]

Fixed Output

array:1 [
  "permission created ok" => array:1 [
     0 => "view clients"
  ]
]
array:1 [
  "updated permissions" => array:1 [
      0 => "manage clients"
      1 => "view clients"
  ]
]

Made a couple of other minor tweaks that do not affect functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant