Inconsitencies in query alters, for the insider role scope.

Very detailed report, thanks for that! I will have a look at the cause you described and see if this can be fixed.

Okay, I think I see what's going on here:

• You have an insider admin role synced with the Drupal admin role (that you have)
• You have an outsider regular role which can view published groups synced with the Authenticated user Drupal role
• Because your insider or outsider status is only confirmed on the query level, we need to add both to the query

Admin roles map to the simpler $allowed_ids in GroupQueryAlter::doAlter(), but then it still processes the outsider role and adds the status condition and contradiction membership condition.

This is because all conditions are glued together using AND rather than OR. When we have both non-status-relevant and status-relevant items, we need to make sure they get added to an OR group.

Then the query becomes:

WHERE
  (
    (`groups`.type IN ('organisation'))
    AND (gcfd.entity_id IS NOT NULL)
  )
  OR # THIS WAS ERRONEOUSLY AND
  (
    (groups_field_data.status = 1)
    AND
    (
      (groups_field_data.type IN ('organisation'))
      AND (gcfd.entity_id IS NULL)
    )
  )

This is how I would probably fix it. Needs tests and we might need to check the other query alters for similar logic.

Okay so those going green is:

• A good thing because it does not break any existing tests
• A bad thing because it clearly still needs a test for the scenario from the issue summary

Signing off for the day, but what we need now is a test that goes red without my patch and green with.

GroupRelationshipQueryAlter suffered from the same problem. EntityQueryAlter did not because it always adds an OR-group at the top level. Still needs tests.

This one might break things and turns off some existing tests, but it's a complete, more thorough approach so we can test mixed cases more easily. The one that was reported here still needs to be added.

This should fix the failures for GroupQueryAlter, still needs others fixed and expanded test coverage.

This adds back the unpublished checks and adds checks for the issue that caused this issue (one scope has admin, the other does not)

This should fix group access. Before we commit, we should take out the fix and see if the mixed cases fail then.

This should fix all of the group tests. Then we can move on to reviewing the other query alters.

Forgot to uncomment a few things to make local testing easier...

Might see some cacheability fails now, but the unsupported actions should go green now.

Copy-paste mistake

Confirmed locally that the 3 view tests fail for mixed scope (admin v regular) when I comment out the new code. So fixed!