diff --git a/app/assets/stylesheets/post_history.scss b/app/assets/stylesheets/post_history.scss index 55d9411b2..b4cd01fc4 100644 --- a/app/assets/stylesheets/post_history.scss +++ b/app/assets/stylesheets/post_history.scss @@ -10,9 +10,24 @@ & > summary { border-bottom: 0; - display: unset; font-weight: unset; color: unset; + + display: flex; + flex-direction: column; + + @media screen and (min-width: $screen-lg) { + flex-direction: row; + justify-content: space-between; + } + + & > div { + margin: 0.2em 0; + + @media screen and (min-width: $screen-lg) { + margin: 0; + } + } } &:last-of-type { @@ -30,4 +45,4 @@ padding: 0.5em; background: lighten($success, 45%); } -} \ No newline at end of file +} diff --git a/app/controllers/concerns/post_actions.rb b/app/controllers/concerns/post_actions.rb new file mode 100644 index 000000000..1b18d02aa --- /dev/null +++ b/app/controllers/concerns/post_actions.rb @@ -0,0 +1,38 @@ +module PostActions + extend ActiveSupport::Concern + + # Updates the given post with the given params and creates the corresponding events. + # + # @param post [Post] + # @param user [User] + # @param params [Hash] + # @param body_rendered [String, Nil] + # @param comment [String, Nil] + # @return [PostHistory, False] if the update succeeded, the history event, false otherwise + def update_post(post, user, params, body_rendered, comment: nil) + before = { body: @post.body_markdown, title: @post.title, tags: @post.tags.to_a } + + params[:body] = body_rendered if body_rendered + params = params.merge(last_edited_at: DateTime.now, last_edited_by: user, + last_activity: DateTime.now, last_activity_by: user) + + if post.update(params) + PostHistory.post_edited(post, user, comment: comment, + before: before[:body], after: post.body_markdown, + before_title: before[:title], after_title: post.title, + before_tags: before[:tags], after_tags: post.tags) + else + false + end + end + + # Whether the given user is allowed to directly edit the given post. + # + # @param user [User] + # @param post [Post] + # @param post_type [PostType] + def can_update_post?(user, post, post_type) + user.privilege?('edit_posts') || user.is_moderator || user == post.user || + (post_type.is_freely_editable && user.privilege?('unrestricted')) + end +end diff --git a/app/controllers/post_history_controller.rb b/app/controllers/post_history_controller.rb index 80a37de1c..6684ea009 100644 --- a/app/controllers/post_history_controller.rb +++ b/app/controllers/post_history_controller.rb @@ -1,4 +1,6 @@ class PostHistoryController < ApplicationController + include PostActions + def post @post = Post.find(params[:id]) @@ -11,4 +13,284 @@ def post .order(created_at: :desc, id: :desc).paginate(per_page: 20, page: params[:page]) render layout: 'without_sidebar' end + + def undo + @history = PostHistory.find(params[:id]) + @post = @history.post + histories = @post.post_histories.order(created_at: :asc).ids + index = histories.index(@history.id) + 1 + + unless @history.can_undo? + flash[:danger] = 'You cannot undo this particular history element.' + redirect_to post_history_url(@post) + return + end + + msg = helpers.disallow_undo_history(@history, current_user) + if msg.present? + flash[:danger] = "You are not allowed to undo this history element: #{msg}" + redirect_to post_history_url(@post) + return + end + + opts = { + before: @post.body_markdown, before_title: @post.title, before_tags: @post.tags.to_a, + comment: "Undo of [##{index}: #{@history.post_history_type.name.humanize}]" \ + "(#{post_history_url(@post, anchor: index)})" + } + + PostHistory.transaction do + unless undo_post_history(@history) + flash[:danger] = "Unable to undo revision: #{@post.errors.full_messages.join(', ')}" + redirect_to post_history_path(@post) + return + end + + opts = opts.merge(after: @post.body_markdown, after_title: @post.title, after_tags: @post.tags.to_a) + + undo_type = @history.post_history_type.name_inverted + + # Record in the history that this element was rolled back + new_history = PostHistory.send(undo_type, @post, current_user, **opts) + + # Set the original to be rolled back + @history.reverted_with_id = new_history.id + @history.save! + end + + flash[:success] = 'History successfully rolled back' + redirect_to post_history_path(@post) + end + + def rollback_overview + @history = PostHistory.find(params[:id]) + @post = @history.post + + @changes = determine_changes_to_restore(@post, @history) + + # Check whether we would actually be making changes + if @changes.empty? + flash[:warning] = 'You cannot rollback to this history element: no changes would be made.' + redirect_to post_history_url(@post) + return + end + + # Check whether the user is allowed to make those changes + unless can_update_post?(current_user, @post, @post.post_type) + flash[:danger] = ability_err_msg(:edit_posts, 'edit this post') + redirect_to post_history_url(@post) + return + end + + @full_history = @post.post_histories + .includes(:post_history_type, :user, post_history_tags: [:tag]) + .order(created_at: :desc, id: :desc) + events_to_undo = determine_edit_events_to_undo(@post, @history) + @any_hidden = events_to_undo.any?(&:hidden) + @undo_history_ids = events_to_undo.ids + + render layout: 'without_sidebar' + end + + def rollback_to + @history = PostHistory.find(params[:id]) + @post = @history.post + + comment = params[:edit_comment] + if comment.blank? + flash[:danger] = 'You need to provide a comment for why you are rolling back.' + redirect_to rollback_overview_post_history_path(@post, @history) + return + end + + # Determine the changes that we need to make + to_change = determine_changes_to_restore(@post, @history) + + if to_change.empty? + flash[:warning] = 'You cannot revert to this history element: no changes would be made.' + redirect_to post_history_url(@post) + return + end + + # Check whether the user is allowed to make those changes + msg = helpers.disallow_roll_back_to_history(@history, current_user) + if msg + flash[:danger] = msg + redirect_to post_history_url(@post) + return + end + + # Actually apply the changes + revert_to_state(to_change, @post, @history, comment) + + flash[:success] = 'Successfully rolled back.' + redirect_to post_history_url(@post) + end + + protected + + # Reverts the given post to the state of the given history item by using the aggregated changes from to_change + # @param to_change [Hash] the changes to make, output of `determine_changes_to_restore` + # @param post [Post] + # @param history [PostHistory] + def revert_to_state(to_change, post, history, comment) + index = post.post_histories.order(created_at: :desc, id: :desc).ids.index(history.id) + revert_comment = "Rolling back to [##{index}: #{history.post_history_type.name.humanize}]" \ + "(#{post_history_url(post, anchor: index)}): #{comment}" + + change_params = {} + change_params[:title] = to_change[:title] if to_change[:title].present? + change_params[:tags_cache] = to_change[:tags].map(&:name) if to_change[:tags].present? + + body_rendered = nil + if to_change[:body].present? + change_params[:body_markdown] = to_change[:body] + body_rendered = ApplicationController.helpers.render_markdown(to_change[:body]) + end + + edit_event = update_post(post, current_user, change_params, body_rendered, comment: revert_comment) + + revert_history_items(history, edit_event) + end + + # Updates the post histories by setting by which event they were reverted. + # @param history [PostHistory] + # @param edit_event [PostHistory, Nil] + def revert_history_items(history, edit_event) + determine_edit_events_to_undo(history.post, history) + .update_all(reverted_with_id: edit_event, updated_at: DateTime.now) + end + + # Determines the edit events to undo to revert to the given history item. + # Events are ordered in the order in which they need to be undone (newest to oldest). + # + # @param post [Post] + # @param history [PostHistory] the history item to revert the state to + def determine_edit_events_to_undo(post, history) + post.post_histories + .where(created_at: (history.created_at + 1.second)..) + .where(post_history_type: PostHistoryType.where(name: 'post_edited')) + .includes(:post_history_type) + .order(created_at: :desc, id: :desc) + end + + # Determines the set of changes to apply to the post to revert back to the state it had at the given history item. + # + # The returned hash can contain: + # deleted [Boolean] - whether the post should now be deleted + # closed [Boolean] - whether the post should be closed + # title [String] - the title the post should be updated to have + # body [String] - the body the post should be updated to have + # tags [Array] - the tags the post should be updated to have + # close_reason [CloseReason] - the close reason that should be set + # duplicate_post [Post] - the post of which this post is a duplicate that should be set + # + # @param post [Post] + # @param history [PostHistory] + # @return [Hash] + def determine_changes_to_restore(post, history) + events_to_undo = determine_edit_events_to_undo(post, history) + + # Aggregate changes from events + to_change = {} + events_to_undo.each do |ph| + case ph.post_history_type.name + when 'post_edited' + to_change[:title] = ph.before_title if ph.before_title.present? + to_change[:body] = ph.before_state if ph.before_state.present? + to_change[:tags] = ph.before_tags if ph.before_tags.present? + end + end + + # Cleanup changes that are already present + to_change.delete(:title) if to_change[:title] == post.title + to_change.delete(:body) if to_change[:body] == post.body_markdown + to_change.delete(:tags) if to_change[:tags]&.map(&:id)&.sort == post.tags.ids.sort + + to_change + end + + private + + # Undoes the given history item. + # @param history [PostHistory] + # @return [Boolean] whether the undo was successfully applied + def undo_post_history(history) + post = history.post + case history.post_history_type.name + when 'post_edited' + undo_post_edit(post, history) + when 'history_hidden' + undo_hide_history(post, history) + when 'history_revealed' + undo_reveal_history(post, history) + else + false + end + end + + # @param post [Post] + # @param history [PostHistory] + # @return [Boolean] whether the undo was successfully applied + def undo_post_edit(post, history) + post.title = history.before_title if history.before_title && history.before_title != history.after_title + if history.before_state && history.before_state != history.after_state + post.body_markdown = history.before_state + post.body = ApplicationController.helpers.render_markdown(history.before_state) + end + post.tags_cache += history.tags_removed.map(&:name) + post.tags_cache -= history.tags_added.map(&:name) + post.last_activity = DateTime.now + post.last_activity_by = current_user + post.save + end + + # @param post [Post] + # @param history [PostHistory] + # @return [Boolean] whether the undo was successfully applied + def undo_hide_history(post, history) + # We need to reveal history for all events that were hidden by this action + histories_to_reveal = post.post_histories + .where(created_at: ..history.created_at) + .where(hidden: true) + .includes(:post_history_type) + .order(created_at: :desc, id: :desc) + + # If there are more history hiding events, only hide until the previous one. + # We need to add one second because we don't want to reveal the edit before the history_hidden event, which + # will have likely occurred in the same second. + # Note that we do want to include the history_hidden item itself, so we add that with or. + predecessor = history.find_predecessor('history_hidden') + if predecessor + histories_to_reveal = histories_to_reveal.where(created_at: (predecessor.created_at + 1.second)..) + .or(PostHistory.where(id: predecessor)) + end + + histories_to_reveal.update_all(hidden: false, updated_at: DateTime.now).positive? + end + + # @param post [Post] + # @param history [PostHistory] + # @return [Boolean] whether the undo was successfully applied + def undo_reveal_history(post, history) + # We need to hide history for all events that were revealed by this action + histories_to_hide = post.post_histories + .where(created_at: ..history.created_at) + .where(hidden: true) + .includes(:post_history_type) + .order(created_at: :desc, id: :desc) + + # If there are more history hiding events, only reveal until the previous previous hiding. + # That is, we revealed hiding 1, now we are hiding again (confusing I know). + # We need to add one second because we don't want to hide the edit before the history_hidden event, which + # will have likely occurred in the same second. + # Note that we do want to include the history_hidden item itself, so we add that with or. + predecessor = history.find_predecessor('history_hidden')&.find_predecessor('history_hidden') + if predecessor + histories_to_hide = histories_to_hide.where(created_at: (predecessor.created_at + 1.second)..) + .or(PostHistory.where(id: predecessor)) + end + + histories_to_hide.update_all(hidden: true, updated_at: DateTime.now).positive? + end end diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 3b32fed5f..f67238e20 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -1,6 +1,8 @@ # rubocop:disable Metrics/ClassLength # rubocop:disable Metrics/MethodLength class PostsController < ApplicationController + include PostActions + before_action :authenticate_user!, except: [:document, :help_center, :show] before_action :set_post, only: [:toggle_comments, :feature, :lock, :unlock] before_action :set_scoped_post, only: [:change_category, :show, :edit, :update, :close, :reopen, :delete, :restore] @@ -169,8 +171,7 @@ def update return redirect_to post_path(@post) end - if current_user.privilege?('edit_posts') || current_user.is_moderator || current_user == @post.user || \ - (@post_type.is_freely_editable && current_user.privilege?('unrestricted')) + if can_update_post?(current_user, @post, @post_type) if ['HelpDoc', 'PolicyDoc'].include?(@post_type.name) && (current_user.is_global_moderator || \ current_user.is_global_admin) && params[:network_push] == 'true' posts = Post.unscoped.where(post_type_id: [PolicyDoc.post_type_id, HelpDoc.post_type_id], @@ -188,15 +189,7 @@ def update flash[:success] = "#{helpers.pluralize(posts.to_a.size, 'post')} updated." redirect_to help_path(slug: @post.doc_slug) else - if @post.update(edit_post_params.merge(body: body_rendered, - last_edited_at: DateTime.now, last_edited_by: current_user, - last_activity: DateTime.now, last_activity_by: current_user)) - - PostHistory.post_edited(@post, current_user, before: before[:body], - after: @post.body_markdown, comment: params[:edit_comment], - before_title: before[:title], after_title: @post.title, - before_tags: before[:tags], after_tags: @post.tags) - + if update_post(@post, current_user, edit_post_params, body_rendered, comment: params[:edit_comment]) if params[:redact] # Hide all previous history PostHistory.where(post: @post).update_all(hidden: true) @@ -280,13 +273,16 @@ def close end duplicate_of = Question.find(params[:other_post]) + comment = "Closed as #{reason.name} of [Question ##{duplicate_of.id}](#{post_path(duplicate_of)})" else duplicate_of = nil + comment = "Closed as #{reason.name}" end if @post.update(closed: true, closed_by: current_user, closed_at: DateTime.now, last_activity: DateTime.now, last_activity_by: current_user, close_reason: reason, duplicate_post: duplicate_of) - PostHistory.question_closed(@post, current_user) + PostHistory.question_closed(@post, current_user, comment: comment, + close_reason_id: reason.id, duplicate_post_id: duplicate_of&.id) render json: { status: 'success' } else render json: { status: 'failed', message: helpers.i18ns('posts.cant_close_post'), diff --git a/app/helpers/post_history_helper.rb b/app/helpers/post_history_helper.rb new file mode 100644 index 000000000..6889ec98f --- /dev/null +++ b/app/helpers/post_history_helper.rb @@ -0,0 +1,88 @@ +module PostHistoryHelper + include PostActions + + # @param history [PostHistory] + # @param user [User, Nil] + # @return [Boolean] whether the given user is allowed to roll back to the given history item + def allow_roll_back_to_history?(history, user) + user.present? && !disallow_roll_back_to_history(history, user) + end + + # @param history [PostHistory] + # @param user [User] + # @return [String, Nil] whether the given user is allowed to roll back to the given history item + def disallow_roll_back_to_history(history, user) + if history.hidden? && history.user_id != user.id && !user.is_admin + return i18ns('post_histories.cant_undo_hidden') + end + + case history.post_history_type.name + when 'post_edited', 'initial_revision', 'imported_from_external_source' + disallow_edit(history.post, user) + else + 'Unsupported history type' + end + end + + # @param history [PostHistory] + # @param user [User, Nil] + # @return [Boolean] whether the given user is allowed to undo the given history item + def allow_undo_history?(history, user) + user.present? && !disallow_undo_history(history, user) + end + + # @param history [PostHistory] + # @param user [User] + # @return [String, Nil] the error message if disallowed, or nil if allowed + def disallow_undo_history(history, user) + if history.hidden? && history.user_id != user.id && !user.is_admin + return i18ns('post_histories.cant_undo_hidden') + end + + case history.post_history_type.name + when 'post_edited' + disallow_edit(history.post, user) + when 'history_hidden' + disallow_reveal_history(history, user) + when 'history_revealed' + disallow_hide_history(history, user) + else + 'Unsupported history type' + end + end + + # @param post [Post] + # @param user [User] + # @return [String, Nil] the error message if disallowed, or nil if allowed + def disallow_edit(post, user) + unless can_update_post?(user, post, post.post_type) + ability_err_msg(:edit_posts, 'edit this post') + end + end + + # @param history [PostHistory] + # @param user [User] + # @return [String, Nil] the error message if disallowed, or nil if allowed + def disallow_reveal_history(history, user) + unless user.is_admin || history.user_id == user.id + i18ns('post_histories.cant_reveal') + end + end + + # @param _history [PostHistory] + # @param _user [User] + # @return [String, Nil] the error message if disallowed, or nil if allowed + def disallow_hide_history(_history, _user); end +end + +class PostHistoryScrubber < Rails::Html::PermitScrubber + def initialize + super + self.tags = %w[a b i em strong s strike del sup sub] + self.attributes = %w[href title lang dir id class start] + end + + def skip_node?(node) + node.text? + end +end diff --git a/app/models/post_history.rb b/app/models/post_history.rb index 73b2e715a..5cc0b8800 100644 --- a/app/models/post_history.rb +++ b/app/models/post_history.rb @@ -2,9 +2,16 @@ class PostHistory < ApplicationRecord include PostRelated belongs_to :post_history_type belongs_to :user - has_many :post_history_tags + has_many :post_history_tags, dependent: :destroy has_many :tags, through: :post_history_tags + belongs_to :close_reason, optional: true + belongs_to :duplicate_post, class_name: 'Post', optional: true + + # This relation must be present to ensure that deletions of post histories always work. It will cause the reverted + # items to no longer appear as reverted. + has_many :reverts_post_histories, class_name: 'PostHistory', foreign_key: :reverted_with_id, dependent: :nullify + def before_tags tags.where(post_history_tags: { relationship: 'before' }) end @@ -13,6 +20,20 @@ def after_tags tags.where(post_history_tags: { relationship: 'after' }) end + # @return [Array] the tags that were removed in this history step + def tags_removed + before_tags - after_tags + end + + # @return [Array] the tags that were added in this history step + def tags_added + after_tags - before_tags + end + + def reverted? + !reverted_with_id.nil? + end + # @param user [User] # @return [Boolean] whether the given user is allowed to see the details of this history item def allowed_to_see_details?(user) @@ -25,7 +46,8 @@ def self.method_missing(name, *args, **opts) end object, user = args - fields = [:before, :after, :comment, :before_title, :after_title, :before_tags, :after_tags, :hidden] + fields = [:before, :after, :comment, :before_title, :after_title, :before_tags, :after_tags, :close_reason_id, + :duplicate_post_id, :hidden] values = fields.to_h { |f| [f, nil] }.merge(opts) history_type_name = name.to_s @@ -37,7 +59,8 @@ def self.method_missing(name, *args, **opts) params = { post_history_type: history_type, user: user, post: object, community_id: object.community_id } { before: :before_state, after: :after_state, comment: :comment, before_title: :before_title, - after_title: :after_title, hidden: :hidden }.each do |arg, attr| + after_title: :after_title, close_reason_id: :close_reason_id, duplicate_post_id: :duplicate_post_id, + hidden: :hidden }.each do |arg, attr| next if values[arg].nil? params = params.merge(attr => values[arg]) @@ -61,4 +84,45 @@ def self.method_missing(name, *args, **opts) def self.respond_to_missing?(method_name, include_private = false) PostHistoryType.exists?(name: method_name.to_s) || super end + + # @return [Boolean] whether this history item can be undone + def can_undo? + return false if reverted? || hidden + + case post_history_type.name + when 'post_edited' + # Post title must be still what it was after the edit + (after_title.nil? || after_title == before_title || after_title == post.title) && + # Post body must be still the same + (after_state.nil? || after_state == before_state || after_state == post.body_markdown) && + # Post tags that were removed must not have been re-added + (tags_removed & post.tags == []) && + # Post tags that were added must not have been removed + (tags_added - post.tags == []) + when 'history_hidden', 'history_revealed' + true + else + false + end + end + + # Attempts to find a predecessor event (event that came before) of the given type. + # This method will return the predecessor with the greatest created_at timestamp. + # + # @param type [PostHistoryType, String] + # @return [PostHistoryType, Nil] the predecessor of this event of the given type, if any exists + def find_predecessor(type) + type = if type.is_a?(PostHistoryType) + type + else + PostHistoryType.find_by(name: type) + end + + post.post_histories + .where(post_history_type: type) + .where(created_at: ..created_at) + .where.not(id: id) + .order(created_at: :desc, id: :desc) + .first + end end diff --git a/app/models/post_history_type.rb b/app/models/post_history_type.rb index fd5929a60..34a8c7f3d 100644 --- a/app/models/post_history_type.rb +++ b/app/models/post_history_type.rb @@ -1,5 +1,38 @@ class PostHistoryType < ApplicationRecord + INVERSES = { + 'post_deleted' => 'post_undeleted', + 'post_undeleted' => 'post_deleted', + 'question_closed' => 'question_reopened', + 'question_reopened' => 'question_closed', + 'attribution_notice_added' => 'attribution_notice_removed', + 'attribution_notice_removed' => 'attribution_notice_added', + 'nominated_for_promotion' => 'promotion_removed', + 'promotion_removed' => 'nominated_for_promotion', + 'history_hidden' => 'history_revealed', + 'history_revealed' => 'history_hidden' + }.freeze + REVERT_TO_TYPES = %w[ + post_edited + initial_revision + imported_from_external_source + ].freeze + has_many :post_histories validates :name, uniqueness: { case_sensitive: false } + + # @return [Boolean] whether one can roll back to an event of this type. + def can_be_rolled_back_to? + REVERT_TO_TYPES.include?(name) + end + + # @return [Boolean] whether this event relates to hiding/revealing of history + def history_hiding_related? + %w[history_hidden history_revealed].include?(name) + end + + # @return [String] the name of the opposite post history type. If there is no opposite, the name is returned unchanged + def name_inverted + INVERSES.fetch(name, name) + end end diff --git a/app/views/post_history/post.html.erb b/app/views/post_history/post.html.erb index 8847e92e5..3db98b1f4 100644 --- a/app/views/post_history/post.html.erb +++ b/app/views/post_history/post.html.erb @@ -5,28 +5,70 @@ <% @history.each.with_index do |event, index| %> -
+ <% revision = @history.size - index %> +
- #<%= @history.size - index %>: <%= event.post_history_type.name.humanize %> - - - by - <% if deleted_user? event.user %> - (deleted user) +
+ + <% if event.reverted? %> + #<%= revision %>: <%= event.post_history_type.name.humanize %> <% else %> - user avatar - <%= user_link event.user %> + #<%= revision %>: <%= event.post_history_type.name.humanize %> <% end %> - · - <%= event.created_at.iso8601 %> (<%= time_ago_in_words(event.created_at) %> ago) - + + + + by + <% if deleted_user? event.user %> + (deleted user) + <% else %> + user avatar + <%= user_link event.user %> + <% end %> + · + <%= event.created_at.iso8601 %> (<%= time_ago_in_words(event.created_at) %> ago) + <% if event.comment.present? %> -
<%= event.comment %> +
<%= sanitize(render_markdown(event.comment), scrubber: PostHistoryScrubber.new) %> <% end %> -
- <%= link_to post_history_url(@post, anchor: @history.size - index), class: 'js-permalink' do %> - copy link - <% end %> + +
+ +
+ <% if event.can_undo? && allow_undo_history?(event, current_user) %> + <% if event.post_history_type.history_hiding_related? %> + <% name = event.post_history_type.name == 'history_hidden' ? 'reveal history' : 're-hide history' %> + <%= link_to name.titlecase, undo_post_history_url(@post, event), + 'aria-label': "#{name.titlecase} at ##{revision}", + data: { + confirm: "Are you sure you want to #{name} at ##{revision}?" + }, + method: :post + %> + — + <% else %> + <%= link_to 'Undo', undo_post_history_url(@post, event), + 'aria-label': "Undo history item ##{revision} #{event.post_history_type.name.humanize}", + data: { + confirm: "Are you sure you want to undo change ##{revision}: #{event.post_history_type.name.humanize}?" + }, + method: :post + %> + — + <% end %> + <% end %> + + <% if index != 0 && event.post_history_type.can_be_rolled_back_to? && allow_roll_back_to_history?(event, current_user) %> + <%= link_to 'Roll back to this Version', rollback_overview_post_history_path(@post, event), + 'aria-label': "Roll back to version ##{revision} #{event.post_history_type.name.humanize}" + %> + — + <% end %> + + <%= link_to post_history_url(@post, anchor: revision), class: 'js-permalink' do %> + Copy Link + <% end %> +
<% if event.allowed_to_see_details?(current_user) %> <% if event.hidden? %> diff --git a/app/views/post_history/rollback_overview.html.erb b/app/views/post_history/rollback_overview.html.erb new file mode 100644 index 000000000..4804bfcf3 --- /dev/null +++ b/app/views/post_history/rollback_overview.html.erb @@ -0,0 +1,116 @@ +

