diff --git a/app/assets/javascripts/discourse/app/controllers/create-invite.js b/app/assets/javascripts/discourse/app/controllers/create-invite.js index 20085801cd0..e043d28bb7e 100644 --- a/app/assets/javascripts/discourse/app/controllers/create-invite.js +++ b/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 ( diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 35aee33d470..8b642ff24ca 100644 --- a/app/controllers/invites_controller.rb +++ b/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 diff --git a/app/models/invite.rb b/app/models/invite.rb index 9b715a5d5a4..c7236e9caa6 100644 --- a/app/models/invite.rb +++ b/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 diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index eadca475e74..fabf94d299c 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -249,7 +249,7 @@ en:

Otherwise please Reset Password.

not_found_template_link: |

This invitation to %{site_name} can no longer be redeemed. Please ask the person who invited you to send you a new invitation.

- user_exists: "There's no need to invite %{email}, they already have an account!" + user_exists: "There's no need to invite %{email}, they already have an account!" invite_exists: "You already invited %{email}." invalid_email: "%{email} isn't a valid email address." rate_limit: diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index 41f38483123..5efa9c02503 100644 --- a/spec/models/invite_spec.rb +++ b/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 diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb index dd5e7d29eba..35e7b4a5ee0 100644 --- a/spec/requests/invites_controller_spec.rb +++ b/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