Fixed that updating the issue form was broken by r4011 when user is not allowed to...

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

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index b0b632f194ddca0eaad2860c126ec18ba0e7d44a..11469a20b78e4134b8d2ab3a4603704ad95ff8a2 100644 (file)
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -21,11 +21,11 @@ class IssuesController < ApplicationController
 
   before_filter :find_issue, :only => [:show, :edit, :update]
   before_filter :find_issues, :only => [:bulk_edit, :bulk_update, :destroy]
-  before_filter :find_project, :only => [:new, :create]
+  before_filter :find_project, :only => [:new, :create, :update_form]
   before_filter :authorize, :except => [:index]
   before_filter :find_optional_project, :only => [:index]
   before_filter :check_for_default_issue_status, :only => [:new, :create]
-  before_filter :build_new_issue_from_params, :only => [:new, :create]
+  before_filter :build_new_issue_from_params, :only => [:new, :create, :update_form]
   accept_rss_auth :index, :show
   accept_api_auth :index, :show, :create, :update, :destroy
 
@@ -132,7 +132,6 @@ class IssuesController < ApplicationController
   def new
     respond_to do |format|
       format.html { render :action => 'new', :layout => !request.xhr? }
-      format.js { render :partial => 'update_form' }
     end
   end
 
@@ -202,6 +201,11 @@ class IssuesController < ApplicationController
     end
   end
 
+  # Updates the issue form when changing the project, status or tracker
+  # on issue creation/update
+  def update_form
+  end
+
   # Bulk edit/copy a set of issues
   def bulk_edit
     @issues.sort!

diff --git a/app/views/issues/_update_form.js.erb b/app/views/issues/_update_form.js.erb
deleted file mode 100644 (file)
index 46fc972..0000000
--- a/app/views/issues/_update_form.js.erb
+++ /dev/null
@@ -1,7 +0,0 @@
-$('#all_attributes').html('<%= escape_javascript(render :partial => 'form') %>');
-
-<% if User.current.allowed_to?(:log_time, @issue.project) %>
-  $('#log_time').show();
-<% else %>
-  $('#log_time').hide();
-<% end %>

diff --git a/app/views/issues/update_form.js.erb b/app/views/issues/update_form.js.erb
new file mode 100644 (file)
index 0000000..46fc972
--- /dev/null
+++ b/app/views/issues/update_form.js.erb
@@ -0,0 +1,7 @@
+$('#all_attributes').html('<%= escape_javascript(render :partial => 'form') %>');
+
+<% if User.current.allowed_to?(:log_time, @issue.project) %>
+  $('#log_time').show();
+<% else %>
+  $('#log_time').hide();
+<% end %>

diff --git a/config/routes.rb b/config/routes.rb
index f157f764c234e708fe2bc97792d6f89f39a62e4c..b89973e55a23e8ec9671157378ee5655d2a2072a 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -116,7 +116,7 @@ RedmineApp::Application.routes.draw do
       end
     end
     # issue form update
-    match 'issues/new', :controller => 'issues', :action => 'new', :via => [:put, :post], :as => 'issue_form'
+    match 'issues/update_form', :controller => 'issues', :action => 'update_form', :via => [:put, :post], :as => 'issue_form'
 
     resources :files, :only => [:index, :new, :create]
 

diff --git a/lib/redmine.rb b/lib/redmine.rb
index 63fbdeb23e5620400d64fe2151464d4d58d79547..43e3f146a33d2c8697eb0d82ed41625eeb8237c3 100644 (file)
--- a/lib/redmine.rb
+++ b/lib/redmine.rb
@@ -115,7 +115,7 @@ Redmine::AccessControl.map do |map|
     map.permission :manage_subtasks, {}
     map.permission :set_issues_private, {}
     map.permission :set_own_issues_private, {}, :require => :loggedin
-    map.permission :add_issue_notes, {:issues => [:edit, :update], :journals => [:new], :attachments => :upload}
+    map.permission :add_issue_notes, {:issues => [:edit, :update, :update_form], :journals => [:new], :attachments => :upload}
     map.permission :edit_issue_notes, {:journals => :edit}, :require => :loggedin
     map.permission :edit_own_issue_notes, {:journals => :edit}, :require => :loggedin
     map.permission :view_private_notes, {}, :read => true, :require => :member

diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml
index 15bf89bc51989cbb2bbc15fe0d1aae494aee52c8..8136b41d93cf18c17ab7f6c1b3af4ce21f5a2cb3 100644 (file)
--- a/test/fixtures/users.yml
+++ b/test/fixtures/users.yml
@@ -105,12 +105,15 @@ users_006:
   login: ''
   type: AnonymousUser
 users_007: 
