Problem/Motivation
Ah. The content_access_author realm. At the moment content access needs to rebuild node access records if a user's role changes.
From my point of view content access has done this wrong, and we should change it. The whole point of the node access system is really to stop having to recompute the node access records when something about users changes.
Steps to reproduce
Use the module.
Proposed resolution
I think we should:
- Remove the
content_access_authorrealm - Introduce new realms for each of the roles, for authors, so something like:
content_access_author__developeror whatever - When computing the node access records, we generate records for each of the configured roles that are allowed to view their own content, and the grant ID would be the node owner ID.
- Then when computing the grants a user has, for every role the user has, they get their own user ID as a the grant ID within the realm for that role.
That avoids having to recompute all the node access records when a user changes a role, and means that access changes are instant, rather than needing to wait for someone to do something too!
Remaining tasks
Probably needs some discussion with @quadrexdev
User interface changes
None.
API changes
I'd consider this a BC break, and thus something that would trigger a 3.0 release. There might be downstream users relying on the names/values of the content access author grants.
Data model changes
We'd be changing the names of our node access realms.
Issue fork content_access-3579507
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
steven jones commentedComment #3
steven jones commentedComment #5
steven jones commentedI've opened a MR, but this thing very much needs some tests since I was running tests part of the way through converting the realms and everything was passing just fine, so we've got some gaps in test coverage I'd imagine.
Comment #6
steven jones commentedI'm not sure we have tests for per-node settings author-type permissions, so we'll need to add that in.
I have no removed the rebuilding of the access records from the tests that we do have that do the node-type author settings changes, and changing the roles a user has still correctly enforces access, showing that we're on the right track here.
I suspect this will introduce some inefficiencies, but maybe we can resolve those later etc. if nothing else it's actually making the code a lot simpler because we're not having to re-implement some parts of the node access system, which is quite nice!
Comment #7
steven jones commentedI've written those tests now.
I wonder if actually I should get these tests only into the 2.x branch of the module in a separate issue, then that makes sure these changes aren't changing any behavior.
Comment #8
steven jones commentedI think this is ready for review now!
Comment #9
steven jones commentedI think if we're really conservative, this is a BC break, so bumping to 3.0.x
Comment #10
steven jones commentedSo...I'm going merge this, because it seems to be working fine.
Comment #12
steven jones commented