From 1c870382556d255102ba72a31695915390458997 Mon Sep 17 00:00:00 2001 From: jbrw Date: Wed, 2 Dec 2020 17:21:59 -0500 Subject: [PATCH] FEATURE: Allow Category Group Moderators to edit topic titles (#11340) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * FEATURE: Allow Category Group Moderators to edit topic titles Adds category group moderators to the topic guardian’s `can_edit` method. The value of `can_edit` is returned by the topic view serializer, and this value determines whether the current user can edit the title/category/tags of the topic directly (which category group moderators could already do by editing the first post of a topic). Note that the value of `can_edit` is now always returned by the topic view serializer (ie, for both true and false values) to cover the case where a topic is moved out of a category that a category group moderator has permissions on, so that when the topic is reloaded the UI picks up that `can_edit` is now false, and thus the edit icon should no longer be displayed. * DEV: Add a comment explaining why `can_edit` is always returned --- .../topic_view_details_serializer.rb | 15 ++++++++----- lib/guardian/topic_guardian.rb | 1 + .../serializers/topic_view_serializer_spec.rb | 21 +++++++++++++++++++ 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/app/serializers/topic_view_details_serializer.rb b/app/serializers/topic_view_details_serializer.rb index ef152591c57..9988675130c 100644 --- a/app/serializers/topic_view_details_serializer.rb +++ b/app/serializers/topic_view_details_serializer.rb @@ -4,7 +4,6 @@ class TopicViewDetailsSerializer < ApplicationSerializer def self.can_attributes [:can_move_posts, - :can_edit, :can_delete, :can_recover, :can_remove_allowed_users, @@ -24,7 +23,10 @@ class TopicViewDetailsSerializer < ApplicationSerializer :can_moderate_category] end + # NOTE: `can_edit` is defined as an attribute because we explicitly want + # it returned even if it has a value of `false` attributes( + :can_edit, :notification_level, :notifications_reason_id, *can_attributes, @@ -87,6 +89,13 @@ class TopicViewDetailsSerializer < ApplicationSerializer define_method(ca) { true } end + # NOTE: A Category Group Moderator moving a topic to a different category + # may result in the 'can_edit?' result changing from `true` to `false`. + # Explictly returning a `false` value is required to update the client UI. + def can_edit + scope.can_edit?(object.topic) + end + def include_can_review_topic? scope.can_review_topic?(object.topic) end @@ -95,10 +104,6 @@ class TopicViewDetailsSerializer < ApplicationSerializer scope.can_move_posts?(object.topic) end - def include_can_edit? - scope.can_edit?(object.topic) - end - def include_can_delete? scope.can_delete?(object.topic) end diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb index fdd13e703fe..92e4c6702a1 100644 --- a/lib/guardian/topic_guardian.rb +++ b/lib/guardian/topic_guardian.rb @@ -79,6 +79,7 @@ module TopicGuardian return true if is_admin? return true if is_moderator? && can_create_post?(topic) + return true if is_category_group_moderator?(topic.category) # can't edit topics in secured categories where you don't have permission to create topics # except for a tiny edge case where the topic is uncategorized and you are trying diff --git a/spec/serializers/topic_view_serializer_spec.rb b/spec/serializers/topic_view_serializer_spec.rb index 421b65ce371..b9a9d03e250 100644 --- a/spec/serializers/topic_view_serializer_spec.rb +++ b/spec/serializers/topic_view_serializer_spec.rb @@ -365,6 +365,27 @@ describe TopicViewSerializer do expect(json[:details][:can_edit_tags]).to eq(true) end end + + context "can_edit" do + fab!(:group_user) { Fabricate(:group_user) } + fab!(:category) { Fabricate(:category, reviewable_by_group: group_user.group) } + fab!(:topic) { Fabricate(:topic, category: category) } + let(:user) { group_user.user } + + before do + SiteSetting.enable_category_group_moderation = true + end + + it 'explicitly returns can_edit' do + json = serialize_topic(topic, user) + expect(json[:details][:can_edit]).to eq(true) + + topic.update!(category: nil) + + json = serialize_topic(topic, user) + expect(json[:details][:can_edit]).to eq(false) + end + end end context "published_page" do