From f47fc9d6522886cf81cfea26ec1f396219c539ba Mon Sep 17 00:00:00 2001
From: Azul <azul@riseup.net>
Date: Mon, 23 May 2016 12:17:31 +0200
Subject: 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.
---
 app/controllers/account_controller.rb              | 17 ++++++++++++++
 app/controllers/users_controller.rb                | 14 +----------
 app/views/account/new.html.haml                    | 27 ++++++++++++++++++++++
 app/views/sessions/_warnings.html.haml             | 12 ++++++++++
 app/views/sessions/new.html.haml                   |  2 +-
 app/views/users/_warnings.html.haml                | 12 ----------
 app/views/users/new.html.haml                      | 27 ----------------------
 config/routes.rb                                   |  4 ++--
 test/functional/account_controller_test.rb         | 26 +++++++++++++++++++++
 test/functional/users_controller_test.rb           | 22 +-----------------
 test/integration/browser/account_livecycle_test.rb |  2 +-
 .../browser/password_validation_test.rb            |  8 +++----
 test/support/browser_integration_test.rb           |  4 ++--
 13 files changed, 94 insertions(+), 83 deletions(-)
 create mode 100644 app/controllers/account_controller.rb
 create mode 100644 app/views/account/new.html.haml
 create mode 100644 app/views/sessions/_warnings.html.haml
 delete mode 100644 app/views/users/_warnings.html.haml
 delete mode 100644 app/views/users/new.html.haml
 create mode 100644 test/functional/account_controller_test.rb

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/account/new.html.haml b/app/views/account/new.html.haml
new file mode 100644
index 0000000..d40259e
--- /dev/null
+++ b/app/views/account/new.html.haml
@@ -0,0 +1,27 @@
+-#
+-# This form is handled entirely by javascript
+-# Please take care when changing element ids.
+-#
+-# The form is hidden when no js is available
+-# to prevent submission in the clear.
+-#
+
+- form_options = {url: '/not-used', html: {id: 'new_user', class: user_form_class('form-horizontal'), style: 'display:none'}, validate: true}
+
+.col-md-1
+.col-md-9
+  %h2=t :signup
+  .lead=t :signup_info
+  = 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 }
+    = f.input :password_confirmation, :label => t(:password_confirmation), :required => false, :validate => true, :input_html => { :id => :srp_password_confirmation }
+
+    - if APP_CONFIG[:invite_required]
+      = f.input :invite_code, :label => t(:invite_code), :input_html => { :id => :srp_invite_code }
+    - else
+      = f.input :invite_code, :as => "hidden", :input_html => { :value => " ", :id => :srp_invite_code }
+
+    = f.button :wrapped, cancel: home_path
+-#
diff --git a/app/views/sessions/_warnings.html.haml b/app/views/sessions/_warnings.html.haml
new file mode 100644
index 0000000..baf80a4
--- /dev/null
+++ b/app/views/sessions/_warnings.html.haml
@@ -0,0 +1,12 @@
+%noscript
+  %div.alert.alert-error=t :js_required_html
+#cookie_warning.alert.alert-error{:style => "display:none"}
+  =t :cookie_disabled_warning
+:javascript
+  document.cookie = "testing=cookies_enabled; path=/";
+  if(document.cookie.indexOf("testing=cookies_enabled") < 0)
+  {
+    document.getElementById('cookie_warning').style.display = 'block';
+  } else {
+    document.getElementById('cookie_warning').style.display = 'none';
+  }
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/app/views/users/_warnings.html.haml b/app/views/users/_warnings.html.haml
deleted file mode 100644
index baf80a4..0000000
--- a/app/views/users/_warnings.html.haml
+++ /dev/null
@@ -1,12 +0,0 @@
-%noscript
-  %div.alert.alert-error=t :js_required_html
-#cookie_warning.alert.alert-error{:style => "display:none"}
-  =t :cookie_disabled_warning
-:javascript
-  document.cookie = "testing=cookies_enabled; path=/";
-  if(document.cookie.indexOf("testing=cookies_enabled") < 0)
-  {
-    document.getElementById('cookie_warning').style.display = 'block';
-  } else {
-    document.getElementById('cookie_warning').style.display = 'none';
-  }
diff --git a/app/views/users/new.html.haml b/app/views/users/new.html.haml
deleted file mode 100644
index 1b257d9..0000000
--- a/app/views/users/new.html.haml
+++ /dev/null
@@ -1,27 +0,0 @@
--#
--# This form is handled entirely by javascript
--# Please take care when changing element ids.
--# 
--# The form is hidden when no js is available 
--# to prevent submission in the clear.
--#
-
-- form_options = {url: '/not-used', html: {id: 'new_user', class: user_form_class('form-horizontal'), style: 'display:none'}, validate: true}
-
-.col-md-1
-.col-md-9
-  %h2=t :signup
-  .lead=t :signup_info
-  = render :partial => '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 }
-    = f.input :password_confirmation, :label => t(:password_confirmation), :required => false, :validate => true, :input_html => { :id => :srp_password_confirmation }
-
-    - if APP_CONFIG[:invite_required]
-      = f.input :invite_code, :label => t(:invite_code), :input_html => { :id => :srp_invite_code }
-    - else
-      = f.input :invite_code, :as => "hidden", :input_html => { :value => " ", :id => :srp_invite_code }
-
-    = f.button :wrapped, cancel: home_path
--#
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
-- 
cgit v1.2.3