RuboCop mechanical cleanup: Lint, Capybara, RSpec, Naming - #2767
Merged
Conversation
… model Extract assignment out of the condition in `redirect_back` and `Feedback.submit_feedback` so the intent is unambiguous.
Defining the class inside the `namespace :setup` block triggered Lint/ConstantDefinitionInBlock. Hoist it to the top of the file so it is a top-level constant.
Capybara/RSpec/NegationMatcherAfterVisit flags `have_no_*` called right after `visit`, because negation matchers return immediately before the page has reached its expected state. Add a positive assertion (current_path or page element) first so the negation only runs once the page has settled. Also switch `visible: true` to `visible: :visible` in chapters_sidebar_component_spec (Capybara/RSpec/VisibilityMatcher).
Replace allow_any_instance_of with the codebase's established pattern of stubbing Services::MailingList.new to return an instance_double. login_helpers is left for a separate refactor.
Wrap the "shows all details" style scenarios in aggregate_failures so they report every mismatch in one run instead of aborting on the first, resolving RSpec/MultipleExpectations.
The mandatory-fields scenario exceeded the example length limit. Pull the repeated form-fill block into fill_in_mandatory_event_fields so the scenario stays focused on assertions.
Naming/AccessorMethodName flags set_-prefixed methods that take an argument. These set associated records rather than a single attribute, so rename to assign_organisers, assign_chapters, assign_host, assign_coaches across their callers.
The method name mixed uppercase (RSVP) and a has_ predicate prefix. Rename to existing_rsvp_on? to satisfy Naming/MethodName and use the conventional predicate suffix.
Renaming has_existing_RSVP_on to existing_rsvp_on? in the MethodName commit also resolved its PredicatePrefix offense, so member.rb no longer needs excluding. The remaining has_/is_ predicates are left for a dedicated refactor (is_logged_in? collides with the existing logged_in? boolean).
mroderick
marked this pull request as ready for review
July 30, 2026 13:49
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.
Problem
.rubocop_todo.ymlcarried 188 offenses. The first batch — the mechanical, low-risk cops across Lint, Capybara, RSpec, and Naming — can be cleared without behavior change, one commit per rule.Changes
11 commits, each fixing one RuboCop rule and removing its
.rubocop_todo.ymlentry:ApplicationController#redirect_backandFeedback.submit_feedback._prefix onmember_no_rsvpingroup_spec.superinEventCardComponent#initialize.SetupCheckerout of thenamespace :setupblock to the top ofsetup.rake.visible: truetovisible: :visibleinchapters_sidebar_component_spec.Services::MailingList.newwith aninstance_doubleinmember/details_controller_spec(the codebase's established pattern).aggregate_failures.fill_in_mandatory_event_fields.set_*writer methods toassign_*across their callers.has_existing_RSVP_ontoexisting_rsvp_on?(snake_case predicate).Deliberately not in this PR
These need judgment or config decisions, not mechanical edits:
page.save_screenshotinspec_helperis a legitimate CI failure-screenshot capture. Whitelisting it means overriding the CapybaraDebuggerMethodsgroup, which is brittle across RuboCop versions. Worth a focused config PR.is_logged_in?would rename tologged_in?, which already exists as a different boolean method (the before-action calls it). Same collision risk for the policyis_admin_or_organiser?/is_chapter_organiser?predicates, which are referenced across many controllers and policies. Needs a dedicated rename PR with caller updates and collision resolution.spec/support/helpers/login_helpers.rb— the app-wideallow_any_instance_of(ApplicationController)login stub used by feature and controller specs. Replacing it touches the test login mechanism for the whole suite.These remain in
.rubocop_todo.yml.Verification
bundle exec rubocop— 360 files, no offenses.bundle exec parallel_rspec spec/ -n 3— 1166 examples, 0 failures.Result
188 to 167 offenses in
.rubocop_todo.yml.