Rolling back post to previous state

+ +
+ <%= render 'posts/type_agnostic', post: @post, show_category_tag: true, show_type_tag: true, last_activity: false %> +
+ +<% revertion_revision = nil %> +<% @full_history.each.with_index do |event, index| %> + <% revision = @full_history.size - index %> + <% revertion_revision = revision if event == @history %> + <% rolling_back = @undo_history_ids.include?(event.id) %> +
+ +
+ + <% if rolling_back %> + #<%= revision %>: <%= event.post_history_type.name.humanize %> + <% elsif event.id == @history.id %> + #<%= revision %>: <%= event.post_history_type.name.humanize %> + <% else %> + #<%= revision %>: <%= event.post_history_type.name.humanize %> + <% end %> + + + + by + <% if deleted_user? event.user %> + (deleted user) + <% else %> + user avatar + <%= user_link event.user %> + <% end %> + · + <%= event.created_at.iso8601 %> (<%= time_ago_in_words(event.created_at) %> ago) + + <% if event.comment.present? %> +
<%= sanitize(render_markdown(event.comment), scrubber: PostHistoryScrubber.new) %> + <% end %> +
+
+
+ <% if event.allowed_to_see_details?(current_user) %> + <% if (event.before_title.present? || event.after_title.present?) && event.before_title != event.after_title %> + <%= render 'diff', before: event.before_title, after: event.after_title, post: @post %> + <% end %> + <% if (event.before_state.present? || event.after_state.present?) && event.before_state != event.after_state %> + <%= render 'diff', before: event.before_state, after: event.after_state, post: @post %> + <% end %> + <% if (event.before_tags.present? || event.after_tags.present?) && event.before_tags != event.after_tags %> + <%= render 'diff', before: event.before_tags, after: event.after_tags, post: @post %> + <% end %> + <% elsif [event.before_title, event.after_title, + event.before_state, event.after_state, + event.before_tags, event.after_tags].any?(&:present?) %> +

