move signup from users to account_controller

There was a lot of special case handling going on in the users_controller
for this. Lot simpler this way.

11 files changed, 58 insertions, 47 deletions

diff --git a/app/controllers/account_controller.rb b/app/controllers/account_controller.rb
new file mode 100644
index 0000000..ee7cca4
--- /dev/null
+++ b/app/controllers/account_controller.rb
@@ -0,0 +1,17 @@
+class AccountController < ApplicationController
+
+  before_filter :require_registration_allowed
+  before_filter :redirect_if_logged_in
+
+  def new
+    @user = User.new
+  end
+
+  protected
+
+  def require_registration_allowed
+    unless APP_CONFIG[:allow_registration]
+      redirect_to home_path
+    end
+  end
+end
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 2816a64..4d198b9 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -5,11 +5,9 @@
 class UsersController < ApplicationController
   include ControllerExtension::FetchUser
 
-  before_filter :require_login, :except => [:new]
-  before_filter :redirect_if_logged_in, :only => [:new]
+  before_filter :require_login
   before_filter :require_admin, :only => [:index, :deactivate, :enable]
   before_filter :fetch_user, :only => [:show, :edit, :destroy, :deactivate, :enable]
-  before_filter :require_registration_allowed, only: :new
 
   respond_to :html
 
@@ -27,10 +25,6 @@ class UsersController < ApplicationController
     @users = @users.limit(100)
   end
 
-  def new
-    @user = User.new
-  end
-
   def show
   end
 
@@ -64,12 +58,6 @@ class UsersController < ApplicationController
 
   protected
 
-  def require_registration_allowed
-    unless APP_CONFIG[:allow_registration]
-      redirect_to home_path
-    end
-  end
-
   def user_params
     if admin?
       params.require(:user).permit(:effective_service_level)
diff --git a/app/views/users/new.html.haml b/app/views/account/new.html.haml
index 1b257d9..d40259e 100644
--- a/app/views/users/new.html.haml
+++ b/app/views/account/new.html.haml
@@ -1,8 +1,8 @@
 -#
 -# This form is handled entirely by javascript
 -# Please take care when changing element ids.
--# 
--# The form is hidden when no js is available 
+-#
+-# The form is hidden when no js is available
 -# to prevent submission in the clear.
 -#
 
@@ -12,7 +12,7 @@
 .col-md-9
   %h2=t :signup
   .lead=t :signup_info
-  = render :partial => 'warnings'
+  = render "sessions/warnings"
   = simple_form_for(@user, form_options) do |f|
     = f.input :login, :label => t(:username), :required => false, :input_html => { :id => :srp_username }
     = f.input :password, :label => t(:password), :required => false, :validate => true, :input_html => { :id => :srp_password }
diff --git a/app/views/users/_warnings.html.haml b/app/views/sessions/_warnings.html.haml
index baf80a4..baf80a4 100644
--- a/app/views/users/_warnings.html.haml
+++ b/app/views/sessions/_warnings.html.haml
diff --git a/app/views/sessions/new.html.haml b/app/views/sessions/new.html.haml
index 942c485..6695123 100644
--- a/app/views/sessions/new.html.haml
+++ b/app/views/sessions/new.html.haml
@@ -2,7 +2,7 @@
 .col-md-9
   %h2=t :login
   .lead=t :login_info
-  = render :partial => 'users/warnings'
+  = render 'warnings'
   = simple_form_for [:api, @session], validate: true, html: { id: :new_session, class: 'form-horizontal hidden js-show', style: "display:none;" } do |f|
     = f.input :login, :required => false, :label => t(:username), :input_html => { :id => :srp_username }
     = f.input :password, :required => false, :input_html => { :id => :srp_password }
diff --git a/config/routes.rb b/config/routes.rb
index 7fbedf2..b152c9c 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -44,8 +44,8 @@ LeapWeb::Application.routes.draw do
     get "login" => "sessions#new", :as => "login"
     delete "logout" => "sessions#destroy", :as => "logout"
 
-    get "signup" => "users#new", :as => "signup"
-    resources :users, :except => [:create, :update] do
+    get "signup" => "account#new", :as => "signup"
+    resources :users, :except => [:new, :create, :update] do
       # resource :email_settings, :only => [:edit, :update]
       # resources :email_aliases, :only => [:destroy], :id => /.*/
       post 'deactivate', on: :member
