7101 - Fix Prosopite N+1 harness and clear datatable + request specs

[suttondemlong]

What github issue is this PR for, if any?

Resolves #7101

What changed, and why?

Part 1 of the Prosopite work: make N+1 detection actually run, then clear spec/datatables and spec/requests and start enforcing them. Three commits, each green on its own so the history bisects.

1. 30f5d4b — fix the harness. spec/.prosopite_ignore documented itself as "scanned but won't raise, only log", but ignored examples ran with Prosopite.enabled = false, so they were not scanned and nothing was logged either. Since every spec directory was listed, nothing was detected anywhere. Raising is now opted into per example via Prosopite.start_raise, so an ignored directory genuinely logs to log/prosopite.log without failing the build.

Two further gaps stopped enforcement from being usable:

• The factory pause patched FactoryBot::SyntaxRunner, but spec/support/factory_bot.rb includes FactoryBot::Syntax::Methods into example groups, so create from a spec or let block never went through it. Patched the module that is actually included — this alone removed 8 false-positive failures in spec/datatables.
• Prosopite reports repeated queries a spec causes in its own expectation loops. Enforcement now fails an example only when the call stack reaches app/ or lib/. (Rails' backtrace cleaner already silences spec/, so "no app frame" means it came from test code.) I verified each residual failure with a widened cleaner rather than assuming — 7 of 9 were spec-side, e.g. volunteer_datatable_spec.rb:270.

Also allowlisted per-record validations/has_one initialisers and the operations that write one record at a time by design (CSV import, org defaults, bulk supervisor assignment, copying a draft contact per case). Each runs one INSERT plus that row's belongs_to checks, so there is no collection to eager load.

2. e657527 — the N+1 fixes.

VolunteerDatatable ran three queries per row. made_contact_with_all_cases_in_days? and hours_spent_in_days are aggregates that cannot be preloaded, so they are computed once per page as grouped queries. Before changing them I probed the existing semantics and preserved them exactly, including one surprise: the contacted-case count is deliberately not DISTINCT, because Volunteer#cases_where_contact_made_in_days counts contact rows rather than cases. Keeping that makes this a performance change only (see the out-of-scope note below).

Languages are preloaded against the already-loaded page rather than through raw_records. Adding includes(:languages) there breaks the extra_languages filter with PG::UndefinedColumn on default_sort_order: that relation carries SELECT aliases, DISTINCT and an ORDER BY on an alias, and an includes makes Rails build an id-lookup query that repeats the alias in its own SELECT list. This is why the existing index_relation scopes its own includes(:languages) the way it does.

CasaCase#next_court_date and #most_recent_past_court_date were scoped queries on the court_dates association, so they re-ran per case and silently defeated any includes(:court_dates). They now filter in Ruby when the association is loaded, which also fixes the missing-data report.

The remainder are missing eager loads on collections rendered per row: the case group form (assigned volunteers for every case in the org), the court report case picker, Supervisor#volunteers (added :supervisor — Volunteer#supervisor is its own has_one :through, so preloading the join row does not satisfy it), notifications' patch note types, the all-CASA dashboard's per-org counts (batched, verified against the per-org methods including the cross-org edge case), the contact type "last logged" hint (one lookup instead of one per type), the contact form's contact_topic_answers topics so nested saves validate belongs_to from memory, the new contacts table's row policy associations, and CaseCourtReportContext (interviewees' contact types and case contacts, plus resolving the last hearing date once instead of per caller).

3. 404825c — drop spec/datatables and spec/requests from the ignore list.

One decision worth a reviewer's opinion

The intentional per-record write loops are declared in spec/support/prosopite.rb via allow_stack_paths, to keep test tooling out of application code. PROSOPITE_TODO.md suggests wrapping them in Prosopite.pause instead, which would put the intent at the call site and also quiet the development rack middleware. Happy to switch if you prefer that.

How is this tested? (please write rspec and jest tests!) 💖💪

The enforcement itself is the test: with these two directories un-ignored, any new N+1 in a datatable or request path fails the build. Baseline before the fixes was 25 failures in spec/datatables and 44 in spec/requests, all Prosopite.

bundle exec rspec spec/datatables spec/requests   # 973 examples, 0 failures, 1 pending
bundle exec rspec                                 # 3721 examples, 0 failures, 21 pending
bundle exec standardrb                            # clean
bundle exec erb_lint <4 changed .erb files>       # no errors

Each commit verified green individually, not just the tip.

Semantics were checked rather than assumed: for the three aggregates I replaced, and for the batched all-CASA counts, I compared the new grouped queries against the original per-record methods on fixtures covering the awkward cases (a volunteer with more contacts than cases, an inactive assigned case, an org-less draft contact, a contact on one org's case created by another org's volunteer). No existing expectations changed, and no new ones were needed — the values are identical by construction.

Not covered: spec/system and the other still-ignored directories remain log-only, so N+1s there are written to log/prosopite.log but do not fail. Those are the later parts described in #7101.

Screenshots please :)

No visual change is intended; the four touched views (all_casa_admins/dashboard/show, case_groups/_form, case_contacts/form/_contact_types, case_contacts/form/details) only change which records are preloaded and where a value is read from, not the markup or the rendered values.

Follow-up

Not in this PR:

• app/views/volunteers/index.html.erb:107 calls hours_spent_in_days(30) inside the row loop — the same N+1 as the datatable, still live on the migrated index. The current specs do not catch it because the fixtures are too small to cross min_n_queries. Fixing it means lifting the batching helpers out of the datatable into shared code.
• User#no_attempt_for_two_weeks and #volunteers_serving_transition_aged_youth have no callers left after the Tailwind migration removed SupervisorDatatable; they look like dead code now.
• The two pre-existing behaviour bugs listed under "Out of scope" in Prosopite N+1 detection is inert: fix the harness and enforce it per directory #7101.

[compwron]

Looks very promising :)

[suttondemlong]

Looks very promising :)

Thanks! Sean asked me to use the resources from the DataAnnotation thing to do some work in here. Once I can verify and add screenshots I’ll mark as ready for review!