Skip to content

IQSS/12742 - Fix for LocallyFAIR indexing for explicit groups - #12749

Open
qqmyers wants to merge 2 commits into
IQSS:developfrom
GlobalDataverseCommunityConsortium:IQSS/12742-LocallyFAIR-indexing-issue-for-explicit-groups
Open

qqmyers wants to merge 2 commits into
IQSS:developfrom
GlobalDataverseCommunityConsortium:IQSS/12742-LocallyFAIR-indexing-issue-for-explicit-groups

Conversation

@qqmyers

@qqmyers qqmyers commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

What this PR does / why we need it: This resolves an issue reported in #12742 that appears to have its root in a difference in how Explicit groups map between alias and identifier that is different from other types. Since the indexing being done for Locally FAIR content was not accounting for this difference, members of explicit groups could go directly to Locally FAIR collection/data/file pages they should have access to (if they knew the alias, DOI, identifier, etc.) but would not see that content in search results or collection page listings (since those are powered by solr. The current PR resolves this issue.

In addition, the PR adds support for the builtin groups (All and Auth users - All would be a useless choice for LF, but AuthenticatedUsers could be useful - must login to see content). They use a different prefix (:) and weren't addressed in the initial LF PR.

Which issue(s) this PR closes:

Special notes for your reviewer: FWIW: The convertToIndexableString method was designed to avoid doing a db loookup of the group to get it's alias (for performance), but that means that the code has to convert from identifier to alias and explicit groups need to drop "&explicit/" for this whereas the alias and identifier for shib, ip, persistent global groups just differ by '&'. Not sure there's any reason for that.

Tests are added to assure that the conversion is correct relative to the internal logic in the various group classes.

W.r.t. release notes, the permissions for LF content have to be reindexed. We now have the permission reindex APIs documented and they can be used, but the per-collection perm reindex api only takes a collection id, whereas the general reindex api (content and permissions) allows you to use the collection alias (easier to find). I'm not sure what the best option is to recommend. The release note currently has both options.

Suggestions on how to test this: Install the PR, enable LocallyFAIR content (if not done already) and create a LF collection, subcollection, and dataset. Verify that you can see the subcollection and dataset in the collection listing as a member of an explicit group defined on the collection. Regression test that you can also see the subcollection and dataset pages, that the public can't see this content, etc. (Regression is unlikely as the change only affects indexing.) If Locally FAIR content already exists on the test machine, you can just add an explicit group to it and test that way, reindexing the LF content after installing the PR.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?: included.

Additional documentation:

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Size: 10 A percentage of a sprint. 7 hours.

Projects

Status: Ready for Review ⏩

Development

Successfully merging this pull request may close these issues.

Locally FAIR: group assignees on the "Published access limited to" list are ignored; only direct user assignees grant access

1 participant