When a user’s membership in an OG group changes, the Edit tab on group content nodes can remain stale (e.g., still hidden after adding a member, or still shown after removal) until cache is
cleared. This appears to be caused by OG access results not carrying cacheability metadata through hook_entity_access() denial paths, and by access results not varying by OG permissions.

Steps to reproduce

1. Create a group content type (e.g., Article) and a group type (e.g., Department).
2. Configure OG permissions so group members have “Edit any article content”.
3. View an Article as a non‑member: no Edit tab.
4. Add membership for the same user to the group (active).
5. Refresh the Article page.

Expected result

The Edit tab appears immediately after membership is added (and disappears immediately after removal).

Actual result

The Edit tab remains in the prior state until caches are cleared.

CommentFileSizeAuthor
Selection_999(5863).jpg28.31 KBamitaibu

Issue fork og-3571426

Command icon 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:

About issue forks

Comments

amitaibu created an issue. See original summary.

amitaibu opened merge request !68

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented
Status: Active » Needs work

New test is still failing

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented
Status: Needs work » Fixed

Fixed tests locally.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented
Status: Fixed » Needs review
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Ok my skills in the access and cache are limited so I leaned heavy on an AI for this review...

“The og_permissions cache context is computed from the current user and the current OG context group. But OgAccess::userAccess() can be called for a different $user or $group (e.g., access checks for other accounts or for group content where the active OG context is different). In those cases, adding og_permissions causes the access result to be cached under the wrong context, so later requests reuse an access decision that was computed for a different user or group.”

Happy to push up the test and fix it came up with but not if that sounds suss to anybody with more knowledge. The fix is:

“The fix only adds the og_permissions cache context when the access check is for the current user and the current OG context group matches the $group being checked. Otherwise, it skips og_permissions so access results aren’t cached under an unrelated user or group context. This avoids cross‑user/group cache pollution while still allowing the edit‑tab cache to update for the active user in the active group.”

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented

Thanks @joelpittet, you and your AI are right :)
We also have to check whether the group being checked is the one from the `ogContext`.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

@amitaibu I took a another review but I need to give it a bit more context because when I spot something like "removing reset() is a BC break" in the response I know it wants a bit of guidance ;) So I will give it some more.

But to keep you not waiting for my post this is it's hot take

In src/OgAccess.php, when isCurrentUserAndGroup() is false but the user is the current user, the code falls back to adding user as a cache context.

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented

> "removing reset() is a BC break"

I think it should have been removed before we tagged 2.0. AFAIK, It was never used, and been hanging there for years.

> In src/OgAccess.php, when isCurrentUserAndGroup() is false but the user is the current user, the code falls back to adding user as a cache context.

I think that's correct. That is, we tell caching to cache by the user. Meaning that if the user is changed, it will call the access function again.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Yeah exactly, removing the reset() not an issue, good clean-up, that's why I want to scrutinize a bit closer what it's saying.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Fixed

The video was a really nice illustration thanks @amitaibu I started the merge train. 🚂

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • joelpittet committed ae32b933 on 2.x authored by amitaibu 
    fix: #3571426 Improve cache context, to allow edit tabs to appear after...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.