Remove user-specific badges from event cards to fix public visibility of Manage (#2869) - #2870
Open
mroderick wants to merge 3 commits into
Open
Remove user-specific badges from event cards to fix public visibility of Manage (#2869)#2870mroderick wants to merge 3 commits into
mroderick wants to merge 3 commits into
Conversation
User-dependent badges (Attending, Manage) were rendered inside the cache fragment of EventCardComponent, whose key contains no user context. Dashboard renders pass a user while public listings do not, so a card cached for an organiser was served to every visitor, including signed-out users, making the Manage badge publicly visible on event listings (issue #2869). Remove the badges and the user parameter entirely: the card renders only event data and is cached per event version and locale. The one-off :v2 key suffix invalidates badge-bearing fragments already in production cache. Pins the user-agnostic contract in specs, asserts cross-locale fragment isolation, and covers the leak scenario at feature level: an attending member warms the card cache and a second member must not see the first member badge. Organisers reach admin pages via the navigation Admin Menu, which makes the per-card Manage shortcut redundant (it was a dead link for meetings anyway). MemberPresenter methods that only served the badges (organiser?, event_organiser?, attending? and private helpers) are removed.
The badge removal deleted the last production caller (MemberPresenter#attending?) of Member#attending_event_ids, leaving a memoized set query and its invitation after_save invalidation hook maintaining a memo nobody reads. Delete the methods, the hook, and their specs.
The event card renders chapter badge, venue and organiser avatars, but /events/upcoming and chapter pages did not preload organisers, which costs per-card queries on cache miss, and workshop queries lacked venue data. Align the queries with DashboardQuery using eager_load so the where-scoped organisers through-association preloads correctly. A request spec asserts the SQL cost of the cache-miss render path stays bounded as the number of rendered cards grows.
mroderick
marked this pull request as ready for review
September 11, 2026 17:34
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2869
Summary
The Manage badge on event cards was publicly visible on event listings, including to signed-out visitors. The card component rendered user-specific badges (Attending, Manage) inside a fragment cache keyed only on the event, while dashboard renders passed a user and public listings did not. A card cached for an organiser was therefore served verbatim to everyone until the event changed.
This removes user-specific content from the card entirely: the badge code, the user parameter, and the MemberPresenter methods that only served them. The card is now cacheable per event version and locale, which is what the caching introduced in #2684 intended.
Changes
user:parameter. Organisers reach admin pages through the navigation Admin Menu, which makes the per-card shortcut redundant (it was a dead link for meetings anyway).[cache_key_with_version, I18n.locale, :v2]. The:v2suffix invalidates badge-bearing fragments already in production cache, so they stop serving after deploy; the locale component stops anfr/delocale cookie from poisoning cached card dates for other visitors.Member#attending_event_ids, its invalidation hook, andMemberPresenter#organiser?/#event_organiser?/#attending?had no remaining callers./events/upcomingand chapter render paths so the cache-miss path does not gain per-card queries.Review notes
app/components/event_card_component.html.erb:1). The:v2bump is a deploy-time invalidation decision: without it, stale badge-bearing fragments keep serving until expiry. If you would rather clear Solid Cache in the deploy task instead, that is the alternative.eager_loadvsincludes— the organisers association has awhere('permissions.name' => ...)scope that only preloads undereager_load(JOIN), which is why the render paths now mirrorDashboardQuery's pattern.Workshophas novenueassociation (venue is thehostsponsor), so workshops preload:workshop_host.member_portal_spec.rb(attending member warms the cache, second member must see no badge); component-level cache isolation and a query-cost bound are covered in the component and request specs.Detail: how the leak worked
EventCardComponentwraps the whole card incache @event.cache_key_with_version. Dashboard views render the component withuser: current_user; public listings render it with no user. Because the fragment key contained no user context, the first render to warm a card's fragment (typically an organiser's dashboard visit, who legitimately saw Manage) cached the badge-bearing HTML, and every later render of that card on any page — including signed-out visits to/events/upcoming— received it. Tests missed this because the test environment uses:null_store, so the component spec exercises fragment caching only after swapping in a real store; the regression tests now do exactly that.