Skip to content

Add support for configurable role identifier key (e.g., slug) #2846

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

philiplamb
Copy link

I needed to use a slug (explicitly not use "name" column), and to check against it, but this package forces you to use the name in your checkRole methods. I also saw many issues asking for slug support, this doesn't add slug so I made this configurable through the config/permission.php.

#1846

You can now update this line to use your preferred column:
'role_identifier' => 'name', // e.g., change to 'slug'

This PR:

  • Updates role matching methods (hasRole, hasAllRoles, hasExactRoles, etc.) to respect the configured identifier.
  • Adds a getRoleKey() helper method to the HasRoles trait.
  • Introduces findByKey() in the Role model to support lookup by the configured identifier.
  • Tests name and slug usage. I had to add slug for testing as nullable.
  • Has backward compatibility. The default behavior (name) remains unchanged.

@parallels999
Copy link
Contributor

parallels999 commented May 21, 2025

Has backward compatibility.
Introduces findByKey() in the Role

Not, it breaks it, spatie/laravel-permission/Contracts/Role.php
It also breaks other things, but why go into details, hehe
getRoleKey seems like a key, really ambigous, same findByKey

@philiplamb
Copy link
Author

Hi @parallels999 , thanks for your feedback, i fixed the naming and some mistakes. I'd appreciate if you'd go into more details, or i need to do something else. This is my first PR. Thank you.

@drbyte
Copy link
Collaborator

drbyte commented May 31, 2025

I needed to use a slug (explicitly not use "name" column), and to check against it

In cases like this, instead of making broad package changes, I typically just use the slug to lookup the model ID, and then explicitly pass the model to all the corresponding functions. No performance hit because lookups of role-related details are user-initiated, not pageload-initiated.

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.

3 participants