diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 87d66fd66d9..ed4844c3573 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -211,12 +211,21 @@ class InvitesController < ApplicationController def post_process_invite(user) user.enqueue_welcome_message('welcome_invite') if user.send_welcome_message + if user.has_password? - email_token = user.email_tokens.create(email: user.email) - Jobs.enqueue(:critical_user_email, type: :signup, user_id: user.id, email_token: email_token.token) + send_activation_email(user) unless user.active elsif !SiteSetting.enable_sso && SiteSetting.enable_local_logins Jobs.enqueue(:invite_password_instructions_email, username: user.username) end end + def send_activation_email(user) + email_token = user.email_tokens.create(email: user.email) + + Jobs.enqueue(:critical_user_email, + type: :signup, + user_id: user.id, + email_token: email_token.token + ) + end end diff --git a/app/models/invite.rb b/app/models/invite.rb index cc44d69151a..2a7f96876aa 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -107,10 +107,19 @@ class Invite < ActiveRecord::Base invite = nil end - invite.update_columns(created_at: Time.zone.now, updated_at: Time.zone.now) if invite + if invite + invite.update_columns( + created_at: Time.zone.now, + updated_at: Time.zone.now, + via_email: invite.via_email && send_email + ) + else + create_args = { + invited_by: invited_by, + email: lower_email, + via_email: send_email + } - if !invite - create_args = { invited_by: invited_by, email: lower_email } create_args[:moderator] = true if opts[:moderator] create_args[:custom_message] = custom_message if custom_message invite = Invite.create!(create_args) @@ -257,6 +266,7 @@ end # invalidated_at :datetime # moderator :boolean default(FALSE), not null # custom_message :text +# via_email :boolean default(FALSE), not null # # Indexes # diff --git a/app/models/invite_redeemer.rb b/app/models/invite_redeemer.rb index 0c1702c3641..ffdd84cc837 100644 --- a/app/models/invite_redeemer.rb +++ b/app/models/invite_redeemer.rb @@ -24,7 +24,7 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f email: invite.email, username: available_username, name: available_name, - active: true, + active: false, trust_level: SiteSetting.default_invitee_trust_level } @@ -57,6 +57,12 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f end user.save! + + if invite.via_email + user.email_tokens.create(email: user.email) + user.activate + end + User.find(user.id) end @@ -82,8 +88,9 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f end def mark_invite_redeemed - Invite.where(['id = ? AND redeemed_at IS NULL AND created_at >= ?', - invite.id, SiteSetting.invite_expiry_days.days.ago]).update_all('redeemed_at = CURRENT_TIMESTAMP') + Invite.where('id = ? AND redeemed_at IS NULL AND created_at >= ?', + invite.id, SiteSetting.invite_expiry_days.days.ago) + .update_all('redeemed_at = CURRENT_TIMESTAMP') end def get_invited_user @@ -119,7 +126,7 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f end def send_welcome_message - if Invite.where(['email = ?', invite.email]).update_all(['user_id = ?', invited_user.id]) == 1 + if Invite.where('email = ?', invite.email).update_all(['user_id = ?', invited_user.id]) == 1 invited_user.send_welcome_message = true end end diff --git a/db/migrate/20181005144357_add_via_email_to_invites.rb b/db/migrate/20181005144357_add_via_email_to_invites.rb new file mode 100644 index 00000000000..144987cff1d --- /dev/null +++ b/db/migrate/20181005144357_add_via_email_to_invites.rb @@ -0,0 +1,5 @@ +class AddViaEmailToInvites < ActiveRecord::Migration[5.2] + def change + add_column :invites, :via_email, :boolean, default: false, null: false + end +end diff --git a/spec/fabricators/invite_fabricator.rb b/spec/fabricators/invite_fabricator.rb index ecb9133f45c..d092cf73703 100644 --- a/spec/fabricators/invite_fabricator.rb +++ b/spec/fabricators/invite_fabricator.rb @@ -1,4 +1,5 @@ Fabricator(:invite) do invited_by(fabricator: :user) email 'iceking@ADVENTURETIME.ooo' + via_email true end diff --git a/spec/models/invite_redeemer_spec.rb b/spec/models/invite_redeemer_spec.rb index ac31409b522..870d2b13a20 100644 --- a/spec/models/invite_redeemer_spec.rb +++ b/spec/models/invite_redeemer_spec.rb @@ -38,7 +38,7 @@ describe InviteRedeemer do expect(user.id).to eq(staged_user.id) expect(user.username).to eq('walter') expect(user.name).to eq('Walter White') - expect(user.active).to eq(false) + expect(user.staged).to eq(false) expect(user.email).to eq('staged@account.com') expect(user.approved).to eq(true) end @@ -99,7 +99,6 @@ describe InviteRedeemer do end it "can set password" do - inviter = invite.invited_by user = InviteRedeemer.new(invite, username, name, password).redeem expect(user).to have_password expect(user.confirm_password?(password)).to eq(true) diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index 33a49cb1f4e..6ed73ee5383 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -142,6 +142,27 @@ describe Invite do expect(invite.topics).to match_array([topic, another_topic]) end end + + it 'correctly marks invite as sent via email' do + expect(invite.via_email).to eq(true) + + Invite.invite_by_email(iceking, inviter, topic) + expect(invite.reload.via_email).to eq(true) + end + + it 'does not mark invite as sent via email after generating invite link' do + expect(invite.via_email).to eq(true) + + Invite.generate_invite_link(iceking, inviter, topic) + expect(invite.reload.via_email).to eq(false) + + Invite.invite_by_email(iceking, inviter, topic) + expect(invite.reload.via_email).to eq(false) + + Invite.generate_invite_link(iceking, inviter, topic) + expect(invite.reload.via_email).to eq(false) + end + end end end diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb index 9663b889e9d..37b2c7ec900 100644 --- a/spec/requests/invites_controller_spec.rb +++ b/spec/requests/invites_controller_spec.rb @@ -296,33 +296,68 @@ describe InvitesController do expect(Jobs::SendSystemMessage.jobs.size).to eq(1) end - it "sends password reset email if password is not set" do - put "/invites/show/#{invite.invite_key}.json" - expect(response.status).to eq(200) - expect(Jobs::InvitePasswordInstructionsEmail.jobs.size).to eq(1) + context "without password" do + it "sends password reset email" do + put "/invites/show/#{invite.invite_key}.json" + expect(response.status).to eq(200) + expect(Jobs::InvitePasswordInstructionsEmail.jobs.size).to eq(1) + expect(Jobs::CriticalUserEmail.jobs.size).to eq(0) + end + + it "does not send password reset email if sso is enabled" do + SiteSetting.sso_url = "https://www.example.com/sso" + SiteSetting.enable_sso = true + put "/invites/show/#{invite.invite_key}.json" + expect(response.status).to eq(200) + expect(Jobs::InvitePasswordInstructionsEmail.jobs.size).to eq(0) + expect(Jobs::CriticalUserEmail.jobs.size).to eq(0) + end + + it "does not send password reset email if local login is disabled" do + SiteSetting.enable_local_logins = false + put "/invites/show/#{invite.invite_key}.json" + expect(response.status).to eq(200) + expect(Jobs::InvitePasswordInstructionsEmail.jobs.size).to eq(0) + expect(Jobs::CriticalUserEmail.jobs.size).to eq(0) + end end - it "does not send password reset email if sso is enabled" do - SiteSetting.sso_url = "https://www.example.com/sso" - SiteSetting.enable_sso = true - put "/invites/show/#{invite.invite_key}.json" - expect(response.status).to eq(200) - expect(Jobs::InvitePasswordInstructionsEmail.jobs.size).to eq(0) + context "with password" do + context "user was invited via email" do + before { invite.update_column(:via_email, true) } + + it "doesn't send an activation email and activates the user" do + put "/invites/show/#{invite.invite_key}.json", params: { password: "verystrongpassword" } + expect(response.status).to eq(200) + + expect(Jobs::InvitePasswordInstructionsEmail.jobs.size).to eq(0) + expect(Jobs::CriticalUserEmail.jobs.size).to eq(0) + + invited_user = User.find_by_email(invite.email) + expect(invited_user.active).to eq(true) + expect(invited_user.email_confirmed?).to eq(true) + end + end + + context "user was invited via link" do + before { invite.update_column(:via_email, false) } + + it "sends an activation email and doesn't activate the user" do + put "/invites/show/#{invite.invite_key}.json", params: { password: "verystrongpassword" } + expect(response.status).to eq(200) + + expect(Jobs::InvitePasswordInstructionsEmail.jobs.size).to eq(0) + expect(Jobs::CriticalUserEmail.jobs.size).to eq(1) + expect(Jobs::CriticalUserEmail.jobs.first["args"].first["type"]).to eq("signup") + + invited_user = User.find_by_email(invite.email) + expect(invited_user.active).to eq(false) + expect(invited_user.email_confirmed?).to eq(false) + end + end + end - it "does not send password reset email if local login is disabled" do - SiteSetting.enable_local_logins = false - put "/invites/show/#{invite.invite_key}.json" - expect(response.status).to eq(200) - expect(Jobs::InvitePasswordInstructionsEmail.jobs.size).to eq(0) - end - - it 'sends an activation email if password is set' do - put "/invites/show/#{invite.invite_key}.json", params: { password: "verystrongpassword" } - expect(response.status).to eq(200) - expect(Jobs::InvitePasswordInstructionsEmail.jobs.size).to eq(0) - expect(Jobs::CriticalUserEmail.jobs.size).to eq(1) - end end end end