FIX: don't raise error when inviting existing user to private topic via email

https://meta.discourse.org/t/inviting-existing-user-to-a-private-topic-message-via-email-shows-error-message/65994

app/assets/javascripts/discourse/controllers/invite.js.es6

@@ -13,6 +13,7 @@ export default Ember.Controller.extend(ModalFunctionality, {
   hasCustomMessage: false,
   customMessage: null,
   inviteIcon: "envelope",
+  invitingExistingUserToTopic: false,
 
   @computed('isMessage', 'invitingToTopic')
   title(isMessage, invitingToTopic) {
@@ -25,9 +26,10 @@ export default Ember.Controller.extend(ModalFunctionality, {
     }
   },
 
-  isAdmin: function(){
+  @computed
+  isAdmin() {
     return Discourse.User.currentProp("admin");
-  }.property(),
+  },
 
   @computed('isAdmin', 'emailOrUsername', 'invitingToTopic', 'isPrivateTopic', 'model.groupNames', 'model.saving', 'model.details.can_invite_to')
   disabled(isAdmin, emailOrUsername, invitingToTopic, isPrivateTopic, groupNames, saving, can_invite_to) {
@@ -46,29 +48,32 @@ export default Ember.Controller.extend(ModalFunctionality, {
     return false;
   },
 
-  disabledCopyLink: function() {
-    if (this.get('hasCustomMessage')) return true;
-    if (this.get('model.saving')) return true;
-    if (Ember.isEmpty(this.get('emailOrUsername'))) return true;
-    const emailOrUsername = this.get('emailOrUsername').trim();
+  @computed('isAdmin', 'emailOrUsername', 'model.saving', 'isPrivateTopic', 'model.groupNames', 'hasCustomMessage')
+  disabledCopyLink(isAdmin, emailOrUsername, saving, isPrivateTopic, groupNames, hasCustomMessage) {
+    if (hasCustomMessage) return true;
+    if (saving) return true;
+    if (Ember.isEmpty(emailOrUsername)) return true;
+    const email = emailOrUsername.trim();
     // email must be valid
-    if (!emailValid(emailOrUsername)) return true;
+    if (!emailValid(email)) return true;
     // normal users (not admin) can't invite users to private topic via email
-    if (!this.get('isAdmin') && this.get('isPrivateTopic') && emailValid(emailOrUsername)) return true;
+    if (!isAdmin && isPrivateTopic && emailValid(email)) return true;
     // when inviting to private topic via email, group name must be specified
-    if (this.get('isPrivateTopic') && Ember.isEmpty(this.get('model.groupNames')) && emailValid(emailOrUsername)) return true;
+    if (isPrivateTopic && Ember.isEmpty(groupNames) && emailValid(email)) return true;
     return false;
-  }.property('emailOrUsername', 'model.saving', 'isPrivateTopic', 'model.groupNames', 'hasCustomMessage'),
+  },
 
-  buttonTitle: function() {
-    return this.get('model.saving') ? 'topic.inviting' : 'topic.invite_reply.action';
-  }.property('model.saving'),
+  @computed('model.saving')
+  buttonTitle(saving) {
+    return saving ? 'topic.inviting' : 'topic.invite_reply.action';
+  },
 
   // We are inviting to a topic if the model isn't the current user.
   // The current user would mean we are inviting to the forum in general.
-  invitingToTopic: function() {
-    return this.get('model') !== this.currentUser;
-  }.property('model'),
+  @computed('model')
+  invitingToTopic(model) {
+    return model !== this.currentUser;
+  },
 
   @computed('model', 'model.details.can_invite_via_email')
   canInviteViaEmail(model, can_invite_via_email) {
@@ -91,9 +96,7 @@ export default Ember.Controller.extend(ModalFunctionality, {
   isMessage: Em.computed.equal('model.archetype', 'private_message'),
 
   // Allow Existing Members? (username autocomplete)
-  allowExistingMembers: function() {
-    return this.get('invitingToTopic');
-  }.property('invitingToTopic'),
+  allowExistingMembers: Ember.computed.alias('invitingToTopic'),
 
   // Show Groups? (add invited user to private group)
   @computed('isAdmin', 'emailOrUsername', 'isPrivateTopic', 'isMessage', 'invitingToTopic', 'canInviteViaEmail')
@@ -141,29 +144,34 @@ export default Ember.Controller.extend(ModalFunctionality, {
     }
   },
 
-  showGroupsClass: function() {
-    return this.get('isPrivateTopic') ? 'required' : 'optional';
-  }.property('isPrivateTopic'),
+  @computed('isPrivateTopic')
+  showGroupsClass(isPrivateTopic) {
+    return isPrivateTopic ? 'required' : 'optional';
+  },
 
   groupFinder(term) {
     return Group.findAll({search: term, ignore_automatic: true});
   },
 
-  successMessage: function() {
+  @computed('isMessage', 'emailOrUsername', 'invitingExistingUserToTopic')
+  successMessage(isMessage, emailOrUsername, invitingExistingUserToTopic) {
     if (this.get('hasGroups')) {
       return I18n.t('topic.invite_private.success_group');
-    } else if (this.get('isMessage')) {
+    } else if (isMessage) {
       return I18n.t('topic.invite_private.success');
-    } else if ( emailValid(this.get('emailOrUsername')) ) {
-      return I18n.t('topic.invite_reply.success_email', { emailOrUsername: this.get('emailOrUsername') });
+    } else if (invitingExistingUserToTopic) {
+      return I18n.t('topic.invite_reply.success_existing_email', { emailOrUsername });
+    } else if (emailValid(emailOrUsername)) {
+      return I18n.t('topic.invite_reply.success_email', { emailOrUsername });
     } else {
       return I18n.t('topic.invite_reply.success_username');
     }
-  }.property('model.inviteLink', 'isMessage', 'emailOrUsername'),
+  },
 
-  errorMessage: function() {
-    return this.get('isMessage') ? I18n.t('topic.invite_private.error') : I18n.t('topic.invite_reply.error');
-  }.property('isMessage'),
+  @computed('isMessage')
+  errorMessage(isMessage) {
+    return isMessage ? I18n.t('topic.invite_private.error') : I18n.t('topic.invite_reply.error');
+  },
 
   @computed('canInviteViaEmail')
   placeholderKey(canInviteViaEmail) {
@@ -172,15 +180,17 @@ export default Ember.Controller.extend(ModalFunctionality, {
             'topic.invite_reply.username_placeholder';
   },
 
-  customMessagePlaceholder: function() {
+  @computed
+  customMessagePlaceholder() {
     return I18n.t('invite.custom_message_placeholder');
-  }.property(),
+  },
 
   // Reset the modal to allow a new user to be invited.
   reset() {
     this.set('emailOrUsername', null);
     this.set('hasCustomMessage', false);
     this.set('customMessage', null);
+    this.set('invitingExistingUserToTopic', false);
     this.get('model').setProperties({
       groupNames: null,
       error: false,
@@ -189,6 +199,7 @@ export default Ember.Controller.extend(ModalFunctionality, {
       inviteLink: null
     });
   },
+
   actions: {
 
     createInvite() {
@@ -230,6 +241,8 @@ export default Ember.Controller.extend(ModalFunctionality, {
               } else if (this.get('isMessage') && result && result.user) {
                 this.get('model.details.allowed_users').pushObject(Ember.Object.create(result.user));
                 this.appEvents.trigger('post-stream:refresh');
+              } else if (this.get('invitingToTopic') && emailValid(this.get('emailOrUsername').trim()) && result && result.user) {
+                this.set('invitingExistingUserToTopic', true);
               }
             }).catch(onerror);
       }

app/models/topic.rb

@@ -744,9 +744,10 @@ SQL
 
   # Invite a user to the topic by username or email. Returns success/failure
   def invite(invited_by, username_or_email, group_ids=nil, custom_message=nil)
+    user = User.find_by_username_or_email(username_or_email)
+
     if private_message?
       # If the user exists, add them to the message.
-      user = User.find_by_username_or_email(username_or_email)
       raise UserExists.new I18n.t("topic_invite.user_exists") if user.present? && topic_allowed_users.where(user_id: user.id).exists?
 
       if user && topic_allowed_users.create!(user_id: user.id)
@@ -764,18 +765,27 @@ SQL
     end
 
     if username_or_email =~ /^.+@.+$/ && Guardian.new(invited_by).can_invite_via_email?(self)
-      # rate limit topic invite
       RateLimiter.new(invited_by, "topic-invitations-per-day", SiteSetting.max_topic_invitations_per_day, 1.day.to_i).performed!
 
-      # NOTE callers expect an invite object if an invite was sent via email
-      invite_by_email(invited_by, username_or_email, group_ids, custom_message)
+      if user.present?
+        # add existing users
+        Invite.extend_permissions(self, user, invited_by)
+
+        # Notify the user they've been invited
+        user.notifications.create(notification_type: Notification.types[:invited_to_topic],
+                                  topic_id: id,
+                                  post_number: 1,
+                                  data: { topic_title: title,
+                                          display_username: invited_by.username }.to_json)
+        return true
+      else
+        # NOTE callers expect an invite object if an invite was sent via email
+        invite_by_email(invited_by, username_or_email, group_ids, custom_message)
+      end
     else
-      # invite existing member to a topic
-      user = User.find_by_username(username_or_email)
       raise UserExists.new I18n.t("topic_invite.user_exists") if user.present? && topic_allowed_users.where(user_id: user.id).exists?
 
       if user && topic_allowed_users.create!(user_id: user.id)
-        # rate limit topic invite
         RateLimiter.new(invited_by, "topic-invitations-per-day", SiteSetting.max_topic_invitations_per_day, 1.day.to_i).performed!
 
         # Notify the user they've been invited

config/locales/client.en.yml

@@ -1724,6 +1724,7 @@ en:
         success_email: "We mailed out an invitation to <b>{{emailOrUsername}}</b>. We'll notify you when the invitation is redeemed. Check the invitations tab on your user page to keep track of your invites."
         success_username: "We've invited that user to participate in this topic."
         error: "Sorry, we couldn't invite that person. Perhaps they have already been invited? (Invites are rate limited)"
+        success_existing_email: "A user with email <b>{{emailOrUsername}}</b> already exists. We've invited that user to participate in this topic."
 
       login_reply: 'Log In to Reply'
 

spec/models/topic_spec.rb

@@ -1748,7 +1748,7 @@ describe Topic do
 
       it "should add user to the group" do
         expect(Guardian.new(walter).can_see?(group_private_topic)).to be_falsey
-        expect { group_private_topic.invite(group_manager, walter.email) }.to raise_error(Invite::UserExists)
+        group_private_topic.invite(group_manager, walter.email)
         expect(walter.groups).to include(group)
         expect(Guardian.new(walter).can_see?(group_private_topic)).to be_truthy
       end