diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e8052c0..3ad0574 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -18,7 +18,7 @@ Metrics/BlockLength: # Offense count: 2 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 128 + Max: 131 # Offense count: 4 Metrics/CyclomaticComplexity: diff --git a/CHANGELOG.md b/CHANGELOG.md index d07a507..9144b8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ +* Nested attributes support reference associations. + +* References many association destruction works now the same way as embedded many one does. If any object was destroyed or marked for destruction - if will be moved to the destroyed array on `apply_changes` call (on the parent object saving). + +* References one and many associations now don't destroy the object if it was marked for destruction, but the association is not autosave. In this case the object will be unlinked from the parent object and moved to the `destroyed` array for the references many association. + * No more `ActiveData.persistence_adapter` method. Define `self.active_data_persistence_adapter` directly in the desired class. * Represented attributes are not provided by default, to add them, `include ActiveData::Model::Representation` -* `include ActiveData::Model::Associations::Validations` is not included by default anymore, to get `validate_ancestry!`, `valid_ancestry?` and `invalid_ancestry?` methods back you neet to include this module manually. +* `include ActiveData::Model::Associations::Validations` is not included by default anymore, to get `validate_ancestry!`, `valid_ancestry?` and `invalid_ancestry?` methods back you need to include this module manually. diff --git a/Gemfile b/Gemfile index 976ecdb..944699e 100644 --- a/Gemfile +++ b/Gemfile @@ -5,4 +5,5 @@ gemspec group :test do gem 'guard' gem 'guard-rspec' + gem 'pry' end diff --git a/lib/active_data/model/associations/nested_attributes.rb b/lib/active_data/model/associations/nested_attributes.rb index afec0cb..87c4c8e 100644 --- a/lib/active_data/model/associations/nested_attributes.rb +++ b/lib/active_data/model/associations/nested_attributes.rb @@ -71,12 +71,8 @@ def self.assign_nested_attributes_for_one_to_one_association(object, association association = object.association(association_name) existing_record = association.target primary_attribute_name = primary_name_for(association.reflection.klass) - if existing_record - primary_attribute = existing_record.attribute(primary_attribute_name) - primary_attribute_value = primary_attribute.typecast(attributes[primary_attribute_name]) if primary_attribute - end - if existing_record && (!primary_attribute || options[:update_only] || existing_record.primary_attribute == primary_attribute_value) + if existing_record && (options[:update_only] || existing_record_matches?(existing_record, primary_attribute_name, attributes)) assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) unless call_reject_if(object, association_name, attributes) elsif attributes[primary_attribute_name].present? raise ActiveData::ObjectNotFound.new(object, association_name, attributes[primary_attribute_name]) @@ -123,9 +119,7 @@ def self.assign_nested_attributes_for_collection_association(object, association end else existing_record = association.target.detect do |record| - primary_attribute_value = record.attribute(primary_attribute_name) - .typecast(attributes[primary_attribute_name]) - record.primary_attribute == primary_attribute_value + existing_record_matches?(record, primary_attribute_name, attributes) end if existing_record unless call_reject_if(object, association_name, attributes) @@ -189,6 +183,17 @@ def self.unassignable_keys(object) def self.primary_name_for(klass) klass < ActiveData::Model ? klass.primary_name : 'id' end + + def self.existing_record_matches?(existing_record, primary_attribute_name, attributes) + if existing_record.is_a?(ActiveData::Model) + primary_attribute = existing_record.attribute(primary_attribute_name) + primary_attribute_value = primary_attribute.typecast(attributes[primary_attribute_name]) if primary_attribute + + !primary_attribute || existing_record.primary_attribute == primary_attribute_value + else + attributes[primary_attribute_name].present? && existing_record.send(primary_attribute_name).to_s == attributes[primary_attribute_name].to_s + end + end end module ClassMethods diff --git a/lib/active_data/model/associations/references_many.rb b/lib/active_data/model/associations/references_many.rb index 32c8113..94f0c03 100644 --- a/lib/active_data/model/associations/references_many.rb +++ b/lib/active_data/model/associations/references_many.rb @@ -18,11 +18,25 @@ def create!(attributes = {}) object end + def destroyed + @destroyed ||= [] + end + def apply_changes - target.all? do |object| + @destroyed = [] + + result = target.all? do |object| if object - if object.marked_for_destruction? && reflection.autosave? - object.destroy + if object.marked_for_destruction? + @destroyed.push(object) + if reflection.autosave? + object.destroy + else + true + end + elsif object.destroyed? + @destroyed.push(object) + true elsif object.new_record? || (reflection.autosave? && object.changed?) persist_object(object) else @@ -32,6 +46,9 @@ def apply_changes true end end + + @target -= @destroyed + result end def target=(object) diff --git a/lib/active_data/model/associations/references_one.rb b/lib/active_data/model/associations/references_one.rb index d5e9827..cd44a9a 100644 --- a/lib/active_data/model/associations/references_one.rb +++ b/lib/active_data/model/associations/references_one.rb @@ -18,8 +18,13 @@ def create!(attributes = {}) def apply_changes if target - if target.marked_for_destruction? && reflection.autosave? - target.destroy + if target.marked_for_destruction? + if reflection.autosave? + target.destroy + else + replace(nil) + true + end elsif target.new_record? || (reflection.autosave? && target.changed?) persist_object(target) else diff --git a/spec/lib/active_data/active_record/nested_attributes_spec.rb b/spec/lib/active_data/active_record/nested_attributes_spec.rb index 7030005..dd3054a 100644 --- a/spec/lib/active_data/active_record/nested_attributes_spec.rb +++ b/spec/lib/active_data/active_record/nested_attributes_spec.rb @@ -2,14 +2,16 @@ require 'shared/nested_attribute_examples' describe ActiveData::ActiveRecord::NestedAttributes do - before do - stub_class(:user, ActiveRecord::Base) do - embeds_one :profile - embeds_many :projects + context 'embedded nested attributes' do + before do + stub_class(:user, ActiveRecord::Base) do + embeds_one :profile + embeds_many :projects - accepts_nested_attributes_for :profile, :projects + accepts_nested_attributes_for :profile, :projects + end end - end - include_examples 'nested attributes' + include_examples 'embedded nested attributes' + end end diff --git a/spec/lib/active_data/model/associations/nested_attributes_spec.rb b/spec/lib/active_data/model/associations/nested_attributes_spec.rb index b741659..5fadb3d 100644 --- a/spec/lib/active_data/model/associations/nested_attributes_spec.rb +++ b/spec/lib/active_data/model/associations/nested_attributes_spec.rb @@ -2,7 +2,7 @@ require 'shared/nested_attribute_examples' describe ActiveData::Model::Associations::NestedAttributes do - context '' do + context 'embedded nested attributes' do before do stub_model :user do include ActiveData::Model::Associations @@ -19,16 +19,17 @@ def save end end - include_examples 'nested attributes' + include_examples 'embedded nested attributes' end - xcontext 'references_one' do + context 'referenced nested attributes' do before do stub_class(:author, ActiveRecord::Base) stub_class(:user, ActiveRecord::Base) stub_model :book do include ActiveData::Model::Associations + include ActiveData::Model::Lifecycle references_one :author references_many :users @@ -57,7 +58,7 @@ def save end context 'existing' do - let(:author) { Author.new(name: 'Author') } + let(:author) { Author.create!(name: 'Author') } let(:book) { Book.new author: author } specify { expect { book.author_attributes = {id: 42, name: 'Author'} }.to raise_error ActiveData::ObjectNotFound } @@ -106,10 +107,166 @@ def save end end end - end - context 'references_many' do - let(:book) { Book.new } + context 'references_many' do + let(:book) { Book.new } + + specify { expect { book.users_attributes = {} }.not_to change { book.users } } + specify do + expect { book.users_attributes = [{email: 'User 1'}, {email: 'User 2'}] } + .to change { book.users.map(&:email) }.to(['User 1', 'User 2']) + end + specify do + expect { book.users_attributes = {1 => {email: 'User 1'}, 2 => {email: 'User 2'}} } + .to change { book.users.map(&:email) }.to(['User 1', 'User 2']) + end + specify do + expect { book.users_attributes = [{id: 33, email: 'User 1'}, {email: 'User 2'}] } + .to raise_error ActiveData::ObjectNotFound + end + specify do + expect { book.users_attributes = [{email: ''}, {email: 'User 2'}] } + .to change { book.users.map(&:email) }.to(['', 'User 2']) + end + + context ':limit' do + before { Book.accepts_nested_attributes_for :users, limit: 1 } + + specify do + expect { book.users_attributes = [{email: 'User 1'}] } + .to change { book.users.map(&:email) }.to(['User 1']) + end + specify do + expect { book.users_attributes = [{email: 'User 1'}, {email: 'User 2'}] } + .to raise_error ActiveData::TooManyObjects + end + end + + context ':reject_if' do + context do + before { Book.accepts_nested_attributes_for :users, reject_if: :all_blank } + specify do + expect { book.users_attributes = [{email: ''}, {email: 'User 2'}] } + .to change { book.users.map(&:email) }.to(['User 2']) + end + end + + context do + before { Book.accepts_nested_attributes_for :users, reject_if: ->(attributes) { attributes['email'].blank? } } + specify do + expect { book.users_attributes = [{email: ''}, {email: 'User 2'}] } + .to change { book.users.map(&:email) }.to(['User 2']) + end + end + + context do + before { Book.accepts_nested_attributes_for :users, reject_if: ->(attributes) { attributes['foobar'].blank? } } + specify do + expect { book.users_attributes = [{email: ''}, {email: 'User 2'}] } + .not_to change { book.users } + end + end + end + + context 'existing' do + let(:users) { Array.new(2) { |i| User.create!(email: "User #{i.next}").tap { |pr| pr.id = 42 + i } } } + let(:book) { Book.new users: users } + + specify do + expect do + book.users_attributes = [ + {id: users.first.id, email: 'User 3'}, + {email: 'User 4'} + ] + end.to change { book.users.map(&:email) }.to(['User 3', 'User 2', 'User 4']) + end + specify do + expect do + book.users_attributes = [ + {id: users.first.id, email: 'User 3'}, + {id: 33, email: 'User 4'} + ] + end.to raise_error ActiveData::ObjectNotFound + end + specify do + expect do + book.users_attributes = [ + {id: users.first.id, email: 'User 3'}, + {id: 33, email: 'User 4', _destroy: 1} + ] + end.to raise_error ActiveData::ObjectNotFound + end + specify do + expect do + book.users_attributes = { + 1 => {id: users.first.id, email: 'User 3'}, + 2 => {email: 'User 4'} + } + end.to change { book.users.map(&:email) }.to(['User 3', 'User 2', 'User 4']) + end + specify do + expect do + book.users_attributes = [ + {id: users.first.id, email: 'User 3', _destroy: '1'}, + {email: 'User 4', _destroy: '1'} + ] + end.to change { book.users.map(&:email) }.to(['User 3', 'User 2']) + end + specify do + expect do + book.users_attributes = [ + {id: users.first.id, email: 'User 3', _destroy: '1'}, + {email: 'User 4', _destroy: '1'} + ] + book.save { true } + end.to change { book.users.map(&:email) }.to(['User 3', 'User 2']) + end + + context ':allow_destroy' do + before { Book.accepts_nested_attributes_for :users, allow_destroy: true } + + specify do + expect do + book.users_attributes = [ + {id: users.first.id, email: 'User 3', _destroy: '1'}, + {email: 'User 4', _destroy: '1'} + ] + end.to change { book.users.map(&:email) }.to(['User 3', 'User 2']) + end + specify do + expect do + book.users_attributes = [ + {id: users.first.id, email: 'User 3', _destroy: '1'}, + {email: 'User 4', _destroy: '1'} + ] + book.save { true } + end.to change { book.users.map(&:email) }.to(['User 2']) + end + end + + context ':update_only' do + before { Book.accepts_nested_attributes_for :users, update_only: true } + + specify do + expect do + book.users_attributes = [ + {id: users.first.id, email: 'User 3'}, + {email: 'User 4'} + ] + end.to change { book.users.map(&:email) }.to(['User 3', 'User 2']) + end + + specify do + expect do + book.users_attributes = [ + {id: users.last.id, email: 'User 3'}, + {id: users.first.id.pred, email: 'User 0'} + ] + end.to raise_error ActiveData::ObjectNotFound + end + end + end + end end end diff --git a/spec/lib/active_data/model/associations/references_many_spec.rb b/spec/lib/active_data/model/associations/references_many_spec.rb index 27b1e6b..c176f09 100644 --- a/spec/lib/active_data/model/associations/references_many_spec.rb +++ b/spec/lib/active_data/model/associations/references_many_spec.rb @@ -235,28 +235,49 @@ existing_association.build(name: 'Morty') expect { existing_association.apply_changes } .to change { existing_book.author_ids } - .from([author.id, nil]).to([author.id, be_a(Integer)]) + .from([author.id, nil]).to([be_a(Integer).and(be > author.id)]) + end + specify do + existing_association.target.first.mark_for_destruction + existing_association.build(name: 'Morty') + expect { existing_association.apply_changes } + .to change { existing_association.destroyed } + .from([]).to([author]) end specify do existing_association.target.first.mark_for_destruction existing_association.build(name: 'Morty') expect { existing_association.apply_changes } .to change { existing_association.target.map(&:persisted?) } - .from([true, false]).to([true, true]) + .from([true, false]).to([true]) + end + specify do + existing_association.target.first.mark_for_destruction + existing_association.build(name: 'Morty') + expect { existing_association.apply_changes } + .to change { existing_association.destroyed.map(&:persisted?) } + .from([]).to([true]) end specify do existing_association.target.first.destroy! existing_association.build(name: 'Morty') expect { existing_association.apply_changes } .to change { existing_book.author_ids } - .from([author.id, nil]).to([author.id, be_a(Integer)]) + .from([author.id, nil]).to([be_a(Integer).and(be > author.id)]) end specify do existing_association.target.first.destroy! existing_association.build(name: 'Morty') expect { existing_association.apply_changes } .to change { existing_association.target.map(&:persisted?) } - .from([false, false]).to([false, true]) + .from([false, false]).to([true]) + end + specify do + existing_association.target.first.destroy! + existing_association.build(name: 'Morty') + expect { existing_association.apply_changes } + .to change { existing_association.destroyed } + .from([]).to([author]) end context ':autosave' do @@ -301,28 +322,49 @@ existing_association.build(name: 'Morty') expect { existing_association.apply_changes } .to change { existing_book.author_ids } - .from([author.id, nil]).to([author.id, be_a(Integer)]) + .from([author.id, nil]).to([be_a(Integer).and(be > author.id)]) + end + specify do + existing_association.target.first.mark_for_destruction + existing_association.build(name: 'Morty') + expect { existing_association.apply_changes } + .to change { existing_association.destroyed } + .from([]).to([author]) end specify do existing_association.target.first.mark_for_destruction existing_association.build(name: 'Morty') expect { existing_association.apply_changes } .to change { existing_association.target.map(&:persisted?) } - .from([true, false]).to([false, true]) + .from([true, false]).to([true]) + end + specify do + existing_association.target.first.mark_for_destruction + existing_association.build(name: 'Morty') + expect { existing_association.apply_changes } + .to change { existing_association.destroyed.map(&:persisted?) } + .from([]).to([false]) end specify do existing_association.target.first.destroy! existing_association.build(name: 'Morty') expect { existing_association.apply_changes } .to change { existing_book.author_ids } - .from([author.id, nil]).to([author.id, be_a(Integer)]) + .from([author.id, nil]).to([be_a(Integer).and(be > author.id)]) end specify do existing_association.target.first.destroy! existing_association.build(name: 'Morty') expect { existing_association.apply_changes } .to change { existing_association.target.map(&:persisted?) } - .from([false, false]).to([false, true]) + .from([false, false]).to([true]) + end + specify do + existing_association.target.first.destroy! + existing_association.build(name: 'Morty') + expect { existing_association.apply_changes } + .to change { existing_association.destroyed } + .from([]).to([author]) end end end diff --git a/spec/lib/active_data/model/associations/references_one_spec.rb b/spec/lib/active_data/model/associations/references_one_spec.rb index 3270fa9..7602991 100644 --- a/spec/lib/active_data/model/associations/references_one_spec.rb +++ b/spec/lib/active_data/model/associations/references_one_spec.rb @@ -179,12 +179,14 @@ specify do existing_association.target.mark_for_destruction expect { existing_association.send(method) } - .not_to change { existing_association.target.destroyed? } + .to change { existing_association.target } + .to(nil) end specify do existing_association.target.mark_for_destruction expect { existing_association.send(method) } - .not_to change { existing_book.author_id } + .to change { existing_book.author_id } + .to(nil) end specify do existing_association.target.destroy! diff --git a/spec/shared/nested_attribute_examples.rb b/spec/shared/nested_attribute_examples.rb index 17d1198..ee877ab 100644 --- a/spec/shared/nested_attribute_examples.rb +++ b/spec/shared/nested_attribute_examples.rb @@ -1,6 +1,6 @@ require 'spec_helper' -shared_examples 'nested attributes' do +shared_examples 'embedded nested attributes' do before do stub_model :project do include ActiveData::Model::Primary @@ -143,7 +143,7 @@ def profile_attributes=(args) .to change { user.projects.map(&:title) }.to(['Project 1', 'Project 2']) end specify do - expect { user.projects_attributes = [{slug: 42, title: 'Project 1'}, {title: 'Project 2'}] } + expect { user.projects_attributes = [{slug: 33, title: 'Project 1'}, {title: 'Project 2'}] } .to change { user.projects.map(&:title) }.to(['Project 1', 'Project 2']) end specify do @@ -200,8 +200,7 @@ def profile_attributes=(args) {slug: projects.first.slug.to_i, title: 'Project 3'}, {title: 'Project 4'} ] - end - .to change { user.projects.map(&:title) }.to(['Project 3', 'Project 2', 'Project 4']) + end.to change { user.projects.map(&:title) }.to(['Project 3', 'Project 2', 'Project 4']) end specify do expect do @@ -209,8 +208,7 @@ def profile_attributes=(args) {slug: projects.first.slug.to_i, title: 'Project 3'}, {slug: 33, title: 'Project 4'} ] - end - .to change { user.projects.map(&:slug) }.to(%w[42 43 33]) + end.to change { user.projects.map(&:slug) }.to(%w[42 43 33]) end specify do expect do @@ -227,8 +225,7 @@ def profile_attributes=(args) 1 => {slug: projects.first.slug.to_i, title: 'Project 3'}, 2 => {title: 'Project 4'} } - end - .to change { user.projects.map(&:title) }.to(['Project 3', 'Project 2', 'Project 4']) + end.to change { user.projects.map(&:title) }.to(['Project 3', 'Project 2', 'Project 4']) end specify do expect do @@ -236,8 +233,7 @@ def profile_attributes=(args) {slug: projects.first.slug.to_i, title: 'Project 3', _destroy: '1'}, {title: 'Project 4', _destroy: '1'} ] - end - .to change { user.projects.map(&:title) }.to(['Project 3', 'Project 2']) + end.to change { user.projects.map(&:title) }.to(['Project 3', 'Project 2']) end specify do expect do @@ -246,8 +242,7 @@ def profile_attributes=(args) {title: 'Project 4', _destroy: '1'} ] user.save { true } - end - .to change { user.projects.map(&:title) }.to(['Project 3', 'Project 2']) + end.to change { user.projects.map(&:title) }.to(['Project 3', 'Project 2']) end context ':allow_destroy' do @@ -259,8 +254,7 @@ def profile_attributes=(args) {slug: projects.first.slug.to_i, title: 'Project 3', _destroy: '1'}, {title: 'Project 4', _destroy: '1'} ] - end - .to change { user.projects.map(&:title) }.to(['Project 3', 'Project 2']) + end.to change { user.projects.map(&:title) }.to(['Project 3', 'Project 2']) end specify do expect do @@ -269,8 +263,7 @@ def profile_attributes=(args) {title: 'Project 4', _destroy: '1'} ] user.save { true } - end - .to change { user.projects.map(&:title) }.to(['Project 2']) + end.to change { user.projects.map(&:title) }.to(['Project 2']) end end @@ -283,8 +276,7 @@ def profile_attributes=(args) {slug: projects.first.slug.to_i, title: 'Project 3'}, {title: 'Project 4'} ] - end - .to change { user.projects.map(&:title) }.to(['Project 3', 'Project 2']) + end.to change { user.projects.map(&:title) }.to(['Project 3', 'Project 2']) end specify do @@ -293,8 +285,7 @@ def profile_attributes=(args) {slug: projects.last.slug.to_i, title: 'Project 3'}, {slug: projects.first.slug.to_i.pred, title: 'Project 0'} ] - end - .to change { user.projects.map(&:title) }.to(['Project 1', 'Project 3']) + end.to change { user.projects.map(&:title) }.to(['Project 1', 'Project 3']) end end end