Refactor: Start to extract IssuesController#edit into #update (REST).

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

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index accfae33c002847fabbf22ff7e83b726556c8565..827186d89b898bf4a9217a3ba8423589db9f2b37 100644 (file)
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -19,7 +19,7 @@ class IssuesController < ApplicationController
   menu_item :new_issue, :only => :new
   default_search_scope :issues
   
-  before_filter :find_issue, :only => [:show, :edit, :reply]
+  before_filter :find_issue, :only => [:show, :edit, :update, :reply]
   before_filter :find_issues, :only => [:bulk_edit, :move, :destroy]
   before_filter :find_project, :only => [:new, :update_form, :preview]
   before_filter :authorize, :except => [:index, :changes, :gantt, :calendar, :preview, :context_menu]
@@ -226,7 +226,7 @@ class IssuesController < ApplicationController
       end
       # failure
       respond_to do |format|
-        format.html { }
+        format.html { render :action => 'edit' }
         format.xml  { render :xml => @issue.errors, :status => :unprocessable_entity }
       end
     end
@@ -237,6 +237,13 @@ class IssuesController < ApplicationController
     attachments.each(&:destroy)
   end
 
+  #--
+  # Start converting to the Rails REST controllers
+  #++
+  def update
+    edit
+  end
+
   def reply
     journal = Journal.find(params[:journal_id]) if params[:journal_id]
     if journal

diff --git a/app/views/issues/_edit.rhtml b/app/views/issues/_edit.rhtml
index 413f21729d7b3cadf1089511af297558767a2130..50a187410b3dd9154cd3c7112ae0dbee33e9978b 100644 (file)
--- a/app/views/issues/_edit.rhtml
+++ b/app/views/issues/_edit.rhtml
@@ -1,7 +1,8 @@
 <% labelled_tabular_form_for :issue, @issue,
-                             :url => {:action => 'edit', :id => @issue},
+                             :url => {:action => 'update', :id => @issue},
                              :html => {:id => 'issue-form',
                                        :class => nil,
+                                       :method => :put,
                                        :multipart => true} do |f| %>
     <%= error_messages_for 'issue' %>
     <%= error_messages_for 'time_entry' %>

diff --git a/lib/redmine.rb b/lib/redmine.rb
index 09ee2f5c9bda86ca6aa7e93abc758d1d68526e30..8b9154500f8a8d0e01e696cc266f7a4811a81546 100644 (file)
--- a/lib/redmine.rb
+++ b/lib/redmine.rb
@@ -52,9 +52,9 @@ Redmine::AccessControl.map do |map|
                                   :queries => :index,
                                   :reports => [:issue_report, :issue_report_details]}
     map.permission :add_issues, {:issues => [:new, :update_form]}
-    map.permission :edit_issues, {:issues => [:edit, :reply, :bulk_edit, :update_form]}
+    map.permission :edit_issues, {:issues => [:edit, :update, :reply, :bulk_edit, :update_form]}
     map.permission :manage_issue_relations, {:issue_relations => [:new, :destroy]}
-    map.permission :add_issue_notes, {:issues => [:edit, :reply]}
+    map.permission :add_issue_notes, {:issues => [:edit, :update, :reply]}
     map.permission :edit_issue_notes, {:journals => :edit}, :require => :loggedin
     map.permission :edit_own_issue_notes, {:journals => :edit}, :require => :loggedin
     map.permission :move_issues, {:issues => :move}, :require => :loggedin

diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index 94a74ba285d1b3e67be88541a5d87f9bd250080d..fd9177a4583247fd5da73af5b1485f874db8e3c0 100644 (file)
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -657,7 +657,7 @@ class IssuesControllerTest < ActionController::TestCase
     assert_select_rjs :show, "update"
   end
 
-  def test_post_edit_without_custom_fields_param
+  def test_put_update_without_custom_fields_param
     @request.session[:user_id] = 2
     ActionMailer::Base.deliveries.clear
     
@@ -668,7 +668,7 @@ class IssuesControllerTest < ActionController::TestCase
     
     assert_difference('Journal.count') do
       assert_difference('JournalDetail.count', 2) do
