From 068771ea0764e326d720455c87e6c8cbc0f825a3 Mon Sep 17 00:00:00 2001 From: Eric Davis Date: Thu, 7 Oct 2010 15:51:09 +0000 Subject: [PATCH] Refactor: extract TimelogController#new from #edit git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4239 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/timelog_controller.rb | 10 +++++++++- app/views/time_entry_reports/report.rhtml | 2 +- app/views/timelog/index.html.erb | 2 +- config/routes.rb | 6 +++--- lib/redmine.rb | 4 ++-- test/functional/timelog_controller_test.rb | 22 +++++++++++----------- test/integration/routing_test.rb | 7 ++++--- 7 files changed, 31 insertions(+), 22 deletions(-) diff --git a/app/controllers/timelog_controller.rb b/app/controllers/timelog_controller.rb index bf571aa59..d15ea6a54 100644 --- a/app/controllers/timelog_controller.rb +++ b/app/controllers/timelog_controller.rb @@ -17,7 +17,7 @@ class TimelogController < ApplicationController menu_item :issues - before_filter :find_project, :authorize, :only => [:edit, :destroy] + before_filter :find_project, :authorize, :only => [:new, :edit, :destroy] before_filter :find_optional_project, :only => [:index] verify :method => :post, :only => :destroy, :redirect_to => { :action => :index } @@ -85,6 +85,14 @@ class TimelogController < ApplicationController end end end + + def new + @time_entry ||= TimeEntry.new(:project => @project, :issue => @issue, :user => User.current, :spent_on => User.current.today) + @time_entry.attributes = params[:time_entry] + + call_hook(:controller_timelog_edit_before_save, { :params => params, :time_entry => @time_entry }) + render :action => 'edit' + end def edit (render_403; return) if @time_entry && !@time_entry.editable_by?(User.current) diff --git a/app/views/time_entry_reports/report.rhtml b/app/views/time_entry_reports/report.rhtml index 0d28d5dc7..5ae9d6550 100644 --- a/app/views/time_entry_reports/report.rhtml +++ b/app/views/time_entry_reports/report.rhtml @@ -1,5 +1,5 @@
-<%= link_to_if_authorized l(:button_log_time), {:controller => 'timelog', :action => 'edit', :project_id => @project, :issue_id => @issue}, :class => 'icon icon-time-add' %> +<%= link_to_if_authorized l(:button_log_time), {:controller => 'timelog', :action => 'new', :project_id => @project, :issue_id => @issue}, :class => 'icon icon-time-add' %>
<%= render_timelog_breadcrumb %> diff --git a/app/views/timelog/index.html.erb b/app/views/timelog/index.html.erb index a17c06e65..737476f35 100644 --- a/app/views/timelog/index.html.erb +++ b/app/views/timelog/index.html.erb @@ -1,5 +1,5 @@
-<%= link_to_if_authorized l(:button_log_time), {:controller => 'timelog', :action => 'edit', :project_id => @project, :issue_id => @issue}, :class => 'icon icon-time-add' %> +<%= link_to_if_authorized l(:button_log_time), {:controller => 'timelog', :action => 'new', :project_id => @project, :issue_id => @issue}, :class => 'icon icon-time-add' %>
<%= render_timelog_breadcrumb %> diff --git a/config/routes.rb b/config/routes.rb index 87aefa806..34ee875ea 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -15,8 +15,8 @@ ActionController::Routing::Routes.draw do |map| map.connect 'help/:ctrl/:page', :controller => 'help' map.connect 'time_entries/:id/edit', :action => 'edit', :controller => 'timelog' - map.connect 'projects/:project_id/time_entries/new', :action => 'edit', :controller => 'timelog' - map.connect 'projects/:project_id/issues/:issue_id/time_entries/new', :action => 'edit', :controller => 'timelog' + map.connect 'projects/:project_id/time_entries/new', :action => 'new', :controller => 'timelog' + map.connect 'projects/:project_id/issues/:issue_id/time_entries/new', :action => 'new', :controller => 'timelog' map.with_options :controller => 'timelog' do |timelog| timelog.connect 'projects/:project_id/time_entries', :action => 'index' @@ -37,7 +37,7 @@ ActionController::Routing::Routes.draw do |map| time_report.connect 'projects/:project_id/time_entries/report.:format' end - timelog.with_options :action => 'edit', :conditions => {:method => :get} do |time_edit| + timelog.with_options :action => 'new', :conditions => {:method => :get} do |time_edit| time_edit.connect 'issues/:issue_id/time_entries/new' end diff --git a/lib/redmine.rb b/lib/redmine.rb index 42f3c47f2..85e07fd2e 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -86,8 +86,8 @@ Redmine::AccessControl.map do |map| map.project_module :time_tracking do |map| map.permission :log_time, {:timelog => :edit}, :require => :loggedin map.permission :view_time_entries, :timelog => [:index], :time_entry_reports => [:report] - map.permission :edit_time_entries, {:timelog => [:edit, :destroy]}, :require => :member - map.permission :edit_own_time_entries, {:timelog => [:edit, :destroy]}, :require => :loggedin + map.permission :edit_time_entries, {:timelog => [:new, :edit, :destroy]}, :require => :member + map.permission :edit_own_time_entries, {:timelog => [:new, :edit, :destroy]}, :require => :loggedin map.permission :manage_project_activities, {:project_enumerations => [:update, :destroy]}, :require => :member end diff --git a/test/functional/timelog_controller_test.rb b/test/functional/timelog_controller_test.rb index 22d14e32e..823650257 100644 --- a/test/functional/timelog_controller_test.rb +++ b/test/functional/timelog_controller_test.rb @@ -31,9 +31,9 @@ class TimelogControllerTest < ActionController::TestCase @response = ActionController::TestResponse.new end - def test_get_edit + def test_get_new @request.session[:user_id] = 3 - get :edit, :project_id => 1 + get :new, :project_id => 1 assert_response :success assert_template 'edit' # Default activity selected @@ -41,6 +41,15 @@ class TimelogControllerTest < ActionController::TestCase :content => 'Development' end + def test_get_new_should_only_show_active_time_entry_activities + @request.session[:user_id] = 3 + get :new, :project_id => 1 + assert_response :success + assert_template 'edit' + assert_no_tag :tag => 'option', :content => 'Inactive Activity' + + end + def test_get_edit_existing_time @request.session[:user_id] = 2 get :edit, :id => 2, :project_id => nil @@ -50,15 +59,6 @@ class TimelogControllerTest < ActionController::TestCase assert_tag :tag => 'form', :attributes => { :action => '/projects/ecookbook/timelog/edit/2' } end - def test_get_edit_should_only_show_active_time_entry_activities - @request.session[:user_id] = 3 - get :edit, :project_id => 1 - assert_response :success - assert_template 'edit' - assert_no_tag :tag => 'option', :content => 'Inactive Activity' - - end - def test_get_edit_with_an_existing_time_entry_with_inactive_activity te = TimeEntry.find(1) te.activity = TimeEntryActivity.find_by_name("Inactive Activity") diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb index d8d036853..3ba01cdc1 100644 --- a/test/integration/routing_test.rb +++ b/test/integration/routing_test.rb @@ -233,11 +233,12 @@ class RoutingTest < ActionController::IntegrationTest should_route :get, "/issues/234/time_entries.atom", :controller => 'timelog', :action => 'index', :issue_id => '234', :format => 'atom' should_route :get, "/projects/ecookbook/issues/123/time_entries", :controller => 'timelog', :action => 'index', :project_id => 'ecookbook', :issue_id => '123' - should_route :get, "/issues/567/time_entries/new", :controller => 'timelog', :action => 'edit', :issue_id => '567' - should_route :get, "/projects/ecookbook/time_entries/new", :controller => 'timelog', :action => 'edit', :project_id => 'ecookbook' - should_route :get, "/projects/ecookbook/issues/567/time_entries/new", :controller => 'timelog', :action => 'edit', :project_id => 'ecookbook', :issue_id => '567' + should_route :get, "/issues/567/time_entries/new", :controller => 'timelog', :action => 'new', :issue_id => '567' + should_route :get, "/projects/ecookbook/time_entries/new", :controller => 'timelog', :action => 'new', :project_id => 'ecookbook' + should_route :get, "/projects/ecookbook/issues/567/time_entries/new", :controller => 'timelog', :action => 'new', :project_id => 'ecookbook', :issue_id => '567' should_route :get, "/time_entries/22/edit", :controller => 'timelog', :action => 'edit', :id => '22' + should_route :post, "/time_entries/22/edit", :controller => 'timelog', :action => 'edit', :id => '22' should_route :post, "/time_entries/55/destroy", :controller => 'timelog', :action => 'destroy', :id => '55' end