refactor: standardize permission layers#5094
Conversation
peterhaochen47
force-pushed
the
pr/main/refactor-standardize-perm-layers-may26
branch
from
2ecd3ff to
672db0d
Compare
|
|
||
| def is_org_manager? | ||
| VCAP::CloudController::OrganizationManager.where(user_id: @user.id).any? | ||
| membership.authorized_org_guids_subquery(VCAP::CloudController::Membership::ORG_MANAGER).any? |
There was a problem hiding this comment.
Don't need the guid, so authorized_orgs_subquery would be simpler.
There was a problem hiding this comment.
Addressed, also made an additional improvement, see message of new commit.
philippthun
left a comment
philippthun
left a comment
There was a problem hiding this comment.
Thanks for addressing the feedback. The second optimization is not needed as we are using the any_not_empty extension:
I don't care about using !empty? or any? in this file, but I think the commit message should be adapted as it might be misleading for others.
- such that the codepaths involved become more aligned with the following layer design/abstraction: ``` | Layer | Responsibility | |-------|---------------| | **Controller** | Asks `Permissions` (via `permission_queryer`) what the current user can see/do | | **Permissions** | Translates "can user do X?" into role-set queries via `Membership` | | **Membership** | Executes role-based database queries (which orgs/spaces does user belong to?) | | **Models** | Data access, no RBAC logic | ```
peterhaochen47
force-pushed
the
pr/main/refactor-standardize-perm-layers-may26
branch
from
6caa741 to
a1c6e76
Compare
I see, updated, simply modified the first commit. |
2 participants
following layer design/abstraction:
I have reviewed the contributing guide
I have viewed, signed, and submitted the Contributor License Agreement
I have made this pull request to the
mainbranchI have run all the unit tests using
bundle exec rakeI have run CF Acceptance Tests