-
-
Notifications
You must be signed in to change notification settings - Fork 112
chore(multiset): restrict the 'Sets' resource in Filament #4142
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
base: master
Are you sure you want to change the base?
chore(multiset): restrict the 'Sets' resource in Filament #4142
Conversation
Jamiras
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Developers can still add/remove sets via the Game management page - and presumably edit them once #4139 is merged.
What is the purpose of the Sets list anyway? Edit just pops open an empty dialog:
And you can get to the Set details from the Sets list on the Game page (which would be the more likely place I'd go do find a set).
Yep, this is definitely desirable and not something we want to prevent.
Right now, it mainly allows you to select certain hashes and opt them out of supporting a subset. I think this ability/customization is far too powerful for developers at the moment and should be limited. |
I must be missing something. I don't see anything related to hashes in that list. If you're referring to the "Incompatible Hashes" tab on the Set page, you can get to the Set page from the Sets tab of the game (which to me is much more intuitive than globally searching for the set). From the list: If I click on a row, it takes me to the same page as clicking a row in the Sets tab of the game. If I click edit, I get an empty dialog. So, restricting access to the list doesn't seem to restrict access to anything other than a global list. And I question why we even need a global list. |
|
I agree that having a list of achievement sets is unintuitive.
This doesn't seem right; the resource should indeed be restricted. You shouldn't be able to access anything for In latest, I've removed the list view for achievement sets. We'll still need to allow hash managers to access the View page though. Maybe a new link for this should be added in #4139. |
| { | ||
| return $user->hasAnyRole([ | ||
| Role::GAME_HASH_MANAGER, | ||
| Role::DEVELOPER, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a developer without hash manager permissions, I can still click on a row in the Game's Set list, and I get a forbidden error. I expect the page should open readonly as view returns true. But if it's not supposed to open, then the click shouldn't do anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disabled the row links in latest. The title labels still have an underline on hover which Filament doesn't seem to give me any way to get rid of, but this will naturally go away as a result of #4139.
Accompanies #4139.
Locks down the "Sets" resource (in the sidebar) in Filament to just users who have
Role::GAME_HASH_MANAGER.I've thought this over, and I don't think we have enough data to make this tool generally available to developers yet. If it isn't used wisely, it can cause more harm than good, and using it should really be an edge case.
If updating the hash compatibility matrix for a game's sets becomes a common ask and we want to make it generally available to full devs, I think we should come up with an easier UX. Otherwise, for now, we can restrict it to a small subset of users.