From f704deca17f34272c4df7d5b50c75d82640e1ef3 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Wed, 9 Feb 2022 10:37:38 +0200 Subject: [PATCH] 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 | 1 + app/models/topic.rb | 3 ++- lib/post_creator.rb | 3 ++- lib/topic_creator.rb | 2 +- spec/components/post_creator_spec.rb | 18 ++++++++++++++++-- spec/models/draft_spec.rb | 6 +++--- spec/requests/posts_controller_spec.rb | 12 ++++++++++++ 7 files changed, 37 insertions(+), 8 deletions(-) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index bdc2823755..da7ecf92fd 100644 --- a/app/controllers/posts_controller.rb +++ b/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) diff --git a/app/models/topic.rb b/app/models/topic.rb index dbac653ba8..d3dc304700 100644 --- a/app/models/topic.rb +++ b/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 diff --git a/lib/post_creator.rb b/lib/post_creator.rb index 8b7f186e27..2e56397c99 100644 --- a/lib/post_creator.rb +++ b/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 diff --git a/lib/topic_creator.rb b/lib/topic_creator.rb index 4b79035b48..9119ec9551 100644 --- a/lib/topic_creator.rb +++ b/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 diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index 484ff8140a..465d83300f 100644 --- a/spec/components/post_creator_spec.rb +++ b/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 diff --git a/spec/models/draft_spec.rb b/spec/models/draft_spec.rb index 538e9c96c5..22052679df 100644 --- a/spec/models/draft_spec.rb +++ b/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 diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index 5ebc38d976..1686be0af1 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/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