+ The detailed changes of this event are hidden because of a redaction. +

+ <% end %> +
+<% end %> + +
+ +<% if @any_hidden %> +
+

Redacted changes

+

+ You are rolling back changes to a point in history before a redaction was made.
+ Please carefully review the changes to see that you do not add back any information which should remain redacted. +

+

+ You may also want to look at the detailed changes of the individual events above to see which ones are + redactions. +

+
+<% end %> + +
+

+ You will be changing the post as follows: +

+ + <% if @changes.include?(:title) %> + <%= render 'diff', before: @post.title, after: @changes[:title], post: @post %> + <% end %> + <% if @changes.include?(:body) %> + <%= render 'diff', before: @post.body_markdown, after: @changes[:body], post: @post %> + <% end %> + <% if @changes.include?(:tags) %> + <%= render 'diff', before: @post.tags, after: @changes[:tags], post: @post %> + <% end %> +
+ +<%= form_for @history, url: rollback_to_post_history_path(@post, @history), method: :post, + html: { class: 'has-margin-top-4' } do |f| %> +
+ <%= label_tag :edit_comment, t('posts.edit_comment_label'), class: 'form-element' %> + <%= text_field_tag :pre_comment, + "Rolling back to ##{revertion_revision} - #{@history.post_history_type.name.humanize}:", + class: 'form-element', disabled: true %> + <%= text_field_tag :edit_comment, nil, class: 'form-element', required: true %> +
+ + <% if @any_hidden %> +
+ <%= check_box_tag :confirm, true, false, class: 'form-checkbox-element', required: true %> + <%= label_tag :confirm, 'I have carefully reviewed history and confirm I want to rollback post content to a point in time which was hidden.' %> +
+ <% end %> + +
+ <%= f.submit 'Confirm Rollback', class: 'button is-filled' %> + <%= link_to t('g.cancel').titlecase, post_history_path(@post), + class: 'button is-muted is-outlined is-danger js-cancel-edit' %> +
+<% end %> diff --git a/config/locales/strings/en.post_histories.yml b/config/locales/strings/en.post_histories.yml new file mode 100644 index 000000000..7c917efc5 --- /dev/null +++ b/config/locales/strings/en.post_histories.yml @@ -0,0 +1,6 @@ +en: + post_histories: + cant_reveal: > + You cannot reveal history that was hidden. Contact an administrator if you believe you need to. + cant_undo_hidden: > + You cannot undo changes whose details are hidden. Contact an administrator if you believe you need to. diff --git a/config/locales/strings/es.post_histories.yml b/config/locales/strings/es.post_histories.yml new file mode 100644 index 000000000..52c2bb998 --- /dev/null +++ b/config/locales/strings/es.post_histories.yml @@ -0,0 +1,6 @@ +es: + post_histories: + cant_reveal: > + No puedes revelar la historia que estaba escondida. Póngase en contacto con un administrador si cree que necesita revelarlo. + cant_undo_hidden: > + No puede deshacer los cambios cuyos detalles están ocultos. Comuníquese con un administrador si cree que deben deshacerse. diff --git a/config/routes.rb b/config/routes.rb index f3eaa3b94..ae9586091 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -141,6 +141,9 @@ get ':id', to: 'posts#show', as: :post get ':id/history', to: 'post_history#post', as: :post_history + post ':post_id/history_undo/:id', to: 'post_history#undo', as: :undo_post_history + get ':post_id/history_rollback/:id', to: 'post_history#rollback_overview', as: :rollback_overview_post_history + post ':post_id/history_rollback/:id', to: 'post_history#rollback_to', as: :rollback_to_post_history get ':id/edit', to: 'posts#edit', as: :edit_post patch ':id/edit', to: 'posts#update', as: :update_post post ':id/close', to: 'posts#close', as: :close_post diff --git a/db/migrate/20230722161509_add_fields_to_post_history.rb b/db/migrate/20230722161509_add_fields_to_post_history.rb new file mode 100644 index 000000000..07938d86c --- /dev/null +++ b/db/migrate/20230722161509_add_fields_to_post_history.rb @@ -0,0 +1,7 @@ +class AddFieldsToPostHistory < ActiveRecord::Migration[7.0] + def change + add_reference :post_histories, :close_reason, foreign_key: true + add_reference :post_histories, :duplicate_post, foreign_key: { to_table: :posts } + add_reference :post_histories, :reverted_with, foreign_key: { to_table: :post_histories } + end +end diff --git a/db/migrate/20230722204756_recover_closed_information_into_history.rb b/db/migrate/20230722204756_recover_closed_information_into_history.rb new file mode 100644 index 000000000..f43dbec23 --- /dev/null +++ b/db/migrate/20230722204756_recover_closed_information_into_history.rb @@ -0,0 +1,52 @@ +class RecoverClosedInformationIntoHistory < ActiveRecord::Migration[7.0] + def change + pht = PostHistoryType.find_by(name: 'question_closed') + + # For all history where the post is still closed, add the close reason and duplicate post id. + # We need to only add it to the last post closure (for close, reopen, close cases), so we do a left join with + # post histories in a way where we only retrieve the closed history event with the last created_at date. + + # NOTE: Ideally we would do a `joins(:post).includes(post: [:close_reason]).find_each` to achieve the same result in + # a pure Rails way as this uncached, unscoped, in_batches, pluck mess. + # However, the default_scopes which limit to a specific community seem to apply on the includes relations, + # adding where clauses on the "current" community (i.e. community IS NULL) on them... + # Given the potential amount of history items we are dealing with, we therefore have to instead do this fun + # workaround. + PostHistory.uncached do + PostHistory.unscoped.in_batches do |relation| + relation.joins(Arel.sql('INNER JOIN `posts` ON `post_histories`.`post_id` = `posts`.`id`')) + .joins(Arel.sql('LEFT JOIN `close_reasons` ON `posts`.`close_reason_id` = `close_reasons`.`id`')) + .where(post_history_type: pht) + .where(posts: { closed: true }) + .joins(Arel.sql(<<-SQL + LEFT JOIN `post_histories` `ph` + ON `ph`.`post_id` = `post_histories`.`post_id` + AND `post_histories`.`created_at` < `ph`.`created_at` + AND `ph`.`post_history_type_id` = #{pht.id} + SQL + )) + .where(Arel.sql('`ph`.`created_at` IS NULL')) + .pluck(Arel.sql('post_histories.id, close_reasons.id, close_reasons.name, posts.duplicate_post_id')) + .each do |r| + post_history_id = r[0] + close_reason_id = r[1] + close_reason_name = r[2] || '' + duplicate_post_id = r[3] + + if duplicate_post_id + comment = "Closed as #{close_reason_name} of [Question ##{duplicate_post_id}](/posts/#{duplicate_post_id})" + else + comment = "Closed as #{close_reason_name}" + end + + # Prevent loading the post history by doing `where(id: ...).update_all`, which will directly run an UPDATE + # query. + PostHistory.where(id: post_history_id) + .update_all(comment: comment, + close_reason_id: close_reason_id, + duplicate_post_id: duplicate_post_id) + end + end + end + end +end diff --git a/db/schema.rb b/db/schema.rb index c606a4ecc..2a07e5b21 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -387,9 +387,15 @@ t.string "before_title" t.string "after_title" t.boolean "hidden", default: false, null: false + t.bigint "close_reason_id" + t.bigint "duplicate_post_id" + t.bigint "reverted_with_id" + t.index ["close_reason_id"], name: "index_post_histories_on_close_reason_id" t.index ["community_id"], name: "index_post_histories_on_community_id" + t.index ["duplicate_post_id"], name: "index_post_histories_on_duplicate_post_id" t.index ["post_history_type_id"], name: "index_post_histories_on_post_history_type_id" t.index ["post_id"], name: "index_post_histories_on_post_type_and_post_id" + t.index ["reverted_with_id"], name: "index_post_histories_on_reverted_with_id" t.index ["user_id"], name: "index_post_histories_on_user_id" end @@ -789,7 +795,10 @@ add_foreign_key "notifications", "communities" add_foreign_key "pinned_links", "communities" add_foreign_key "pinned_links", "posts" + add_foreign_key "post_histories", "close_reasons" add_foreign_key "post_histories", "communities" + add_foreign_key "post_histories", "post_histories", column: "reverted_with_id" + add_foreign_key "post_histories", "posts", column: "duplicate_post_id" add_foreign_key "post_history_tags", "post_histories" add_foreign_key "post_history_tags", "tags" add_foreign_key "post_types", "post_types", column: "answer_type_id" diff --git a/db/seeds/post_history_types.yml b/db/seeds/post_history_types.yml index d12b6e28c..fd30b9d86 100644 --- a/db/seeds/post_history_types.yml +++ b/db/seeds/post_history_types.yml @@ -10,4 +10,5 @@ - name: imported_from_external_source - name: nominated_for_promotion - name: promotion_removed -- name: history_hidden \ No newline at end of file +- name: history_hidden +- name: history_revealed \ No newline at end of file diff --git a/test/controllers/post_history_controller_test.rb b/test/controllers/post_history_controller_test.rb index c86257132..6200e3bd4 100644 --- a/test/controllers/post_history_controller_test.rb +++ b/test/controllers/post_history_controller_test.rb @@ -29,4 +29,87 @@ class PostHistoryControllerTest < ActionController::TestCase assert_not_nil assigns(:history) assert_not_nil assigns(:post) end + + # ----------------------------------------------------------------------------------------------- + # Rollbacks + # ----------------------------------------------------------------------------------------------- + + test 'user without edit permissions cannot undo post edit' do + event = post_histories(:q1_edit) + sign_in users(:standard_user2) + + assert_no_difference 'PostHistory.count' do + post :undo, params: { post_id: event.post_id, id: event.id } + assert_not_nil flash[:danger] + end + + # Verify no changes were made + assert_not_equal event.before_state, assigns(:post).body_markdown + end + + test 'privileged user can undo post edit' do + event = post_histories(:q1_edit) + user = users(:editor) + + sign_in user + + # Rollback and check success, a new history event should be created + assert_difference 'PostHistory.count' do + post :undo, params: { post_id: event.post_id, id: event.id } + assert_not_nil flash[:success] + end + + # New edit event should have been made, which should be the reverse of our event + new_event = PostHistory.last + assert_equal 'post_edited', new_event.post_history_type.name + assert_equal user.id, new_event.user_id + assert_equal event.before_state, new_event.after_state + assert_equal event.after_state, new_event.before_state + assert_equal event.before_title, new_event.after_title + assert_equal event.after_title, new_event.before_title + # The following doesn't always hold for a rollback, but should hold for this particular event + assert_equal event.before_tags.to_a.sort_by(&:id), new_event.after_tags.to_a.sort_by(&:id) + assert_equal event.after_tags.to_a.sort_by(&:id), new_event.before_tags.to_a.sort_by(&:id) + + # Verify that the post was restored + post = assigns(:post) + assert_equal event.before_state, post.body_markdown + assert_equal event.before_title, post.title + + # Verify that all the tags that were there before are now again present on the post + assert_equal event.before_tags.to_a.sort_by(&:id), (post.tags & event.before_tags).sort_by(&:id) + end + + test 'unprivileged user cannot rollback to state' do + event = posts(:question_one).post_histories.first + user = users(:standard_user2) + + sign_in user + assert_no_difference 'PostHistory.count' do + post :rollback_to, params: { post_id: event.post_id, id: event.id } + assert_not_nil flash[:danger] + end + + # Assert no changes were made + assert_not_equal event.before_state, assigns(:post).body_markdown + end + + test 'privileged user can rollback to initial state if no hiding is involved' do + event = posts(:question_one).post_histories + .find_by(post_history_type: PostHistoryType.find_by(name: 'initial_revision')) + user = users(:moderator) + + sign_in user + assert_difference 'PostHistory.count' do + post :rollback_to, params: { post_id: event.post_id, id: event.id, edit_comment: 'Suffs' } + assert_not_nil flash[:success] + end + + post = Post.find(assigns(:post).id) + + # Assert post was rolled back to state of initial + assert_equal event.after_title, post.title + assert_equal event.after_state, post.body_markdown + assert_equal event.after_tags.ids.sort, post.tags.ids.sort + end end diff --git a/test/fixtures/post_histories.yml b/test/fixtures/post_histories.yml index e69de29bb..b095a13cf 100644 --- a/test/fixtures/post_histories.yml +++ b/test/fixtures/post_histories.yml @@ -0,0 +1,87 @@ +# Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html + +# There are tags updated too +q1_initial: + post_history_type: initial_revision + user: standard_user + post: question_one + community: sample + after_state: This is the original body of test question one. No HTML here. + after_title: Q1 - Original title for question nr one + created_at: 2022-01-01T00:00:00.000000Z + +q1_edit: + post_history_type: post_edited + user: editor + post: question_one + comment: Fixed incorrect information + community: sample + before_state: This is the original body of test question one. No HTML here. + after_state: This is the body of test question one. Note that we did not include any markdown or HTML in here. + before_title: Q1 - Original title for question nr one + after_title: Q1 - This is test question number one + created_at: 2023-01-01T00:00:00.000000Z + +q1_close: + post_history_type: question_closed + user: global_moderator + post: question_one + community: sample + close_reason: duplicate + duplicate_post: question_two + created_at: 2023-01-02T00:00:00.000000Z + +q1_reopen: + post_history_type: question_reopened + user: global_moderator + post: question_one + community: sample + created_at: 2023-01-03T00:00:00.000000Z + +q1_close2: + post_history_type: question_closed + user: global_moderator + post: question_one + community: sample + close_reason: duplicate + duplicate_post: question_two + created_at: 2023-01-04T00:00:00.000000Z + +q1_reopen2: + post_history_type: question_reopened + user: global_moderator + post: question_one + community: sample + created_at: 2023-01-05T00:00:00.000000Z + +q2_delete: + post_history_type: post_deleted + user: standard_user + post: question_two + community: sample + created_at: 2023-01-01T00:00:00.000000Z + +q2_undelete: + post_history_type: post_undeleted + user: standard_user + post: question_two + community: sample + created_at: 2023-01-02T00:00:00.000000Z + +# Do redaction for Q2 +q2_edit: + post_history_type: post_edited + user: editor + post: question_two + community: sample + before_title: Q2 - This is test@test.nl question number two + after_title: Q2 - This is test question number two + comment: Remove PII + created_at: 2023-01-02T00:00:00.000000Z + +q2_hidden: + post_history_type: history_hidden + user: standard_user + post: question_two + community: sample + created_at: 2023-01-03T00:00:00.000000Z diff --git a/test/fixtures/post_history_tags.yml b/test/fixtures/post_history_tags.yml index f61b3311b..1b252b139 100644 --- a/test/fixtures/post_history_tags.yml +++ b/test/fixtures/post_history_tags.yml @@ -1,11 +1,39 @@ # Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html -one: - post_history: one - tag: one - relationship: MyString - -two: - post_history: two - tag: two - relationship: MyString +q1_initial_discussion: + post_history: q1_initial + tag: discussion + relationship: 'after' + +q1_initial_fr: + post_history: q1_initial + tag: feature-request + relationship: 'after' + +q1_edit_before_t1: + post_history: q1_edit + tag: discussion + relationship: 'before' + +q1_edit_before_t2: + post_history: q1_edit + tag: feature-request + relationship: 'before' + +# Leave tag "discussion" unchanged +q1_edit_after_t1: + post_history: q1_edit + tag: discussion + relationship: 'after' + +# Add tag "support" +q1_edit_after_t2: + post_history: q1_edit + tag: support + relationship: 'after' + +# Replace feature-request with bug +q1_edit_after_t3: + post_history: q1_edit + tag: bug + relationship: 'after' diff --git a/test/fixtures/post_history_types.yml b/test/fixtures/post_history_types.yml index 330742f43..f54caf51d 100644 --- a/test/fixtures/post_history_types.yml +++ b/test/fixtures/post_history_types.yml @@ -1,3 +1,24 @@ initial_revision: name: initial_revision - description: initial revision \ No newline at end of file + description: initial revision +post_edited: + name: post_edited + description: post edited +post_deleted: + name: post_deleted + description: post deleted +post_undeleted: + name: post_undeleted + description: post restored +question_closed: + name: question_closed + description: question closed +question_reopened: + name: question_reopened + description: question reopened +history_hidden: + name: history_hidden + description: history hidden +history_revealed: + name: history_revealed + description: history revealed diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 9cd0a928f..d805577c2 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -111,3 +111,12 @@ deleted_profile: sign_in_count: 1337 username: deleted_profile confirmed_at: 2020-01-01T00:00:00.000000Z + +standard_user2: + email: standard2@qpixel-test.net + encrypted_password: '$2a$11$roUHXKxecjyQ72Qn7DWs3.9eRCCoRn176kX/UNb/xiue3aGqf7xEW' + sign_in_count: 1337 + username: standard_user_2 + is_global_admin: false + is_global_moderator: false + confirmed_at: 2023-01-01T00:00:00.000000Z \ No newline at end of file diff --git a/test/models/post_history_test.rb b/test/models/post_history_test.rb index 156f729d2..154095bcf 100644 --- a/test/models/post_history_test.rb +++ b/test/models/post_history_test.rb @@ -36,4 +36,48 @@ class PostHistoryTest < ActiveSupport::TestCase assert_equal post.body_markdown, event.after_state assert_nil event.before_state end + + test 'edit event matching current post can be rolled back' do + event = post_histories(:q1_edit) + assert event.can_undo? + end + + test 'if body does not match, edit event cannot be rolled back' do + event = post_histories(:q1_edit) + event.update!(after_state: "#{event.after_state} - not matching") + assert_not event.can_undo? + end + + test 'if title does not match, edit event cannot be rolled back' do + event = post_histories(:q1_edit) + event.update!(after_title: "#{event.after_title} - not matching") + assert_not event.can_undo? + end + + test 'if an added tag is missing, edit event cannot be rolled back' do + event = post_histories(:q1_edit) + + # Delete the tag that the event adds from the post + post = event.post + post.tags_cache.delete(event.tags_added.first.name) + post.save! + + assert_not event.can_undo? + end + + test 'if additional unrelated tags are on post, edit event can still be rolled back' do + event = post_histories(:q1_edit) + post = event.post + post.tags_cache << tags(:child).name + post.save! + + # Refresh event, should still be allowed to rollback + event = PostHistory.find(event.id) + assert event.can_undo? + end + + test 'predecessor of event finds closest predecessor' do + event = post_histories(:q1_reopen2) + assert_equal post_histories(:q1_close2), event.find_predecessor('question_closed') + end end