Conversation
|
Thanks for this @schwarzgeist , do you mind if I open this PR and use it for the rails upgrade ticket that you were working on? |
|
@klaiv please do! You're welcome to duplicate it, overwrite it, and use it however is the most helpful! |
|
Same goes for this work: https://github.com/spreedly/core/pull/10015 |
Keeping up with the latest version during the upgrade
- CallbackChain and compile are deprecated/removed in Rails 7.2
klaiv
force-pushed
the
PLAT-573-upgrade-rails
branch
from
e9889bb to
58ba817
Compare
There was a problem hiding this comment.
Pull request overview
Updates this Ripple gem to align with a newer Ruby/Rails stack needed by spreedly/core upgrade work (PLAT-573), including dependency/version bumps and a callbacks implementation change intended to work with newer ActiveSupport internals.
Changes:
- Bump
activesupportdependency to~> 7.2.2. - Bump Ripple gem version to
5.4.0and update local Ruby toolchain to3.3.7. - Simplify embedded-association callback propagation to use
run_callbacks.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
ripple.gemspec |
Updates runtime dependency on ActiveSupport (and retains older dev dependencies). |
lib/ripple/version.rb |
Bumps gem version to 5.4.0. |
lib/ripple/associations.rb |
Changes how callbacks are propagated to embedded documents. |
.tool-versions |
Updates the repo’s Ruby version used by asdf/tooling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def propagate_callbacks_to_embedded_associations(name, _kind) | ||
| self.class.embedded_associations.each do |association| | ||
| documents = instance_variable_get(association.ivar) | ||
| # We must explicitly check #nil? (rather than just saying `if documents`) | ||
| # because documents can be an association proxy that is proxying nil. | ||
| # In this case ruby treats documents as true because it is not _really_ nil, | ||
| # but #nil? will tell us if it is proxying nil. | ||
| next if documents.nil? | ||
|
|
||
| Array(documents).each do |doc| | ||
| cbs = doc.send("_#{name}_callbacks") | ||
| filtered_cbs = ActiveSupport::Callbacks::CallbackChain.new( | ||
| cbs.name, | ||
| cbs.config | ||
| ) | ||
| filtered_cbs.append( | ||
| *cbs.select do |callback| | ||
| callback.kind == kind | ||
| end | ||
| ) | ||
| runner = filtered_cbs.compile | ||
|
|
||
| env = ActiveSupport::Callbacks::Filters::Environment.new(doc, false, nil) | ||
| if kind == :before | ||
| runner.invoke_before(env) | ||
| else | ||
| runner.invoke_after(env) | ||
| end | ||
| doc.run_callbacks(name) {} | ||
| end |
There was a problem hiding this comment.
propagate_callbacks_to_embedded_associations now ignores kind and calls doc.run_callbacks(name); since run_callbacks executes both before+after, embedded docs will run their after_* callbacks during the :before propagation and then again during the :after propagation. This changes callback ordering/semantics and should fail the existing spec that expects [:embedded_before_save, :save, :embedded_after_save] for embedded associations (see spec/ripple/callbacks_spec.rb). Please update this to execute only the matching callback kind in each pass (or restructure so embedded callbacks wrap the parent operation once) so embedded before_* run before the parent and embedded after_* run after the parent.
| gem.add_dependency "riak-client", "~> 2.6" | ||
| gem.add_dependency "activesupport", "~> 6.1.5" | ||
| gem.add_dependency "activesupport", "~> 7.2.2" | ||
| gem.add_dependency "activemodel", ">= 5.2.4" | ||
| gem.add_dependency "tzinfo" |
There was a problem hiding this comment.
Bumping activesupport to ~> 7.2.2 makes the existing matrix Gemfiles (Gemfile.rails30/31/32/50) that pin activemodel ~> 3.x/5.0 impossible to resolve (those activemodel versions require a matching activesupport major/minor). If those Gemfiles/CI paths are still supported, they need to be updated/removed alongside this dependency bump to keep bundle install working.
| gem.add_development_dependency "rspec", "~>2.8.0" | ||
| gem.add_development_dependency "rake" | ||
|
|
||
| gem.add_dependency "riak-client", "~> 2.6" | ||
| gem.add_dependency "activesupport", "~> 6.1.5" | ||
| gem.add_dependency "activesupport", "~> 7.2.2" |
There was a problem hiding this comment.
With .tool-versions moving to Ruby 3.3, the dev dependency rspec ~> 2.8.0 is extremely old and is not compatible with modern Ruby (and will likely prevent bundle exec rake spec from running). To support the Ruby upgrade, bump RSpec (and any related tooling) to a version that supports Ruby 3.3, and adjust specs as needed.
Upgrades Ruby/Rails/Ripple versions to support a Ruby/Rails upgrade for https://github.com/spreedly/core/pull/10015
PLAT-573