Skip to content

Expand Prosopite N+1 enforcement: 12 newly-surfaced N+1s across 6 directories #6912

@compwron

Description

@compwron

Context

Prosopite is wired to raise on N+1 in tests (`spec/support/prosopite.rb`), but `spec/.prosopite_ignore` currently opts out every spec directory — so the gem catches nothing in CI. `PROSOPITE_TODO.md` tracks known N+1s but is incomplete.

I tried removing the six "unflagged" directories from `.prosopite_ignore` (`helpers`, `notifications`, `seeds`, `mailers`, `presenters`, `config`) and running those specs. 12 failures surfaced across all six — meaning none of them are clean today.

What surfaced

`spec/presenters` — 4 failures

  • `spec/presenters/case_contact_presenter_spec.rb:18`, `:25`, `:33`, `:40` — `CaseContactPresenter#display_case_number`. Every call iterates and hits the DB. Likely fix: preload at the controller level or change the lookup data structure.
  • (Note: this is also why issue Retire app/presenters in favor of Draper decorators #6909 — retire presenters in favor of Draper — is worth doing; the rewrite can fix this in passing.)

`spec/helpers` — 2 failures

  • `spec/helpers/ui_helper_spec.rb:12` — `UiHelper#grouped_options_for_assigning_case` doesn't eager-load.
  • `spec/helpers/notifications_helper_spec.rb:56` — `NotificationsHelper#patch_notes_as_hash_keyed_by_type_name`.

`spec/mailers` — 2 failures

  • `spec/mailers/supervisor_mailer_spec.rb:122`, `:134` — `SupervisorMailer.weekly_digest` N+1s when iterating volunteers' `case_contact`s.

`spec/notifications` — 1 failure

  • `spec/notifications/reimbursement_complete_notifier_spec.rb:17` — `ReimbursementCompleteNotifier#message` queries mileage rate per call.

`spec/seeds` — 1 failure

  • `spec/seeds/seeds_spec.rb:33` — seeds run once; this may be acceptable to wrap in `Prosopite.pause` rather than fix.

`spec/config` — 2 failures

  • `spec/config/initializers/rack_attack_spec.rb` — `ApplicationController#set_current_user` and `Accessible#check_user` repeat user/admin lookups across the test's repeated requests. This is likely a false positive from the rack_attack throttle test pattern (each request legitimately re-looks-up the user). Probably fix with `disable_prosopite: true` on those examples.

Proposal

Treat each directory as a separate small PR:

  1. `spec/seeds` and `spec/config` — cheap wins; mostly false positives that just need a metadata flag.
  2. `spec/notifications` — single N+1, scoped fix.
  3. `spec/mailers` — eager-load in the weekly digest query.
  4. `spec/helpers` — fix at the helper level or the call site.
  5. `spec/presenters` — folds into Retire app/presenters in favor of Draper decorators #6909.

Each PR removes the relevant directory from `.prosopite_ignore` and updates `PROSOPITE_TODO.md`.

Why

Once a directory is enforced, no future PR can silently introduce an N+1 there. That's the compounding value — it grows as the schema grows. Today the gem is decorative because the ignore list is total.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    Status

    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions