From 25b4e82601abae40b2a82ac592f9a8153364fdf8 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Fri, 8 Jan 2021 20:35:13 +0530 Subject: [PATCH] FEATURE: allow disabling self-deletions of posts (#11668) https://meta.discourse.org/t/restoring-deleted-messages/173647/6?u=techapj --- app/controllers/posts_controller.rb | 8 +++++--- config/locales/server.en.yml | 4 ++-- lib/guardian/post_guardian.rb | 23 +++++++++++++++-------- spec/components/guardian_spec.rb | 6 +++++- spec/requests/posts_controller_spec.rb | 18 ++++++++++++++++++ 5 files changed, 45 insertions(+), 14 deletions(-) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index cfb25ee9094..19e42aa1205 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -299,13 +299,13 @@ class PostsController < ApplicationController def destroy post = find_post_from_params + guardian.ensure_can_delete!(post) + unless guardian.can_moderate_topic?(post.topic) RateLimiter.new(current_user, "delete_post_per_min", SiteSetting.max_post_deletions_per_minute, 1.minute).performed! RateLimiter.new(current_user, "delete_post_per_day", SiteSetting.max_post_deletions_per_day, 1.day).performed! end - guardian.ensure_can_delete!(post) - destroyer = PostDestroyer.new(current_user, post, context: params[:context]) destroyer.destroy @@ -320,11 +320,13 @@ class PostsController < ApplicationController def recover post = find_post_from_params + guardian.ensure_can_recover_post!(post) + unless guardian.can_moderate_topic?(post.topic) RateLimiter.new(current_user, "delete_post_per_min", SiteSetting.max_post_deletions_per_minute, 1.minute).performed! RateLimiter.new(current_user, "delete_post_per_day", SiteSetting.max_post_deletions_per_day, 1.day).performed! end - guardian.ensure_can_recover_post!(post) + destroyer = PostDestroyer.new(current_user, post) destroyer.recover post.reload diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 1f0e1def60f..ec32420b17d 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1746,8 +1746,8 @@ en: max_logins_per_ip_per_hour: "Maximum number of logins allowed per IP address per hour" max_logins_per_ip_per_minute: "Maximum number of logins allowed per IP address per minute" - max_post_deletions_per_minute: "Maximum number of posts a user can delete per minute." - max_post_deletions_per_day: "Maximum number of posts a user can delete per day." + max_post_deletions_per_minute: "Maximum number of posts a user can delete per minute. Set to 0 to disable post deletions." + max_post_deletions_per_day: "Maximum number of posts a user can delete per day. Set to 0 to disable post deletions." invite_link_max_redemptions_limit: "Maximum redemptions allowed for invite links can't be more than this value." diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb index ca10a2924a9..7af04242671 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -185,14 +185,19 @@ module PostGuardian # Can't delete the first post return false if post.is_first_post? - # Can't delete posts in archived topics unless you are staff can_moderate = can_moderate_topic?(post.topic) - return false if !can_moderate && post.topic&.archived? + return true if can_moderate + + # Can't delete posts in archived topics unless you are staff + return false if post.topic&.archived? # You can delete your own posts - return !post.user_deleted? if is_my_own?(post) + if is_my_own?(post) + return false if (SiteSetting.max_post_deletions_per_minute < 1 || SiteSetting.max_post_deletions_per_day < 1) + return true if !post.user_deleted? + end - can_moderate + false end def can_recover_post?(post) @@ -200,12 +205,14 @@ module PostGuardian # PERF, vast majority of the time topic will not be deleted topic = (post.topic || Topic.with_deleted.find(post.topic_id)) if post.topic_id + return true if can_moderate_topic?(topic) && !!post.deleted_at - if can_moderate_topic?(topic) - !!post.deleted_at - else - is_my_own?(post) && post.user_deleted && !post.deleted_at + if is_my_own?(post) + return false if (SiteSetting.max_post_deletions_per_minute < 1 || SiteSetting.max_post_deletions_per_day < 1) + return true if post.user_deleted && !post.deleted_at end + + false end def can_delete_post_action?(post_action) diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index e2a2a9bca06..68b39f89fc9 100644 --- a/spec/components/guardian_spec.rb +++ b/spec/components/guardian_spec.rb @@ -2081,6 +2081,11 @@ describe Guardian do expect(Guardian.new(user).can_delete?(post)).to be_truthy end + it 'returns false when self deletions are disabled' do + SiteSetting.max_post_deletions_per_day = 0 + expect(Guardian.new(user).can_delete?(post)).to be_falsey + end + it "returns false when trying to delete another user's own post" do expect(Guardian.new(Fabricate(:user)).can_delete?(post)).to be_falsey end @@ -2119,7 +2124,6 @@ describe Guardian do it "doesn't allow a regular user to delete it" do expect(Guardian.new(post.user).can_delete?(post)).to be_falsey end - end end diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index d4b4decac48..1d22435ac21 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/spec/requests/posts_controller_spec.rb @@ -203,6 +203,15 @@ describe PostsController do expect(response).to be_forbidden end + it "raises an error when the self deletions are disabled" do + SiteSetting.max_post_deletions_per_day = 0 + post = Fabricate(:post, user: user, topic: topic, post_number: 3) + sign_in(user) + + delete "/posts/#{post.id}.json" + expect(response).to be_forbidden + end + it "uses a PostDestroyer" do post = Fabricate(:post, topic_id: topic.id, post_number: 3) sign_in(moderator) @@ -306,6 +315,15 @@ describe PostsController do expect(response).to be_forbidden end + it "raises an error when self deletion/recovery is disabled" do + SiteSetting.max_post_deletions_per_day = 0 + post = Fabricate(:post, user: user, topic: topic, post_number: 3) + sign_in(user) + + put "/posts/#{post.id}/recover.json" + expect(response).to be_forbidden + end + it "recovers a post correctly" do topic_id = create_post.topic_id post = create_post(topic_id: topic_id)