From 0dc96ce817b68df7edefb6eaa9f8d56f93b58378 Mon Sep 17 00:00:00 2001 From: Guhyoun Nam <70915823+rngus2344@users.noreply.github.com> Date: Tue, 13 Jul 2021 09:40:11 -0500 Subject: [PATCH] FEATURE: Setting to allow moderators to change post ownership (#13708) --- .../discourse/app/widgets/post-admin-menu.js | 5 +- config/locales/server.en.yml | 1 + config/site_settings.yml | 3 + lib/guardian/post_guardian.rb | 4 +- spec/requests/topics_controller_spec.rb | 150 ++++++++++-------- 5 files changed, 92 insertions(+), 71 deletions(-) diff --git a/app/assets/javascripts/discourse/app/widgets/post-admin-menu.js b/app/assets/javascripts/discourse/app/widgets/post-admin-menu.js index f689d5a6761..e6b58b6a41b 100644 --- a/app/assets/javascripts/discourse/app/widgets/post-admin-menu.js +++ b/app/assets/javascripts/discourse/app/widgets/post-admin-menu.js @@ -74,7 +74,10 @@ export function buildManageButtons(attrs, currentUser, siteSettings) { }); } - if (currentUser.admin) { + if ( + currentUser.admin || + (siteSettings.moderators_change_post_ownership && currentUser.staff) + ) { contents.push({ icon: "user", label: "post.controls.change_owner", diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index e8a0a751ce1..1633d234f49 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1612,6 +1612,7 @@ en: gtm_container_id: "Google Tag Manager container id. eg: GTM-ABCDEF.
Note: Third-party scripts loaded by GTM may need to be allowlisted in 'content security policy script src'." enable_escaped_fragments: "Fall back to Google's Ajax-Crawling API if no webcrawler is detected. See https://developers.google.com/webmasters/ajax-crawling/docs/learn-more" moderators_manage_categories_and_groups: "Allow moderators to manage categories and groups" + moderators_change_post_ownership: "Allow moderators to change post ownership" cors_origins: "Allowed origins for cross-origin requests (CORS). Each origin must include http:// or https://. The DISCOURSE_ENABLE_CORS env variable must be set to true to enable CORS." use_admin_ip_allowlist: "Admins can only log in if they are at an IP address defined in the Screened IPs list (Admin > Logs > Screened Ips)." blocked_ip_blocks: "A list of private IP blocks that should never be crawled by Discourse" diff --git a/config/site_settings.yml b/config/site_settings.yml index f76484642c2..45f5d2a0f57 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1566,6 +1566,9 @@ security: enable_escaped_fragments: true allow_index_in_robots_txt: true moderators_manage_categories_and_groups: false + moderators_change_post_ownership: + client: true + default: false moderators_view_emails: client: true default: false diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb index 99ac97ca2e5..fc3bbeb2d05 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -251,7 +251,9 @@ module PostGuardian end def can_change_post_owner? - is_admin? + return true if is_admin? + + SiteSetting.moderators_change_post_ownership && is_staff? end def can_change_post_timestamps? diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index ccc4bf121c2..e5e2c356f40 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -625,18 +625,6 @@ RSpec.describe TopicsController do expect(response).to be_forbidden end - describe 'forbidden to moderators' do - before do - sign_in(moderator) - end - it 'correctly denies' do - post "/t/111/change-owner.json", params: { - topic_id: 111, username: 'user_a', post_ids: [1, 2, 3] - } - expect(response).to be_forbidden - end - end - describe 'forbidden to trust_level_4s' do before do sign_in(trust_level_4) @@ -651,80 +639,104 @@ RSpec.describe TopicsController do end describe 'changing ownership' do - let!(:editor) { sign_in(admin) } fab!(:topic) { Fabricate(:topic) } fab!(:user_a) { Fabricate(:user) } fab!(:p1) { Fabricate(:post, topic: topic) } fab!(:p2) { Fabricate(:post, topic: topic) } - it "raises an error with a parameter missing" do - [ - { post_ids: [1, 2, 3] }, - { username: 'user_a' } - ].each do |params| - post "/t/111/change-owner.json", params: params - expect(response.status).to eq(400) + describe 'moderator signed in' do + let!(:editor) { sign_in(moderator) } + + it "returns 200 when moderators_change_post_ownership is true" do + SiteSetting.moderators_change_post_ownership = true + + post "/t/#{topic.id}/change-owner.json", params: { + username: user_a.username_lower, post_ids: [p1.id] + } + expect(response.status).to eq(200) + end + + it "returns 403 when moderators_change_post_ownership is false" do + SiteSetting.moderators_change_post_ownership = false + + post "/t/#{topic.id}/change-owner.json", params: { + username: user_a.username_lower, post_ids: [p1.id] + } + expect(response.status).to eq(403) end end + describe 'admin signed in' do + let!(:editor) { sign_in(admin) } - it "changes the topic and posts ownership" do - post "/t/#{topic.id}/change-owner.json", params: { - username: user_a.username_lower, post_ids: [p1.id] - } - topic.reload - p1.reload - expect(response.status).to eq(200) - expect(topic.user.username).to eq(user_a.username) - expect(p1.user.username).to eq(user_a.username) - end + it "raises an error with a parameter missing" do + [ + { post_ids: [1, 2, 3] }, + { username: 'user_a' } + ].each do |params| + post "/t/111/change-owner.json", params: params + expect(response.status).to eq(400) + end + end - it "changes multiple posts" do - post "/t/#{topic.id}/change-owner.json", params: { - username: user_a.username_lower, post_ids: [p1.id, p2.id] - } + it "changes the topic and posts ownership" do + post "/t/#{topic.id}/change-owner.json", params: { + username: user_a.username_lower, post_ids: [p1.id] + } + topic.reload + p1.reload + expect(response.status).to eq(200) + expect(topic.user.username).to eq(user_a.username) + expect(p1.user.username).to eq(user_a.username) + end - expect(response.status).to eq(200) + it "changes multiple posts" do + post "/t/#{topic.id}/change-owner.json", params: { + username: user_a.username_lower, post_ids: [p1.id, p2.id] + } - p1.reload - p2.reload + expect(response.status).to eq(200) - expect(p1.user).to_not eq(nil) - expect(p1.reload.user).to eq(p2.reload.user) - end + p1.reload + p2.reload - it "works with deleted users" do - deleted_user = user - t2 = Fabricate(:topic, user: deleted_user) - p3 = Fabricate(:post, topic: t2, user: deleted_user) + expect(p1.user).to_not eq(nil) + expect(p1.reload.user).to eq(p2.reload.user) + end - UserDestroyer.new(editor).destroy(deleted_user, delete_posts: true, context: 'test', delete_as_spammer: true) + it "works with deleted users" do + deleted_user = user + t2 = Fabricate(:topic, user: deleted_user) + p3 = Fabricate(:post, topic: t2, user: deleted_user) - post "/t/#{t2.id}/change-owner.json", params: { - username: user_a.username_lower, post_ids: [p3.id] - } + UserDestroyer.new(editor).destroy(deleted_user, delete_posts: true, context: 'test', delete_as_spammer: true) - expect(response.status).to eq(200) - t2.reload - p3.reload - expect(t2.deleted_at).to be_nil - expect(p3.user).to eq(user_a) - end + post "/t/#{t2.id}/change-owner.json", params: { + username: user_a.username_lower, post_ids: [p3.id] + } - it "removes likes by new owner" do - now = Time.zone.now - freeze_time(now - 1.day) - PostActionCreator.like(user_a, p1) - p1.reload - freeze_time(now) - post "/t/#{topic.id}/change-owner.json", params: { - username: user_a.username_lower, post_ids: [p1.id] - } - topic.reload - p1.reload - expect(response.status).to eq(200) - expect(topic.user.username).to eq(user_a.username) - expect(p1.user.username).to eq(user_a.username) - expect(p1.like_count).to eq(0) + expect(response.status).to eq(200) + t2.reload + p3.reload + expect(t2.deleted_at).to be_nil + expect(p3.user).to eq(user_a) + end + + it "removes likes by new owner" do + now = Time.zone.now + freeze_time(now - 1.day) + PostActionCreator.like(user_a, p1) + p1.reload + freeze_time(now) + post "/t/#{topic.id}/change-owner.json", params: { + username: user_a.username_lower, post_ids: [p1.id] + } + topic.reload + p1.reload + expect(response.status).to eq(200) + expect(topic.user.username).to eq(user_a.username) + expect(p1.user.username).to eq(user_a.username) + expect(p1.like_count).to eq(0) + end end end end