FIX: approve invited user

This commit fixes the case where invited users who typed in a password
would not be approved by default. Because we moved the user create logic
for an invited user there was a clash with the `save` in the user model
and the `save` in the invite_redeemer class.

- added approve logic into invite_redeemer class.
- added tests to verify that the user is approved
- added a check to see if must_approve_users is on
- added a check to see if the inviter is staff
- go ahead and approve the user if must_approve_users is off
- keep existing User.approve workflow if user exists
- improve if/else logic to remove duplicate code
- use `Time.zone.now`

app/models/invite_redeemer.rb

@@ -36,6 +36,12 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password) do
       user.password = password
     end
 
+    if !SiteSetting.must_approve_users? || (SiteSetting.must_approve_users? && invite.invited_by.staff?)
+      user.approved = true
+      user.approved_by_id = invite.invited_by_id
+      user.approved_at = Time.zone.now
+    end
+
     user.moderator = true if invite.moderator? && invite.invited_by.staff?
     user.save!
 
@@ -49,11 +55,11 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password) do
   end
 
   def process_invitation
+    approve_account_if_needed
     add_to_private_topics_if_invited
     add_user_to_invited_topics
     add_user_to_groups
     send_welcome_message
-    approve_account_if_needed
     notify_invitee
     send_password_instructions
     delete_duplicate_invites
@@ -109,7 +115,9 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password) do
   end
 
   def approve_account_if_needed
-    invited_user.approve(invite.invited_by_id, false)
+    if get_existing_user
+      invited_user.approve(invite.invited_by_id, false)
+    end
   end
 
   def send_password_instructions

spec/models/invite_redeemer_spec.rb

@@ -9,6 +9,7 @@ describe InviteRedeemer do
       expect(user.name).to eq('Walter White')
       expect(user).to be_active
       expect(user.email).to eq('walter.white@email.com')
+      expect(user.approved).to eq(true)
     end
 
     it "can set the password too" do
@@ -16,6 +17,7 @@ describe InviteRedeemer do
       user = InviteRedeemer.create_user_from_invite(Fabricate(:invite, email: 'walter.white@email.com'), 'walter', 'Walter White', password)
       expect(user).to have_password
       expect(user.confirm_password?(password)).to eq(true)
+      expect(user.approved).to eq(true)
     end
 
     it "raises exception with record and errors" do
@@ -37,7 +39,21 @@ describe InviteRedeemer do
     let(:password) { 'know5nOthiNG'}
     let(:invite_redeemer) { InviteRedeemer.new(invite, username, name) }
 
-    it "should redeem the invite" do
+    it "should redeem the invite if invited by staff" do
+      SiteSetting.must_approve_users = true
+      inviter = invite.invited_by
+      inviter.admin = true
+      user = invite_redeemer.redeem
+
+      expect(user.name).to eq(name)
+      expect(user.username).to eq(username)
+      expect(user.invited_by).to eq(inviter)
+      expect(inviter.notifications.count).to eq(1)
+      expect(user.approved).to eq(true)
+    end
+
+    it "should redeem the invite if invited by non staff but not approve" do
+      SiteSetting.must_approve_users = true
       inviter = invite.invited_by
       user = invite_redeemer.redeem
 
@@ -45,6 +61,18 @@ describe InviteRedeemer do
       expect(user.username).to eq(username)
       expect(user.invited_by).to eq(inviter)
       expect(inviter.notifications.count).to eq(1)
+      expect(user.approved).to eq(false)
+    end
+
+    it "should redeem the invite if invited by non staff and approve if staff not required to approve" do
+      inviter = invite.invited_by
+      user = invite_redeemer.redeem
+
+      expect(user.name).to eq(name)
+      expect(user.username).to eq(username)
+      expect(user.invited_by).to eq(inviter)
+      expect(inviter.notifications.count).to eq(1)
+      expect(user.approved).to eq(true)
     end
 
     it "should not blow up if invited_by user has been removed" do
@@ -63,6 +91,7 @@ describe InviteRedeemer do
       user = InviteRedeemer.new(invite, username, name, password).redeem
       expect(user).to have_password
       expect(user.confirm_password?(password)).to eq(true)
+      expect(user.approved).to eq(true)
     end
   end
 end

spec/models/invite_spec.rb

@@ -273,6 +273,7 @@ describe Invite do
 
     context 'inviting when must_approve_users? is enabled' do
       it 'correctly activates accounts' do
+        invite.invited_by = Fabricate(:admin)
         SiteSetting.stubs(:must_approve_users).returns(true)
         user = invite.redeem
         expect(user.approved?).to eq(true)