From 3da7b1bc2882cfb02c5068680979158322242398 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sat, 14 Feb 2015 08:03:51 +0000 Subject: [PATCH] Implements /issues/new form for creating issues outside a project (#1003). git-svn-id: http://svn.redmine.org/redmine/trunk@13999 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/issues_controller.rb | 55 ++++++++++-------- app/helpers/issues_helper.rb | 9 ++- app/models/issue.rb | 13 +---- app/views/issues/_form.html.erb | 2 +- app/views/issues/new.html.erb | 4 +- config/routes.rb | 2 + test/functional/issues_controller_test.rb | 69 +++++++++++++++++++++++ test/integration/routing/issues_test.rb | 4 ++ 8 files changed, 119 insertions(+), 39 deletions(-) diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 8c1d73751..1cf909294 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -21,9 +21,8 @@ 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 :authorize, :except => [:index] - before_filter :find_optional_project, :only => [:index] + before_filter :authorize, :except => [:index, :new, :create] + before_filter :find_optional_project, :only => [:index, :new, :create] before_filter :build_new_issue_from_params, :only => [:new, :create] accept_rss_auth :index, :show accept_api_auth :index, :show, :create, :update, :destroy @@ -154,12 +153,7 @@ class IssuesController < ApplicationController format.html { render_attachment_warning_if_needed(@issue) flash[:notice] = l(:notice_issue_successful_create, :id => view_context.link_to("##{@issue.id}", issue_path(@issue), :title => @issue.subject)) - if params[:continue] - attrs = {:tracker_id => @issue.tracker, :parent_issue_id => @issue.parent_issue_id}.reject {|k,v| v.nil?} - redirect_to new_project_issue_path(@issue.project, :issue => attrs) - else - redirect_to issue_path(@issue) - end + redirect_after_create } format.api { render :action => 'show', :status => :created, :location => issue_url(@issue) } end @@ -359,13 +353,6 @@ class IssuesController < ApplicationController private - def find_project - project_id = params[:project_id] || (params[:issue] && params[:issue][:project_id]) - @project = Project.find(project_id) - rescue ActiveRecord::RecordNotFound - render_404 - end - def retrieve_previous_and_next_issue_ids retrieve_query_from_session if @query @@ -434,6 +421,9 @@ class IssuesController < ApplicationController end end @issue.project = @project + if request.get? + @issue.project ||= @issue.allowed_target_projects.first + end @issue.author ||= User.current @issue.start_date ||= Date.today if Setting.default_issue_start_date_to_creation_date? @@ -443,14 +433,16 @@ class IssuesController < ApplicationController end @issue.safe_attributes = attrs end - @issue.tracker ||= @project.trackers.first - if @issue.tracker.nil? - render_error l(:error_no_tracker_in_project) - return false - end - if @issue.status.nil? - render_error l(:error_no_default_issue_status) - return false + if @issue.project + @issue.tracker ||= @issue.project.trackers.first + if @issue.tracker.nil? + render_error l(:error_no_tracker_in_project) + return false + end + if @issue.status.nil? + render_error l(:error_no_default_issue_status) + return false + end end @priorities = IssuePriority.active @@ -505,4 +497,19 @@ class IssuesController < ApplicationController param == '1' end end + + # Redirects user after a successful issue creation + def redirect_after_create + if params[:continue] + attrs = {:tracker_id => @issue.tracker, :parent_issue_id => @issue.parent_issue_id}.reject {|k,v| v.nil?} + if params[:project_id] + redirect_to new_project_issue_path(@issue.project, :issue => attrs) + else + attrs.merge! :project_id => @issue.project_id + redirect_to new_issue_path(:issue => attrs) + end + else + redirect_to issue_path(@issue) + end + end end diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index e34039d76..a88adceb9 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -202,10 +202,15 @@ module IssuesHelper # Returns the path for updating the issue form # with project as the current project def update_issue_form_path(project, issue) + options = {:format => 'js'} if issue.new_record? - new_project_issue_path(project, :format => 'js') + if project + new_project_issue_path(project, options) + else + new_issue_path(options) + end else - edit_issue_path(issue, :format => 'js') + edit_issue_path(issue, options) end end diff --git a/app/models/issue.rb b/app/models/issue.rb index 1e1fb9afc..5a49b0d7b 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -380,15 +380,7 @@ class Issue < ActiveRecord::Base end safe_attributes 'project_id', - :if => lambda {|issue, user| - if issue.new_record? - issue.copy? - else - user.allowed_to?(:edit_issues, issue.project) - end - } - - safe_attributes 'tracker_id', + 'tracker_id', 'status_id', 'category_id', 'assigned_to_id', @@ -429,7 +421,8 @@ class Issue < ActiveRecord::Base names = super names -= disabled_core_fields names -= read_only_attribute_names(user) - if new_record? && copy? + if new_record? + # Make sure that project_id can always be set for new issues names |= %w(project_id) end names diff --git a/app/views/issues/_form.html.erb b/app/views/issues/_form.html.erb index 44578610c..c071cb0e1 100644 --- a/app/views/issues/_form.html.erb +++ b/app/views/issues/_form.html.erb @@ -7,7 +7,7 @@

<% end %> -<% if @issue.safe_attribute? 'project_id' %> +<% if @issue.safe_attribute?('project_id') && (!@issue.new_record? || @project.nil? || @issue.copy?) %>

<%= f.select :project_id, project_tree_options_for_select(@issue.allowed_target_projects, :selected => @issue.project), {:required => true}, :onchange => "updateIssueFrom('#{escape_javascript update_issue_form_path(@project, @issue)}')" %>

<% end %> diff --git a/app/views/issues/new.html.erb b/app/views/issues/new.html.erb index f168c39a4..c93a1fc1f 100644 --- a/app/views/issues/new.html.erb +++ b/app/views/issues/new.html.erb @@ -2,7 +2,7 @@ <%= call_hook(:view_issues_new_top, {:issue => @issue}) %> -<%= labelled_form_for @issue, :url => project_issues_path(@project), +<%= labelled_form_for @issue, :url => _project_issues_path(@project), :html => {:id => 'issue-form', :multipart => true} do |f| %> <%= error_messages_for 'issue' %> <%= hidden_field_tag 'copy_from', params[:copy_from] if params[:copy_from] %> @@ -49,7 +49,7 @@ <%= submit_tag l(:button_create) %> <%= submit_tag l(:button_create_and_continue), :name => 'continue' %> - <%= preview_link preview_new_issue_path(:project_id => @project), 'issue-form' %> + <%= preview_link preview_new_issue_path(:project_id => @issue.project), 'issue-form' %> <% end %>
diff --git a/config/routes.rb b/config/routes.rb index a32ec76ef..142fa915f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -185,6 +185,8 @@ Rails.application.routes.draw do resources :relations, :controller => 'issue_relations', :only => [:index, :show, :create, :destroy] end end + # Used when updating the form of a new issue outside a project + post '/issues/new', :to => 'issues#new' match '/issues', :controller => 'issues', :action => 'destroy', :via => :delete resources :queries, :except => [:show] diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 5e8762e08..3d048edbc 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -1497,6 +1497,7 @@ class IssuesControllerTest < ActionController::TestCase assert_response :success assert_template 'new' + assert_select 'form#issue-form[action=?]', '/projects/ecookbook/issues' assert_select 'form#issue-form' do assert_select 'input[name=?]', 'issue[is_private]' assert_select 'select[name=?]', 'issue[project_id]', 0 @@ -1552,6 +1553,21 @@ class IssuesControllerTest < ActionController::TestCase end end + def test_new_without_project_id + @request.session[:user_id] = 2 + get :new + assert_response :success + assert_template 'new' + + assert_select 'form#issue-form[action=?]', '/issues' + assert_select 'form#issue-form' do + assert_select 'select[name=?]', 'issue[project_id]' + end + + assert_nil assigns(:project) + assert_not_nil assigns(:issue) + end + def test_new_should_select_default_status @request.session[:user_id] = 2 @@ -2164,6 +2180,59 @@ class IssuesControllerTest < ActionController::TestCase assert issue.is_private? end + def test_create_without_project_id + @request.session[:user_id] = 2 + + assert_difference 'Issue.count' do + post :create, + :issue => {:project_id => 3, + :tracker_id => 2, + :subject => 'Foo'} + assert_response 302 + end + issue = Issue.order('id DESC').first + assert_equal 3, issue.project_id + assert_equal 2, issue.tracker_id + end + + def test_create_without_project_id_and_continue_should_redirect_without_project_id + @request.session[:user_id] = 2 + + assert_difference 'Issue.count' do + post :create, + :issue => {:project_id => 3, + :tracker_id => 2, + :subject => 'Foo'}, + :continue => '1' + assert_redirected_to '/issues/new?issue%5Bproject_id%5D=3&issue%5Btracker_id%5D=2' + end + end + + def test_create_without_project_id_should_be_denied_without_permission + Role.non_member.remove_permission! :add_issues + Role.anonymous.remove_permission! :add_issues + @request.session[:user_id] = 2 + + assert_no_difference 'Issue.count' do + post :create, + :issue => {:project_id => 3, + :tracker_id => 2, + :subject => 'Foo'} + assert_response 403 + end + end + + def test_create_without_project_id_with_failure + @request.session[:user_id] = 2 + + post :create, + :issue => {:project_id => 3, + :tracker_id => 2, + :subject => ''} + assert_response :success + assert_nil assigns(:project) + end + def test_post_create_should_send_a_notification ActionMailer::Base.deliveries.clear @request.session[:user_id] = 2 diff --git a/test/integration/routing/issues_test.rb b/test/integration/routing/issues_test.rb index 1b59f3214..c7d7523a7 100644 --- a/test/integration/routing/issues_test.rb +++ b/test/integration/routing/issues_test.rb @@ -27,6 +27,9 @@ class RoutingIssuesTest < Redmine::RoutingTest should_route 'GET /issues/64.pdf' => 'issues#show', :id => '64', :format => 'pdf' should_route 'GET /issues/64.atom' => 'issues#show', :id => '64', :format => 'atom' + should_route 'GET /issues/new' => 'issues#new' + should_route 'POST /issues' => 'issues#create' + should_route 'GET /issues/64/edit' => 'issues#edit', :id => '64' should_route 'PUT /issues/64' => 'issues#update', :id => '64' should_route 'DELETE /issues/64' => 'issues#destroy', :id => '64' @@ -50,6 +53,7 @@ class RoutingIssuesTest < Redmine::RoutingTest end def test_issues_form_update + should_route 'POST /issues/new' => 'issues#new' should_route 'POST /projects/23/issues/new' => 'issues#new', :project_id => '23' should_route 'PATCH /issues/23/edit' => 'issues#edit', :id => '23' end -- 2.39.5