Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use expect for nested params in auth/setup#update #33657

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

mjankowski
Copy link
Contributor

Background: #33644 (comment)

I'd been planning to do some of these as a post-rails-8 thing, and that rubocop PR reminded me. I suspect they will add/expand the cop over time to catch more conditions ... and we actually have a lot less of these than I would have guessed, since much of the API controllers are not nested (ie, just have top-level params.permit(...) but not nested require/permit combo).

For this first PR, I've done the simplest example I could find along with a spec to show what changes (prior to change, this would error out as 5xx response, post-change it's handled as 4xx due to invalid params).

Assuming we're good with the change and not worried about issues from the changing (but probably improved?) response codes, I'll do larger future batches, and/or just bulk autocorrect.

@mjankowski mjankowski added ruby Pull requests that update Ruby code lint fix 💅 labels Jan 20, 2025
@ClearlyClaire ClearlyClaire added this pull request to the merge queue Jan 21, 2025
Merged via the queue into mastodon:main with commit 45149cd Jan 21, 2025
37 checks passed
@mjankowski mjankowski deleted the expect-first-one branch January 21, 2025 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lint fix 💅 ruby Pull requests that update Ruby code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants