From 844ab92517cb6257654bf5e50891646e78772393 Mon Sep 17 00:00:00 2001 From: Andrew Pratley Date: Sun, 20 May 2018 21:20:42 +1000 Subject: [PATCH 1/4] Add link table test for update method call --- test/paranoia_test.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/paranoia_test.rb b/test/paranoia_test.rb index e08f9a7b..5cc8cf09 100644 --- a/test/paranoia_test.rb +++ b/test/paranoia_test.rb @@ -376,6 +376,20 @@ def test_default_scope_for_has_many_through_relationships assert_equal 0, employee.employers.count end + def test_link_table_for_has_many_through_relationships_with_update_method_call + employer = Employer.create + employee = Employee.create + job = Job.create :employer => employer, :employee => employee + assert_equal 1, employer.jobs.count + assert_equal 1, employer.employees.count + assert_equal 1, employee.jobs.count + assert_equal 1, employee.employers.count + + employer.update(employees: []) + assert_equal 1, Employee.unscoped.count + assert_equal 1, Job.unscoped.count + end + def test_delete_behavior_for_callbacks model = CallbackModel.new model.save From c9bd309b13588215cfadf5a3fd6b5e8ecdaacaa3 Mon Sep 17 00:00:00 2001 From: Andrew Pratley Date: Tue, 29 May 2018 11:55:12 +1000 Subject: [PATCH 2/4] Add paranoia_update method --- lib/paranoia.rb | 40 ++++++++++++++++++++++++++++++++++++++++ test/paranoia_test.rb | 27 +++++++++++++++++---------- 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/lib/paranoia.rb b/lib/paranoia.rb index 4ba4c389..1b04a0d4 100644 --- a/lib/paranoia.rb +++ b/lib/paranoia.rb @@ -56,6 +56,46 @@ def restore(id_or_ids, opts = {}) end end + def paranoia_update(attributes) + attributes = attributes.with_indifferent_access + object_class = self.class + + has_many_through_associations = object_class.reflect_on_all_associations.select do |association| + association.is_a?(ActiveRecord::Reflection::ThroughReflection) + end + + transaction do + run_callbacks(:update) do + has_many_through_associations.each do |association| + association_table_plural_name = association.source_reflection.plural_name.to_sym + association_key = + "#{object_class.reflect_on_association(association_table_plural_name).foreign_key.pluralize}" + next if !attributes.key?(association_key) + association_id_array = attributes[association_key].reject(&:blank?) + link_table_plural_name = association.through_reflection.plural_name.to_sym + object_key = "#{object_class.reflect_on_association(link_table_plural_name).foreign_key}" + link_table_class = association.through_reflection.klass + object_primary_key = association.active_record.primary_key.to_sym + + link_table_objects_to_be_soft_deleted = + link_table_class + .where(object_key => public_send(object_primary_key)) + .where.not(association_key.singularize => association_id_array) + link_table_objects_to_be_soft_deleted.map(&:paranoia_destroy) + end + + self.attributes = attributes + self.save + end + end + end + alias_method :update, :paranoia_update + + def paranoia_update!(attributes) + paranoia_update(attributes) || + raise(ActiveRecord::RecordNotDestroyed.new("Failed to update the record", self)) + end + def paranoia_destroy transaction do run_callbacks(:destroy) do diff --git a/test/paranoia_test.rb b/test/paranoia_test.rb index 5cc8cf09..040ca065 100644 --- a/test/paranoia_test.rb +++ b/test/paranoia_test.rb @@ -378,16 +378,23 @@ def test_default_scope_for_has_many_through_relationships def test_link_table_for_has_many_through_relationships_with_update_method_call employer = Employer.create - employee = Employee.create - job = Job.create :employer => employer, :employee => employee - assert_equal 1, employer.jobs.count - assert_equal 1, employer.employees.count - assert_equal 1, employee.jobs.count - assert_equal 1, employee.employers.count - - employer.update(employees: []) - assert_equal 1, Employee.unscoped.count - assert_equal 1, Job.unscoped.count + employee_1 = Employee.create(id: 1) + employee_2 = Employee.create(id: 2) + employee_3 = Employee.create(id: 3) + employee_4 = Employee.create(id: 4) + employee_5 = Employee.create(id: 5) + job_1 = Job.create :employer => employer, :employee => employee_1 + job_2 = Job.create :employer => employer, :employee => employee_2 + job_3 = Job.create :employer => employer, :employee => employee_3 + assert_equal 3, employer.jobs.count + assert_equal 3, employer.employees.count + assert_equal 1, employee_1.jobs.count + assert_equal 1, employee_2.employers.count + + employer.update(employee_ids: [3,4,5]) + assert_equal 5, Employee.count + assert_equal [3,4,5], Job.all.map(&:employee_id) + assert_equal [1,2], Job.deleted.map(&:employee_id) end def test_delete_behavior_for_callbacks From 939c87fb24416ff4916c8c2195e584c339fa3fd3 Mon Sep 17 00:00:00 2001 From: Andrew Pratley Date: Tue, 29 May 2018 11:56:24 +1000 Subject: [PATCH 3/4] only apply to paranoid link tables --- lib/paranoia.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/paranoia.rb b/lib/paranoia.rb index 1b04a0d4..1d49b780 100644 --- a/lib/paranoia.rb +++ b/lib/paranoia.rb @@ -70,11 +70,12 @@ def paranoia_update(attributes) association_table_plural_name = association.source_reflection.plural_name.to_sym association_key = "#{object_class.reflect_on_association(association_table_plural_name).foreign_key.pluralize}" - next if !attributes.key?(association_key) + link_table_class = association.through_reflection.klass + next if !attributes.key?(association_key) || !link_table_class.paranoid? + association_id_array = attributes[association_key].reject(&:blank?) link_table_plural_name = association.through_reflection.plural_name.to_sym object_key = "#{object_class.reflect_on_association(link_table_plural_name).foreign_key}" - link_table_class = association.through_reflection.klass object_primary_key = association.active_record.primary_key.to_sym link_table_objects_to_be_soft_deleted = From 8a004caa46ef1b89e62c8d96507138cccd8dc5af Mon Sep 17 00:00:00 2001 From: Andrew Pratley Date: Tue, 5 Jun 2018 11:48:20 +1000 Subject: [PATCH 4/4] Change value reference and add more specs --- lib/paranoia.rb | 3 +- test/paranoia_test.rb | 177 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 164 insertions(+), 16 deletions(-) diff --git a/lib/paranoia.rb b/lib/paranoia.rb index 1d49b780..e05a87f8 100644 --- a/lib/paranoia.rb +++ b/lib/paranoia.rb @@ -67,9 +67,8 @@ def paranoia_update(attributes) transaction do run_callbacks(:update) do has_many_through_associations.each do |association| - association_table_plural_name = association.source_reflection.plural_name.to_sym association_key = - "#{object_class.reflect_on_association(association_table_plural_name).foreign_key.pluralize}" + "#{object_class.reflect_on_association(association.name).foreign_key.pluralize}" link_table_class = association.through_reflection.klass next if !attributes.key?(association_key) || !link_table_class.paranoid? diff --git a/test/paranoia_test.rb b/test/paranoia_test.rb index 040ca065..978843f3 100644 --- a/test/paranoia_test.rb +++ b/test/paranoia_test.rb @@ -33,9 +33,15 @@ def setup! 'fail_callback_models' => 'deleted_at DATETIME', 'related_models' => 'parent_model_id INTEGER, parent_model_with_counter_cache_column_id INTEGER, deleted_at DATETIME', 'asplode_models' => 'parent_model_id INTEGER, deleted_at DATETIME', - 'employers' => 'name VARCHAR(32), deleted_at DATETIME', + 'employers' => 'name VARCHAR(32), employee_count INTEGER, date_established DATETIME, number_of_offices INTEGER, deleted_at DATETIME', 'employees' => 'deleted_at DATETIME', 'jobs' => 'employer_id INTEGER NOT NULL, employee_id INTEGER NOT NULL, deleted_at DATETIME', + 'purchases' => 'deleted_at DATETIME', + 'purchase_orders' => 'employer_id INTEGER NOT NULL, purchase_id INTEGER NOT NULL, deleted_at DATETIME', + 'suppliers' => 'deleted_at DATETIME', + 'accounts' => 'employer_id INTEGER NOT NULL, supplier_id INTEGER NOT NULL, deleted_at DATETIME', + 'candidates' => 'deleted_at DATETIME', + 'interviews' => 'employer_id INTEGER NOT NULL, candidate_id INTEGER NOT NULL, deleted_at DATETIME', 'custom_column_models' => 'destroyed_at DATETIME', 'custom_sentinel_models' => 'deleted_at DATETIME NOT NULL', 'non_paranoid_models' => 'parent_model_id INTEGER', @@ -376,25 +382,91 @@ def test_default_scope_for_has_many_through_relationships assert_equal 0, employee.employers.count end - def test_link_table_for_has_many_through_relationships_with_update_method_call - employer = Employer.create - employee_1 = Employee.create(id: 1) - employee_2 = Employee.create(id: 2) - employee_3 = Employee.create(id: 3) - employee_4 = Employee.create(id: 4) - employee_5 = Employee.create(id: 5) - job_1 = Job.create :employer => employer, :employee => employee_1 - job_2 = Job.create :employer => employer, :employee => employee_2 - job_3 = Job.create :employer => employer, :employee => employee_3 + def test_link_table_retains_records_with_update_call + employer = employer_with_associations + assert_equal 3, employer.jobs.count assert_equal 3, employer.employees.count - assert_equal 1, employee_1.jobs.count - assert_equal 1, employee_2.employers.count + assert_equal 1, employer.employees.first.jobs.count + assert_equal 1, employer.employees.second.jobs.count + + employer.update(employee_ids: [3,4,5], name: 'Google') - employer.update(employee_ids: [3,4,5]) assert_equal 5, Employee.count assert_equal [3,4,5], Job.all.map(&:employee_id) assert_equal [1,2], Job.deleted.map(&:employee_id) + assert_equal 'Google', employer.name + end + + def test_link_table_does_not_retains_records_with_update_call_when_not_paranoid + employer = employer_with_associations + + assert_equal 2, employer.purchases.count + assert_equal 2, employer.purchase_orders.count + + employer.update(purchase_ids: [], name: 'Apple') + + assert_equal 0, PurchaseOrder.unscoped.count + assert_equal [1,2], Purchase.all.map(&:id) + assert_equal 'Apple', employer.name + end + + def test_update_on_associations_with_dependent_destroy_when_not_paranoid + employer = employer_with_associations + + assert_equal 2, PurchaseOrder.count + assert_equal 2, Purchase.count + + employer.update(purchase_ids: []) + + assert_equal 0, PurchaseOrder.count + assert_equal 2, Purchase.count + end + + def test_update_on_associations_with_dependent_destroy_when_paranoid + employer = employer_with_associations + + assert_equal 1, Interview.count + assert_equal 1, Candidate.count + + employer.update(candidate_ids: []) + + assert_equal 1, Interview.unscoped.count + assert_equal 1, Candidate.count + end + + def test_update_on_associations_without_dependent_destroy_when_paranoid + employer = employer_with_associations + + assert_equal 3, Job.count + assert_equal 5, Employee.count + + employer.update(employee_ids: []) + + assert_equal 0, Job.count + assert_equal 5, Employee.count + end + + def test_update_on_associations_without_dependent_destroy_when_not_paranoid + employer = employer_with_associations + + assert_equal 1, Account.count + assert_equal 1, Supplier.count + + employer.update(supplier_ids: []) + + assert_equal 0, Account.unscoped.count + assert_equal 1, Supplier.count + end + + def test_update_on_top_level_attributes + employer = employer_with_associations + + employer.update(name: 'Apple', employee_count: 123, date_established: Date.yesterday) + assert_equal 'Apple', employer.name + assert_equal 123, employer.employee_count + assert_equal Date.yesterday, employer.date_established + assert_equal 6, employer.number_of_offices # remains unchanged end def test_delete_behavior_for_callbacks @@ -1077,9 +1149,41 @@ def test_counter_cache_column_on_restore end private + def get_featureful_model FeaturefulModel.new(:name => "not empty") end + + def employer_with_associations + employer = Employer.create( + name: 'Bellroy', + employee_count: 100, + date_established: Date.today, + number_of_offices: 6 + ) + + employee_1 = Employee.create(id: 1) + employee_2 = Employee.create(id: 2) + employee_3 = Employee.create(id: 3) + employee_4 = Employee.create(id: 4) + employee_5 = Employee.create(id: 5) + job_1 = Job.create :employer => employer, :employee => employee_1 + job_2 = Job.create :employer => employer, :employee => employee_2 + job_3 = Job.create :employer => employer, :employee => employee_3 + + purchase_1 = Purchase.create(id: 1) + purchase_2 = Purchase.create(id: 2) + purchase_order_1 = PurchaseOrder.create :employer => employer, :purchase => purchase_1 + purchase_order_2 = PurchaseOrder.create :employer => employer, :purchase => purchase_2 + + candidate = Candidate.create + inteview = Interview.create :employer => employer, :candidate => candidate + + supplier = Supplier.create + account = Account.create :employer => employer, :supplier => supplier + + employer + end end # Helper classes @@ -1176,11 +1280,24 @@ def set_after_commit_on_destroy_callback_called end end +# Classes | Paranoid? | Dependant Destroy? +#-------------------------------------------------------- +# Account/Supplier | X | X +# Employee/Job | ✓ | X +# Purchase/PurchaseOrder | X | ✓ +# Interview/Candidate | ✓ | ✓ + class Employer < ActiveRecord::Base acts_as_paranoid validates_uniqueness_of :name has_many :jobs has_many :employees, :through => :jobs + has_many :purchase_orders, dependent: :destroy + has_many :purchases, dependent: :destroy, :through => :purchase_orders + has_many :interviews, dependent: :destroy + has_many :candidates, dependent: :destroy, :through => :interviews + has_many :accounts + has_many :suppliers, :through => :accounts end class Employee < ActiveRecord::Base @@ -1195,6 +1312,38 @@ class Job < ActiveRecord::Base belongs_to :employee end +class PurchaseOrder < ActiveRecord::Base + belongs_to :employer + belongs_to :purchase +end + +class Purchase < ActiveRecord::Base + has_many :purchase_orders + has_many :employers, :through => :purchase_orders +end + +class Interview < ActiveRecord::Base + acts_as_paranoid + belongs_to :employer + belongs_to :candidate +end + +class Candidate < ActiveRecord::Base + acts_as_paranoid + has_many :interviews + has_many :employers, :through => :interviews +end + +class Account < ActiveRecord::Base + belongs_to :employer + belongs_to :supplier +end + +class Supplier < ActiveRecord::Base + has_many :accounts + has_many :employers, :through => :accounts +end + class CustomColumnModel < ActiveRecord::Base acts_as_paranoid column: :destroyed_at end