From 2aec92d0b4262a58cce86ed4c25a3346e77198f1 Mon Sep 17 00:00:00 2001 From: jbrw Date: Thu, 23 Jul 2020 09:50:00 -0400 Subject: [PATCH] FEATURE - allow Group Moderators to edit category description (#10292) Co-authored-by: Alan Guo Xiang Tan --- app/models/post.rb | 4 ++ lib/guardian/category_guardian.rb | 4 ++ lib/guardian/post_guardian.rb | 4 ++ lib/post_revisor.rb | 9 +++- spec/components/post_revisor_spec.rb | 22 +++++++++ spec/requests/posts_controller_spec.rb | 47 +++++++++++++++++++ spec/services/staff_action_logger_spec.rb | 22 +++++++++ .../acceptance/post-admin-menu-test.js | 2 + 8 files changed, 112 insertions(+), 2 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index a9880a8d26d..766703268b0 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -485,6 +485,10 @@ class Post < ActiveRecord::Base post_number == 1 end + def is_category_description? + topic.present? && topic.is_category_topic? && is_first_post? + end + def is_reply_by_email? via_email && post_number.present? && post_number > 1 end diff --git a/lib/guardian/category_guardian.rb b/lib/guardian/category_guardian.rb index 5787642161d..3699bcea2c9 100644 --- a/lib/guardian/category_guardian.rb +++ b/lib/guardian/category_guardian.rb @@ -54,6 +54,10 @@ module CategoryGuardian secure_category_ids.include?(category.id) end + def can_edit_category_description?(category) + can_perform_action_available_to_group_moderators?(category.topic) + end + def secure_category_ids @secure_category_ids ||= @user.secure_category_ids end diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb index d05296ef336..2827f5462bf 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -161,6 +161,10 @@ module PostGuardian return !post.edit_time_limit_expired?(@user) end + if post.is_category_description? + return true if can_edit_category_description?(post.topic.category) + end + false end diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb index fc4abeeb2c3..0b7a6502f1d 100644 --- a/lib/post_revisor.rb +++ b/lib/post_revisor.rb @@ -192,8 +192,13 @@ class PostRevisor PostLocker.new(@post, @editor).lock end - # We log staff edits to posts - if @editor.staff? && @editor.id != @post.user_id && @fields.has_key?('raw') && !@opts[:skip_staff_log] + # We log staff/group moderator edits to posts + if ( + (@editor.staff? || (@post.is_category_description? && Guardian.new(@editor).can_edit_category_description?(@post.topic.category))) && + @editor.id != @post.user_id && + @fields.has_key?('raw') && + !@opts[:skip_staff_log] + ) StaffActionLogger.new(@editor).log_post_edit( @post, old_raw: old_raw diff --git a/spec/components/post_revisor_spec.rb b/spec/components/post_revisor_spec.rb index b79490b1393..bb8c0751756 100644 --- a/spec/components/post_revisor_spec.rb +++ b/spec/components/post_revisor_spec.rb @@ -598,6 +598,28 @@ describe PostRevisor do end end + context "logging group moderator edits" do + fab!(:group_user) { Fabricate(:group_user) } + fab!(:category) { Fabricate(:category, reviewable_by_group_id: group_user.group.id, topic: topic) } + + before do + SiteSetting.enable_category_group_moderation = true + topic.update!(category: category) + post.update!(topic: topic) + end + + it "logs an edit when a group moderator revises the category description" do + PostRevisor.new(post).revise!(group_user.user, raw: "a group moderator can update the description") + + log = UserHistory.where( + acting_user_id: group_user.user.id, + action: UserHistory.actions[:post_edit] + ).first + expect(log).to be_present + expect(log.details).to eq("Hello world\n\n---\n\na group moderator can update the description") + end + end + context "staff_edit_locks_post" do context "disabled" do diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index ef5bf4e46f4..6622b42acf6 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/spec/requests/posts_controller_spec.rb @@ -426,6 +426,53 @@ describe PostsController do end end + describe "when logged in as group moderator" do + fab!(:topic) { Fabricate(:topic, category: category) } + fab!(:post) { Fabricate(:post, user: user, topic: topic) } + fab!(:group_user) { Fabricate(:group_user) } + let(:user_gm) { group_user.user } + let(:group) { group_user.group } + + before do + SiteSetting.enable_category_group_moderation = true + post.topic.category.update!(reviewable_by_group_id: group.id, topic_id: topic.id) + sign_in(user_gm) + end + + it "allows updating the category description" do + put "/posts/#{post.id}.json", params: update_params + expect(response.status).to eq(200) + + post.reload + expect(post.raw).to eq('edited body') + expect(UserHistory.where(action: UserHistory.actions[:post_edit]).count).to eq(1) + end + + it "can not update other posts within the primary category topic" do + second_post = Fabricate(:post, user: user, topic: topic) + + put "/posts/#{second_post.id}.json", params: update_params + expect(response.status).to eq(403) + end + + it "can not update other first posts of topics in the same category" do + second_topic_in_category = Fabricate(:topic, category: category) + post_in_second_topic = Fabricate(:post, user: user, topic: second_topic_in_category) + + put "/posts/#{post_in_second_topic.id}.json", params: update_params + expect(response.status).to eq(403) + end + + it "can not update category descriptions in other categories" do + second_category = Fabricate(:category) + topic.update!(category: second_category) + + put "/posts/#{post.id}.json", params: update_params + expect(response.status).to eq(403) + end + + end + it 'can not change category to a disallowed category' do post = create_post sign_in(post.user) diff --git a/spec/services/staff_action_logger_spec.rb b/spec/services/staff_action_logger_spec.rb index 1d6bb3bea76..09729848890 100644 --- a/spec/services/staff_action_logger_spec.rb +++ b/spec/services/staff_action_logger_spec.rb @@ -595,4 +595,26 @@ describe StaffActionLogger do end end + describe 'log_post_staff_note' do + fab!(:post) { Fabricate(:post) } + + it "raises an error when argument is missing" do + expect { logger.log_topic_archived(nil) }.to raise_error(Discourse::InvalidParameters) + end + + it "creates a new UserHistory record" do + expect { logger.log_post_staff_note(post, { new_raw_value: 'my note', old_value: nil }) }.to change { UserHistory.count }.by(1) + user_history = UserHistory.last + expect(user_history.action).to eq(UserHistory.actions[:post_staff_note_create]) + expect(user_history.new_value).to eq('my note') + expect(user_history.previous_value).to eq(nil) + + expect { logger.log_post_staff_note(post, { new_raw_value: nil, old_value: 'my note' }) }.to change { UserHistory.count }.by(1) + user_history = UserHistory.last + expect(user_history.action).to eq(UserHistory.actions[:post_staff_note_destroy]) + expect(user_history.new_value).to eq(nil) + expect(user_history.previous_value).to eq('my note') + end + end + end diff --git a/test/javascripts/acceptance/post-admin-menu-test.js b/test/javascripts/acceptance/post-admin-menu-test.js index 277fc9e3d98..c17e96bb7fa 100644 --- a/test/javascripts/acceptance/post-admin-menu-test.js +++ b/test/javascripts/acceptance/post-admin-menu-test.js @@ -7,6 +7,7 @@ QUnit.test("Enter as a anon user", async assert => { await click(".show-more-actions"); assert.ok(exists("#topic"), "The topic was rendered"); + assert.ok(exists("#post_1 .post-controls .edit"), "The edit button was not rendered"); assert.ok(!exists(".show-post-admin-menu"), "The wrench button was not rendered"); }); @@ -17,5 +18,6 @@ QUnit.test("Enter as a user with group moderator permissions", async assert => { await click(".show-more-actions"); await click(".show-post-admin-menu"); + assert.ok(exists("#post_1 .post-controls .edit"), "The edit button was rendered"); assert.ok(exists(".add-notice"), "The add notice button was rendered"); });