Refactor: split UsersController#add into #add and #create

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4215 e93f8b46-1217-0410-a6f0-8f06a7374b81

6 files changed, 64 insertions, 20 deletions

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index b854850a3..f22427a66 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -72,22 +72,26 @@ class UsersController < ApplicationController
   end
 
   def add
-    if request.get?
-      @user = User.new(:language => Setting.default_language)
+    @user = User.new(:language => Setting.default_language)
+    @auth_sources = AuthSource.find(:all)
+  end
+  
+  verify :method => :post, :only => :create, :render => {:nothing => true, :status => :method_not_allowed }
+  def create
+    @user = User.new(params[:user])
+    @user.admin = params[:user][:admin] || false
+    @user.login = params[:user][:login]
+    @user.password, @user.password_confirmation = params[:password], params[:password_confirmation] unless @user.auth_source_id
+    if @user.save
+      Mailer.deliver_account_information(@user, params[:password]) if params[:send_information]
+      flash[:notice] = l(:notice_successful_create)
+      redirect_to(params[:continue] ? {:controller => 'users', :action => 'add'} : 
+                                      {:controller => 'users', :action => 'edit', :id => @user})
+      return
     else
-      @user = User.new(params[:user])
-      @user.admin = params[:user][:admin] || false
-      @user.login = params[:user][:login]
-      @user.password, @user.password_confirmation = params[:password], params[:password_confirmation] unless @user.auth_source_id
-      if @user.save
-        Mailer.deliver_account_information(@user, params[:password]) if params[:send_information]
-        flash[:notice] = l(:notice_successful_create)
-        redirect_to(params[:continue] ? {:controller => 'users', :action => 'add'} : 
-                                        {:controller => 'users', :action => 'edit', :id => @user})
-        return
-      end
+      @auth_sources = AuthSource.find(:all)
+      render :action => 'add'
     end
-    @auth_sources = AuthSource.find(:all)
   end
 
   def edit
diff --git a/app/views/users/add.rhtml b/app/views/users/add.rhtml
index 2e0743e87..0e7a33319 100644
--- a/app/views/users/add.rhtml
+++ b/app/views/users/add.rhtml
@@ -1,6 +1,6 @@
 <h2><%= link_to l(:label_user_plural), :controller => 'users', :action => 'index' %> &#187; <%=l(:label_user_new)%></h2>
 
-<% labelled_tabular_form_for :user, @user, :url => { :action => "add" }, :html => { :class => nil } do |f| %>
+<% labelled_tabular_form_for :user, @user, :url => { :action => "create" }, :html => { :class => nil } do |f| %>
 	<%= render :partial => 'form', :locals => { :f => f } %>
 	<p><label><%= check_box_tag 'send_information', 1, true %> <%= l(:label_send_information) %></label></p>
 	<p>
diff --git a/config/routes.rb b/config/routes.rb
index eab2157d8..0e60c555f 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -148,7 +148,7 @@ ActionController::Routing::Routes.draw do |map|
     end
     users.with_options :conditions => {:method => :post} do |user_actions|
       user_actions.connect 'users', :action => 'add'
-      user_actions.connect 'users/new', :action => 'add'
+      user_actions.connect 'users/new', :action => 'create'
       user_actions.connect 'users/:id/edit', :action => 'edit'
       user_actions.connect 'users/:id/memberships', :action => 'edit_membership'
       user_actions.connect 'users/:id/memberships/:membership_id', :action => 'edit_membership'
diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb
index 0e4c14c79..8e115eac6 100644
--- a/test/functional/users_controller_test.rb
+++ b/test/functional/users_controller_test.rb
@@ -24,7 +24,7 @@ class UsersController; def rescue_action(e) raise e end; end
 class UsersControllerTest < ActionController::TestCase
   include Redmine::I18n
   
-  fixtures :users, :projects, :members, :member_roles, :roles
+  fixtures :users, :projects, :members, :member_roles, :roles, :auth_sources
   
   def setup
     @controller = UsersController.new
@@ -107,6 +107,46 @@ class UsersControllerTest < ActionController::TestCase
     assert project_ids.include?(2) #private project admin can see
   end
 
+  context "GET :add" do
+    setup do
+      get :add
+    end
+
+    should_assign_to :user
+    should_respond_with :success
+    should_render_template :add
+  end
+
+  context "POST :create" do
+    context "when successful" do
+      setup do
+        post :create, :user => {
+          :firstname => 'John',
+          :lastname => 'Doe',
+          :login => 'jdoe',
+          :password => 'test',
+          :password_confirmation => 'test',
+          :mail => 'jdoe@gmail.com'
+        }
+      end
+
+      should_assign_to :user
+      should_respond_with :redirect
+      should_redirect_to('user edit') { {:controller => 'users', :action => 'edit', :id => User.find_by_login('jdoe')}}
+    end
+
+    context "when unsuccessful" do
+      setup do
+        post :create, :user => {}
+      end
+
+      should_assign_to :user
+      should_respond_with :success
+      should_render_template :add
+    end
+
+  end
+
   def test_edit
     ActionMailer::Base.deliveries.clear
     post :edit, :id => 2, :user => {:firstname => 'Changed'}
diff --git a/test/integration/admin_test.rb b/test/integration/admin_test.rb
index 9ea9e9809..62ddc533f 100644
--- a/test/integration/admin_test.rb
+++ b/test/integration/admin_test.rb
@@ -25,7 +25,7 @@ class AdminTest < ActionController::IntegrationTest
     get "/users/add"
     assert_response :success
     assert_template "users/add"
-    post "/users/add", :user => { :login => "psmith", :firstname => "Paul", :lastname => "Smith", :mail => "psmith@somenet.foo", :language => "en" }, :password => "psmith09", :password_confirmation => "psmith09"
+    post "/users/create", :user => { :login => "psmith", :firstname => "Paul", :lastname => "Smith", :mail => "psmith@somenet.foo", :language => "en" }, :password => "psmith09", :password_confirmation => "psmith09"
     
     user = User.find_by_login("psmith")
     assert_kind_of User, user
@@ -42,7 +42,7 @@ class AdminTest < ActionController::IntegrationTest
   end
 
   test "Add a user as an anonymous user should fail" do
-    post '/users/add', :user => { :login => 'psmith', :firstname => 'Paul'}, :password => "psmith09", :password_confirmation => "psmith09"
+    post '/users/create', :user => { :login => 'psmith', :firstname => 'Paul'}, :password => "psmith09", :password_confirmation => "psmith09"
     assert_response :redirect
     assert_redirected_to "/login?back_url=http%3A%2F%2Fwww.example.com%2Fusers%2Fnew"
   end
diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb
index 37f262909..74b7d990d 100644
--- a/test/integration/routing_test.rb
+++ b/test/integration/routing_test.rb
@@ -250,7 +250,7 @@ class RoutingTest < ActionController::IntegrationTest
     should_route :get, "/users/444/edit", :controller => 'users', :action => 'edit', :id => '444'
     should_route :get, "/users/222/edit/membership", :controller => 'users', :action => 'edit', :id => '222', :tab => 'membership'
 
-    should_route :post, "/users/new", :controller => 'users', :action => 'add'
+    should_route :post, "/users/new", :controller => 'users', :action => 'create'
     should_route :post, "/users/444/edit", :controller => 'users', :action => 'edit', :id => '444'
     should_route :post, "/users/123/memberships", :controller => 'users', :action => 'edit_membership', :id => '123'
     should_route :post, "/users/123/memberships/55", :controller => 'users', :action => 'edit_membership', :id => '123', :membership_id => '55'