Merge pull request #4346 from tgxworld/adrapereira-ap_merge_multiple_responses

FEATURE: Allow staff users to merge posts.

app/assets/javascripts/discourse/controllers/topic.js.es6

@@ -8,6 +8,7 @@ import computed from 'ember-addons/ember-computed-decorators';
 import Composer from 'discourse/models/composer';
 import DiscourseURL from 'discourse/lib/url';
 import { categoryBadgeHTML } from 'discourse/helpers/category-link';
+import Post from 'discourse/models/post';
 
 export default Ember.Controller.extend(SelectedPostsCount, BufferedContent, {
   needs: ['modal', 'composer', 'quote-button', 'application'],
@@ -517,6 +518,16 @@ export default Ember.Controller.extend(SelectedPostsCount, BufferedContent, {
       });
     },
 
+    mergePosts() {
+      bootbox.confirm(I18n.t("post.merge.confirm", { count: this.get('selectedPostsCount') }), result => {
+        if (result) {
+          const selectedPosts = this.get('selectedPosts');
+          Post.mergePosts(selectedPosts);
+          this.send('toggleMultiSelect');
+        }
+      });
+    },
+
     expandHidden(post) {
       post.expandHidden();
     },
@@ -692,6 +703,13 @@ export default Ember.Controller.extend(SelectedPostsCount, BufferedContent, {
     return this.get('selectedPostsUsername') !== undefined;
   }.property('selectedPostsUsername'),
 
+  @computed('selectedPosts', 'selectedPostsCount', 'selectedPostsUsername')
+  canMergePosts(selectedPosts, selectedPostsCount, selectedPostsUsername) {
+    if (selectedPostsCount < 2) return false;
+    if (!selectedPosts.every(p => p.get('can_delete'))) return false;
+    return selectedPostsUsername !== undefined;
+  },
+
   categories: function() {
     return Discourse.Category.list();
   }.property(),

app/assets/javascripts/discourse/models/post.js.es6

@@ -328,6 +328,15 @@ Post.reopenClass({
     });
   },
 
+  mergePosts(selectedPosts) {
+    return Discourse.ajax("/posts/merge_posts", {
+      type: 'PUT',
+      data: { post_ids: selectedPosts.map(p => p.get('id')) }
+    }).catch(() => {
+      self.flash(I18n.t('topic.merge_posts.error'));
+    });
+  },
+
   loadRevision(postId, version) {
     return ajax("/posts/" + postId + "/revisions/" + version + ".json")
                     .then(result => Ember.Object.create(result));

app/assets/javascripts/discourse/templates/selected-posts.hbs

@@ -24,4 +24,8 @@
   {{d-button action="changeOwner" icon="user" label="topic.change_owner.action"}}
 {{/if}}
 
+{{#if canMergePosts}}
+  {{d-button action="mergePosts" icon="arrows-v" label="topic.merge_posts.action"}}
+{{/if}}
+
 <p class='cancel'><a href {{action "toggleMultiSelect"}}>{{i18n 'topic.multi_select.cancel'}}</a></p>

app/controllers/posts_controller.rb

@@ -1,6 +1,7 @@
 require_dependency 'new_post_manager'
 require_dependency 'post_creator'
 require_dependency 'post_destroyer'
+require_dependency 'post_merger'
 require_dependency 'distributed_memoizer'
 require_dependency 'new_post_result_serializer'
 
@@ -273,6 +274,14 @@ class PostsController < ApplicationController
     render nothing: true
   end
 
+  def merge_posts
+    params.require(:post_ids)
+    posts = Post.where(id: params[:post_ids]).order(:id)
+    raise Discourse::InvalidParameters.new(:post_ids) if posts.pluck(:id) == params[:post_ids]
+    PostMerger.new(current_user, posts).merge
+    render nothing: true
+  end
+
   # Direct replies to this post
   def replies
     post = find_post_from_params

config/locales/client.en.yml

@@ -1525,6 +1525,11 @@ en:
           one: "Please choose the topic you'd like to move that post to."
           other: "Please choose the topic you'd like to move those <b>{{count}}</b> posts to."
 
+      merge_posts:
+        title: "Merge Selected Posts"
+        action: "merge selected posts"
+        error: "There was an error merging the selected posts."
+
       change_owner:
         title: "Change Owner of Posts"
         action: "change ownership"
@@ -1744,6 +1749,11 @@ en:
           one: "Are you sure you want to delete that post?"
           other: "Are you sure you want to delete all those posts?"
 
+      merge:
+        confirm:
+          one: "Are you sure you want merge those posts?"
+          other: "Are you sure you want to merge those {{count}} posts?"
+
       revisions:
         controls:
           first: "First revision"

config/locales/server.en.yml

@@ -1418,6 +1418,14 @@ en:
     new_user: "Welcome to our community! These are the most popular recent topics."
     not_seen_in_a_month: "Welcome back! We haven't seen you in a while. These are the most popular topics since you've been away."
 
+  merge_posts:
+    edit_reason:
+      one: "A post was merged in by %{username}"
+      other: "%{count} posts were merged in by %{username}"
+    errors:
+      different_topics: "Posts belonging to different topics cannot be merged."
+      different_users: "Posts belonging to different users cannot be merged."
+
   move_posts:
     new_topic_moderator_post:
       one: "A post was split to a new topic: %{topic_link}"

config/routes.rb

@@ -410,6 +410,7 @@ Discourse::Application.routes.draw do
     put "recover"
     collection do
       delete "destroy_many"
+      put "merge_posts"
     end
   end
 

lib/post_merger.rb

@@ -0,0 +1,58 @@
+class PostMerger
+  class CannotMergeError < StandardError; end
+
+  def initialize(user, posts)
+    @user = user
+    @posts = posts
+  end
+
+  def merge
+    return unless ensure_at_least_two_posts
+    ensure_same_topic!
+    ensure_same_user!
+
+    guardian = Guardian.new(@user)
+    ensure_staff_user!(guardian)
+
+    posts = @posts.sort_by do |post|
+      guardian.ensure_can_delete!(post)
+      post.post_number
+    end
+
+    post_content = posts.map(&:raw)
+    post = posts.pop
+
+    changes = {
+      raw: post_content.join("\n\n"),
+      edit_reason: I18n.t("merge_posts.edit_reason", count: posts.length, username: @user.username)
+    }
+
+    Post.transaction do
+      revisor = PostRevisor.new(post, post.topic)
+      revisor.revise!(@user, changes, {})
+      posts.each { |p| PostDestroyer.new(@user, p).destroy }
+    end
+  end
+
+  private
+
+  def ensure_at_least_two_posts
+    @posts.count >= 2
+  end
+
+  def ensure_same_topic!
+    unless @posts.map(&:topic_id).uniq.length == 1
+      raise CannotMergeError.new(I18n.t("merge_posts.errors.different_topics"))
+    end
+  end
+
+  def ensure_same_user!
+    unless @posts.map(&:user_id).uniq.length == 1
+      raise CannotMergeError.new(I18n.t("merge_posts.errors.different_users"))
+    end
+  end
+
+  def ensure_staff_user!(guardian)
+    raise Discourse::InvalidAccess unless guardian.is_staff?
+  end
+end

spec/components/post_merger_spec.rb

@@ -0,0 +1,66 @@
+require 'rails_helper'
+require 'post_merger'
+
+describe PostMerger do
+  let(:moderator) { Fabricate(:moderator) }
+  let(:admin) { Fabricate(:admin) }
+  let(:user) { Fabricate(:user) }
+  let(:post) { create_post }
+  let(:topic) { post.topic }
+
+  describe ".merge" do
+    it "should merge posts into the latest post correctly" do
+      reply1 = create_post(topic: topic, raw: 'The first reply', post_number: 2, user: user)
+      reply2 = create_post(topic: topic, raw: "The second reply\nSecond line", post_number: 3, user: user)
+      reply3 = create_post(topic: topic, raw: 'The third reply', post_number: 4, user: user)
+      replies = [reply3, reply2, reply1]
+
+      PostMerger.new(admin, replies).merge
+
+      expect(reply1.trashed?).to eq(true)
+      expect(reply2.trashed?).to eq(true)
+      expect(reply3.deleted_at).to eq(nil)
+
+      expect(reply3.edit_reason).to eq(I18n.t(
+        "merge_posts.edit_reason",
+        count: replies.count - 1, username: admin.username
+      ))
+
+      expect(reply3.raw).to eq(
+        "The first reply\n\nThe second reply\nSecond line\n\nThe third reply"
+      )
+    end
+
+    it "should not allow the first post in a topic to be merged" do
+      post.update_attributes!(user: user)
+      reply1 = create_post(topic: topic, post_number: post.post_number, user: user)
+      reply2 = create_post(topic: topic, post_number: post.post_number, user: user)
+
+      expect{ PostMerger.new(admin, [reply2, post, reply1]).merge }.to raise_error(Discourse::InvalidAccess)
+    end
+
+    it "should only allow staff user to merge posts" do
+      reply1 = create_post(topic: topic, post_number: post.post_number, user: user)
+      reply2 = create_post(topic: topic, post_number: post.post_number, user: user)
+
+      expect{ PostMerger.new(user, [reply2, reply1]).merge }.to raise_error(Discourse::InvalidAccess)
+    end
+
+    it "should not allow posts from different topics to be merged" do
+      another_post = create_post(user: post.user)
+
+      expect { PostMerger.new(user, [another_post, post]).merge }.to raise_error(
+        PostMerger::CannotMergeError, I18n.t("merge_posts.errors.different_topics")
+      )
+    end
+
+    it "should not allow posts from different users to be merged" do
+      another_post = create_post(user: user, topic_id: topic.id)
+
+      expect { PostMerger.new(user, [another_post, post]).merge }.to raise_error(
+        PostMerger::CannotMergeError, I18n.t("merge_posts.errors.different_users")
+      )
+    end
+  end
+end
+