Skip to content

Fix unrelated N+1 queries found during the bucket N+1 investigation - #11883

Merged
nbudin merged 2 commits into
fix-user-signups-bucket-n-plus-onefrom
investigate-unrelated-n-plus-ones
Aug 6, 2026
Merged

Fix unrelated N+1 queries found during the bucket N+1 investigation#11883
nbudin merged 2 commits into
fix-user-signups-bucket-n-plus-onefrom
investigate-unrelated-n-plus-ones

Conversation

@nbudin

@nbudin nbudin commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

While chasing the bucket/registration_policy N+1s, the Sentry sweep also turned up a few other N+1 issues clearly unrelated to that work. This PR fixes the two with real scaling impact:

1. active_storage_attachments N+1 (INTERCODE-183/188)

EventDrop's description/short_blurb/content_warnings/age_restrictions/participant_communications methods each independently called event.images.includes(:blob) to resolve inline image references in markdown. .includes always issues a fresh query regardless of Rails' association cache, so rendering N of these fields on the same event fired N separate queries -- not just one per event as it might look at a glance. Memoized the attachment lookup once per EventDrop instance.

2. user_con_profiles N+1 (INTERCODE-184/185)

SignupBucketFinder::FakeSignup.from_signup reads signup.user_con_profile for every existing signup on a run when building its comparison set for bucket-finding. EventChangeRegistrationPolicyService already preloaded :user_con_profile before doing this (from an earlier review pass); EventSignupService#bucket_finder and ExecuteRankedChoiceSignupService#actual_bucket didn't. Brought both in line with the existing pattern.

Not fixed here

Also found: cms_partials (INTERCODE-182, single occurrence) and deprecated_graph_ql_usages insert (INTERCODE-181, not actually an N+1 -- Sentry's detector flagged deprecated-field-usage logging, which is intentional per-operation behavior, not a bug). Neither had enough signal or scaling impact to justify a fix; noting them here for visibility rather than silently dropping them.

Test plan

  • New regression tests for both fixes, each verified to fail without the fix and pass with it:
    • EventDrop: 2 queries → 1, across markdown field calls on the same event
    • ExecuteRankedChoiceSignupService: query count no longer scales with existing-signup count (was 6→26 for 1→20 existing signups; now flat)
    • EventSignupService: 11 queries → 2 for 10 existing signups
  • Full test/liquid_drops/, test/services/event_signup_service_test.rb, test/services/execute_ranked_choice_signup_service_test.rb (107 tests)
  • rubocop / stree clean

🤖 Generated with Claude Code

nbudin and others added 2 commits August 6, 2026 12:01
Sentry showed an active_storage_attachments N+1 (INTERCODE-183/188,
unrelated to the bucket/registration_policy work) under Event.runs and
Event.team_members. Traced it to EventDrop's description/short_blurb/
content_warnings/age_restrictions/participant_communications methods,
which each independently called event.images.includes(:blob) to
resolve inline image references in markdown -- and since #includes
always issues a fresh query regardless of Rails' association cache,
rendering N of these fields on the same event fired N separate queries,
not just one per event.

Memoized the attachment lookup once per EventDrop instance instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ChoiceSignupService

Sentry showed a user_con_profiles N+1 (INTERCODE-184/185, unrelated to
the bucket/registration_policy work) under Mutation.createMySignup,
scaling with the number of existing signups on the run.

SignupBucketFinder::FakeSignup.from_signup reads signup.user_con_profile
for every existing signup on the run when building its comparison set.
EventChangeRegistrationPolicyService already preloaded :user_con_profile
before doing this, but EventSignupService#bucket_finder and
ExecuteRankedChoiceSignupService#actual_bucket didn't -- brought both in
line with the established pattern.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@nbudin
nbudin merged commit ac68f4e into main Aug 6, 2026
@nbudin
nbudin deleted the investigate-unrelated-n-plus-ones branch August 6, 2026 23:55
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