-        post :edit, :id => 1, :issue => {:subject => new_subject,
+        put :update, :id => 1, :issue => {:subject => new_subject,
                                          :priority_id => '6',
                                          :category_id => '1' # no change
                                         }
@@ -686,14 +686,14 @@ class IssuesControllerTest < ActionController::TestCase
     assert mail.body.include?("Subject changed from #{old_subject} to #{new_subject}")
   end
   
-  def test_post_edit_with_custom_field_change
+  def test_put_update_with_custom_field_change
     @request.session[:user_id] = 2
     issue = Issue.find(1)
     assert_equal '125', issue.custom_value_for(2).value
     
     assert_difference('Journal.count') do
       assert_difference('JournalDetail.count', 3) do
-        post :edit, :id => 1, :issue => {:subject => 'Custom field change',
+        put :update, :id => 1, :issue => {:subject => 'Custom field change',
                                          :priority_id => '6',
                                          :category_id => '1', # no change
                                          :custom_field_values => { '2' => 'New custom value' }
@@ -709,12 +709,12 @@ class IssuesControllerTest < ActionController::TestCase
     assert mail.body.include?("Searchable field changed from 125 to New custom value")
   end
   
-  def test_post_edit_with_status_and_assignee_change
+  def test_put_update_with_status_and_assignee_change
     issue = Issue.find(1)
     assert_equal 1, issue.status_id
     @request.session[:user_id] = 2
     assert_difference('TimeEntry.count', 0) do
-      post :edit,
+      put :update,
            :id => 1,
            :issue => { :status_id => 2, :assigned_to_id => 3 },
            :notes => 'Assigned to dlopper',
@@ -733,10 +733,10 @@ class IssuesControllerTest < ActionController::TestCase
     assert mail.subject.include?("(#{ IssueStatus.find(2).name })")
   end
   
-  def test_post_edit_with_note_only
+  def test_put_update_with_note_only
     notes = 'Note added by IssuesControllerTest#test_update_with_note_only'
     # anonymous user
-    post :edit,
+    put :update,
          :id => 1,
          :notes => notes
     assert_redirected_to :action => 'show', :id => '1'
@@ -749,11 +749,11 @@ class IssuesControllerTest < ActionController::TestCase
     assert mail.body.include?(notes)
   end
   
-  def test_post_edit_with_note_and_spent_time
+  def test_put_update_with_note_and_spent_time
     @request.session[:user_id] = 2
     spent_hours_before = Issue.find(1).spent_hours
     assert_difference('TimeEntry.count') do
-      post :edit,
+      put :update,
            :id => 1,
            :notes => '2.5 hours added',
            :time_entry => { :hours => '2.5', :comments => '', :activity_id => TimeEntryActivity.first }
@@ -772,7 +772,7 @@ class IssuesControllerTest < ActionController::TestCase
     assert_equal spent_hours_before + 2.5, issue.spent_hours
   end
   
-  def test_post_edit_with_attachment_only
+  def test_put_update_with_attachment_only
     set_tmp_attachments_directory
     
     # Delete all fixtured journals, a race condition can occur causing the wrong
@@ -780,7 +780,7 @@ class IssuesControllerTest < ActionController::TestCase
     Journal.delete_all
 
     # anonymous user
-    post :edit,
+    put :update,
          :id => 1,
          :notes => '',
          :attachments => {'1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain')}}
@@ -795,12 +795,12 @@ class IssuesControllerTest < ActionController::TestCase
     assert mail.body.include?('testfile.txt')
   end
   
-  def test_post_edit_with_no_change
+  def test_put_update_with_no_change
     issue = Issue.find(1)
     issue.journals.clear
     ActionMailer::Base.deliveries.clear
     
-    post :edit,
+    put :update,
          :id => 1,
          :notes => ''
     assert_redirected_to :action => 'show', :id => '1'
@@ -811,26 +811,26 @@ class IssuesControllerTest < ActionController::TestCase
     assert ActionMailer::Base.deliveries.empty?
   end
 
-  def test_post_edit_should_send_a_notification
+  def test_put_update_should_send_a_notification
     @request.session[:user_id] = 2
     ActionMailer::Base.deliveries.clear
     issue = Issue.find(1)
     old_subject = issue.subject
     new_subject = 'Subject modified by IssuesControllerTest#test_post_edit'
     
-    post :edit, :id => 1, :issue => {:subject => new_subject,
+    put :update, :id => 1, :issue => {:subject => new_subject,
                                      :priority_id => '6',
                                      :category_id => '1' # no change
                                     }
     assert_equal 1, ActionMailer::Base.deliveries.size
   end
   
-  def test_post_edit_with_invalid_spent_time
+  def test_put_update_with_invalid_spent_time
     @request.session[:user_id] = 2
     notes = 'Note added by IssuesControllerTest#test_post_edit_with_invalid_spent_time'
     
     assert_no_difference('Journal.count') do
-      post :edit,
+      put :update,
            :id => 1,
            :notes => notes,
            :time_entry => {"comments"=>"", "activity_id"=>"", "hours"=>"2z"}
@@ -843,11 +843,11 @@ class IssuesControllerTest < ActionController::TestCase
     assert_tag :input, :attributes => { :name => 'time_entry[hours]', :value => "2z" }
   end
   
-  def test_post_edit_should_allow_fixed_version_to_be_set_to_a_subproject
+  def test_put_update_should_allow_fixed_version_to_be_set_to_a_subproject
     issue = Issue.find(2)
     @request.session[:user_id] = 2
 
-    post :edit,
+    put :update,
          :id => issue.id,
          :issue => {
            :fixed_version_id => 4
@@ -859,11 +859,11 @@ class IssuesControllerTest < ActionController::TestCase
     assert_not_equal issue.project_id, issue.fixed_version.project_id
   end
 
-  def test_post_edit_should_redirect_back_using_the_back_url_parameter
+  def test_put_update_should_redirect_back_using_the_back_url_parameter
     issue = Issue.find(2)
     @request.session[:user_id] = 2
 
-    post :edit,
+    put :update,
          :id => issue.id,
          :issue => {
            :fixed_version_id => 4
@@ -874,11 +874,11 @@ class IssuesControllerTest < ActionController::TestCase
     assert_redirected_to '/issues'
   end
   
-  def test_post_edit_should_not_redirect_back_using_the_back_url_parameter_off_the_host
+  def test_put_update_should_not_redirect_back_using_the_back_url_parameter_off_the_host
     issue = Issue.find(2)
     @request.session[:user_id] = 2
 
-    post :edit,
+    put :update,
          :id => issue.id,
          :issue => {
            :fixed_version_id => 4