diff --git a/app/assets/javascripts/discourse/models/category.js.es6 b/app/assets/javascripts/discourse/models/category.js.es6
index c8531986d40..2e6928abfe4 100644
--- a/app/assets/javascripts/discourse/models/category.js.es6
+++ b/app/assets/javascripts/discourse/models/category.js.es6
@@ -99,7 +99,7 @@ const Category = RestModel.extend({
color: this.get("color"),
text_color: this.get("text_color"),
secure: this.get("secure"),
- permissions: this.get("permissionsForUpdate"),
+ permissions: this._permissionsForUpdate(),
auto_close_hours: this.get("auto_close_hours"),
auto_close_based_on_last_post: this.get(
"auto_close_based_on_last_post"
@@ -135,8 +135,8 @@ const Category = RestModel.extend({
});
},
- @computed("permissions")
- permissionsForUpdate(permissions) {
+ _permissionsForUpdate() {
+ const permissions = this.get("permissions");
let rval = {};
permissions.forEach(p => (rval[p.group_name] = p.permission.id));
return rval;
diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb
index 96d358b0abd..4e6a6da7c84 100644
--- a/app/controllers/categories_controller.rb
+++ b/app/controllers/categories_controller.rb
@@ -140,7 +140,7 @@ class CategoriesController < ApplicationController
render_serialized(@category, CategorySerializer)
else
- return render_json_error(@category) unless @category.save
+ return render_json_error(@category)
end
end
diff --git a/app/models/category.rb b/app/models/category.rb
index a99117812cf..b404389e723 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -55,6 +55,8 @@ class Category < ActiveRecord::Base
validate :ensure_slug
+ validate :permissions_compatibility_validator
+
validates :auto_close_hours, numericality: { greater_than: 0, less_than_or_equal_to: 87600 }, allow_nil: true
after_create :create_category_definition
@@ -631,6 +633,43 @@ class Category < ActiveRecord::Base
true
end
end
+
+ def permissions_compatibility_validator
+ # when saving subcategories
+ if @permissions && parent_category_id.present?
+ return if parent_category.category_groups.empty?
+
+ parent_permissions = parent_category.category_groups.pluck(:group_id, :permission_type)
+ child_permissions = @permissions.empty? ? [[Group[:everyone].id, CategoryGroup.permission_types[:full]]] : @permissions
+ check_permissions_compatibility(parent_permissions, child_permissions)
+
+ # when saving parent category
+ elsif @permissions && subcategories.present?
+ return if @permissions.empty?
+
+ parent_permissions = @permissions
+ child_permissions = subcategories_permissions.uniq
+
+ check_permissions_compatibility(parent_permissions, child_permissions)
+ end
+ end
+
+ private
+
+ def check_permissions_compatibility(parent_permissions, child_permissions)
+ parent_groups = parent_permissions.map(&:first)
+ child_groups = child_permissions.map(&:first)
+ only_in_subcategory = child_groups - parent_groups
+
+ errors.add(:base, I18n.t("category.errors.permission_conflict")) if only_in_subcategory.present?
+ end
+
+ def subcategories_permissions
+ CategoryGroup.joins(:category)
+ .where(['categories.parent_category_id = ?', self.id])
+ .pluck(:group_id, :permission_type)
+ .uniq
+ end
end
# == Schema Information
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 022642e1233..649875891be 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -586,6 +586,7 @@ en:
email_already_used_in_group: "'%{email}' is already used by the group '%{group_name}'."
email_already_used_in_category: "'%{email}' is already used by the category '%{category_name}'."
description_incomplete: "The category description post must have at least one paragraph."
+ permission_conflict: "Subcategory permissions cannot be less restrictive than parent's."
cannot_delete:
uncategorized: "Can't delete Uncategorized"
has_subcategories: "Can't delete this category because it has sub-categories."
diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb
index 2406c25a403..a5f8a48402f 100644
--- a/spec/models/category_spec.rb
+++ b/spec/models/category_spec.rb
@@ -751,4 +751,77 @@ describe Category do
end
end
+ describe "validate permissions compatibility" do
+ let(:admin) { Fabricate(:admin) }
+ let(:group) { Fabricate(:group) }
+ let(:group2) { Fabricate(:group) }
+ let(:parent_category) { Fabricate(:category, name: "parent") }
+ let(:subcategory) { Fabricate(:category, name: "child1", parent_category_id: parent_category.id) }
+ let(:subcategory2) { Fabricate(:category, 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
+ parent_category.set_permissions(group => :readonly)
+ parent_category.save!
+
+ subcategory.set_permissions(group => :full, group2 => :readonly)
+
+ expect(subcategory.valid?).to eq(false)
+ expect(subcategory.errors.full_messages).to eq([I18n.t("category.errors.permission_conflict")])
+ end
+
+ it "is valid if permissions are same or more restrictive" do
+ parent_category.set_permissions(group => :full, group2 => :create_post)
+ parent_category.save!
+
+ subcategory.set_permissions(group => :create_post, group2 => :full)
+
+ expect(subcategory.valid?).to eq(true)
+ end
+
+ it "is valid if no permissions are set on parent" do
+ parent_category.set_permissions(everyone: :full)
+ parent_category.save!
+
+ subcategory.set_permissions(group => :create_post, group2 => :create_post)
+
+ expect(subcategory.valid?).to eq(true)
+ end
+ end
+
+ context "when changing parent category permissions" do
+ it "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)
+ subcategory2.save!
+
+ parent_category.set_permissions(group => :readonly)
+
+ expect(parent_category.valid?).to eq(false)
+ expect(parent_category.errors.full_messages).to eq([I18n.t("category.errors.permission_conflict")])
+ end
+
+ it "is valid if subcategory permissions are same or more restrictive" do
+ subcategory.set_permissions(group => :create_post)
+ subcategory.save!
+ subcategory2.set_permissions(group => :create_post, group2 => :create_post)
+ subcategory2.save!
+
+ parent_category.set_permissions(group => :full, group2 => :create_post)
+
+ expect(parent_category.valid?).to eq(true)
+
+ end
+
+ it "is valid if no permissions set on parent" do
+ subcategory.set_permissions(group => :create_post)
+ subcategory.save
+ parent_category.set_permissions(everyone: :full)
+
+ expect(parent_category.valid?).to eq(true)
+ end
+ end
+ end
+
end