Skip to content

Store collection title on Recent Activity share rows - #26893

Open
patrickmann wants to merge 4 commits into
masterfrom
fix/collections-share-recent-activity
Open

Store collection title on Recent Activity share rows#26893
patrickmann wants to merge 4 commits into
masterfrom
fix/collections-share-recent-activity

Conversation

@patrickmann

@patrickmann patrickmann commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Closes Graylog2/graylog-plugin-enterprise#14983

Description

Sharing a collection produced a Welcome page Recent Activity row showing the collection's raw ObjectId instead of its title, for example The collection 689abc... was shared by Jane Doe.

Recent Activity resolves titles at read time through the Catalog, which is built from content pack entity excerpts. Collections have no content pack facade, so the lookup misses and the list falls back to the title stored on the row. The share and unshare paths never stored one, so the title was null and the UI rendered the id. This is the same root cause as #14932, on a different, shared core code path (RecentActivityUpdatesListener).

RecentActivityUpdatesListener.createRecentActivityFor(EntitySharesUpdateEvent) now resolves the entity title once via GRNDescriptorService and stores it on the SHARE and UNSHARE rows. The change is additive: read-time catalog resolution still wins for facade-backed types (streams, dashboards, and the rest), so only facade-less types like collections use the stored title.

The subscriber also returns early when the event has no creates and no deletes. EntitySharesService posts the event unconditionally, so a capability-only change previously resolved a title that was then discarded.

Resolution is guarded. GRNDescriptorService.getDescriptor throws for a GRN type with no registered provider, and this runs inside an event subscriber, so a failure falls back to a null title (today's behavior) rather than dropping the activity. Only that missing-provider case is expected and logged at debug level; anything else, such as a database failure, is logged at warn.

How Tested

  • New RecentActivityUpdatesListenerTest: share and unshare store the resolved title and resolve it only once per event; the duplicate grantee in creates() is filtered out; a GRN type with no descriptor provider (output, using a real provider-less GRNDescriptorService rather than a stubbed throw) still records the row with a null title and does not throw; a provider placeholder title is stored as-is; a capability-only change records nothing and resolves no title.
  • StartPageServiceTest: a catalog hit wins over the stored title, and only a catalog miss falls back to it. Both new assertions were checked against a mutation: flipping the precedence reddens only the new case, and moving the title resolution inside the per-row loop trips the new single-resolution check.
  • Manual (enterprise, collections on by default):
    1. Create a collection.
    2. Share it with another user or team.
    3. Open the Welcome page and check Recent Activity: the row reads The collection <title> was shared by <user>, not a raw id.
    4. Unshare it and confirm the unshare row also shows the title.

Notes

Resolving the descriptor at write time is safe here in a way it would not be at read time. #26785 rejected a GRNDescriptorService fallback inside StartPageItemTitleRetriever because a provider reports a missing entity with a placeholder title rather than an absent one, which would have surfaced those placeholders for every entity type on every read. At write time the shared entity normally exists, so the resolved title is real.

Two known limitations, both accepted:

  • The stored title is a snapshot. Renaming a facade-less entity after it was shared leaves the old share row showing the old title. This matches the create, update and delete rows for the same types, and is still better than the raw ObjectId shown today.
  • If the entity is already gone when the title is resolved (a share submitted from a stale Share modal, or a delete racing the async event bus), the row stores the provider's placeholder string instead of a title. Filtering it is not worth it: the placeholder is an ordinary display string with no marker, so a prefix filter would blank legitimate titles such as a stream named ERROR: prod ingest. These placeholders are already visible elsewhere, for example on the Grants Overview page.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • My code follows the code style of this project.
  • I have added tests to cover my changes.

patrickmann and others added 4 commits August 5, 2026 10:24
Sharing or unsharing a collection recorded a Recent Activity row with no
item title, so the Welcome page rendered the raw ObjectId. Collections
have no content pack facade, so the read-time catalog lookup misses and
falls back to the stored title, which was null for the share and unshare
paths.

Resolve the entity title at write time via GRNDescriptorService and store
it on the SHARE and UNSHARE rows, guarded so a type without a descriptor
provider still records the activity (title stays null and is resolved
from the catalog at read time, as before).

Closes Graylog2/graylog-plugin-enterprise#14983

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Skip the event entirely when there is nothing to record, so a
capability-only share change no longer resolves a title it discards.

Log an unexpected descriptor failure at WARN. Only the missing-provider
case is expected (outputs shared via a content pack install have none),
so a Mongo failure no longer degrades silently at DEBUG.

Fix the resolveTitle comment: the read path prefers catalog resolution
and only falls back to the stored title, not the other way round.

Tests: resolve the title once per event rather than per row, cover the
duplicate-grantee filter and the capability-only branch, use OUTPUT with
a real provider-less GRNDescriptorService instead of stubbing a throw for
a type that does have one, and pin that a provider's placeholder title is
stored as-is. Add a StartPageService case pinning that a catalog hit wins
over the stored title and only a miss falls back to it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@patrickmann
patrickmann requested a review from a team August 5, 2026 12:09
@patrickmann
patrickmann marked this pull request as ready for review August 5, 2026 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant