Show 'waiting approval' and don't send email

When 'must approve users' in enabled, we don't want to send an
activation email to users after they sign up. Instead, we will show them
'waiting approval' and not take an action until their account is
approved by an admin.

app/controllers/users_controller.rb

@@ -161,35 +161,27 @@ class UsersController < ApplicationController
     end
 
     if user.save
-      msg = nil
-      active_user = user.active?
-
-      if active_user
-        # If the user is active (remote authorized email)
-        if SiteSetting.must_approve_users?
-          msg = I18n.t("login.wait_approval")
-          active_user = false
-        else
-          log_on_user(user)
-          user.enqueue_welcome_message('welcome_user')
-          msg = I18n.t("login.active")
-        end
-      else
-        msg = I18n.t("login.activate_email", email: user.email)
-        Jobs.enqueue(
-          :user_email, type: :signup, user_id: user.id,
+      if SiteSetting.must_approve_users?
+        message = I18n.t("login.wait_approval")
+      elsif !user.active?
+        message = I18n.t("login.activate_email", email: user.email)
+        Jobs.enqueue(:user_email,
+          type: :signup,
+          user_id: user.id,
           email_token: user.email_tokens.first.token
         )
+      else
+        message = I18n.t("login.active")
+        log_on_user(user)
+        user.enqueue_welcome_message('welcome_user')
       end
 
-      # Create 3rd party auth records (Twitter, Facebook, GitHub)
       create_third_party_auth_records(user, auth) if auth.present?
 
       # Clear authentication session.
       session[:authentication] = nil
 
-      # JSON result
-      render json: { success: true, active: active_user, message: msg }
+      render json: { success: true, active: user.active?, message: message }
     else
       render json: {
         success: false,

spec/controllers/users_controller_spec.rb

@@ -287,62 +287,74 @@ describe UsersController do
     end
 
     context 'when creating a non active user (unconfirmed email)' do
-      it 'should enqueue a signup email' do
+      it 'enqueues a signup email' do
         Jobs.expects(:enqueue).with(:user_email, has_entries(type: :signup))
         post_user
       end
 
-      it "doesn't send a welcome email" do
+      it 'does not enqueue a welcome email' do
         User.any_instance.expects(:enqueue_welcome_message).with('welcome_user').never
         post_user
       end
 
+      it 'indicates the user is not active in the response' do
+        post_user
+        expect(JSON.parse(response.body)['active']).to be_false
+      end
+
+      context "and 'must approve users' site setting is enabled" do
+        before { SiteSetting.expects(:must_approve_users).returns(true) }
+
+        it 'does not enqueue an email' do
+          Jobs.expects(:enqueue).never
+          post_user
+        end
+
+        it 'does not login the user' do
+          post_user
+          expect(session[:current_user_id]).to be_blank
+        end
+
+        it 'indicates the user is not active in the response' do
+          post_user
+          expect(JSON.parse(response.body)['active']).to be_false
+        end
+
+        it "shows the 'waiting approval' message" do
+          post_user
+          expect(JSON.parse(response.body)['message']).to eq(
+            I18n.t 'login.wait_approval'
+          )
+        end
       end
     end
 
     context 'when creating an active user (confirmed email)' do
-
-      before do
-        User.any_instance.stubs(:active?).returns(true)
-      end
+      before { User.any_instance.stubs(:active?).returns(true) }
 
       it 'enqueues a welcome email' do
         User.any_instance.expects(:enqueue_welcome_message).with('welcome_user')
-
         post_user
       end
+
+      it "shows the 'active' message" do
+        User.any_instance.expects(:enqueue_welcome_message)
+        post_user
+        expect(JSON.parse(response.body)['message']).to eq(
+          I18n.t 'login.active'
+        )
       end
 
       it "should be logged in" do
         User.any_instance.expects(:enqueue_welcome_message)
-
         post_user
-
         session[:current_user_id].should be_present
       end
 
-      it "returns true in the active part of the JSON" do
+      it 'indicates the user is active in the response' do
         User.any_instance.expects(:enqueue_welcome_message)
-
         post_user
-
-        ::JSON.parse(response.body)['active'].should == true
-      end
-
-
-      context 'when approving of users is required' do
-        before do
-          SiteSetting.expects(:must_approve_users).returns(true)
-          post_user
-        end
-
-        it "doesn't log in the user" do
-          session[:current_user_id].should be_blank
-        end
-
-        it "doesn't return active in the JSON" do
-          ::JSON.parse(response.body)['active'].should == false
-        end
+        expect(JSON.parse(response.body)['active']).to be_true
       end
 
       context 'authentication records for' do