From eef21625c6b9dd589573bc1771c7075a12e0abc9 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Tue, 7 Jan 2020 15:33:48 +0200 Subject: [PATCH] Rename 'target usernames' with 'target recipients' in Composer (#8606) * Reapply "Rename 'target usernames' with 'target recipients' in Composer" This reverts commit 9fe11d0fc33850d46d4e1261ba6f66f187652613 which reverted ebb288dc2cec966fd26ee46f10e1e4d8a596ba5a. * DEV: Add test for replying to PM --- .../components/composer-messages.js.es6 | 11 ++-- .../discourse/controllers/composer.js.es6 | 22 +++++--- .../discourse/mixins/open-composer.js.es6 | 4 +- .../discourse/models/composer.js.es6 | 53 +++++++++++++++---- .../discourse/routes/application.js.es6 | 24 ++++----- .../discourse/templates/composer.hbs | 2 +- app/controllers/posts_controller.rb | 22 +++++--- .../acceptance/composer-test.js.es6 | 17 ++++++ test/javascripts/models/composer-test.js.es6 | 12 +++++ 9 files changed, 120 insertions(+), 47 deletions(-) diff --git a/app/assets/javascripts/discourse/components/composer-messages.js.es6 b/app/assets/javascripts/discourse/components/composer-messages.js.es6 index e9860973a2a..cafae730587 100644 --- a/app/assets/javascripts/discourse/components/composer-messages.js.es6 +++ b/app/assets/javascripts/discourse/components/composer-messages.js.es6 @@ -97,16 +97,11 @@ export default Component.extend({ const composer = this.composer; if (composer.get("privateMessage")) { - let usernames = composer.get("targetUsernames"); - - if (usernames) { - usernames = usernames.split(","); - } + const recipients = composer.targetRecipientsArray; if ( - usernames && - usernames.length === 1 && - usernames[0] === this.currentUser.get("username") + recipients.length > 0 && + recipients.every(r => r.name === this.currentUser.get("username")) ) { const message = this._yourselfConfirm || diff --git a/app/assets/javascripts/discourse/controllers/composer.js.es6 b/app/assets/javascripts/discourse/controllers/composer.js.es6 index a379d8f7e11..1e2414894ae 100644 --- a/app/assets/javascripts/discourse/controllers/composer.js.es6 +++ b/app/assets/javascripts/discourse/controllers/composer.js.es6 @@ -25,6 +25,7 @@ import { SAVE_LABELS, SAVE_ICONS } from "discourse/models/composer"; import { Promise } from "rsvp"; import ENV from "discourse-common/config/environment"; import EmberObject, { computed } from "@ember/object"; +import deprecated from "discourse-common/lib/deprecated"; function loadDraft(store, opts) { opts = opts || {}; @@ -129,7 +130,7 @@ export default Controller.extend({ @discourseComputed( "model.replyingToTopic", "model.creatingPrivateMessage", - "model.targetUsernames", + "model.targetRecipients", "model.composeState" ) focusTarget(replyingToTopic, creatingPM, usernames, composeState) { @@ -294,7 +295,7 @@ export default Controller.extend({ } }, - @discourseComputed("model.creatingPrivateMessage", "model.targetUsernames") + @discourseComputed("model.creatingPrivateMessage", "model.targetRecipients") showWarning(creatingPrivateMessage, usernames) { if (!this.get("currentUser.staff")) { return false; @@ -909,19 +910,24 @@ export default Controller.extend({ isWarning: false }); - if (opts.usernames && !this.get("model.targetUsernames")) { - this.set("model.targetUsernames", opts.usernames); + if (!this.model.targetRecipients) { + if (opts.usernames) { + deprecated("`usernames` is deprecated, use `recipients` instead."); + this.model.set("targetRecipients", opts.usernames); + } else if (opts.recipients) { + this.model.set("targetRecipients", opts.recipients); + } } if ( opts.topicTitle && opts.topicTitle.length <= this.siteSettings.max_topic_title_length ) { - this.set("model.title", opts.topicTitle); + this.model.set("title", opts.topicTitle); } if (opts.topicCategoryId) { - this.set("model.categoryId", opts.topicCategoryId); + this.model.set("categoryId", opts.topicCategoryId); } if (opts.topicTags && !this.site.mobileView && this.site.can_tag_topics) { @@ -934,11 +940,11 @@ export default Controller.extend({ (array[index] = tag.substring(0, this.siteSettings.max_tag_length)) ); - this.set("model.tags", tags); + this.model.set("tags", tags); } if (opts.topicBody) { - this.set("model.reply", opts.topicBody); + this.model.set("reply", opts.topicBody); } }, diff --git a/app/assets/javascripts/discourse/mixins/open-composer.js.es6 b/app/assets/javascripts/discourse/mixins/open-composer.js.es6 index 136cca6d50a..480eae57835 100644 --- a/app/assets/javascripts/discourse/mixins/open-composer.js.es6 +++ b/app/assets/javascripts/discourse/mixins/open-composer.js.es6 @@ -39,10 +39,10 @@ export default Mixin.create({ }); }, - openComposerWithMessageParams(usernames, topicTitle, topicBody) { + openComposerWithMessageParams(recipients, topicTitle, topicBody) { this.controllerFor("composer").open({ action: Composer.PRIVATE_MESSAGE, - usernames, + recipients, topicTitle, topicBody, archetypeId: "private_message", diff --git a/app/assets/javascripts/discourse/models/composer.js.es6 b/app/assets/javascripts/discourse/models/composer.js.es6 index a630f521c41..3f8c75d9fc8 100644 --- a/app/assets/javascripts/discourse/models/composer.js.es6 +++ b/app/assets/javascripts/discourse/models/composer.js.es6 @@ -14,13 +14,18 @@ import { observes, on } from "discourse-common/utils/decorators"; -import { escapeExpression, tinyAvatar } from "discourse/lib/utilities"; +import { + escapeExpression, + tinyAvatar, + emailValid +} from "discourse/lib/utilities"; import { propertyNotEqual } from "discourse/lib/computed"; import { throttle } from "@ember/runloop"; import { Promise } from "rsvp"; import { set } from "@ember/object"; import Site from "discourse/models/site"; import User from "discourse/models/user"; +import deprecated from "discourse-common/lib/deprecated"; // The actions the composer can take export const CREATE_TOPIC = "createTopic", @@ -51,7 +56,7 @@ const CLOSED = "closed", is_warning: "isWarning", whisper: "whisper", archetype: "archetypeId", - target_usernames: "targetUsernames", + target_recipients: "targetRecipients", typing_duration_msecs: "typingTime", composer_open_duration_msecs: "composerTime", tags: "tags", @@ -77,7 +82,9 @@ const CLOSED = "closed", composerTime: "composerTime", typingTime: "typingTime", postId: "post.id", - usernames: "targetUsernames" + // TODO remove together with 'targetUsername' deprecations + usernames: "targetUsernames", + recipients: "targetRecipients" }, _add_draft_fields = {}, FAST_REPLY_LENGTH_THRESHOLD = 10000; @@ -340,11 +347,36 @@ const Composer = RestModel.extend({ return options; }, + @discourseComputed("targetRecipients") + targetUsernames(targetRecipients) { + deprecated( + "`targetUsernames` is deprecated, use `targetRecipients` instead." + ); + return targetRecipients; + }, + + @discourseComputed("targetRecipients") + targetRecipientsArray(targetRecipients) { + const recipients = targetRecipients ? targetRecipients.split(",") : []; + const groups = new Set(this.site.groups.map(g => g.name)); + + return recipients.map(item => { + if (groups.has(item)) { + return { type: "group", name: item }; + } else if (emailValid(item)) { + return { type: "email", name: item }; + } else { + return { type: "user", name: item }; + } + }); + }, + @discourseComputed( "loading", "canEditTitle", "titleLength", - "targetUsernames", + "targetRecipients", + "targetRecipientsArray", "replyLength", "categoryId", "missingReplyCharacters", @@ -357,7 +389,8 @@ const Composer = RestModel.extend({ loading, canEditTitle, titleLength, - targetUsernames, + targetRecipients, + targetRecipientsArray, replyLength, categoryId, missingReplyCharacters, @@ -402,9 +435,7 @@ const Composer = RestModel.extend({ if (this.privateMessage) { // need at least one user when sending a PM - return ( - targetUsernames && (targetUsernames.trim() + ",").indexOf(",") === 0 - ); + return targetRecipients && targetRecipientsArray.length === 0; } else { // has a category? (when needed) return this.requiredCategoryMissing; @@ -667,13 +698,17 @@ const Composer = RestModel.extend({ throw new Error("draft sequence is required"); } + if (opts.usernames) { + deprecated("`usernames` is deprecated, use `recipients` instead."); + } + this.setProperties({ draftKey: opts.draftKey, draftSequence: opts.draftSequence, composeState: opts.composerState || OPEN, action: opts.action, topic: opts.topic, - targetUsernames: opts.usernames, + targetRecipients: opts.usernames || opts.recipients, composerTotalOpened: opts.composerTime, typingTime: opts.typingTime, whisper: opts.whisper, diff --git a/app/assets/javascripts/discourse/routes/application.js.es6 b/app/assets/javascripts/discourse/routes/application.js.es6 index 793c8a64923..e877d8ceff2 100644 --- a/app/assets/javascripts/discourse/routes/application.js.es6 +++ b/app/assets/javascripts/discourse/routes/application.js.es6 @@ -71,20 +71,20 @@ const ApplicationRoute = DiscourseRoute.extend(OpenComposer, { }, composePrivateMessage(user, post) { - const recipient = user ? user.get("username") : "", - reply = post - ? `${window.location.protocol}//${window.location.host}${post.url}` - : null, - title = post - ? I18n.t("composer.reference_topic_title", { - title: post.topic.title - }) - : null; + const recipients = user ? user.get("username") : ""; + const reply = post + ? `${window.location.protocol}//${window.location.host}${post.url}` + : null; + const title = post + ? I18n.t("composer.reference_topic_title", { + title: post.topic.title + }) + : null; // used only once, one less dependency return this.controllerFor("composer").open({ action: Composer.PRIVATE_MESSAGE, - usernames: recipient, + recipients, archetypeId: "private_message", draftKey: Composer.NEW_PRIVATE_MESSAGE_KEY, reply, @@ -221,8 +221,8 @@ const ApplicationRoute = DiscourseRoute.extend(OpenComposer, { ); }, - createNewMessageViaParams(username, title, body) { - this.openComposerWithMessageParams(username, title, body); + createNewMessageViaParams(recipients, title, body) { + this.openComposerWithMessageParams(recipients, title, body); } }, diff --git a/app/assets/javascripts/discourse/templates/composer.hbs b/app/assets/javascripts/discourse/templates/composer.hbs index f738efc3625..4a5a1789462 100644 --- a/app/assets/javascripts/discourse/templates/composer.hbs +++ b/app/assets/javascripts/discourse/templates/composer.hbs @@ -53,7 +53,7 @@ {{#if model.creatingPrivateMessage}}
{{composer-user-selector topicId=topicModel.id - usernames=model.targetUsernames + usernames=model.targetRecipients hasGroups=model.hasTargetGroups focusTarget=focusTarget class="users-input"}} diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 580d10a9943..caef113e502 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -675,7 +675,9 @@ class PostsController < ApplicationController :topic_id, :archetype, :category, + # TODO remove together with 'targetUsername' deprecations :target_usernames, + :target_recipients, :reply_to_post_number, :auto_track, :typing_duration_msecs, @@ -758,13 +760,19 @@ class PostsController < ApplicationController result[:user_agent] = request.user_agent result[:referrer] = request.env["HTTP_REFERER"] - if usernames = result[:target_usernames] - usernames = usernames.split(",") - groups = Group.messageable(current_user).where('name in (?)', usernames).pluck('name') - usernames -= groups - emails = usernames.select { |user| user.match(/@/) } - usernames -= emails - result[:target_usernames] = usernames.join(",") + if recipients = result[:target_usernames] + Discourse.deprecate("`target_usernames` is deprecated, use `target_recipients` instead.", output_in_test: true) + else + recipients = result[:target_recipients] + end + + if recipients + recipients = recipients.split(",") + groups = Group.messageable(current_user).where('name in (?)', recipients).pluck('name') + recipients -= groups + emails = recipients.select { |user| user.match(/@/) } + recipients -= emails + result[:target_usernames] = recipients.join(",") result[:target_emails] = emails.join(",") result[:target_group_names] = groups.join(",") end diff --git a/test/javascripts/acceptance/composer-test.js.es6 b/test/javascripts/acceptance/composer-test.js.es6 index d85bb9a2785..0a5d4d8baec 100644 --- a/test/javascripts/acceptance/composer-test.js.es6 +++ b/test/javascripts/acceptance/composer-test.js.es6 @@ -831,3 +831,20 @@ QUnit.test("Image resizing buttons", async assert => { "it does not unescapes script tags in code blocks" ); }); + +QUnit.test("can reply to a private message", async assert => { + let submitted; + + /* global server */ + server.post("/posts", () => { + submitted = true; + return [200, { "Content-Type": "application/json" }, {}]; + }); + + await visit("/t/34"); + await click(".topic-post:eq(0) button.reply"); + await fillIn(".d-editor-input", "this is the *content* of the reply"); + await click("#reply-control button.create"); + + assert.ok(submitted); +}); diff --git a/test/javascripts/models/composer-test.js.es6 b/test/javascripts/models/composer-test.js.es6 index cd3135b74f6..e2e35fa3463 100644 --- a/test/javascripts/models/composer-test.js.es6 +++ b/test/javascripts/models/composer-test.js.es6 @@ -396,3 +396,15 @@ QUnit.test("allows featured link before choosing a category", assert => { ); assert.ok(composer.get("canEditTopicFeaturedLink"), "can paste link"); }); + +QUnit.test("targetRecipientsArray contains types", assert => { + let composer = createComposer({ + targetRecipients: "test,codinghorror,staff,foo@bar.com" + }); + assert.ok(composer.targetRecipientsArray, [ + { type: "group", name: "test" }, + { type: "user", name: "codinghorror" }, + { type: "group", name: "staff" }, + { type: "email", name: "foo@bar.com" } + ]); +});