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

Fix $user->roles($purpose) #6653

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

distantnative
Copy link
Member

@distantnative distantnative commented Sep 7, 2024

Description

Summary of changes

  • User::roles() has a new optional $purpose parameter to pass a user action (create or change). If one is passed, the roles collection is filtered via $roles->canBeChanged()/$roles->canBeCreated() which checks if the current user has the permissions to apply the action to each role.
  • ::roles() doesn't return only the current role for non-admin users but all available roles

Reasoning

Available roles depend on the context as they depend on whether the current user has permissions to e.g. create a user with a certain role or change the role to a certain role.

Additional context

We need to pass the $purpose parameters, e.g. in user dialogs, in a subsequent PR.

Ready?

  • In-code documentation (wherever needed)
  • Unit tests for fixed bug/feature
  • Tests and CI checks all pass

@distantnative distantnative added the type: enhancement ✨ Suggests an enhancement; improves Kirby label Sep 7, 2024
@distantnative distantnative self-assigned this Sep 7, 2024
@distantnative distantnative marked this pull request as ready for review September 7, 2024 20:35
@distantnative distantnative requested a review from a team September 7, 2024 20:36
@distantnative distantnative added this to the 4.5.0 milestone Sep 7, 2024
@distantnative distantnative marked this pull request as draft September 8, 2024 07:42
@distantnative distantnative changed the title ->roles($context) to limit available roles Fix $user->roles($purpose) Sep 8, 2024
@distantnative distantnative added the type: bug 🐛 Is a bug; fixes a bug label Sep 8, 2024
@distantnative distantnative marked this pull request as ready for review September 8, 2024 07:56
@distantnative distantnative requested a review from a team September 8, 2024 07:57
Copy link
Member

@bastianallgeier bastianallgeier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've come to the point where I really see the argument as a valid solution and would not overthink this anymore tbh.

@bastianallgeier bastianallgeier merged commit 8b3cddf into develop-minor Sep 16, 2024
14 checks passed
@bastianallgeier bastianallgeier deleted the enhancement/app-roles-context branch September 16, 2024 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug 🐛 Is a bug; fixes a bug type: enhancement ✨ Suggests an enhancement; improves Kirby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants