FIX: Apply 'hide email account' for invites

app/assets/javascripts/discourse/app/controllers/create-invite.js

@@ -123,6 +123,10 @@ export default Controller.extend(
       return this.invite
         .save(data)
         .then(() => {
+          if (!this.invite.id) {
+            return;
+          }
+
           this.rollbackBuffer();
 
           if (

app/controllers/invites_controller.rb

@@ -143,29 +143,28 @@ class InvitesController < ApplicationController
       return render_json_error(I18n.t("invite.requires_groups", groups: editable_topic_groups.pluck(:name).join(", ")))
     end
 
-    begin
-      invite = Invite.generate(current_user,
-        email: params[:email],
-        domain: params[:domain],
-        skip_email: params[:skip_email],
-        invited_by: current_user,
-        custom_message: params[:custom_message],
-        max_redemptions_allowed: params[:max_redemptions_allowed],
-        topic_id: topic&.id,
-        group_ids: groups&.map(&:id),
-        expires_at: params[:expires_at],
-      )
+    invite = Invite.generate(current_user,
+      email: params[:email],
+      domain: params[:domain],
+      skip_email: params[:skip_email],
+      invited_by: current_user,
+      custom_message: params[:custom_message],
+      max_redemptions_allowed: params[:max_redemptions_allowed],
+      topic_id: topic&.id,
+      group_ids: groups&.map(&:id),
+      expires_at: params[:expires_at],
+    )
 
-      if invite.present?
-        render_serialized(invite, InviteSerializer, scope: guardian, root: nil, show_emails: params.has_key?(:email), show_warnings: true)
-      else
-        render json: failed_json, status: 422
-      end
-    rescue Invite::UserExists => e
-      render_json_error(e.message)
-    rescue ActiveRecord::RecordInvalid => e
-      render_json_error(e.record.errors.full_messages.first)
+    if invite.present?
+      render_serialized(invite, InviteSerializer, scope: guardian, root: nil, show_emails: params.has_key?(:email), show_warnings: true)
+    else
+      render json: failed_json, status: 422
     end
+  rescue Invite::UserExists => e
+    return render json: {}, status: 200 if SiteSetting.hide_email_address_taken?
+    render_json_error(e.message)
+  rescue ActiveRecord::RecordInvalid => e
+    render_json_error(e.record.errors.full_messages.first)
   end
 
   def retrieve
@@ -259,6 +258,7 @@ class InvitesController < ApplicationController
       begin
         invite.update!(params.permit(:email, :custom_message, :max_redemptions_allowed, :expires_at))
       rescue ActiveRecord::RecordInvalid => e
+        return render json: {}, status: 200 if SiteSetting.hide_email_address_taken? && e.record.email_already_exists?
         return render_json_error(e.record.errors.full_messages.first)
       end
     end

app/models/invite.rb

@@ -49,20 +49,20 @@ class Invite < ActiveRecord::Base
     self.email = Email.downcase(email) unless email.nil?
   end
 
-  attr_accessor :email_already_exists
+  attribute :email_already_exists
 
   def self.emailed_status_types
     @emailed_status_types ||= Enum.new(not_required: 0, pending: 1, bulk_pending: 2, sending: 3, sent: 4)
   end
 
   def user_doesnt_already_exist
-    @email_already_exists = false
+    self.email_already_exists = false
     return if email.blank?
     user = Invite.find_user_by_email(email)
 
     if user && user.id != self.invited_users&.first&.user_id
-      @email_already_exists = true
-      errors.add(:base, user_exists_error_msg(email, user.username))
+      self.email_already_exists = true
+      errors.add(:base, user_exists_error_msg(email))
     end
   end
 
@@ -100,9 +100,7 @@ class Invite < ActiveRecord::Base
 
     email = Email.downcase(opts[:email]) if opts[:email].present?
 
-    if user = find_user_by_email(email)
-      raise UserExists.new(new.user_exists_error_msg(email, user.username))
-    end
+    raise UserExists.new(new.user_exists_error_msg(email)) if find_user_by_email(email)
 
     if email.present?
       invite = Invite
@@ -270,14 +268,8 @@ class Invite < ActiveRecord::Base
     end
   end
 
-  def user_exists_error_msg(email, username)
-    sanitized_email = CGI.escapeHTML(email)
-    sanitized_username = CGI.escapeHTML(username)
-
-    I18n.t(
-      "invite.user_exists",
-      email: sanitized_email, username: sanitized_username, base_path: Discourse.base_path
-    )
+  def user_exists_error_msg(email)
+    I18n.t("invite.user_exists", email: CGI.escapeHTML(email))
   end
 end
 

config/locales/server.en.yml

@@ -249,7 +249,7 @@ en:
       <p>Otherwise please <a href="%{base_url}/password-reset">Reset Password</a>.</p>
     not_found_template_link: |
       <p>This invitation to <a href="%{base_url}">%{site_name}</a> can no longer be redeemed. Please ask the person who invited you to send you a new invitation.</p>
-    user_exists: "There's no need to invite <b>%{email}</b>, they <a href='%{base_path}/u/%{username}/summary'>already have an account!</a>"
+    user_exists: "There's no need to invite <b>%{email}</b>, they already have an account!"
     invite_exists: "You already invited <b>%{email}</b>."
     invalid_email: "%{email} isn't a valid email address."
     rate_limit:

spec/models/invite_spec.rb

@@ -90,10 +90,7 @@ describe Invite do
       expect { Invite.generate(user, email: user.email) }
         .to raise_error(
           Invite::UserExists,
-          I18n.t(
-            'invite.user_exists',
-            email: escaped_email, username: user.username, base_path: Discourse.base_path
-          )
+          I18n.t('invite.user_exists', email: escaped_email)
         )
     end
 

spec/requests/invites_controller_spec.rb

@@ -296,34 +296,77 @@ describe InvitesController do
     end
 
     context 'email invite' do
-      it 'creates invite once and updates it on successive calls' do
+      subject(:create_invite) { post '/invites.json', params: params }
+
+      let(:params) { { email: email } }
+      let(:email) { 'test@example.com' }
+
+      before do
         sign_in(user)
-
-        post '/invites.json', params: { email: 'test@example.com' }
-        expect(response.status).to eq(200)
-        expect(Jobs::InviteEmail.jobs.size).to eq(1)
-
-        invite = Invite.last
-
-        post '/invites.json', params: { email: 'test@example.com' }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body['id']).to eq(invite.id)
       end
 
-      it 'accept skip_email parameter' do
-        sign_in(user)
+      context 'when doing successive calls' do
+        let(:invite) { Invite.last }
 
-        post '/invites.json', params: { email: 'test@example.com', skip_email: true }
-        expect(response.status).to eq(200)
-        expect(Jobs::InviteEmail.jobs.size).to eq(0)
+        it 'creates invite once and updates it after' do
+          create_invite
+          expect(response).to have_http_status :ok
+          expect(Jobs::InviteEmail.jobs.size).to eq(1)
+
+          create_invite
+          expect(response).to have_http_status :ok
+          expect(response.parsed_body['id']).to eq(invite.id)
+        end
       end
 
-      it 'fails in case of validation failure' do
-        sign_in(admin)
+      context 'when "skip_email" parameter is provided' do
+        before do
+          params[:skip_email] = true
+        end
 
-        post '/invites.json', params: { email: 'test@mailinator.com' }
-        expect(response.status).to eq(422)
-        expect(response.parsed_body['errors']).to be_present
+        it 'accepts the parameter' do
+          create_invite
+          expect(response).to have_http_status :ok
+          expect(Jobs::InviteEmail.jobs.size).to eq(0)
+        end
+      end
+
+      context 'when validations fail' do
+        let(:email) { 'test@mailinator.com' }
+
+        it 'fails' do
+          create_invite
+          expect(response).to have_http_status :unprocessable_entity
+          expect(response.parsed_body['errors']).to be_present
+        end
+      end
+
+      context 'when providing an email belonging to an existing user' do
+        let(:email) { user.email }
+
+        before do
+          SiteSetting.hide_email_address_taken = hide_email_address_taken
+        end
+
+        context 'when "hide_email_address_taken" setting is disabled' do
+          let(:hide_email_address_taken) { false }
+
+          it 'returns an error' do
+            create_invite
+            expect(response).to have_http_status :unprocessable_entity
+            expect(body).to match(/no need to invite/)
+          end
+        end
+
+        context 'when "hide_email_address_taken" setting is enabled' do
+          let(:hide_email_address_taken) { true }
+
+          it 'doesn’t inform the user' do
+            create_invite
+            expect(response).to have_http_status :ok
+            expect(response.parsed_body).to be_blank
+          end
+        end
       end
     end
 
@@ -441,6 +484,34 @@ describe InvitesController do
         put "/invites/#{invite.id}.json", params: { email: 'test2@example.com' }
         expect(response.status).to eq(409)
       end
+
+      context "when providing an email belonging to an existing user" do
+        subject(:update_invite) { put "/invites/#{invite.id}.json", params: { email: admin.email } }
+
+        before do
+          SiteSetting.hide_email_address_taken = hide_email_address_taken
+        end
+
+        context "when 'hide_email_address_taken' setting is disabled" do
+          let(:hide_email_address_taken) { false }
+
+          it "returns an error" do
+            update_invite
+            expect(response).to have_http_status :unprocessable_entity
+            expect(body).to match(/no need to invite/)
+          end
+        end
+
+        context "when 'hide_email_address_taken' setting is enabled" do
+          let(:hide_email_address_taken) { true }
+
+          it "doesn't inform the user" do
+            update_invite
+            expect(response).to have_http_status :ok
+            expect(response.parsed_body).to be_blank
+          end
+        end
+      end
     end
   end