+  # A user who does not belong to any project
   id: 7
   created_on: 2006-07-19 19:33:19 +02:00
   status: 1
   last_login_on: 
-  language: ''
-  hashed_password: 1
+  language: 'en'
+  # password = foo
+  salt: 7599f9963ec07b5a3b55b354407120c0
+  hashed_password: 8f659c8d7c072f189374edacfa90d6abbc26d8ed
   updated_on: 2006-07-19 19:33:19 +02:00
   admin: false
   mail: someone@foo.bar

diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index de4c717c2738690e66669baa5641c992ece2ed28..8a147ad49a6ff929aa572284cd634e2122498a9f 100644 (file)
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -1671,9 +1671,9 @@ class IssuesControllerTest < ActionController::TestCase
     assert_error_tag :content => /No tracker/
   end
 
-  def test_update_new_form
+  def test_update_form_for_new_issue
     @request.session[:user_id] = 2
-    xhr :post, :new, :project_id => 1,
+    xhr :post, :update_form, :project_id => 1,
                      :issue => {:tracker_id => 2,
                                 :subject => 'This is the test_new issue',
                                 :description => 'This is the description',
@@ -1690,14 +1690,14 @@ class IssuesControllerTest < ActionController::TestCase
     assert_equal 'This is the test_new issue', issue.subject
   end
 
-  def test_update_new_form_should_propose_transitions_based_on_initial_status
+  def test_update_form_for_new_issue_should_propose_transitions_based_on_initial_status
     @request.session[:user_id] = 2
     WorkflowTransition.delete_all
     WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 1, :new_status_id => 2)
     WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 1, :new_status_id => 5)
     WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 5, :new_status_id => 4)
 
-    xhr :post, :new, :project_id => 1,
+    xhr :post, :update_form, :project_id => 1,
                      :issue => {:tracker_id => 1,
                                 :status_id => 5,
                                 :subject => 'This is an issue'}
@@ -2626,9 +2626,9 @@ class IssuesControllerTest < ActionController::TestCase
     end
   end
 
-  def test_update_edit_form
+  def test_update_form_for_existing_issue
     @request.session[:user_id] = 2
