From 41b06925434de39db81d8cbc9ffff3b57a34b22f Mon Sep 17 00:00:00 2001
From: Chris Hunt <c@chrishunt.co>
Date: Wed, 5 Jun 2013 18:19:27 -0700
Subject: [PATCH] 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       | 32 ++++-------
 spec/controllers/users_controller_spec.rb | 70 +++++++++++++----------
 2 files changed, 53 insertions(+), 49 deletions(-)

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 88ed557d8..f9684a99b 100644
--- a/app/controllers/users_controller.rb
+++ b/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,
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index b8980c357..85c67c458 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/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