From 0ea11a9d497c504cf640fa9dea073cffcc43f265 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Mon, 24 Feb 2020 15:55:12 +0100 Subject: [PATCH] FIX: ensures we don't attempt to create a new PM on an existing topic (#9029) This fix attempts to both fix it at UI level and server side. A previous attempt related to this behavior has been made in commit: https://github.com/discourse/discourse/commit/49c750ca7890490b1405f940b1eb2e7a4f3f69f6 --- .../discourse/models/composer.js.es6 | 2 +- config/locales/server.en.yml | 1 + lib/post_creator.rb | 6 ++++++ .../advanced_user_narrative.rb | 6 +++--- .../new_user_narrative.rb | 4 +++- spec/requests/posts_controller_spec.rb | 21 +++++++++++++++++++ 6 files changed, 35 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/discourse/models/composer.js.es6 b/app/assets/javascripts/discourse/models/composer.js.es6 index 0d8f533f1e2..2150396102b 100644 --- a/app/assets/javascripts/discourse/models/composer.js.es6 +++ b/app/assets/javascripts/discourse/models/composer.js.es6 @@ -912,7 +912,7 @@ const Composer = RestModel.extend({ }, createPost(opts) { - if (this.action === CREATE_TOPIC) { + if (CREATE_TOPIC === this.action || PRIVATE_MESSAGE === this.action) { this.set("topic", null); } diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 577180be98b..00e24df01cb 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -339,6 +339,7 @@ en: pm_reached_recipients_limit: "Sorry, you can't have more than %{recipients_limit} recipients in a message." removed_direct_reply_full_quotes: "Automatically removed quote of whole previous post." secure_upload_not_allowed_in_public_topic: "Sorry, the following secure upload(s) cannot be used in a public topic: %{upload_filenames}." + create_pm_on_existing_topic: "Sorry, you can't create a PM on an existing topic." just_posted_that: "is too similar to what you recently posted" invalid_characters: "contains invalid characters" diff --git a/lib/post_creator.rb b/lib/post_creator.rb index 816132475eb..8340c6c7b7a 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -136,6 +136,12 @@ class PostCreator return false unless skip_validations? || validate_child(topic_creator) else @topic = Topic.find_by(id: @opts[:topic_id]) + + if @topic.present? && @opts[:archetype] == Archetype.private_message + errors.add(:base, I18n.t(:create_pm_on_existing_topic)) + return false + end + unless @topic.present? && (@opts[:skip_guardian] || guardian.can_create?(Post, @topic)) errors.add(:base, I18n.t(:topic_not_found)) return false diff --git a/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/advanced_user_narrative.rb b/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/advanced_user_narrative.rb index 5dda2de49f3..8da5cdc059b 100644 --- a/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/advanced_user_narrative.rb +++ b/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/advanced_user_narrative.rb @@ -176,12 +176,12 @@ module DiscourseNarrativeBot if @post && @post.topic.private_message? && @post.topic.topic_allowed_users.pluck(:user_id).include?(@user.id) - - opts = opts.merge(topic_id: @post.topic_id) end if @data[:topic_id] - opts = opts.merge(topic_id: @data[:topic_id]) + opts = opts + .merge(topic_id: @data[:topic_id]) + .except(:title, :target_usernames, :archetype) end post = reply_to(@post, raw, opts) diff --git a/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/new_user_narrative.rb b/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/new_user_narrative.rb index f85854c32da..8d033ac6be9 100644 --- a/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/new_user_narrative.rb +++ b/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/new_user_narrative.rb @@ -208,7 +208,9 @@ module DiscourseNarrativeBot end if @data[:topic_id] - opts = opts.merge(topic_id: @data[:topic_id]) + opts = opts + .merge(topic_id: @data[:topic_id]) + .except(:title, :target_usernames, :archetype) end post = reply_to(@post, raw, opts) diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index 57d9cf5d00e..59042ed8e80 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/spec/requests/posts_controller_spec.rb @@ -1126,6 +1126,27 @@ describe PostsController do end end + context "when topic_id is set" do + fab!(:topic) { Fabricate(:topic) } + + it "errors when creating a private post" do + user_2 = Fabricate(:user) + + post "/posts.json", params: { + raw: 'this is the test content', + archetype: 'private_message', + title: "this is some post", + target_recipients: user_2.username, + topic_id: topic.id + } + + expect(response.status).to eq(422) + expect(JSON.parse(response.body)["errors"]).to include( + I18n.t("create_pm_on_existing_topic") + ) + end + end + context "errors" do it "does not succeed" do post "/posts.json", params: { raw: 'test' }