FEATURE: Activate users invited via email when invite is redeemed

Do not send an activation email to users invited via email. They
already confirmed their email address by clicking the invite link.
Users invited via link will need to confirm their email address before
they can login.

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

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
 #

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

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

spec/fabricators/invite_fabricator.rb

@@ -1,4 +1,5 @@
 Fabricator(:invite) do
   invited_by(fabricator: :user)
   email 'iceking@ADVENTURETIME.ooo'
+  via_email true
 end

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)

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

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