FIX: Validation of category tree depth
This prevents the creation of sub-sub-categories in multiple tabs
This commit is contained in:
parent
9845963105
commit
c49b20a1a2
|
@ -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
|
||||
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
Loading…
Reference in New Issue