From ef78cbed5c22aa9f7a0d16178227758c64d4c38e Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Tue, 8 Jun 2021 18:08:38 -0400 Subject: [PATCH] only run validation on ancestry column was running into issues with pre-validation callbacks. they were running a bunch of queries and requiring columns to be in the model when we were only focused on the ancestry columns this fixes that before: - run all prevalidation call backs - run all validations - check that ancestry did not thow an exception after: - run only ancestry validations also consolidates 2 different ancestry checks --- lib/ancestry/instance_methods.rb | 17 +++++++++++++++-- lib/ancestry/materialized_path.rb | 7 ------- lib/ancestry/materialized_path_pg.rb | 2 +- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/lib/ancestry/instance_methods.rb b/lib/ancestry/instance_methods.rb index cf5babeb..8e0fcf79 100644 --- a/lib/ancestry/instance_methods.rb +++ b/lib/ancestry/instance_methods.rb @@ -8,7 +8,7 @@ def ancestry_exclude_self # Update descendants with new ancestry (before save) def update_descendants_with_new_ancestry # If enabled and node is existing and ancestry was updated and the new ancestry is sane ... - if !ancestry_callbacks_disabled? && !new_record? && ancestry_changed? && sane_ancestry? + if !ancestry_callbacks_disabled? && !new_record? && ancestry_changed? && sane_ancestor_ids? # ... for each descendant ... unscoped_descendants.each do |descendant| # ... replace old ancestry with new ancestry @@ -108,7 +108,20 @@ def ancestry_changed? end def sane_ancestor_ids? - valid? || errors[self.ancestry_base_class.ancestry_column].blank? + current_context, self.validation_context = validation_context, nil + errors.clear + + attribute = ancestry_base_class.ancestry_column + ancestry_value = send(attribute) + return true unless ancestry_value + + self.class.validators_on(attribute).each do |validator| + validator.validate_each(self, attribute, ancestry_value) + end + ancestry_exclude_self + errors.none? + ensure + self.validation_context = current_context end def ancestors depth_options = {} diff --git a/lib/ancestry/materialized_path.rb b/lib/ancestry/materialized_path.rb index c537a4e2..91f23adc 100644 --- a/lib/ancestry/materialized_path.rb +++ b/lib/ancestry/materialized_path.rb @@ -82,13 +82,6 @@ def ordered_by_ancestry_and(order) end module InstanceMethods - - # Validates the ancestry, but can also be applied if validation is bypassed to determine if children should be affected - def sane_ancestry? - ancestry_value = read_attribute(self.ancestry_base_class.ancestry_column) - ancestry_value.nil? || (!ancestor_ids.include?(self.id) && (valid? || errors[self.ancestry_base_class.ancestry_column].blank?)) - end - # optimization - better to go directly to column and avoid parsing def ancestors? read_attribute(self.ancestry_base_class.ancestry_column).present? diff --git a/lib/ancestry/materialized_path_pg.rb b/lib/ancestry/materialized_path_pg.rb index 07237b8e..98cec645 100644 --- a/lib/ancestry/materialized_path_pg.rb +++ b/lib/ancestry/materialized_path_pg.rb @@ -3,7 +3,7 @@ module MaterializedPathPg # Update descendants with new ancestry (before save) def update_descendants_with_new_ancestry # If enabled and node is existing and ancestry was updated and the new ancestry is sane ... - if !ancestry_callbacks_disabled? && !new_record? && ancestry_changed? && sane_ancestry? + if !ancestry_callbacks_disabled? && !new_record? && ancestry_changed? && sane_ancestor_ids? ancestry_column = ancestry_base_class.ancestry_column old_ancestry = path_ids_in_database.join(Ancestry::MaterializedPath::ANCESTRY_DELIMITER) new_ancestry = path_ids.join(Ancestry::MaterializedPath::ANCESTRY_DELIMITER)