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 access control in menu and routes #9

Merged
merged 2 commits into from
Oct 5, 2024

Conversation

estivenm0
Copy link
Contributor

No description provided.

Copy link
Collaborator

@daniel-cintra daniel-cintra left a comment

Choose a reason for hiding this comment

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

Hi! Thanks for your time in this Pull Request :)

For better consistence across the permissions system, we can adopt the following pattern:

->can('Acl: Permission - List')

Starting the permission name always with capital letters and ending with the CRUD method when possible.

So:
can('Acl: permission - List') becomes can('Acl: Permission - List')
can('Acl: Role - Manage Permissions') becomes can('Acl: Role: Permission - Edit')

Thanks for your collaboration!

Fixed permission issues affecting certain areas of the package and added
logic to show or hide a button based on the authenticated user's
permissions.
@estivenm0
Copy link
Contributor Author

Hi, I’ve made the requested changes to the pull request, including the adjustments you mentioned. Please review and let me know if further modifications are needed.

Copy link
Contributor

@mehedijaman mehedijaman left a comment

Choose a reason for hiding this comment

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

The changes seems fine to me. It can be merged now.

@daniel-cintra
Copy link
Collaborator

Thanks @stivenm0 and @mehedijaman

Copy link
Collaborator

@daniel-cintra daniel-cintra left a comment

Choose a reason for hiding this comment

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

Thanks!

@daniel-cintra daniel-cintra merged commit dcf9573 into ModularThink:main Oct 5, 2024
1 check passed
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