FIX: only send originalText when we need to

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

@@ -975,9 +975,18 @@ const Composer = RestModel.extend({
       if (this.get("replyLength") < this.siteSettings.min_post_length) return;
     }
 
+    this.setProperties({
+      draftStatus: I18n.t("composer.saving_draft_tip"),
+      draftConflictUser: null
+    });
+
+    if (this._clearingStatus) {
+      Em.run.cancel(this._clearingStatus);
+      this._clearingStatus = null;
+    }
+
     const data = {
       reply: this.get("reply"),
-      originalText: this.get("originalText"),
       action: this.get("action"),
       title: this.get("title"),
       categoryId: this.get("categoryId"),
@@ -992,14 +1001,8 @@ const Composer = RestModel.extend({
       noBump: this.get("noBump")
     };
 
-    this.setProperties({
-      draftStatus: I18n.t("composer.saving_draft_tip"),
-      draftConflictUser: null
-    });
-
-    if (this._clearingStatus) {
-      Em.run.cancel(this._clearingStatus);
-      this._clearingStatus = null;
+    if (this.get("post.id") && !Ember.isEmpty(this.get("originalText"))) {
+      data["originalText"] = this.get("originalText");
     }
 
     return Draft.save(this.get("draftKey"), this.get("draftSequence"), data)

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

@@ -5,10 +5,7 @@ Draft.reopenClass({
   clear(key, sequence) {
     return ajax("/draft.json", {
       type: "DELETE",
-      data: {
-        draft_key: key,
-        sequence: sequence
-      }
+      data: { draft_key: key, sequence }
     });
   },
 
@@ -25,16 +22,10 @@ Draft.reopenClass({
   },
 
   save(key, sequence, data) {
-    const dataJson = typeof data === "string" ? dataJson : JSON.stringify(data);
+    data = typeof(data) === "string" ? data : JSON.stringify(data);
     return ajax("/draft.json", {
       type: "POST",
-      data: {
-        draft_key: key,
-        data: dataJson,
-        sequence,
-        post_id: data.postId,
-        original_text: data.originalText
-      }
+      data: { draft_key: key, sequence, data }
     });
   }
 });

app/controllers/draft_controller.rb

@@ -11,11 +11,13 @@ class DraftController < ApplicationController
   def update
     Draft.set(current_user, params[:draft_key], params[:sequence].to_i, params[:data])
 
-    if params[:post_id] && params[:original_text]
-      post = Post.find_by(id: params[:post_id])
-      if post && post.raw != params[:original_text]
-        conflict_user = BasicUserSerializer.new(post.last_editor, root: false)
-        return render json: success_json.merge(conflict_user: conflict_user)
+    if data = JSON::parse(params[:data])
+      if data["postId"].present? && data["originalText"].present?
+        post = Post.find_by(id: data["postId"])
+        if post && post.raw != data["originalText"]
+          conflict_user = BasicUserSerializer.new(post.last_editor, root: false)
+          return render json: success_json.merge(conflict_user: conflict_user)
+        end
       end
     end
 

app/models/draft.rb

@@ -1,19 +1,21 @@
 # frozen_string_literal: true
 
 class Draft < ActiveRecord::Base
-  NEW_TOPIC = 'new_topic'
-  NEW_PRIVATE_MESSAGE = 'new_private_message'
-  EXISTING_TOPIC = 'topic_'
+  NEW_TOPIC ||= 'new_topic'
+  NEW_PRIVATE_MESSAGE ||= 'new_private_message'
+  EXISTING_TOPIC ||= 'topic_'
 
   def self.set(user, key, sequence, data)
-    d = find_draft(user, key)
-    if d
+    if d = find_draft(user, key)
       return if d.sequence > sequence
-      DB.exec("UPDATE drafts
-               SET  data = :data,
-                    sequence = :sequence,
-                    revisions = revisions + 1
-               WHERE id = :id", id: d.id, sequence: sequence, data: data)
+
+      DB.exec(<<~SQL, id: d.id, sequence: sequence, data: data)
+        UPDATE drafts
+           SET sequence = :sequence
+             , data = :data
+             , revisions = revisions + 1
+         WHERE id = :id
+      SQL
     else
       Draft.create!(user_id: user.id, draft_key: key, data: data, sequence: sequence)
     end
@@ -23,16 +25,12 @@ class Draft < ActiveRecord::Base
 
   def self.get(user, key, sequence)
     d = find_draft(user, key)
-    if d && d.sequence == sequence
-      d.data
-    end
+    d.data if d && d.sequence == sequence
   end
 
   def self.clear(user, key, sequence)
     d = find_draft(user, key)
-    if d && d.sequence <= sequence
-      d.destroy
-    end
+    d.destroy if d && d.sequence <= sequence
   end
 
   def self.find_draft(user, key)
@@ -83,11 +81,15 @@ class Draft < ActiveRecord::Base
   end
 
   def self.cleanup!
-    DB.exec("DELETE FROM drafts where sequence < (
-               SELECT max(s.sequence) from draft_sequences s
-               WHERE s.draft_key = drafts.draft_key AND
-                     s.user_id = drafts.user_id
-            )")
+    DB.exec(<<~SQL)
+      DELETE FROM drafts
+       WHERE sequence < (
+        SELECT MAX(s.sequence)
+          FROM draft_sequences s
+         WHERE s.draft_key = drafts.draft_key
+           AND s.user_id = drafts.user_id
+      )
+    SQL
 
     # remove old drafts
     delete_drafts_older_than_n_days = SiteSetting.delete_drafts_older_than_n_days.days.ago

spec/requests/draft_controller_spec.rb

@@ -8,9 +8,15 @@ describe DraftController do
 
   it 'saves a draft on update' do
     user = sign_in(Fabricate(:user))
-    post "/draft.json", params: { draft_key: 'xyz', data: 'my data', sequence: 0 }
+
+    post "/draft.json", params: {
+      draft_key: 'xyz',
+      data: { my: "data" }.to_json,
+      sequence: 0
+    }
+
     expect(response.status).to eq(200)
-    expect(Draft.get(user, 'xyz', 0)).to eq('my data')
+    expect(Draft.get(user, 'xyz', 0)).to eq(%q({"my":"data"}))
   end
 
   it 'checks for an conflict on update' do
@@ -21,9 +27,10 @@ describe DraftController do
       username: user.username,
       draft_key: "topic",
       sequence: 0,
-      data: "{}",
-      post_id: post.id,
-      original_text: post.raw
+      data: {
+        postId: post.id,
+        originalText: post.raw
+      }.to_json
     }
 
     expect(JSON.parse(response.body)['conflict_user']).to eq(nil)
@@ -32,13 +39,16 @@ describe DraftController do
       username: user.username,
       draft_key: "topic",
       sequence: 0,
-      data: "{}",
-      post_id: post.id,
-      original_text: "something else"
+      data: {
+        postId: post.id,
+        originalText: "something else"
+      }.to_json
     }
 
-    expect(JSON.parse(response.body)['conflict_user']['id']).to eq(post.last_editor.id)
-    expect(JSON.parse(response.body)['conflict_user']).to include('avatar_template')
+    json = JSON.parse(response.body)
+
+    expect(json['conflict_user']['id']).to eq(post.last_editor.id)
+    expect(json['conflict_user']).to include('avatar_template')
   end
 
   it 'destroys drafts when required' do