diff --git a/app/models/category.rb b/app/models/category.rb index a7d2fe10a17..0037cc96b44 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -12,6 +12,7 @@ class Category < ActiveRecord::Base include AnonCacheInvalidator include HasDestroyedWebHook + MAX_NESTING = 2 # category + subcategory REQUIRE_TOPIC_APPROVAL = 'require_topic_approval' REQUIRE_REPLY_APPROVAL = 'require_reply_approval' NUM_AUTO_BUMP_DAILY = 'num_auto_bump_daily' @@ -320,13 +321,70 @@ class Category < ActiveRecord::Base MessageBus.publish('/categories', deleted_categories: [self.id]) end + # This is used in a validation so has to produce accurate results before the + # record has been saved + def height_of_ancestors(max_height = MAX_NESTING) + parent_id = self.parent_category_id + + return max_height if parent_id == id + + DB.query(<<~SQL, id: id, parent_id: parent_id, max_height: max_height)[0].max + WITH RECURSIVE ancestors(parent_category_id, height) AS ( + SELECT :parent_id :: integer, 0 + + UNION ALL + + SELECT + categories.parent_category_id, + CASE + WHEN categories.parent_category_id = :id THEN :max_height + ELSE ancestors.height + 1 + END + FROM categories, ancestors + WHERE categories.id = ancestors.parent_category_id + AND ancestors.height < :max_height + ) + + SELECT max(height) FROM ancestors + SQL + end + + # This is used in a validation so has to produce accurate results before the + # record has been saved + def depth_of_descendants(max_depth = MAX_NESTING) + parent_id = self.parent_category_id + + return max_depth if parent_id == id + + DB.query(<<~SQL, id: id, parent_id: parent_id, max_depth: max_depth)[0].max + WITH RECURSIVE descendants(id, depth) AS ( + SELECT :id :: integer, 0 + + UNION ALL + + SELECT + categories.id, + CASE + WHEN categories.id = :parent_id THEN :max_depth + ELSE descendants.depth + 1 + END + FROM categories, descendants + WHERE categories.parent_category_id = descendants.id + AND descendants.depth < :max_depth + ) + + SELECT max(depth) FROM descendants + SQL + end + def parent_category_validator if parent_category_id - errors.add(:base, I18n.t("category.errors.self_parent")) if parent_category_id == id errors.add(:base, I18n.t("category.errors.uncategorized_parent")) if uncategorized? - grandfather_id = Category.where(id: parent_category_id).pluck(:parent_category_id).first - errors.add(:base, I18n.t("category.errors.depth")) if grandfather_id + errors.add(:base, I18n.t("category.errors.self_parent")) if parent_category_id == id + + total_depth = height_of_ancestors + 1 + depth_of_descendants + errors.add(:base, I18n.t("category.errors.depth")) if total_depth > MAX_NESTING end end diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb index 581d20b82ea..dcbfe2b7881 100644 --- a/spec/models/category_spec.rb +++ b/spec/models/category_spec.rb @@ -937,6 +937,101 @@ describe Category do end end + describe "tree metrics" do + fab!(:category) do + Category.create!( + user: user, + name: "foo" + ) + end + + fab!(:subcategory) do + Category.create!( + user: user, + name: "bar", + parent_category: category + ) + end + + context "with a self-parent" do + before_all do + DB.exec(<<-SQL, id: category.id) + UPDATE categories + SET parent_category_id = :id + WHERE id = :id + SQL + end + + context "#depth_of_descendants" do + it "should produce max_depth" do + expect(category.depth_of_descendants(3)).to eq(3) + end + end + + context "#height_of_ancestors" do + it "should produce max_height" do + expect(category.height_of_ancestors(3)).to eq(3) + end + end + end + + context "with a prospective self-parent" do + before do + category.parent_category_id = category.id + end + + context "#depth_of_descendants" do + it "should produce max_depth" do + expect(category.depth_of_descendants(3)).to eq(3) + end + end + + context "#height_of_ancestors" do + it "should produce max_height" do + expect(category.height_of_ancestors(3)).to eq(3) + end + end + end + + context "with a prospective loop" do + before do + category.parent_category_id = subcategory.id + end + + context "#depth_of_descendants" do + it "should produce max_depth" do + expect(category.depth_of_descendants(3)).to eq(3) + end + end + + context "#height_of_ancestors" do + it "should produce max_height" do + expect(category.height_of_ancestors(3)).to eq(3) + end + end + end + + context "#depth_of_descendants" do + it "should be 0 when the category has no descendants" do + expect(subcategory.depth_of_descendants).to eq(0) + end + + it "should be 1 when the category has a descendant" do + expect(category.depth_of_descendants).to eq(1) + end + end + + context "#height_of_ancestors" do + it "should be 0 when the category has no ancestors" do + expect(category.height_of_ancestors).to eq(0) + end + + it "should be 1 when the category has an ancestor" do + expect(subcategory.height_of_ancestors).to eq(1) + end + end + end + describe "#ensure_consistency!" do it "creates category topic" do