Making these checks more granular will help in a future where read only is not a single flag.
BK, existing read-only tests
This is a really good change - we should more explicitly test it and be a little bit more defensive w/r/t passing strings around.
I think this util would benefit from a few explicit unit test cases:
- test_check_permission_has_permission() - test_check_permission_does_not_have_permission() - test_check_permission_permission_does_not_exist()
Instead of implicitly testing it via our read-only tests. At that point, we could probably even remove some of the read-only tests (unless there's specific behavior they test beyond just whether or not read-only permissions are applied).
Smart to generalize this 🤓
I think we want to raise a different, more general error class. Perhaps we need a function that maps error class to permission? Or a more general PermissionDeniedError?
Given that we're just passing strings in, do we want to add an additional check inside of check_permission to ensure that the permission actually exists? That you didn't typo the permission name.
Alternatively, we could pass in an enum or a constant to make it less prone to typos.