FIX: Subcategory permissions validation

FIX: Subcategory permissions validation

When a category has a subcategory, we ensure that no one who can see the subcategory cannot see the parent. However, we don’t take into account the fact that, when no CategoryGroups exist, the default is that everyone has full permissions.

diff --git a/app/models/category.rb b/app/models/category.rb
index 0037cc96b4..c0f66dbea9 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -802,10 +802,27 @@ class Category < ActiveRecord::Base
   end
 
   def subcategories_permissions
-    CategoryGroup.joins(:category)
-      .where(['categories.parent_category_id = ?', self.id])
-      .pluck(:group_id, :permission_type)
-      .uniq
+    everyone = Group[:everyone].id
+    full = CategoryGroup.permission_types[:full]
+
+    result =
+      DB.query(<<-SQL, id: id, everyone: everyone, full: full)
+        SELECT category_groups.group_id, category_groups.permission_type
+        FROM categories, category_groups
+        WHERE categories.parent_category_id = :id
+        AND categories.id = category_groups.category_id
+        UNION
+        SELECT :everyone, :full
+        FROM categories
+        WHERE categories.parent_category_id = :id
+        AND (
+          SELECT DISTINCT 1
+          FROM category_groups
+          WHERE category_groups.category_id = categories.id
+        ) IS NULL
+      SQL
+
+    result.map { |row| [row.group_id, row.permission_type] }
   end
 end
 
diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb
index dcbfe2b788..35a58303df 100644
--- a/spec/models/category_spec.rb
+++ b/spec/models/category_spec.rb
@@ -870,10 +870,12 @@ describe Category do
     fab!(:group2) { Fabricate(:group) }
     fab!(:parent_category) { Fabricate(:category_with_definition, name: "parent") }
     fab!(:subcategory) { Fabricate(:category_with_definition, name: "child1", parent_category_id: parent_category.id) }
-    fab!(:subcategory2) { Fabricate(:category_with_definition, name: "child2", parent_category_id: parent_category.id) }
 
     context "when changing subcategory permissions" do
       it "it is not valid if permissions are less restrictive" do
+        subcategory.set_permissions(group => :readonly)
+        subcategory.save!
+
         parent_category.set_permissions(group => :readonly)
         parent_category.save!
 
@@ -884,6 +886,9 @@ describe Category do
       end
 
       it "is valid if permissions are same or more restrictive" do
+        subcategory.set_permissions(group => :full, group2 => :create_post)
+        subcategory.save!
+
         parent_category.set_permissions(group => :full, group2 => :create_post)
         parent_category.save!
 
@@ -903,7 +908,9 @@ describe Category do
     end
 
     context "when changing parent category permissions" do
-      it "it is not valid if subcategory permissions are less restrictive" do
+      fab!(:subcategory2) { Fabricate(:category_with_definition, name: "child2", parent_category_id: parent_category.id) }
+
+      it "is not valid if subcategory permissions are less restrictive" do
         subcategory.set_permissions(group => :create_post)
         subcategory.save!
         subcategory2.set_permissions(group => :create_post, group2 => :create_post)
@@ -915,6 +922,12 @@ describe Category do
         expect(parent_category.errors.full_messages).to contain_exactly(I18n.t("category.errors.permission_conflict", group_names: group2.name))
       end
 
+      it "is not valid if the subcategory has no category groups, but the parent does" do
+        parent_category.set_permissions(group => :readonly)
+
+        expect(parent_category).not_to be_valid
+      end
+
       it "is valid if subcategory permissions are same or more restrictive" do
         subcategory.set_permissions(group => :create_post)
         subcategory.save!

GitHub sha: 0de7e433

1 Like