-    xhr :put, :new, :project_id => 1,
+    xhr :put, :update_form, :project_id => 1,
                              :id => 1,
                              :issue => {:tracker_id => 2,
                                         :subject => 'This is the test_new issue',
@@ -2647,9 +2647,9 @@ class IssuesControllerTest < ActionController::TestCase
     assert_equal 'This is the test_new issue', issue.subject
   end
 
-  def test_update_edit_form_should_keep_issue_author
+  def test_update_form_for_existing_issue_should_keep_issue_author
     @request.session[:user_id] = 3
-    xhr :put, :new, :project_id => 1, :id => 1, :issue => {:subject => 'Changed'}
+    xhr :put, :update_form, :project_id => 1, :id => 1, :issue => {:subject => 'Changed'}
     assert_response :success
     assert_equal 'text/javascript', response.content_type
 
@@ -2659,14 +2659,14 @@ class IssuesControllerTest < ActionController::TestCase
     assert_not_equal User.current, issue.author
   end
 
-  def test_update_edit_form_should_propose_transitions_based_on_initial_status
+  def test_update_form_for_existing_issue_should_propose_transitions_based_on_initial_status
     @request.session[:user_id] = 2
     WorkflowTransition.delete_all
     WorkflowTransition.create!(:role_id => 1, :tracker_id => 2, :old_status_id => 2, :new_status_id => 1)
     WorkflowTransition.create!(:role_id => 1, :tracker_id => 2, :old_status_id => 2, :new_status_id => 5)
     WorkflowTransition.create!(:role_id => 1, :tracker_id => 2, :old_status_id => 5, :new_status_id => 4)
 
-    xhr :put, :new, :project_id => 1,
+    xhr :put, :update_form, :project_id => 1,
                     :id => 2,
                     :issue => {:tracker_id => 2,
                                :status_id => 5,
@@ -2676,9 +2676,9 @@ class IssuesControllerTest < ActionController::TestCase
     assert_equal [1,2,5], assigns(:allowed_statuses).map(&:id).sort
   end
 
-  def test_update_edit_form_with_project_change
+  def test_update_form_for_existing_issue_with_project_change
     @request.session[:user_id] = 2
-    xhr :put, :new, :project_id => 1,
+    xhr :put, :update_form, :project_id => 1,
                              :id => 1,
                              :issue => {:project_id => 2,
                                         :tracker_id => 2,

diff --git a/test/integration/issues_test.rb b/test/integration/issues_test.rb
index 549c1cae0fd2183ee15ab1d973df6a5230ab3dea..e0659b43c869f7f2a6c1d42b72637ea9674e5af1 100644 (file)
--- a/test/integration/issues_test.rb
+++ b/test/integration/issues_test.rb
@@ -65,15 +65,6 @@ class IssuesTest < ActionController::IntegrationTest
     assert_equal 1, issue.status.id
   end
 
-  def test_update_issue_form
-    log_user('jsmith', 'jsmith')
-    post 'projects/ecookbook/issues/new', :issue => { :tracker_id => "2"}
-    assert_response :success
-    assert_tag 'select',
-      :attributes => {:name => 'issue[tracker_id]'},
-      :child => {:tag => 'option', :attributes => {:value => '2', :selected => 'selected'}}
-  end
-
   # add then remove 2 attachments to an issue
   def test_issue_attachments
     log_user('jsmith', 'jsmith')

diff --git a/test/integration/routing/issues_test.rb b/test/integration/routing/issues_test.rb
index 68ed436bfda42619219f5645744de53e7336c3c3..72760ede140d2095d8e9e0355c8d7e65b1559776 100644 (file)
--- a/test/integration/routing/issues_test.rb
+++ b/test/integration/routing/issues_test.rb
@@ -107,8 +107,8 @@ class RoutingIssuesTest < ActionController::IntegrationTest
   def test_issues_form_update
     ["post", "put"].each do |method|
       assert_routing(
-          { :method => method, :path => "/projects/23/issues/new" },
-          { :controller => 'issues', :action => 'new', :project_id => '23' }
+          { :method => method, :path => "/projects/23/issues/update_form" },
+          { :controller => 'issues', :action => 'update_form', :project_id => '23' }
         )
     end
   end

diff --git a/test/ui/issues_test.rb b/test/ui/issues_test.rb
index b0243bfda6b0868c44428b9bb9f7f4765dc9735b..5a4f49dd5a2ff8cfafe0a585c65419ddd117fce3 100644 (file)
--- a/test/ui/issues_test.rb
+++ b/test/ui/issues_test.rb
@@ -56,6 +56,35 @@ class Redmine::UiTest::IssuesTest < Redmine::UiTest::Base
     assert_equal 'Value for field 2', issue.custom_field_value(CustomField.find_by_name('Searchable field'))
   end
 
+  def test_create_issue_with_form_update
+    field = IssueCustomField.create!(
+      :field_format => 'string',
+      :name => 'Form update CF',
+      :is_for_all => true,
+      :trackers => Tracker.find_all_by_name('Feature request')
+    )
+
+    Role.non_member.add_permission! :add_issues
+    Role.non_member.remove_permission! :edit_issues, :add_issue_notes
+
+    log_user('someone', 'foo')
+    visit '/projects/ecookbook/issues/new'
+    assert page.has_no_content?('Form update CF')
+
+    fill_in 'Subject', :with => 'new test issue'
+    # the custom field should show up when changing tracker
+    select 'Feature request', :from => 'Tracker'
+    assert page.has_content?('Form update CF')
+
+    fill_in 'Form update', :with => 'CF value'
+    assert_difference 'Issue.count' do
+      find('input[name=commit]').click
+    end
+
+    issue = Issue.order('id desc').first
+    assert_equal 'CF value', issue.custom_field_value(field)
+  end
+
   def test_create_issue_with_watchers
     User.generate!(:firstname => 'Some', :lastname => 'Watcher')
 
@@ -98,6 +127,35 @@ class Redmine::UiTest::IssuesTest < Redmine::UiTest::Base
     assert_equal 'new issue description', issue.description
   end
 
+  def test_update_issue_with_form_update
+    field = IssueCustomField.create!(
+      :field_format => 'string',
+      :name => 'Form update CF',
+      :is_for_all => true,
+      :trackers => Tracker.find_all_by_name('Feature request')
+    )
+
+    Role.non_member.add_permission! :edit_issues
+    Role.non_member.remove_permission! :add_issues, :add_issue_notes
+
+    log_user('someone', 'foo')
+    visit '/issues/1'
+    assert page.has_no_content?('Form update CF')
+
+    page.first(:link, 'Update').click
+    # the custom field should show up when changing tracker
+    select 'Feature request', :from => 'Tracker'
+    assert page.has_content?('Form update CF')
+
+    fill_in 'Form update', :with => 'CF value'
+    assert_no_difference 'Issue.count' do
+      page.first(:button, 'Submit').click
+    end
+
+    issue = Issue.find(1)
+    assert_equal 'CF value', issue.custom_field_value(field)
+  end
+
   def test_watch_issue_via_context_menu
     log_user('jsmith', 'jsmith')
     visit '/issues'