Refactor: extract TimelogController#edit to #update

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

app/controllers/timelog_controller.rb

@@ -17,7 +17,7 @@
 
 class TimelogController < ApplicationController
   menu_item :issues
-  before_filter :find_project, :authorize, :only => [:new, :create, :edit, :destroy]
+  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 }
@@ -111,18 +111,26 @@ class TimelogController < ApplicationController
   
   def edit
     (render_403; return) if @time_entry && !@time_entry.editable_by?(User.current)
-    @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 })
+  end
+
+  verify :method => :put, :only => :update, :render => {:nothing => true, :status => :method_not_allowed }
+  def update
+    (render_403; return) if @time_entry && !@time_entry.editable_by?(User.current)
     @time_entry.attributes = params[:time_entry]
     
     call_hook(:controller_timelog_edit_before_save, { :params => params, :time_entry => @time_entry })
     
-    if request.post? and @time_entry.save
+    if @time_entry.save
       flash[:notice] = l(:notice_successful_update)
       redirect_back_or_default :action => 'index', :project_id => @time_entry.project
-      return
+    else
+      render :action => 'edit'
     end    
   end
-  
+
   def destroy
     (render_404; return) unless @time_entry
     (render_403; return) unless @time_entry.editable_by?(User.current)

app/views/timelog/edit.rhtml

@@ -1,6 +1,11 @@
 <h2><%= l(:label_spent_time) %></h2>
 
-<% labelled_tabular_form_for :time_entry, @time_entry, :url => {:action => (@time_entry.new_record? ? 'create' : 'edit'), :id => @time_entry, :project_id => @time_entry.project} do |f| %>
+<% labelled_tabular_form_for(:time_entry, @time_entry, :url => {
+                               :action => (@time_entry.new_record? ? 'create' : 'update'),
+                               :id => @time_entry,
+                               :project_id => @time_entry.project
+                             },
+                             :html => {:method => @time_entry.new_record? ? :post : :put}) do |f| %>
 <%= error_messages_for 'time_entry' %>
 <%= back_url_hidden_field_tag %>
 

config/routes.rb

@@ -15,6 +15,7 @@ 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 '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'
   

lib/redmine.rb

@@ -84,10 +84,10 @@ Redmine::AccessControl.map do |map|
   end
   
   map.project_module :time_tracking do |map|
-    map.permission :log_time, {:timelog => [:new, :create, :edit]}, :require => :loggedin
+    map.permission :log_time, {:timelog => [:new, :create, :edit, :update]}, :require => :loggedin
     map.permission :view_time_entries, :timelog => [:index], :time_entry_reports => [:report]
-    map.permission :edit_time_entries, {:timelog => [:new, :create, :edit, :destroy]}, :require => :member
-    map.permission :edit_own_time_entries, {:timelog => [:new, :create, :edit, :destroy]}, :require => :loggedin
+    map.permission :edit_time_entries, {:timelog => [:new, :create, :edit, :update, :destroy]}, :require => :member
+    map.permission :edit_own_time_entries, {:timelog => [:new, :create, :edit, :update, :destroy]}, :require => :loggedin
     map.permission :manage_project_activities, {:project_enumerations => [:update, :destroy]}, :require => :member
   end
   

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/edit/2' }
+    assert_tag :tag => 'form', :attributes => { :action => '/projects/ecookbook/timelog/update/2' }
   end
   
   def test_get_edit_with_an_existing_time_entry_with_inactive_activity
@@ -101,7 +101,7 @@ class TimelogControllerTest < ActionController::TestCase
     assert_equal 2, entry.user_id
     
     @request.session[:user_id] = 1
-    post :edit, :id => 1,
+    put :update, :id => 1,
                 :time_entry => {:issue_id => '2',
                                 :hours => '8'}
     assert_redirected_to :action => 'index', :project_id => 'ecookbook'

test/integration/routing_test.rb

@@ -239,8 +239,9 @@ class RoutingTest < ActionController::IntegrationTest
     should_route :get, "/time_entries/22/edit", :controller => 'timelog', :action => 'edit', :id => '22'
 
     should_route :post, "/projects/ecookbook/timelog/edit", :controller => 'timelog', :action => 'create', :project_id => 'ecookbook'
-    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'
+
+    should_route :put, "/time_entries/22", :controller => 'timelog', :action => 'update', :id => '22'
   end
 
   context "time_entry_reports" do