diff --git a/test/functional/account_controller_test.rb b/test/functional/account_controller_test.rb
new file mode 100644
index 0000000..f5f1446
--- /dev/null
+++ b/test/functional/account_controller_test.rb
@@ -0,0 +1,26 @@
+require 'test_helper'
+
+class AccountControllerTest < ActionController::TestCase
+
+  test "should get new" do
+    get :new
+    assert_equal User, assigns(:user).class
+    assert_response :success
+  end
+
+  test "new should redirect logged in users" do
+    login
+    get :new
+    assert_response :redirect
+    assert_redirected_to home_path
+  end
+
+  test "new redirects if registration is closed" do
+    with_config(allow_registration: false) do
+      get :new
+      assert_response :redirect
+      assert_redirected_to home_path
+    end
+  end
+end
+
diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb
index 6029c83..2794422 100644
--- a/test/functional/users_controller_test.rb
+++ b/test/functional/users_controller_test.rb
@@ -1,20 +1,7 @@
-require_relative '../test_helper'
+require 'test_helper'
 
 class UsersControllerTest < ActionController::TestCase
 
-  test "should get new" do
-    get :new
-    assert_equal User, assigns(:user).class
-    assert_response :success
-  end
-
-  test "new should redirect logged in users" do
-    login
-    get :new
-    assert_response :redirect
-    assert_redirected_to home_path
-  end
-
   test "failed show without login" do
     user = find_record :user
     get :show, :id => user.id
@@ -163,11 +150,4 @@ class UsersControllerTest < ActionController::TestCase
     assert !assigns(:user).enabled?
   end
 
-  test "new redirects if registration is closed" do
-    with_config(allow_registration: false) do
-      get :new
-      assert_response :redirect
-      assert_redirected_to home_path
-    end
-  end
 end
diff --git a/test/integration/browser/account_livecycle_test.rb b/test/integration/browser/account_livecycle_test.rb
index 604f456..85dbf13 100644
--- a/test/integration/browser/account_livecycle_test.rb
+++ b/test/integration/browser/account_livecycle_test.rb
@@ -22,7 +22,7 @@ class AccountLivecycleTest < BrowserIntegrationTest
       username ||= "test_#{SecureRandom.urlsafe_base64}".downcase
       password ||= SecureRandom.base64
 
-      visit '/users/new'
+      visit '/signup'
       fill_in 'Username', with: username
       fill_in 'Password', with: password
       fill_in 'Password confirmation', with: password
diff --git a/test/integration/browser/password_validation_test.rb b/test/integration/browser/password_validation_test.rb
index 45eb0bf..51fcc5d 100644
--- a/test/integration/browser/password_validation_test.rb
+++ b/test/integration/browser/password_validation_test.rb
@@ -5,26 +5,26 @@ class PasswordValidationTest < BrowserIntegrationTest
   test "password confirmation is validated" do
     username ||= "test_#{SecureRandom.urlsafe_base64}".downcase
     password ||= SecureRandom.base64
-    visit '/users/new'
+    visit '/signup'
     fill_in 'Username', with: username
     fill_in 'Password', with: password
     fill_in 'Password confirmation', with: password + "-typo"
     click_on 'Sign Up'
     assert page.has_content? "does not match."
-    assert_equal '/users/new', current_path
+    assert_equal '/signup', current_path
     assert page.has_selector? ".error #srp_password_confirmation"
   end
 
   test "password needs to be at least 8 chars long" do
     username ||= "test_#{SecureRandom.urlsafe_base64}".downcase
     password ||= SecureRandom.base64[0,7]
-    visit '/users/new'
+    visit '/signup'
     fill_in 'Username', with: username
     fill_in 'Password', with: password
     fill_in 'Password confirmation', with: password
     click_on 'Sign Up'
     assert page.has_content? "needs to be at least 8 characters long"
-    assert_equal '/users/new', current_path
+    assert_equal '/signup', current_path
     assert page.has_selector? ".error #srp_password"
   end
 end
diff --git a/test/support/browser_integration_test.rb b/test/support/browser_integration_test.rb
index 84440a1..70161f9 100644
--- a/test/support/browser_integration_test.rb
+++ b/test/support/browser_integration_test.rb
@@ -52,7 +52,7 @@ class BrowserIntegrationTest < ActionDispatch::IntegrationTest
 
       username ||= "test_#{SecureRandom.urlsafe_base64}".downcase
       password ||= SecureRandom.base64
-      visit '/users/new'
+      visit '/signup'
       fill_in 'Username', with: username
       fill_in 'Password', with: password
       fill_in 'Invite code', with: @testcode.invite_code
@@ -65,7 +65,7 @@ class BrowserIntegrationTest < ActionDispatch::IntegrationTest
 
       username ||= "test_#{SecureRandom.urlsafe_base64}".downcase
       password ||= SecureRandom.base64
-      visit '/users/new'
+      visit '/signup'
       fill_in 'Username', with: username
       fill_in 'Password', with: password
       fill_in 'Password confirmation', with: password