From ff1c53dd6fa2a882ef104117274a0b77dddd424a Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Mon, 28 Jun 2021 10:39:13 +0800 Subject: [PATCH] FIX: Missing category edit icon. Follow-up to 0e4b8c5318569ef7e7a111563709699e3b9ce219 --- app/models/site.rb | 22 ++++++++----- app/serializers/site_category_serializer.rb | 1 - lib/guardian/category_guardian.rb | 9 ++++++ spec/models/site_spec.rb | 35 +++++++++++++++------ 4 files changed, 48 insertions(+), 19 deletions(-) diff --git a/app/models/site.rb b/app/models/site.rb index ced763c06df..338091faeed 100644 --- a/app/models/site.rb +++ b/app/models/site.rb @@ -67,14 +67,14 @@ class Site self.class.all_categories_cache.each do |category| if @guardian.can_see_serialized_category?(category_id: category[:id], read_restricted: category[:read_restricted]) - categories << OpenStruct.new(category) + categories << category end end with_children = Set.new categories.each do |c| - if c.parent_category_id - with_children << c.parent_category_id + if c[:parent_category_id] + with_children << c[:parent_category_id] end end @@ -91,13 +91,19 @@ class Site default_notification_level = CategoryUser.default_notification_level categories.each do |category| - category.notification_level = notification_levels[category.id] || default_notification_level - category.permission = CategoryGroup.permission_types[:full] if allowed_topic_create&.include?(category.id) || @guardian.is_admin? - category.has_children = with_children.include?(category.id) - by_id[category.id] = category + category[:notification_level] = notification_levels[category[:id]] || default_notification_level + category[:permission] = CategoryGroup.permission_types[:full] if allowed_topic_create&.include?(category[:id]) || @guardian.is_admin? + category[:has_children] = with_children.include?(category[:id]) + + category[:can_edit] = @guardian.can_edit_serialized_category?( + category_id: category[:id], + read_restricted: category[:read_restricted] + ) + + by_id[category[:id]] = category end - categories.reject! { |c| c.parent_category_id && !by_id[c.parent_category_id] } + categories.reject! { |c| c[:parent_category_id] && !by_id[c[:parent_category_id]] } categories end end diff --git a/app/serializers/site_category_serializer.rb b/app/serializers/site_category_serializer.rb index 89868e93895..8634ba38740 100644 --- a/app/serializers/site_category_serializer.rb +++ b/app/serializers/site_category_serializer.rb @@ -32,5 +32,4 @@ class SiteCategorySerializer < BasicCategorySerializer def required_tag_group_name object.required_tag_group&.name end - end diff --git a/lib/guardian/category_guardian.rb b/lib/guardian/category_guardian.rb index ef8441071b6..94a48466d6a 100644 --- a/lib/guardian/category_guardian.rb +++ b/lib/guardian/category_guardian.rb @@ -22,6 +22,15 @@ module CategoryGuardian ) end + def can_edit_serialized_category?(category_id:, read_restricted:) + is_admin? || + ( + SiteSetting.moderators_manage_categories_and_groups && + is_moderator? && + can_see_serialized_category?(category_id: category_id, read_restricted: read_restricted) + ) + end + def can_delete_category?(category) can_edit_category?(category) && category.topic_count <= 0 && diff --git a/spec/models/site_spec.rb b/spec/models/site_spec.rb index 3e2e0c2e81b..5ce3426a19f 100644 --- a/spec/models/site_spec.rb +++ b/spec/models/site_spec.rb @@ -54,11 +54,11 @@ describe Site do it "returns correct notification level for categories" do category = Fabricate(:category) guardian = Guardian.new - expect(Site.new(guardian).categories.last.notification_level).to eq(1) + expect(Site.new(guardian).categories.last[:notification_level]).to eq(1) SiteSetting.mute_all_categories_by_default = true - expect(Site.new(guardian).categories.last.notification_level).to eq(0) + expect(Site.new(guardian).categories.last[:notification_level]).to eq(0) SiteSetting.default_categories_tracking = category.id.to_s - expect(Site.new(guardian).categories.last.notification_level).to eq(1) + expect(Site.new(guardian).categories.last[:notification_level]).to eq(1) end describe '#categories' do @@ -67,13 +67,13 @@ describe Site do fab!(:guardian) { Guardian.new(user) } it "omits read restricted categories" do - expect(Site.new(guardian).categories.map(&:id)).to contain_exactly( + expect(Site.new(guardian).categories.map { |c| c[:id] }).to contain_exactly( SiteSetting.uncategorized_category_id, category.id ) category.update!(read_restricted: true) - expect(Site.new(guardian).categories.map(&:id)).to contain_exactly( + expect(Site.new(guardian).categories.map { |c| c[:id] }).to contain_exactly( SiteSetting.uncategorized_category_id ) end @@ -83,13 +83,13 @@ describe Site do category.update!(read_restricted: true) category.groups << group - expect(Site.new(guardian).categories.map(&:id)).to contain_exactly( + expect(Site.new(guardian).categories.map { |c| c[:id] }).to contain_exactly( SiteSetting.uncategorized_category_id ) group.add(user) - expect(Site.new(Guardian.new(user)).categories.map(&:id)).to contain_exactly( + expect(Site.new(Guardian.new(user)).categories.map { |c| c[:id] }).to contain_exactly( SiteSetting.uncategorized_category_id, category.id ) end @@ -104,9 +104,8 @@ describe Site do expect(Site.new(guardian) .categories - .keep_if { |c| c.name == category.name } - .first - .permission) + .keep_if { |c| c[:name] == category.name } + .first[:permission]) .not_to eq(CategoryGroup.permission_types[:full]) # If a parent category is not visible, the child categories should not be returned @@ -138,6 +137,22 @@ describe Site do ensure Site.preloaded_category_custom_fields.clear end + + it 'sets the can_edit field for categories correctly' do + categories = Site.new(Guardian.new).categories + + expect(categories.map { |c| c[:can_edit] }).to contain_exactly(false, false) + + site = Site.new(Guardian.new(Fabricate(:moderator))) + + expect(site.categories.map { |c| c[:can_edit] }).to contain_exactly(false, false) + + SiteSetting.moderators_manage_categories_and_groups = true + + site = Site.new(Guardian.new(Fabricate(:moderator))) + + expect(site.categories.map { |c| c[:can_edit] }).to contain_exactly(true, true) + end end it "omits groups user can not see" do