Repository navigation
Bullet loads on its own now that it knows Active Record 8.2 - #1409
Merged
Merged
Conversation
Bullet 8.2.0 raised at load on Active Record 8.2, so since the move to Rails main it has loaded through config/bullet_on_rails_main.rb, a shim that hands it its 8.1 patches, with `require: false` on the Gemfile line so Bundler would not load it first (#1337). A canary spec, "is still needed: Bullet does not know this Active Record by itself", was written to fail the day Bullet supported 8.2 itself, and its failure message listed the removal steps. Bullet 8.3.0 does. Dependabot's grouped bundler pull request (#1408) carried it, and the canary went red there exactly as designed. This commit bumps bullet alone and follows the canary's list: the shim and its require in config/application.rb are gone, and the Gemfile line loads bullet normally. The canary example is deleted; the spec's two behaviour examples stay, renamed to spec/config/bullet_spec.rb, because after the shim goes they are what proves Bullet still runs and still catches an N+1 on this Active Record. The removal had its own red: with the shim deleted and `require: false` still set, the suite cannot boot (`uninitialized constant Bullet`), and dropping it is the green. Bumping bullet here rather than on #1408's branch keeps Dependabot's pull request untouched: once this merges, Dependabot rebases the group and drops bullet from it.
dschmura
added a commit
that referenced
this pull request
Oct 16, 2026
) #1402 shipped because nothing about the broken template looked broken. The identity picker's crop footer sent `DELETE` to a URL it was handed; one partial up, the workspace Profile handed it `workspace_path(workspace)`, an ordinary route helper whose DELETE is `workspaces#destroy`. **Remove photo** discarded the workspace, with no confirmation, and every file involved read as correct on its own. A scan after the fix (#1412) enumerated all 29 DELETE sites in the templates and found no second instance — but a hand scan proves one tree on one day. This makes the scan a code-smell spec, `spec/code_smells/destroy_requests_name_their_target_spec.rb`. ## What it checks The census reads every request a template makes — `button_to`, `link_to`, `form_with`, `form_tag`, under `app/views` and `app/components` — and when the URL or the method is one of the template's declared locals, it follows the local to every `render` that passes it, then up again until it reaches the template that names a route helper. Declared defaults are read too, which is how the confirm dialog's `confirm_method: :delete` default resolves per caller. Each DELETE that comes out is checked against the real route table: 1. it lands on a named route with a DELETE action; 2. if that action soft-deletes — a controller `destroy` that calls `discard!`, a set **derived from `app/controllers`**, not written down — the chain passes through `shared/confirm_dialog`. Today's tree passes: **33 DELETE entry points** — the 29 sites, with the archived banner's one site fanning out to its five callers and the crop footer's to its two, and the confirm dialog's declaration replaced by its seven DELETE callers. The workspace's destroy is reached only from the danger zone's dialog. The floor is 33. ## Red, two ways - **Negative control** (committed): rewrites the Profile page's `remove_url` to `workspace_path(workspace)` and must find the resulting chain reaching `workspaces#destroy` outside the dialog. - **The real bug** (one-off, not committed — CI's shallow clones can't read history): the census run over the four pre-#1412 view files from `78fbb4cd^` failed on exactly the chain an owner's click took: ```text app/views/workspaces/_profile_section.html.erb > app/views/shared/_identity_picker.html.erb > app/views/shared/identity_picker/_crop_footer.html.erb button_to workspace_path(workspace) ``` Positive controls: the census follows the identity picker through both partials to `workspaces/logos#destroy` and `settings/avatars#destroy`, and the derived soft-deleting set includes `workspaces#destroy` and `workspaces/projects#destroy`. ## Blind spots It reads template text, and prints these whenever a rule fails: collection renders (`render partial:`, four in the tree, all passing records), URLs built in Ruby, hand-written HTML method overrides, and partials nothing renders. `/docs/developer/testing` has a section on reading its failures. ## Changelog The seven pull requests merged since 2026-10-14 (#1401, #1405, #1406, #1407, #1409, #1410, #1412) had entries written before their numbers existed; each now cites its number. The five Dependabot PRs in the same window are left out, matching how routine bumps have been treated: none of the recent ones (#1385–#1389, #1396) is cited. ## Verification - The guard: 7 examples, 0 failures; with the docs-citation spec, 10 examples. - The census's 33 entries reconciled line by line against the 29-site hand inventory. - `bin/comment-block-check` (staged), rubocop and `rake markdown:check` exit 0. - Full suite: see the commit's gate.
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
1 participant
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.
Bullet 8.2.0 raised at load on Active Record 8.2, so since the move to Rails main it has loaded through
config/bullet_on_rails_main.rb, a shim that hands it its 8.1 patches, withrequire: falseon the Gemfile line so Bundler would not load it first (#1337). A canary spec — "is still needed: Bullet does not know this Active Record by itself" — was written to fail the day Bullet supported 8.2 itself, and its failure message listed the removal steps.Bullet 8.3.0 does. Dependabot's grouped bundler pull request (#1408) carried it, and the canary went red there (
test_shard (3)) exactly as designed.This bumps bullet alone and follows the canary's list: the shim and its require in
config/application.rbare gone, and the Gemfile line loads bullet normally. The canary example is deleted; the spec's two behaviour examples stay, renamed tospec/config/bullet_spec.rb, because after the shim goes they are what proves Bullet still runs and still catches an N+1 on this Active Record.Bumping bullet here rather than on #1408's branch keeps Dependabot's pull request untouched: once this merges, Dependabot rebases the group and drops bullet from it.
Closes #1337
Verification
require: falsestill set → the suite cannot boot (NameError: uninitialized constant Bullet). Droppingrequire: false→spec/config/bullet_spec.rb2 examples, 0 failures.bullet_on_rails_main), with a positive control onbullet_spec.