Issue #3458767: Replace node grants with entity query access #3961
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
volodymyr-sydor
added
team: enterprise
volodymyr-sydor
changed the title
volodymyr-sydor
force-pushed
the
feature/3458767-replace-node-grants
branch
2 times, most recently
from
3e99fe6 to
e1b9924
Compare
volodymyr-sydor
force-pushed
the
feature/3458767-replace-node-grants
branch
7 times, most recently
from
62ea541 to
4e48cbd
Compare
This was referenced Oct 17, 2024
nechai
force-pushed
the
feature/3458767-replace-node-grants
branch
from
3b2a0de to
b52e1bb
Compare
| 'gnode:b' => [], | ||
| ]; | ||
| $this->assertEquals($expected_result_groups_none, $altered_grants_groups_none); | ||
| // @todo Rewrite test with entity query access check. |
There was a problem hiding this comment.
I can't approve without any test coverage :)
| * @return string | ||
| * The node field join alias. | ||
| */ | ||
| public function ensureNodeFieldTableJoin(string $field_name): string { |
There was a problem hiding this comment.
I don't like the overall usage of the term Ensure here, I don't think it make it very clear/usable but I can't come with another naming so I guess it will be fine :)
nechai
force-pushed
the
feature/3458767-replace-node-grants
branch
from
ec9b509 to
a882e33
Compare
ribel
added
needs work: rebase
|
Tugboat has finished building the preview for this pull request! Link: Dashboard: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
We have the
entity_access_by_fieldmodule which uses node grants to limit the visibility of entities that users can see in different places on the platform but we want to get rid of node grants and replace it with entity query access.Our local tests show that
hook_node_grants()is slower than query builders on node entity queries.This PR contains the basic (core) tools that allow the use of query builders rather than node grants.
We implement query alter for node entities. It contains the query condition group (with
ORoperator) (see there).The idea is to allow other modules (distro or external) to change only this one query conditions group. For this, developer should implement event subscriber with SocialNodeQueryAccessAlterInterface interface. You can find examples of this event subscriber implementations:
One important thing - if we want to use query builders then we should get rid
hook_node_grants().If we have at least one hook implementation the query will be broken and no results returned. For this reason we added social_node_requirements() to notify developers to avoid using this hook with Social.
Solution
Replace node grants with entity query access to improve performance and ensure access is centrally controllable
PRs:
Issue tracker
Theme issue tracker
N/A
How to test
Screenshots
N/A
Release notes
N/A
Change Record
N/A
Translations
N/A