Skip to content
This repository has been archived by the owner on Aug 18, 2024. It is now read-only.

Split up OgAccess::userAccessEntity() into two methods #672

Closed
4 tasks done
pfrenssen opened this issue Jul 29, 2020 · 0 comments · Fixed by #673
Closed
4 tasks done

Split up OgAccess::userAccessEntity() into two methods #672

pfrenssen opened this issue Jul 29, 2020 · 0 comments · Fixed by #673

Comments

@pfrenssen
Copy link
Contributor

pfrenssen commented Jul 29, 2020

This is part of the OgAccess revamp.

As detailed in earlier issues some inconsistencies were introduced in the documentation of the OgAccess methods during the early days of the D7 > D8 port and over time we have deviated from the original functionality as designed in D7.

In short, the original D7 functions og_user_access() and og_user_access_entity() were designed to check user permissions on groups and group content, but during the early port the methods were accidentally documented as dealing with entity operations rather than user permissions. Later on in the D8 port we have been confused by this wrong documentation and have introduced code that deals with entity operations. The current state is that these methods deal with both concepts but this is incorrect and problematic. For more details see #659.

Current state

OgAccess::userAccessEntity() accepts both user permissions (e.g. subscribe without approval) and entity operations (e.g. create or update). The code has been mangled to work with both. This has resulted in some strange code acrobatics, like calling into ::userAccess() twice.

There is test coverage for both scenarios, and somehow tests pass for both. The coverage for the entity operations case is very extensive, but for user permissions is basic.

Scope

  • Split off the code that deals with entity operations to a new method OgAccess::userAccessEntityOperation().
  • Reduce the original method to align closely to the D7 functionality, only dealing with user permissions.
  • Fix the documentation and ensure the difference between both is completely clear.
  • Ensure existing tests are calling into the respective functions and all tests are passing.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant