FIX: Validation of category tree depth

FIX: Validation of category tree depth

This prevents the creation of sub-sub-categories in multiple tabs

diff --git a/app/models/category.rb b/app/models/category.rb
index a7d2fe10a1..0037cc96b4 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 581d20b82e..dcbfe2b788 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

GitHub sha: c49b20a1

1 Like