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

Create usePermissions UI hook #413

Merged
merged 20 commits into from
Mar 8, 2024
Merged

Create usePermissions UI hook #413

merged 20 commits into from
Mar 8, 2024

Conversation

maffkipp
Copy link
Contributor

Description

  • Adds a reusable usePermissions hook that checks permissions of the current user against a provided list of permissions
  • Creates a custom renderHook method override for testing library set up in the same way as our custom render method
  • Adds permission definition enums to the shared UI project

Motivation and Context

  • We want a reusable solution for restricting UI elements based on the current user's permissions

How Has This Been Tested?

  • Added tests for usePermissions hook

Types of changes

  • Chore (a change that does not modify the application functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Documentation updates are needed, and have been made accordingly.
  • I have added and/or updated tests to cover my changes.
  • All new and existing tests passed.
  • My changes include a database migration.

@maffkipp maffkipp self-assigned this Feb 12, 2024
@elikmiller
Copy link
Collaborator

elikmiller commented Feb 15, 2024

It might be more versatile to have this hook return the list of permissions for the currently logged in user alongside a few helper functions to help determine if a user has a list of required permissions or at least one from a list of permissions. I could see wanting to use different lists of permissions for different checks on the same page or even within the same component. Instead of needing to call this hook multiple times you could call it once and compute what you needed as needed.

What are your thoughts?

@maffkipp
Copy link
Contributor Author

@elikmiller I don't think thats a bad idea at all, we may not even need to return the user's current permissions -- just a set of helper functions that can be called with a list of permissions to match against as the argument.

Copy link
Contributor

@mistahj67 mistahj67 left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning up the permissions callback! 🧹 🧼

Copy link
Collaborator

@elikmiller elikmiller left a comment

Choose a reason for hiding this comment

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

Looks good!

@maffkipp maffkipp merged commit abb5ff7 into main Mar 8, 2024
3 checks passed
@maffkipp maffkipp deleted the use-permissions-hook branch March 8, 2024 00:13
@github-actions github-actions bot locked and limited conversation to collaborators Mar 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants