[Views] Declare strict locals on the remaining partials that take locals

[Luke-Oldenburg]

Note

This PR's last two commits (removing local_assigns reads, and passing instance variables to partials as locals) moved into the stacked PRs below, so this PR is back to the version that was approved (its first two commits). Review and merge them in order. Each PR's base branch is the PR before it, so its diff shows only its own change.

1. [Views] Declare strict locals on the remaining partials that take locals #15191 Declare strict locals on the remaining partials that take locals ← this PR
2. [Views] Read strict locals instead of local_assigns #15200 Read strict locals instead of local_assigns
3. [Views] Remove partial ivars that can't simply become locals #15201 Remove partial ivars that can't simply become locals
4. [Views] Pass layout, login and mailer ivars to partials as strict locals #15202 Pass layout, login and mailer ivars to partials as strict locals
5. [Views] Pass donation page and FullStory ivars to partials as strict locals #15203 Pass donation page and FullStory ivars to partials as strict locals
6. [Views] Pass filter values to the filter partials as strict locals #15204 Pass filter values to the filter partials as strict locals
7. [Views] Pass card, card grant and transfer ivars to partials as strict locals #15205 Pass card, card grant and transfer ivars to partials as strict locals
8. [Views] Pass event home, settings, invoice and G Suite ivars to partials as strict locals #15206 Pass event home, settings, invoice and G Suite ivars to partials as strict locals
9. [Views] Pass comment, receipt form and HCB code history ivars to partials as strict locals #15207 Pass comment, receipt form and HCB code history ivars to partials as strict locals
10. [Views] Pass @event and row options through navs and transaction rows as strict locals #15208 Pass @event and row options through navs and transaction rows as strict locals

Summary of the problem

This finishes the strict locals work from #15188, #15189 and #15190. The 100 partials left were the ones that take locals, plus comments/reactions/react.turbo_stream.erb, a full template that Comment::ReactionsController renders with locals: { comment: }. After this, all 481 partials under app/views declare strict locals.

Most of them read their locals in ways that stop working once a declaration exists:

• defined?(x) is true as soon as x is declared, so it can't mean "the caller passed x" anymore.
• local_assigns[:x] only holds what the caller passed. It never sees a declared default.
• Callers pass locals that the partial never reads, and strict locals rejects those.

A few of these hid bugs, and fixing them naively would have changed what renders.

Describe your changes

Each declaration comes from the partial's callers. A local is required when every caller passes it and the body reads it. Otherwise it gets a default: the file's existing defined?(x) ? x : … or x ||= … value, or nil. As in #15188, the declaration goes on line 1 with a blank line after it.

Reading locals directly

• defined?(x) becomes a plain check on x, with a nil default. I checked the values every caller passes. Existing callers get the same result, except the three changed below and the hcb_codes/_icon attribute under Rendered output.
• local_assigns[:x] becomes x. [Views] Read strict locals instead of local_assigns #15200 does the same in the partials that already declared their locals.
• x ||= default moves into the declaration where the default is a literal and no caller passes nil. _callout's icon ||= stays, because its default depends on type.

Callers that relied on defined? treating false as present. Each now passes the value that keeps today's output:

• reimbursement/expenses/_receipts passed hide_info: false, and receipts/_receipt hid the info anyway. It now passes hide_info: true.
• payments/show passed enable_linking: false, and receipts/_form_v3 showed "Select from Receipt Bin" anyway. It now passes enable_linking: true.
• CommentsController#destroy passed show_blankslate: commentable.comments.empty?. Because defined? treated any value as set, the blank slate showed whenever the viewer's policy_scope(comments) was empty. It now passes show_blankslate: true.

Other fixes the declarations needed

• application/_footer used a help_message local as an on/off flag, but help_message is also an ApplicationHelper method. When a layout rendered the footer without that local, the bare name called the helper and printed the message. When logins/_footer passed help_message: nil or false, the message was hidden. A help_message: nil declaration would have blanked the footer everywhere, so the flag is now show_help_message: true and logins/_footer passes false. I checked every other declared name against the view's methods. The others (current_user, open, possessive, session, tag, title) are either always passed or never used to call the helper.
• users/_nav passed one local literally named local_assigns: { selected:, user:, open: } to users/_settings_dropdown. That only worked because non-strict compilation swaps in the inner hash. It now passes the three locals directly.
• card_grant/pre_authorizations/_organizer_info declared its locals with braces, which Rails ignores. It now uses parentheses.
• hcb_codes/_tags read @event, even though all 14 callers pass event: @event || @hcb_code.event. It now declares event: and reads the local. The transaction page and the admin ledger-audit task page both set @event to that same value, so nothing renders differently. The nested hcb_codes/_create_tag still reads @event itself, as it does for its other four callers.

Unused locals dropped from callers

