Rename 'target usernames' with 'target recipients' in Composer (#8606)

* Reapply "Rename 'target usernames' with 'target recipients' in Composer"

This reverts commit 9fe11d0fc3 which
reverted ebb288dc2c.

* DEV: Add test for replying to PM

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");
+      const recipients = composer.targetRecipientsArray;
-
-      if (usernames) {
-        usernames = usernames.split(",");
-      }
 
       if (
-        usernames &&
+        recipients.length > 0 &&
-        usernames.length === 1 &&
+        recipients.every(r => r.name === this.currentUser.get("username"))
-        usernames[0] === this.currentUser.get("username")
       ) {
         const message =
           this._yourselfConfirm ||

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")) {
+    if (!this.model.targetRecipients) {
-      this.set("model.targetUsernames", opts.usernames);
+      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);
     }
   },
 

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",

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 (
+      return targetRecipients && targetRecipientsArray.length === 0;
-        targetUsernames && (targetUsernames.trim() + ",").indexOf(",") === 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,

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") : "",
+      const recipients = user ? user.get("username") : "";
-        reply = post
+      const reply = post
-          ? `${window.location.protocol}//${window.location.host}${post.url}`
+        ? `${window.location.protocol}//${window.location.host}${post.url}`
-          : null,
+        : null;
-        title = post
+      const title = post
-          ? I18n.t("composer.reference_topic_title", {
+        ? I18n.t("composer.reference_topic_title", {
-              title: post.topic.title
+            title: post.topic.title
-            })
+          })
-          : null;
+        : 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) {
+    createNewMessageViaParams(recipients, title, body) {
-      this.openComposerWithMessageParams(username, title, body);
+      this.openComposerWithMessageParams(recipients, title, body);
     }
   },
 

app/assets/javascripts/discourse/templates/composer.hbs

@@ -53,7 +53,7 @@
                 {{#if model.creatingPrivateMessage}}
                   <div class='user-selector'>
                     {{composer-user-selector topicId=topicModel.id
-                                             usernames=model.targetUsernames
+                                             usernames=model.targetRecipients
                                              hasGroups=model.hasTargetGroups
                                              focusTarget=focusTarget
                                              class="users-input"}}

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]
+    if recipients = result[:target_usernames]
-      usernames = usernames.split(",")
+      Discourse.deprecate("`target_usernames` is deprecated, use `target_recipients` instead.", output_in_test: true)
-      groups = Group.messageable(current_user).where('name in (?)', usernames).pluck('name')
+    else
-      usernames -= groups
+      recipients = result[:target_recipients]
-      emails = usernames.select { |user| user.match(/@/) }
+    end
-      usernames -= emails
+
-      result[:target_usernames] = usernames.join(",")
+    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

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);
+});

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" }
+  ]);
+});