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

Propagate save(validate:) option for :has_many associations on au… #43525

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,3 +1,12 @@
* Propagate `save(validate:)` option for `:has_many` associations on autosave.

`save(validate:)` option is propagated to associated records during `autosave` when `autosave` option is
`nil`. Affects only `:has_many` associations.

Fixes #43400

*Jacopo Beschi*

* Add support for setting the filename of the schema or structure dump in the database config.

Applications may now set their the filename or path of the schema / structure dump file in their database configuration.
Expand Down
6 changes: 5 additions & 1 deletion activerecord/lib/active_record/autosave_association.rb
Expand Up @@ -409,7 +409,11 @@ def save_collection_association(reflection)
if autosave
saved = association.insert_record(record, false)
elsif !reflection.nested?
association_saved = association.insert_record(record)
association_saved = if @_save_options.key?(:validate)
association.insert_record(record, @_save_options[:validate])
else
association.insert_record(record)
end

if reflection.validate?
errors.add(reflection.name) unless association_saved
Expand Down
19 changes: 15 additions & 4 deletions activerecord/lib/active_record/validations.rb
Expand Up @@ -44,13 +44,17 @@ module Validations
# The regular {ActiveRecord::Base#save}[rdoc-ref:Persistence#save] method is replaced
# with this when the validations module is mixed in, which it is by default.
def save(**options)
perform_validations(options) ? super : false
with_save_options(options) do
perform_validations ? super : false
end
end

# Attempts to save the record just like {ActiveRecord::Base#save}[rdoc-ref:Base#save] but
# will raise an ActiveRecord::RecordInvalid exception instead of returning +false+ if the record is not valid.
def save!(**options)
perform_validations(options) ? super : raise_validation_error
with_save_options(options) do
perform_validations ? super : raise_validation_error
end
end

# Runs all the validations within the specified context. Returns +true+ if
Expand All @@ -72,6 +76,13 @@ def valid?(context = nil)
alias_method :validate, :valid?

private
def with_save_options(options)
@_save_options, previous_save_options = options, @_save_options
yield
ensure
@_save_options = previous_save_options
end

def default_validation_context
new_record? ? :create : :update
end
Expand All @@ -80,8 +91,8 @@ def raise_validation_error
raise(RecordInvalid.new(self))
end

def perform_validations(options = {})
options[:validate] == false || valid?(options[:context])
def perform_validations
@_save_options[:validate] == false || valid?(@_save_options[:context])
end
end
end
Expand Down
24 changes: 24 additions & 0 deletions activerecord/test/cases/autosave_association_test.rb
Expand Up @@ -1847,6 +1847,30 @@ def test_validations_still_fire_on_unchanged_association_with_custom_validation_
assert pirate.valid?
assert_not pirate.valid?(:conference)
end

test "propagates (validate: false) option to associations when autosave is nil" do
pirate = SpacePirate.create!
pirate.birds.build

assert_equal false, pirate.save
assert_equal true, pirate.save(validate: false)
end

test "propagates (validate: false) option to associations when autosave is true" do
pirate = SpacePirate.create!
pirate.birds_with_autosave.build

assert_equal false, pirate.save
assert_equal true, pirate.save(validate: false)
end

test "propagates (validate: false) option to associations when autosave is false" do
pirate = SpacePirate.create!
pirate.birds_without_autosave.build

assert_equal false, pirate.save
assert_equal true, pirate.save(validate: false)
end
end

class TestAutosaveAssociationValidationsOnAHasOneAssociation < ActiveRecord::TestCase
Expand Down
2 changes: 2 additions & 0 deletions activerecord/test/models/pirate.rb
Expand Up @@ -109,6 +109,8 @@ class SpacePirate < ActiveRecord::Base
has_one :ship, foreign_key: :pirate_id
has_one :ship_with_annotation, -> { annotate("that is a rocket") }, class_name: :Ship, foreign_key: :pirate_id
has_many :birds, foreign_key: :pirate_id
has_many :birds_with_autosave, foreign_key: :pirate_id, autosave: true, class_name: :Bird
has_many :birds_without_autosave, foreign_key: :pirate_id, autosave: false, class_name: :Bird
has_many :birds_with_annotation, -> { annotate("that are also parrots") }, class_name: :Bird, foreign_key: :pirate_id
has_many :treasures, as: :looter
has_many :treasure_estimates, through: :treasures, source: :price_estimates
Expand Down