FIX: post merging was failing silently (#12566)

https://meta.discourse.org/t/merging-very-long-posts-removes-them/183597

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

@@ -400,7 +400,7 @@ Post.reopenClass({
     return ajax("/posts/merge_posts", {
       type: "PUT",
       data: { post_ids },
-    });
+    }).catch(popupAjaxError);
   },
 
   loadRevision(postId, version) {

app/controllers/posts_controller.rb

@@ -363,6 +363,8 @@ class PostsController < ApplicationController
     raise Discourse::InvalidParameters.new(:post_ids) if posts.pluck(:id) == params[:post_ids]
     PostMerger.new(current_user, posts).merge
     render body: nil
+  rescue PostMerger::CannotMergeError => e
+    render_json_error(e.message)
   end
 
   # Direct replies to this post

config/locales/server.en.yml

@@ -2382,6 +2382,7 @@ en:
     errors:
       different_topics: "Posts belonging to different topics cannot be merged."
       different_users: "Posts belonging to different users cannot be merged."
+      max_post_length: "Posts cannot be merged because the combined post length is more than allowed."
 
   move_posts:
     new_topic_moderator_post:

lib/post_merger.rb

@@ -24,14 +24,14 @@ class PostMerger
     post_content = posts.map(&:raw)
     post = posts.pop
 
+    merged_post_raw = post_content.join("\n\n")
     changes = {
-      raw: post_content.join("\n\n"),
+      raw: merged_post_raw,
       edit_reason: I18n.t("merge_posts.edit_reason", count: posts.length, username: @user.username)
     }
 
-    revisor = PostRevisor.new(post, post.topic)
+    ensure_max_post_length!(merged_post_raw)
-
+    PostRevisor.new(post, post.topic).revise!(@user, changes) do
-    revisor.revise!(@user, changes) do
       posts.each { |p| PostDestroyer.new(@user, p).destroy }
     end
   end
@@ -57,4 +57,11 @@ class PostMerger
   def ensure_staff_user!(guardian)
     raise Discourse::InvalidAccess unless guardian.is_staff?
   end
+
+  def ensure_max_post_length!(raw)
+    value = StrippedLengthValidator.get_sanitized_value(raw)
+    if value.length > SiteSetting.max_post_length
+      raise CannotMergeError.new(I18n.t("merge_posts.errors.max_post_length"))
+    end
+  end
 end

lib/validators/stripped_length_validator.rb

@@ -3,13 +3,7 @@
 class StrippedLengthValidator < ActiveModel::EachValidator
   def self.validate(record, attribute, value, range)
     if !value.nil?
-      value = value.dup
+      value = get_sanitized_value(value)
-      value.gsub!(/<!--(.*?)-->/, '') # strip HTML comments
-      value.gsub!(/:\w+(:\w+)?:/, "X") # replace emojis with a single character
-      value.gsub!(/\.{2,}/, '…') # replace multiple ... with …
-      value.gsub!(/\,{2,}/, ',') # replace multiple ,,, with ,
-      value.strip!
-
       record.errors.add attribute, (I18n.t('errors.messages.too_short', count: range.begin)) if value.length < range.begin
       record.errors.add attribute, (I18n.t('errors.messages.too_long_validation', max: range.end, length: value.length)) if value.length > range.end
     else
@@ -22,4 +16,13 @@ class StrippedLengthValidator < ActiveModel::EachValidator
     range = options[:in].lambda? ? options[:in].call : options[:in]
     self.class.validate(record, attribute, value, range)
   end
+
+  def self.get_sanitized_value(value)
+    value = value.dup
+    value.gsub!(/<!--(.*?)-->/, '') # strip HTML comments
+    value.gsub!(/:\w+(:\w+)?:/, "X") # replace emojis with a single character
+    value.gsub!(/\.{2,}/, '…') # replace multiple ... with …
+    value.gsub!(/\,{2,}/, ',') # replace multiple ,,, with ,
+    value.strip
+  end
 end

spec/components/post_merger_spec.rb

@@ -66,5 +66,18 @@ describe PostMerger do
         PostMerger::CannotMergeError, I18n.t("merge_posts.errors.different_users")
       )
     end
+
+    it "should not allow posts with length greater than max_post_length" do
+      SiteSetting.max_post_length = 60
+
+      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]
+
+      expect { PostMerger.new(admin, replies).merge }.to raise_error(
+        PostMerger::CannotMergeError, I18n.t("merge_posts.errors.max_post_length")
+      )
+    end
   end
 end