Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reset relations when permissions are updated or deleted #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pktharindu
Copy link

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