• events_controller: the 9 events/home/* frames read ivars, so only tags_chart (tags:) and users_chart (users:, event:) still get locals.
• The transaction row partials overwrite event on their second line, so event: and show_amount: are gone from events/account_number, events/transactions_list and both ReceiptsController branches.
• Also dropped: show_new (admin/balances), upload_method: and shimmer: (pre-authorization page), donation: (donation goal and tiers), select_target: (hcb_codes/link_receipt), target: (deletion request form), current_user: (the receipts/extracted broadcast), stripe_card: StripeCard.new (card forms), and whats_hcb:/help_message: (logins/_footer).
• events/home/_balance_transactions and _team_stats passed event: @event outside locals:, where Rails drops it. I removed it.

Related fixes

• events/donation_overview passed is_blankslate: true outside locals:, so the blank-slate share button never got w-full. It's now inside locals:. This is the one visible change.
• hcb_codes/_decline_reason and ledger/items/_decline_reason read local_assigns[:include_external], which ignores the declared include_external: true default. They now read their locals directly. All six callers pass both flags, so nothing renders differently.

Rendered output

• The share button change above is the only visible difference.
• hcb_codes/_icon no longer adds data-action="click->transactions#select" to rows that aren't selectable. Both callers always passed selects:, so defined?(selects) was always true and those rows got data-transaction="" or "false". Cmd-clicking one inside the transactions controller ran document.getElementById("") and threw. Nothing on screen changes.
• The declaration line adds a leading newline to each partial's output. That's whitespace only, as in [Views] Declare strict locals on 85 partials that take no locals #15188.

The erb_lint StrictLocals linter is still off. Turning it on can be its own PR.

Testing

• The static render-site checker from the audit reports no mismatches between any statically resolved render call and its target's declaration. I grepped app/, lib/, config/ and spec/ for any other reference to these partials, including broadcasts and render @collection. Every one passes the required locals.
• Loaded every changed template through ActionView in rails runner. All 118 changed templates that declare strict locals compile, and so do the 13 changed full-page templates.
• Rendered 423 pages through the full Rack stack as a signed-in admin, before and after, inside a rolled-back transaction on the dev DB. Once random text, ids and timestamps are normalized, the only differences are the share button and the removed transactions#select attribute.
• Rendered 796 partial cases directly, replaying each caller's locals. With the old locals on main and the new ones here, these cover:
  • every receipts/_receipt, receipts/_form_v3 and transaction-row caller
  • the comment list and form variants, footers, headers, callouts and navs
  • all 209 activities through render_activities and the Discord fallback
• I also rendered the tag turbo streams with a temporary tag. The only difference across all of these is the transactions#select attribute.
• For the hcb_codes/_tags change, I attached temporary tags to all 116 HCB codes. I rendered each transaction page as an admin and as a regular user who was a reader on some events and a member on others, plus 40 admin ledger-audit task pages. All 272 pages match the first commit.
• bin/erb_lint and rubocop on the changed files: no offenses.
• rspec: 2474 examples, 0 failures on the first commit. After the _tags change, spec/controllers spec/requests spec/mailers spec/integration: 558 examples, 0 failures.

🤖 Generated with Claude Code

[Luke-Oldenburg]

@claude review

[claude]

Claude finished @Luke-Oldenburg's task in 18s —— View job

---

Review of #15191

I found no blocking issues. I read the diff for the controllers and for the application, logins, users, hcb_codes and comments views, but not all 100 files. I didn't run the app, the specs or the linters, so the render and test results in the description are unverified by me.

What I checked

• CommentsController#destroy: show_blankslate: true matches what the old defined? check did. Before, any value counted as set, so the blank slate showed whenever policy_scope(comments) was empty. The description explains this, and it's better than leaving a misleading commentable.comments.empty? in place.
• application/_footer and logins/_footer:
  • Renaming the flag to show_help_message: true fixes the clash with the help_message helper.
  • logins/_footer passes false, which hides the message as before.
  • users/logout used to pass help_message: false. It now relies on the footer's own false, so the outcome is the same.
• users/_nav:
  • Passing selected:, user: and open: directly instead of the odd local_assigns: hash is correct.
  • open as a strict-local name is fine here. The partial's body only uses it as a value, and the description says you checked declared names against helper methods.
• hcb_codes/_icon: if selects is a real behavior change from defined?(selects), but only for rows that pass nil or false. The description covers it and it fixes a latent JS error.
• Direct local_assigns[:x] → x rewrites in _icon, _settings_dropdown and the others I read: these are equivalent when the declared default is nil.

Minor, non-blocking

• Comment scope: the show_blankslate: true change is easy to miss when scanning the controller diff. A one-line comment there would help. Alternatively, drop the local from comments/_list and pass nothing.
• Follow-up PR: local_assigns.__id__ and local_assigns[:class] stay as the description says. Enabling the StrictLocals linter in the follow-up PR will catch regressions.

[manu-gurudath]

thank you.

[Luke-Oldenburg]

@claude review

[claude]

Claude finished @Luke-Oldenburg's task in 14s —— View job

---

Review of #15191

I found no blocking issues. I sampled the diff (the controllers, _callout, _footer, _fullstory and the transfer layout) and did not read all 392 changed files. I didn't run the app, specs or linters, so the render and test results in the description are unverified by me. The diff against origin/main is 392 files, not the 100 in the header, because it includes the earlier commits on this branch.

What I checked

• CommentsController#destroy: show_blankslate: true matches the old defined? behavior, as the description explains.
• application/_footer: show_help_message: true fixes the clash with the help_message helper. The ivars @sql_count and @sql_cached_count are now plain locals.
• application/_callout: klass replaces local_assigns[:class]. color ||= is not a declared local, but no caller passes color:. A grep for color: on application/callout renders found none. If a caller starts passing color:, strict locals will reject it.
• application/_fullstory: the content_for?(:fullstory_rendered) guard replaces the ivar and works.
• layouts/transfer: content_for?(:transfer_disabled) replaces @can_transfer. No @can_transfer references remain in app.

Minor, non-blocking

• comments_controller.rb:63: add a one-line comment on why show_blankslate: true is hardcoded, because it is easy to miss. Alternatively, drop the local if comments/_list defaults to it.
• _callout: if you want to support color: later, declare it in the strict locals.