From 292d3677e95300088307b4c85672ac3ab9c4ddd2 Mon Sep 17 00:00:00 2001 From: Isaac Janzen <50783505+janzenisaac@users.noreply.github.com> Date: Thu, 19 Jan 2023 15:09:01 -0600 Subject: [PATCH] FEATURE: Allow admins to permanently delete revisions (#19913) # Context This PR introduces the ability to permanently delete revisions from a post while maintaining the changes implemented by the revisions. Additional Context: /t/90301 # Functionality In the case a staff member wants to _remove the visual cue_ that a post has been edited eg. Screenshot 2023-01-18 at 2 59 12 PM while maintaining the changes made in the edits, they can enable the (hidden) site setting of `can_permanently_delete`. When this is enabled, after _hiding_ the revisions Screenshot 2023-01-19 at 1 53 35 PM there will be an additional button in the history modal to Delete revisions on a post. Screenshot 2023-01-19 at 1 49 51 PM Since this action is permanent, we display a confirmation dialog prior to triggering the destroy call Screenshot 2023-01-19 at 1 55 59 PM Once confirmed the history modal will close and the post will `rebake` to display an _unedited_ post. Screenshot 2023-01-19 at 1 56 35 PM see that there is not a visual que for _revision have been made on this post_ for a post that **HAS** been edited. In addition to this, a user history log for `purge_post_revisions` will be added for each action completed. # Limits - Admins are rate limited to 20 posts per minute --- .../discourse/app/controllers/history.js | 24 +++++++ .../javascripts/discourse/app/models/post.js | 6 ++ .../discourse/app/templates/modal/history.hbs | 12 +++- app/controllers/posts_controller.rb | 27 +++++++ app/models/user_history.rb | 2 + app/services/staff_action_logger.rb | 10 +++ config/locales/client.en.yml | 4 +- config/routes.rb | 1 + lib/guardian/post_revision_guardian.rb | 4 ++ spec/requests/posts_controller_spec.rb | 70 +++++++++++++++++++ 10 files changed, 158 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/discourse/app/controllers/history.js b/app/assets/javascripts/discourse/app/controllers/history.js index 00e64f86da3..0d428b226b0 100644 --- a/app/assets/javascripts/discourse/app/controllers/history.js +++ b/app/assets/javascripts/discourse/app/controllers/history.js @@ -114,6 +114,17 @@ export default Controller.extend(ModalFunctionality, { ); }, + permanentlyDeleteRevisions(postId) { + this.dialog.yesNoConfirm({ + message: I18n.t("post.revisions.controls.destroy_confirm"), + didConfirm: () => { + Post.permanentlyDeleteRevisions(postId).then(() => { + this.send("closeModal"); + }); + }, + }); + }, + show(postId, postVersion) { Post.showRevision(postId, postVersion).then(() => this.refresh(postId, postVersion) @@ -162,6 +173,7 @@ export default Controller.extend(ModalFunctionality, { }, displayRevisions: gt("model.version_count", 2), + displayGoToFirst: propertyGreaterThan( "model.current_revision", "model.first_revision" @@ -215,6 +227,15 @@ export default Controller.extend(ModalFunctionality, { return this.currentUser && this.currentUser.get("staff"); }, + @discourseComputed("model.previous_hidden") + displayPermanentlyDeleteButton(previousHidden) { + return ( + this.siteSettings.can_permanently_delete && + this.currentUser?.staff && + previousHidden + ); + }, + isEitherRevisionHidden: or("model.previous_hidden", "model.current_hidden"), @discourseComputed( @@ -352,6 +373,9 @@ export default Controller.extend(ModalFunctionality, { hideVersion() { this.hide(this.get("model.post_id"), this.get("model.current_revision")); }, + permanentlyDeleteVersions() { + this.permanentlyDeleteRevisions(this.get("model.post_id")); + }, showVersion() { this.show(this.get("model.post_id"), this.get("model.current_revision")); }, diff --git a/app/assets/javascripts/discourse/app/models/post.js b/app/assets/javascripts/discourse/app/models/post.js index 25a8f401686..1f9f1573ddc 100644 --- a/app/assets/javascripts/discourse/app/models/post.js +++ b/app/assets/javascripts/discourse/app/models/post.js @@ -461,6 +461,12 @@ Post.reopenClass({ }); }, + permanentlyDeleteRevisions(postId) { + return ajax(`/posts/${postId}/revisions/permanently_delete`, { + type: "DELETE", + }); + }, + showRevision(postId, version) { return ajax(`/posts/${postId}/revisions/${version}/show`, { type: "PUT", diff --git a/app/assets/javascripts/discourse/app/templates/modal/history.hbs b/app/assets/javascripts/discourse/app/templates/modal/history.hbs index 4a55aae7874..175a3dbc1f5 100644 --- a/app/assets/javascripts/discourse/app/templates/modal/history.hbs +++ b/app/assets/javascripts/discourse/app/templates/modal/history.hbs @@ -250,6 +250,16 @@ @disabled={{this.loading}} /> {{/if}} + + {{#if this.displayPermanentlyDeleteButton}} + + {{/if}} -{{/if}} \ No newline at end of file +{{/if}} diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index b5c9ad2e0e2..a9f061d426f 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -466,6 +466,33 @@ class PostsController < ApplicationController render body: nil end + def permanently_delete_revisions + guardian.ensure_can_permanently_delete_post_revisions! + + post = find_post_from_params + raise Discourse::InvalidParameters.new(:post) if post.blank? + raise Discourse::NotFound unless post.revisions.present? + + RateLimiter.new( + current_user, + "admin_permanently_delete_post_revisions", + 20, + 1.minute, + apply_limit_to_staff: true, + ).performed! + + ActiveRecord::Base.transaction do + updated_at = Time.zone.now + post.revisions.destroy_all + post.update(version: 1, public_version: 1, last_version_at: updated_at) + StaffActionLogger.new(current_user).log_permanently_delete_post_revisions(post) + end + + post.rebake! + + render body: nil + end + def show_revision post_revision = find_post_revision_from_params guardian.ensure_can_show_post_revision!(post_revision) diff --git a/app/models/user_history.rb b/app/models/user_history.rb index e3a98fe9b27..53cae312490 100644 --- a/app/models/user_history.rb +++ b/app/models/user_history.rb @@ -119,6 +119,7 @@ class UserHistory < ActiveRecord::Base watched_word_create: 97, watched_word_destroy: 98, delete_group: 99, + permanently_delete_post_revisions: 100, ) end @@ -213,6 +214,7 @@ class UserHistory < ActiveRecord::Base watched_word_create watched_word_destroy delete_group + permanently_delete_post_revisions ] end diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index 70c937e5613..9de363d855a 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -954,6 +954,16 @@ class StaffActionLogger ) end + def log_permanently_delete_post_revisions(post) + raise Discourse::InvalidParameters.new(:post) if post.nil? + + UserHistory.create!( + action: UserHistory.actions[:permanently_delete_post_revisions], + acting_user_id: @admin.id, + post_id: post.id, + ) + end + private def get_changes(changes) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 308b01f608f..3f3c0a71454 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1121,7 +1121,7 @@ en: perm_denied_expl: "You denied permission for notifications. Allow notifications via your browser settings." disable: "Disable Notifications" enable: "Enable Notifications" - each_browser_note: 'Note: You have to change this setting on every browser you use. All notifications will be disabled if you pause notifications from user menu, regardless of this setting.' + each_browser_note: "Note: You have to change this setting on every browser you use. All notifications will be disabled if you pause notifications from user menu, regardless of this setting." consent_prompt: "Do you want live notifications when people reply to your posts?" dismiss: "Dismiss" dismiss_notifications: "Dismiss All" @@ -3548,6 +3548,8 @@ en: last: "Last revision" hide: "Hide revision" show: "Show revision" + destroy: "Delete revisions" + destroy_confirm: "Are you sure you want to delete all of the revisions on this post? This action is permanent." revert: "Revert to revision %{revision}" edit_wiki: "Edit Wiki" edit_post: "Edit Post" diff --git a/config/routes.rb b/config/routes.rb index 8531638a997..606fffd6d50 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1082,6 +1082,7 @@ Discourse::Application.routes.draw do put "revisions/:revision/hide" => "posts#hide_revision", :constraints => { revision: /\d+/ } put "revisions/:revision/show" => "posts#show_revision", :constraints => { revision: /\d+/ } put "revisions/:revision/revert" => "posts#revert", :constraints => { revision: /\d+/ } + delete "revisions/permanently_delete" => "posts#permanently_delete_revisions" put "recover" collection do delete "destroy_many" diff --git a/lib/guardian/post_revision_guardian.rb b/lib/guardian/post_revision_guardian.rb index 1e61b19e746..9d2108ba2eb 100644 --- a/lib/guardian/post_revision_guardian.rb +++ b/lib/guardian/post_revision_guardian.rb @@ -13,6 +13,10 @@ module PostRevisionGuardian is_staff? end + def can_permanently_delete_post_revisions? + is_staff? && SiteSetting.can_permanently_delete + end + def can_show_post_revision?(post_revision) is_staff? end diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index a8e12fcf0b6..3feb0b52b4a 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/spec/requests/posts_controller_spec.rb @@ -2049,6 +2049,76 @@ RSpec.describe PostsController do end end + describe "#permanently_delete_revisions" do + before { SiteSetting.can_permanently_delete = true } + + fab!(:post) do + Fabricate( + :post, + user: Fabricate(:user), + raw: "Lorem ipsum dolor sit amet, cu nam libris tractatos, ancillae senserit ius ex", + ) + end + + fab!(:post_with_no_revisions) do + Fabricate( + :post, + user: Fabricate(:user), + raw: "Lorem ipsum dolor sit amet, cu nam libris tractatos, ancillae senserit ius ex", + ) + end + + fab!(:post_revision) { Fabricate(:post_revision, post: post) } + fab!(:post_revision_2) { Fabricate(:post_revision, post: post) } + + let(:post_id) { post.id } + + describe "when logged in as a regular user" do + it "does not delete revisions" do + sign_in(user) + delete "/posts/#{post_id}/revisions/permanently_delete.json" + expect(response).to_not be_successful + end + end + + describe "when logged in as staff" do + before { sign_in(admin) } + + it "fails when post record is not found" do + delete "/posts/#{post_id + 1}/revisions/permanently_delete.json" + expect(response).to_not be_successful + end + + it "fails when no post revisions are found" do + delete "/posts/#{post_with_no_revisions.id}/revisions/permanently_delete.json" + expect(response).to_not be_successful + end + + it "fails when 'can_permanently_delete' setting is false" do + SiteSetting.can_permanently_delete = false + delete "/posts/#{post_id}/revisions/permanently_delete.json" + expect(response).to_not be_successful + end + + it "permanently deletes revisions from post and adds a staff log" do + delete "/posts/#{post_id}/revisions/permanently_delete.json" + expect(response.status).to eq(200) + + # It creates a staff log + logs = + UserHistory.find_by( + action: UserHistory.actions[:permanently_delete_post_revisions], + acting_user_id: admin.id, + post_id: post_id, + ) + expect(logs).to be_present + + # ensure post revisions are deleted + expect(PostRevision.where(post: post)).to eq([]) + end + end + end + describe "#revert" do include_examples "action requires login", :put, "/posts/123/revisions/2/revert.json"