- πΊπΈUnited States m.stenta
@pcambra @paul121 and I have been discussing this at length, and we've come to the conclusion that this should *not* be implemented in the Log module. Rather, the farmOS distribution can make modifications to the log entity definition if it so desires.
See #3415653: [META] Allow more granular access on default farmOS Views β
Closing this as "won't fix".
- Status changed to Active
11 months ago 7:42pm 24 January 2024 - πΊπΈUnited States m.stenta
Reopening this. @pcambra @paul121 and I discussed more today and discovered that
accessCheck(TRUE)
is having no effect on log queries. So we decided to move forward with this after all as a bug fix.We *may* need to consider this a breaking change, though, which would necessitate a major version release.
- π«π·France andypost
As change notice said each query should use
accessCheck(False)
as log recods has no view permissions vary - πΊπΈUnited States m.stenta
Thanks @andypost!
The reason we should consider this a breaking change is that right now
accessCheck(TRUE)
andaccessCheck(FALSE)
make no difference to the list of logs returned based on user permissions. This is because the Log entity uses the Entity API module'spermissions_provider
handler, but not itsquery_access
handler, which is responsible for modifying the Log entity queries based on user permissions.We are going to start using the Entity API
query_access
handler, which means that any lists of logs that are currently being built withaccessCheck(TRUE)
(including Views) may change for users. They will arguably change for the better, because they will actually be respecting user permissions better, so this is a bug fix. But nevertheless it is a bug fix that changes current behavior, so I think it's worth a major version bump to make it clear to users of the Log module that they might want to check their implementations. - Merge request !14Issue #3172320 by paul121, m.stenta, pcambra: Use Entity API query_access handler β (Open) created by m.stenta
- last update
11 months ago 13 pass, 4 fail - Status changed to Needs review
11 months ago 3:54pm 26 January 2024 - Status changed to Needs work
11 months ago 4:13pm 26 January 2024 - πΊπΈUnited States m.stenta
Looks like we'll need to fix some failing tests...
- Status changed to Needs review
11 months ago 8:15pm 26 January 2024 - πΊπΈUnited States m.stenta
Hmm drupal.org tests are failing on 2.x in general... not specific to this MR. They are all passing locally for me (except for this know psql issue: π LogActionsTest::testRescheduleMultipleLogsRelative() fails on PostgreSQL Needs work ).
So I think this is good. Setting back to Needs Review.