Skip to content

Conversation

@tom93
Copy link
Contributor

@tom93 tom93 commented Jul 3, 2025

Reminder for myself to look into this (help welcome). I thought it would be a simple but I get errors; maybe this needs to wait for Rails 5.1, or maybe I misunderstood the documentation, specifically what they mean by "return". Relevant Rails code: deprecation, removal.

@tom93
Copy link
Contributor Author

tom93 commented Jul 3, 2025 via email

@Holmes98
Copy link
Member

Holmes98 commented Jul 5, 2025

I think I tried the configuration to get the new behavior and it didn't help

If you were setting it in config/application.rb, it's being overridden by this:

# Do not halt callback chains when a callback returns false. Previous versions had true.
ActiveSupport.halt_callback_chains_on_return_false = true

I'll push some changes here to opt into the new behavior, hopefully that's fine.

@Holmes98 Holmes98 force-pushed the rails-5-before-callback-return branch from 35de223 to d950b8c Compare July 5, 2025 13:07
@Holmes98 Holmes98 marked this pull request as ready for review July 5, 2025 13:07
@tom93
Copy link
Contributor Author

tom93 commented Jul 5, 2025 via email

@coveralls
Copy link

coveralls commented Jul 5, 2025

Coverage Status

coverage: 37.627% (-0.03%) from 37.654%
when pulling 798324f on rails-5-before-callback-return
into 91646f5 on master.

Holmes98 and others added 2 commits July 11, 2025 18:57
In Rails 5.0, halting a 'before' callback by returning false is
deprecated in favour of `throw(:abort)`, and in Rails 5.1 that feature
is removed.

All our callbacks currently return true, so this shouldn't cause any
changes in behaviour.

See https://guides.rubyonrails.org/v5.2/upgrading_ruby_on_rails.html#halting-callback-chains-via-throw-abort
These were only necessary to avoid halting callback chains under
the previous behaviour.
@Holmes98 Holmes98 force-pushed the rails-5-before-callback-return branch from d950b8c to 798324f Compare July 11, 2025 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants