FIX: Clear drafts only when post is created by real user (#15720)

This commits adds a new advance_draft to PostCreator that controls if
the draft sequence will be advanced or not. If the draft sequence is
advanced then the old drafts will be cleared. This used to happen for
posts created by plugins or through the API and cleared user drafts
by mistake.

app/controllers/posts_controller.rb

@@ -182,6 +182,7 @@ class PostsController < ApplicationController
   def create
     @manager_params = create_params
     @manager_params[:first_post_checks] = !is_api?
+    @manager_params[:advance_draft] = !is_api?
 
     manager = NewPostManager.new(current_user, @manager_params)
 

app/models/topic.rb

@@ -330,6 +330,7 @@ class Topic < ActiveRecord::Base
 
   attr_accessor :ignore_category_auto_close
   attr_accessor :skip_callbacks
+  attr_accessor :advance_draft
 
   before_create do
     initialize_default_values
@@ -338,7 +339,7 @@ class Topic < ActiveRecord::Base
   after_create do
     unless skip_callbacks
       changed_to_category(category)
-      advance_draft_sequence
+      advance_draft_sequence if advance_draft
     end
   end
 

lib/post_creator.rb

@@ -36,6 +36,7 @@ class PostCreator
   #   hidden_reason_id        - Reason for hiding the post (optional)
   #   skip_validations        - Do not validate any of the content in the post
   #   draft_key               - the key of the draft we are creating (will be deleted on success)
+  #   advance_draft           - Destroy draft after creating post or topic
   #   silent                  - Do not update topic stats and fields like last_post_user_id
   #
   #   When replying to a topic:
@@ -218,7 +219,7 @@ class PostCreator
         delete_owned_bookmarks
         ensure_in_allowed_users if guardian.is_staff?
         unarchive_message if !@opts[:import_mode]
-        DraftSequence.next!(@user, draft_key) if !@opts[:import_mode]
+        DraftSequence.next!(@user, draft_key) if !@opts[:import_mode] && @opts[:advance_draft]
         @post.save_reply_relationships
       end
     end

lib/topic_creator.rb

@@ -111,7 +111,7 @@ class TopicCreator
       visible: @opts[:visible]
     }
 
-    [:subtype, :archetype, :meta_data, :import_mode].each do |key|
+    [:subtype, :archetype, :meta_data, :import_mode, :advance_draft].each do |key|
       topic_params[key] = @opts[key] if @opts[key].present?
     end
 

spec/components/post_creator_spec.rb

@@ -14,7 +14,7 @@ describe PostCreator do
 
   context "new topic" do
     fab!(:category) { Fabricate(:category, user: user) }
-    let(:basic_topic_params) { { title: "hello world topic", raw: "my name is fred", archetype_id: 1 } }
+    let(:basic_topic_params) { { title: "hello world topic", raw: "my name is fred", archetype_id: 1, advance_draft: true } }
     let(:image_sizes) { { 'http://an.image.host/image.jpg' => { "width" => 111, "height" => 222 } } }
 
     let(:creator) { PostCreator.new(user, basic_topic_params) }
@@ -150,7 +150,7 @@ describe PostCreator do
 
         messages = MessageBus.track_publish do
           created_post = PostCreator.new(admin, basic_topic_params.merge(category: cat.id)).create
-          _reply = PostCreator.new(admin, raw: "this is my test reply 123 testing", topic_id: created_post.topic_id).create
+          _reply = PostCreator.new(admin, raw: "this is my test reply 123 testing", topic_id: created_post.topic_id, advance_draft: true).create
         end
 
         messages.filter! { |m| m.channel != "/distributed_hash" }
@@ -303,6 +303,20 @@ describe PostCreator do
         end
       end
 
+      it 'clears the draft if advanced_draft is true' do
+        creator = PostCreator.new(user, basic_topic_params.merge(advance_draft: true))
+        Draft.set(user, Draft::NEW_TOPIC, 0, 'test')
+        expect(Draft.where(user: user).size).to eq(1)
+        expect { creator.create }.to change { Draft.count }.by(-1)
+      end
+
+      it 'does not clear the draft if advanced_draft is false' do
+        creator = PostCreator.new(user, basic_topic_params.merge(advance_draft: false))
+        Draft.set(user, Draft::NEW_TOPIC, 0, 'test')
+        expect(Draft.where(user: user).size).to eq(1)
+        expect { creator.create }.to change { Draft.count }.by(0)
+      end
+
       it "updates topic stats" do
         first_post = creator.create
         topic = first_post.topic.reload

spec/models/draft_spec.rb

@@ -227,7 +227,7 @@ describe Draft do
   context 'key expiry' do
     it 'nukes new topic draft after a topic is created' do
       Draft.set(user, Draft::NEW_TOPIC, 0, 'my draft')
-      _t = Fabricate(:topic, user: user)
+      _t = Fabricate(:topic, user: user, advance_draft: true)
       s = DraftSequence.current(user, Draft::NEW_TOPIC)
       expect(Draft.get(user, Draft::NEW_TOPIC, s)).to eq nil
       expect(Draft.count).to eq 0
@@ -235,7 +235,7 @@ describe Draft do
 
     it 'nukes new pm draft after a pm is created' do
       Draft.set(user, Draft::NEW_PRIVATE_MESSAGE, 0, 'my draft')
-      t = Fabricate(:topic, user: user, archetype: Archetype.private_message, category_id: nil)
+      t = Fabricate(:topic, user: user, archetype: Archetype.private_message, category_id: nil, advance_draft: true)
       s = DraftSequence.current(t.user, Draft::NEW_PRIVATE_MESSAGE)
       expect(Draft.get(user, Draft::NEW_PRIVATE_MESSAGE, s)).to eq nil
     end
@@ -252,7 +252,7 @@ describe Draft do
 
       Draft.set(user, topic.draft_key, 0, 'hello')
 
-      p = PostCreator.new(user, raw: Fabricate.build(:post).raw, topic_id: topic.id).create
+      p = PostCreator.new(user, raw: Fabricate.build(:post).raw, topic_id: topic.id, advance_draft: true).create
 
       expect(Draft.get(p.user, p.topic.draft_key, DraftSequence.current(p.user, p.topic.draft_key))).to eq nil
     end

spec/requests/posts_controller_spec.rb

@@ -861,6 +861,18 @@ describe PostsController do
         expect(response.status).to eq(403)
       end
 
+      it 'does not advance draft' do
+        Draft.set(user, Draft::NEW_TOPIC, 0, "test")
+        user_key = ApiKey.create!(user: user).key
+
+        post "/posts.json",
+          params: { title: 'this is a test topic', raw: 'this is test whisper' },
+          headers: { HTTP_API_USERNAME: user.username, HTTP_API_KEY: user_key }
+
+        expect(response.status).to eq(200)
+        expect(Draft.get(user, Draft::NEW_TOPIC, 0)).to eq("test")
+      end
+
       it 'will raise an error if specified category cannot be found' do
         user = Fabricate(:admin)
         master_key = Fabricate(:api_key).key