From: Eric Davis Date: Tue, 12 Oct 2010 15:55:21 +0000 (+0000) Subject: Refactor: convert timelogs to a REST resource (:time_entries) X-Git-Tag: 1.1.0~285 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=718816c5d4b0c45ce1def155c9f517fb5ac91e35;p=redmine.git Refactor: convert timelogs to a REST resource (:time_entries) git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4250 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- diff --git a/app/controllers/timelog_controller.rb b/app/controllers/timelog_controller.rb index d533b10ef..3030bb06e 100644 --- a/app/controllers/timelog_controller.rb +++ b/app/controllers/timelog_controller.rb @@ -20,8 +20,6 @@ class TimelogController < ApplicationController before_filter :find_project, :authorize, :only => [:new, :create, :edit, :update, :destroy] before_filter :find_optional_project, :only => [:index] - verify :method => :post, :only => :destroy, :redirect_to => { :action => :index } - helper :sort include SortHelper helper :issues @@ -131,6 +129,7 @@ class TimelogController < ApplicationController end end + verify :method => :delete, :only => :destroy, :render => {:nothing => true, :status => :method_not_allowed } def destroy (render_404; return) unless @time_entry (render_403; return) unless @time_entry.editable_by?(User.current) diff --git a/app/views/timelog/_list.rhtml b/app/views/timelog/_list.rhtml index e11695ebf..28f1f5239 100644 --- a/app/views/timelog/_list.rhtml +++ b/app/views/timelog/_list.rhtml @@ -31,7 +31,7 @@ :title => l(:button_edit) %> <%= link_to image_tag('delete.png'), {:controller => 'timelog', :action => 'destroy', :id => entry, :project_id => nil}, :confirm => l(:text_are_you_sure), - :method => :post, + :method => :delete, :title => l(:button_delete) %> <% end -%> diff --git a/config/routes.rb b/config/routes.rb index 9e8a75a53..0d52476d0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -13,38 +13,16 @@ ActionController::Routing::Routes.draw do |map| map.connect 'roles/workflow/:id/:role_id/:tracker_id', :controller => 'roles', :action => 'workflow' map.connect 'help/:ctrl/:page', :controller => 'help' - - map.connect 'time_entries/:id/edit', :action => 'edit', :controller => 'timelog' - map.connect 'time_entries/:id', :action => 'update', :controller => 'timelog', :conditions => {:method => :put} - 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' - - timelog.with_options :action => 'index', :conditions => {:method => :get} do |time_details| - time_details.connect 'time_entries' - time_details.connect 'time_entries.:format' - time_details.connect 'issues/:issue_id/time_entries' - time_details.connect 'issues/:issue_id/time_entries.:format' - time_details.connect 'projects/:project_id/time_entries.:format' - time_details.connect 'projects/:project_id/issues/:issue_id/time_entries' - time_details.connect 'projects/:project_id/issues/:issue_id/time_entries.:format' - end - timelog.connect 'projects/:project_id/time_entries/report', :controller => 'time_entry_reports', :action => 'report' - timelog.with_options :controller => 'time_entry_reports', :action => 'report',:conditions => {:method => :get} do |time_report| - time_report.connect 'time_entries/report' - time_report.connect 'time_entries/report.:format' - time_report.connect 'projects/:project_id/time_entries/report.:format' - end - - timelog.with_options :action => 'new', :conditions => {:method => :get} do |time_edit| - time_edit.connect 'issues/:issue_id/time_entries/new' - end - timelog.connect 'projects/:project_id/timelog/edit', :action => 'create', :conditions => {:method => :post} - timelog.connect 'time_entries/:id/destroy', :action => 'destroy', :conditions => {:method => :post} + map.connect 'projects/:project_id/time_entries/report', :controller => 'time_entry_reports', :action => 'report' + map.with_options :controller => 'time_entry_reports', :action => 'report',:conditions => {:method => :get} do |time_report| + time_report.connect 'time_entries/report' + time_report.connect 'time_entries/report.:format' + time_report.connect 'projects/:project_id/time_entries/report.:format' end + + # TODO: wasteful since this is also nested under issues, projects, and projects/issues + map.resources :time_entries, :controller => 'timelog' map.connect 'projects/:id/wiki', :controller => 'wikis', :action => 'edit', :conditions => {:method => :post} map.connect 'projects/:id/wiki/destroy', :controller => 'wikis', :action => 'destroy', :conditions => {:method => :get} @@ -131,8 +109,13 @@ ActionController::Routing::Routes.draw do |map| map.connect '/issues', :controller => 'issues', :action => 'index', :conditions => { :method => :post } map.connect '/issues/create', :controller => 'issues', :action => 'index', :conditions => { :method => :post } - map.resources :issues, :member => { :edit => :post }, :collection => {} - map.resources :issues, :path_prefix => '/projects/:project_id', :collection => { :create => :post } + map.resources :issues, :member => { :edit => :post }, :collection => {} do |issues| + issues.resources :time_entries, :controller => 'timelog' + end + + map.resources :issues, :path_prefix => '/projects/:project_id', :collection => { :create => :post } do |issues| + issues.resources :time_entries, :controller => 'timelog' + end map.with_options :controller => 'issue_relations', :conditions => {:method => :post} do |relations| relations.connect 'issues/:issue_id/relations/:id', :action => 'new' @@ -177,6 +160,9 @@ ActionController::Routing::Routes.draw do |map| project.resources :files, :only => [:index, :new, :create] project.resources :versions, :collection => {:close_completed => :put}, :member => {:status_by => :post} project.resources :news, :shallow => true + project.resources :time_entries, :controller => 'timelog', :path_prefix => 'projects/:project_id' + + end # Destroy uses a get request to prompt the user before the actual DELETE request diff --git a/test/functional/timelog_controller_test.rb b/test/functional/timelog_controller_test.rb index 28ca93531..d35b98c71 100644 --- a/test/functional/timelog_controller_test.rb +++ b/test/functional/timelog_controller_test.rb @@ -56,7 +56,7 @@ class TimelogControllerTest < ActionController::TestCase assert_response :success assert_template 'edit' # Default activity selected - assert_tag :tag => 'form', :attributes => { :action => '/projects/ecookbook/timelog/update/2' } + assert_tag :tag => 'form', :attributes => { :action => '/projects/ecookbook/time_entries/2' } end def test_get_edit_with_an_existing_time_entry_with_inactive_activity @@ -114,7 +114,7 @@ class TimelogControllerTest < ActionController::TestCase def test_destroy @request.session[:user_id] = 2 - post :destroy, :id => 1 + delete :destroy, :id => 1 assert_redirected_to :action => 'index', :project_id => 'ecookbook' assert_equal I18n.t(:notice_successful_delete), flash[:notice] assert_nil TimeEntry.find_by_id(1) @@ -128,7 +128,7 @@ class TimelogControllerTest < ActionController::TestCase end @request.session[:user_id] = 2 - post :destroy, :id => 1 + delete :destroy, :id => 1 assert_redirected_to :action => 'index', :project_id => 'ecookbook' assert_equal I18n.t(:notice_unable_delete_time_entry), flash[:error] assert_not_nil TimeEntry.find_by_id(1) diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb index 401ce960f..25755dc36 100644 --- a/test/integration/routing_test.rb +++ b/test/integration/routing_test.rb @@ -221,27 +221,60 @@ class RoutingTest < ActionController::IntegrationTest should_route :post, "/projects/redmine/repository/edit", :controller => 'repositories', :action => 'edit', :id => 'redmine' end - context "timelogs" do + context "timelogs (global)" do should_route :get, "/time_entries", :controller => 'timelog', :action => 'index' should_route :get, "/time_entries.csv", :controller => 'timelog', :action => 'index', :format => 'csv' should_route :get, "/time_entries.atom", :controller => 'timelog', :action => 'index', :format => 'atom' + should_route :get, "/time_entries/new", :controller => 'timelog', :action => 'new' + should_route :get, "/time_entries/22/edit", :controller => 'timelog', :action => 'edit', :id => '22' + + should_route :post, "/time_entries", :controller => 'timelog', :action => 'create' + + should_route :put, "/time_entries/22", :controller => 'timelog', :action => 'update', :id => '22' + + should_route :delete, "/time_entries/55", :controller => 'timelog', :action => 'destroy', :id => '55' + end + + context "timelogs (scoped under project)" do should_route :get, "/projects/567/time_entries", :controller => 'timelog', :action => 'index', :project_id => '567' should_route :get, "/projects/567/time_entries.csv", :controller => 'timelog', :action => 'index', :project_id => '567', :format => 'csv' should_route :get, "/projects/567/time_entries.atom", :controller => 'timelog', :action => 'index', :project_id => '567', :format => 'atom' + should_route :get, "/projects/567/time_entries/new", :controller => 'timelog', :action => 'new', :project_id => '567' + should_route :get, "/projects/567/time_entries/22/edit", :controller => 'timelog', :action => 'edit', :id => '22', :project_id => '567' + + should_route :post, "/projects/567/time_entries", :controller => 'timelog', :action => 'create', :project_id => '567' + + should_route :put, "/projects/567/time_entries/22", :controller => 'timelog', :action => 'update', :id => '22', :project_id => '567' + + should_route :delete, "/projects/567/time_entries/55", :controller => 'timelog', :action => 'destroy', :id => '55', :project_id => '567' + end + + context "timelogs (scoped under issues)" do should_route :get, "/issues/234/time_entries", :controller => 'timelog', :action => 'index', :issue_id => '234' should_route :get, "/issues/234/time_entries.csv", :controller => 'timelog', :action => 'index', :issue_id => '234', :format => 'csv' 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/234/time_entries/new", :controller => 'timelog', :action => 'new', :issue_id => '234' + should_route :get, "/issues/234/time_entries/22/edit", :controller => 'timelog', :action => 'edit', :id => '22', :issue_id => '234' - 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, "/issues/234/time_entries", :controller => 'timelog', :action => 'create', :issue_id => '234' - should_route :post, "/projects/ecookbook/timelog/edit", :controller => 'timelog', :action => 'create', :project_id => 'ecookbook' - should_route :post, "/time_entries/55/destroy", :controller => 'timelog', :action => 'destroy', :id => '55' + should_route :put, "/issues/234/time_entries/22", :controller => 'timelog', :action => 'update', :id => '22', :issue_id => '234' - should_route :put, "/time_entries/22", :controller => 'timelog', :action => 'update', :id => '22' + should_route :delete, "/issues/234/time_entries/55", :controller => 'timelog', :action => 'destroy', :id => '55', :issue_id => '234' + end + + context "timelogs (scoped under project and issues)" do + should_route :get, "/projects/ecookbook/issues/234/time_entries", :controller => 'timelog', :action => 'index', :issue_id => '234', :project_id => 'ecookbook' + should_route :get, "/projects/ecookbook/issues/234/time_entries.csv", :controller => 'timelog', :action => 'index', :issue_id => '234', :project_id => 'ecookbook', :format => 'csv' + should_route :get, "/projects/ecookbook/issues/234/time_entries.atom", :controller => 'timelog', :action => 'index', :issue_id => '234', :project_id => 'ecookbook', :format => 'atom' + should_route :get, "/projects/ecookbook/issues/234/time_entries/new", :controller => 'timelog', :action => 'new', :issue_id => '234', :project_id => 'ecookbook' + should_route :get, "/projects/ecookbook/issues/234/time_entries/22/edit", :controller => 'timelog', :action => 'edit', :id => '22', :issue_id => '234', :project_id => 'ecookbook' + + should_route :post, "/projects/ecookbook/issues/234/time_entries", :controller => 'timelog', :action => 'create', :issue_id => '234', :project_id => 'ecookbook' + + should_route :put, "/projects/ecookbook/issues/234/time_entries/22", :controller => 'timelog', :action => 'update', :id => '22', :issue_id => '234', :project_id => 'ecookbook' + + should_route :delete, "/projects/ecookbook/issues/234/time_entries/55", :controller => 'timelog', :action => 'destroy', :id => '55', :issue_id => '234', :project_id => 'ecookbook' end context "time_entry_reports" do