From 7799788b3de62a6af32c9d573904a929992d7ab0 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sat, 16 Feb 2013 11:35:18 +0000 Subject: [PATCH] Fixed that updating the issue form was broken by r4011 when user is not allowed to add issues (#13188). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@11405 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/issues_controller.rb | 10 +++- ..._update_form.js.erb => update_form.js.erb} | 0 config/routes.rb | 2 +- lib/redmine.rb | 2 +- test/fixtures/users.yml | 7 ++- test/functional/issues_controller_test.rb | 24 ++++---- test/integration/issues_test.rb | 9 --- test/integration/routing/issues_test.rb | 4 +- test/ui/issues_test.rb | 58 +++++++++++++++++++ 9 files changed, 86 insertions(+), 30 deletions(-) rename app/views/issues/{_update_form.js.erb => update_form.js.erb} (100%) diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index b0b632f19..11469a20b 100644 --- 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 similarity index 100% rename from app/views/issues/_update_form.js.erb rename to app/views/issues/update_form.js.erb diff --git a/config/routes.rb b/config/routes.rb index f157f764c..b89973e55 100644 --- 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 63fbdeb23..43e3f146a 100644 --- 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 15bf89bc5..8136b41d9 100644 --- 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 de4c717c2..8a147ad49 100644 --- 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 549c1cae0..e0659b43c 100644 --- 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 68ed436bf..72760ede1 100644 --- 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 b0243bfda..5a4f49dd5 100644 --- 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' -- 2.39.5