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

Filter by global roles #4728

Merged

Conversation

KatieB5
Copy link
Contributor

@KatieB5 KatieB5 commented Nov 8, 2024

Fixes #4632

Previously the Users page in the Staff Area had a "Filter by global roles" option which displayed all roles rather than just globally assignable roles. This commit adds a global_roles module so that the view does not need to know the details of the implementation.

Fixes #4632

Previously the Users page in the Staff Area had a "Filter by global roles" option which displayed all roles rather than just globally assignable roles. This commit adds a `global_roles` module so that the view does not need to know the details of the implementation.
Copy link
Contributor

@mikerkelly mikerkelly left a comment

Choose a reason for hiding this comment

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

Thanks Katie.

Katie and I paired on this ticket apart from writing the test.

I noticed the unit test includes an explicit list of expected role names. This made me wonder if the GLOBAL_ROLES definition could simply use the same list instead of being generated by roles_for. However, I think it's beneficial to have both approaches—one in the code and one in the test—for better verification. The minor downside is needing to update the test whenever GLOBAL_ROLES changes.

@KatieB5 KatieB5 merged commit 04f199e into main Nov 8, 2024
7 checks passed
@KatieB5 KatieB5 deleted the KatieB5/filter-by-global-role-shows-roles-that-arent-global branch November 8, 2024 14:16
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.

Filter by global role shows roles that aren't global
2 participants