From 44ac1a0debc802b2ecbaa7cf7f763b373bf0fbb4 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 20 Jan 2008 11:30:57 +0000 Subject: [PATCH] ProjectsController#add_issue moved to IssuesController#new. Tracker can now be changed/selected on the new issue form. This action can be invoked without the tracker_id parameter (the first enabled tracker will be used by default). git-svn-id: http://redmine.rubyforge.org/svn/trunk@1080 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/issues_controller.rb | 64 ++++++++++++++++++- app/controllers/projects_controller.rb | 42 +----------- app/helpers/projects_helper.rb | 2 +- app/views/issues/_form.rhtml | 7 ++ app/views/issues/_sidebar.rhtml | 2 +- app/views/issues/context_menu.rhtml | 2 +- .../add_issue.rhtml => issues/new.rhtml} | 2 - app/views/issues/show.rhtml | 2 +- app/views/projects/show.rhtml | 2 +- lib/redmine.rb | 2 +- test/functional/issues_controller_test.rb | 50 +++++++++++++++ test/functional/projects_controller_test.rb | 19 ------ test/integration/issues_test.rb | 6 +- 13 files changed, 128 insertions(+), 74 deletions(-) rename app/views/{projects/add_issue.rhtml => issues/new.rhtml} (88%) diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 11112289e..7a48282c9 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -17,11 +17,13 @@ class IssuesController < ApplicationController layout 'base' - before_filter :find_project, :authorize, :except => [:index, :changes, :preview] + before_filter :find_issue, :except => [:index, :changes, :preview, :new, :update_form] + before_filter :find_project, :only => [:new, :update_form] + before_filter :authorize, :except => [:index, :changes, :preview, :update_form] before_filter :find_optional_project, :only => [:index, :changes] accept_key_auth :index, :changes - cache_sweeper :issue_sweeper, :only => [ :edit, :update, :destroy ] + cache_sweeper :issue_sweeper, :only => [ :new, :edit, :update, :destroy ] helper :projects include ProjectsHelper @@ -90,6 +92,51 @@ class IssuesController < ApplicationController end end + # Add a new issue + # The new issue will be created from an existing one if copy_from parameter is given + def new + @issue = params[:copy_from] ? Issue.new.copy_from(params[:copy_from]) : Issue.new(params[:issue]) + @issue.project = @project + @issue.author = User.current + @issue.tracker ||= @project.trackers.find(params[:tracker_id] ? params[:tracker_id] : :first) + if @issue.tracker.nil? + flash.now[:error] = 'No tracker is associated to this project. Please check the Project settings.' + render :nothing => true, :layout => true + return + end + + default_status = IssueStatus.default + unless default_status + flash.now[:error] = 'No default issue status is defined. Please check your configuration (Go to "Administration -> Issue statuses").' + render :nothing => true, :layout => true + return + end + @issue.status = default_status + @allowed_statuses = ([default_status] + default_status.find_new_statuses_allowed_to(User.current.role_for_project(@project), @issue.tracker)) + + if request.get? || request.xhr? + @issue.start_date ||= Date.today + @custom_values = @issue.custom_values.empty? ? + @project.custom_fields_for_issues(@issue.tracker).collect { |x| CustomValue.new(:custom_field => x, :customized => @issue) } : + @issue.custom_values + else + requested_status = IssueStatus.find_by_id(params[:issue][:status_id]) + # Check that the user is allowed to apply the requested status + @issue.status = (@allowed_statuses.include? requested_status) ? requested_status : default_status + @custom_values = @project.custom_fields_for_issues(@issue.tracker).collect { |x| CustomValue.new(:custom_field => x, :customized => @issue, :value => params["custom_fields"][x.id.to_s]) } + @issue.custom_values = @custom_values + if @issue.save + attach_files(@issue, params[:attachments]) + flash[:notice] = l(:notice_successful_create) + Mailer.deliver_issue_add(@issue) if Setting.notified_events.include?('issue_added') + redirect_to :controller => 'issues', :action => 'index', :project_id => @project + return + end + end + @priorities = Enumeration::get_values('IPRI') + render :layout => !request.xhr? + end + def edit @priorities = Enumeration::get_values('IPRI') @custom_values = [] @@ -185,6 +232,11 @@ class IssuesController < ApplicationController render :layout => false end + def update_form + @issue = Issue.new(params[:issue]) + render :action => :new, :layout => false + end + def preview issue = Issue.find_by_id(params[:id]) @attachements = issue.attachments if issue @@ -193,13 +245,19 @@ class IssuesController < ApplicationController end private - def find_project + def find_issue @issue = Issue.find(params[:id], :include => [:project, :tracker, :status, :author, :priority, :category]) @project = @issue.project rescue ActiveRecord::RecordNotFound render_404 end + def find_project + @project = Project.find(params[:project_id]) + rescue ActiveRecord::RecordNotFound + render_404 + end + def find_optional_project return true unless params[:project_id] @project = Project.find(params[:project_id]) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index f38ca828e..2342e8074 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -22,7 +22,7 @@ class ProjectsController < ApplicationController menu_item :roadmap, :only => :roadmap menu_item :files, :only => [:list_files, :add_file] menu_item :settings, :only => :settings - menu_item :issues, :only => [:add_issue, :bulk_edit_issues, :changelog, :move_issues] + menu_item :issues, :only => [:bulk_edit_issues, :changelog, :move_issues] before_filter :find_project, :except => [ :index, :list, :add ] before_filter :authorize, :except => [ :index, :list, :add, :archive, :unarchive, :destroy ] @@ -30,7 +30,6 @@ class ProjectsController < ApplicationController accept_key_auth :activity, :calendar cache_sweeper :project_sweeper, :only => [ :add, :edit, :archive, :unarchive, :destroy ] - cache_sweeper :issue_sweeper, :only => [ :add_issue ] cache_sweeper :version_sweeper, :only => [ :add_version ] helper :sort @@ -184,45 +183,6 @@ class ProjectsController < ApplicationController end end - # Add a new issue to @project - # The new issue will be created from an existing one if copy_from parameter is given - def add_issue - @issue = params[:copy_from] ? Issue.new.copy_from(params[:copy_from]) : Issue.new(params[:issue]) - @issue.project = @project - @issue.author = User.current - @issue.tracker ||= @project.trackers.find(params[:tracker_id]) - - default_status = IssueStatus.default - unless default_status - flash.now[:error] = 'No default issue status is defined. Please check your configuration (Go to "Administration -> Issue statuses").' - render :nothing => true, :layout => true - return - end - @issue.status = default_status - @allowed_statuses = ([default_status] + default_status.find_new_statuses_allowed_to(User.current.role_for_project(@project), @issue.tracker)) - - if request.get? - @issue.start_date ||= Date.today - @custom_values = @issue.custom_values.empty? ? - @project.custom_fields_for_issues(@issue.tracker).collect { |x| CustomValue.new(:custom_field => x, :customized => @issue) } : - @issue.custom_values - else - requested_status = IssueStatus.find_by_id(params[:issue][:status_id]) - # Check that the user is allowed to apply the requested status - @issue.status = (@allowed_statuses.include? requested_status) ? requested_status : default_status - @custom_values = @project.custom_fields_for_issues(@issue.tracker).collect { |x| CustomValue.new(:custom_field => x, :customized => @issue, :value => params["custom_fields"][x.id.to_s]) } - @issue.custom_values = @custom_values - if @issue.save - attach_files(@issue, params[:attachments]) - flash[:notice] = l(:notice_successful_create) - Mailer.deliver_issue_add(@issue) if Setting.notified_events.include?('issue_added') - redirect_to :controller => 'issues', :action => 'index', :project_id => @project - return - end - end - @priorities = Enumeration::get_values('IPRI') - end - # Bulk edit issues def bulk_edit_issues if request.post? diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index df4b9c334..ee61d88dd 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -194,6 +194,6 @@ module ProjectsHelper # can't use form tag inside helper content_tag('form', select_tag('tracker_id', '' + options_from_collection_for_select(trackers, 'id', 'name'), :onchange => "if (this.value != '') {this.form.submit()}"), - :action => url_for(:controller => 'projects', :action => 'add_issue', :id => @project), :method => 'get') + :action => url_for(:controller => 'issues', :action => 'new', :project_id => @project), :method => 'get') end end diff --git a/app/views/issues/_form.rhtml b/app/views/issues/_form.rhtml index 203d1cca3..779846f4d 100644 --- a/app/views/issues/_form.rhtml +++ b/app/views/issues/_form.rhtml @@ -1,6 +1,13 @@ <%= error_messages_for 'issue' %>
+<% if @issue.new_record? %> +

<%= f.select :tracker_id, @project.trackers.collect {|t| [t.name, t.id]}, :required => true %>

+<%= observe_field :issue_tracker_id, :url => { :action => :new }, + :update => :content, + :with => "Form.serialize('issue-form')" %> +<% end %> +
<% if @issue.new_record? %>

<%= f.select :status_id, (@allowed_statuses.collect {|p| [p.name, p.id]}), :required => true %>

diff --git a/app/views/issues/_sidebar.rhtml b/app/views/issues/_sidebar.rhtml index e6c63896b..1e1fc87d0 100644 --- a/app/views/issues/_sidebar.rhtml +++ b/app/views/issues/_sidebar.rhtml @@ -1,4 +1,4 @@ -<% if authorize_for('projects', 'add_issue') && @project.trackers.any? %> +<% if authorize_for('issues', 'new') && @project.trackers.any? %>

<%= l(:label_issue_new) %>

<%= l(:label_tracker) %>: <%= new_issue_selector %> <% end %> diff --git a/app/views/issues/context_menu.rhtml b/app/views/issues/context_menu.rhtml index 0f11bb943..9691a7713 100644 --- a/app/views/issues/context_menu.rhtml +++ b/app/views/issues/context_menu.rhtml @@ -31,7 +31,7 @@ :selected => @issue.assigned_to.nil?, :disabled => !@can[:assign] %> -
  • <%= context_menu_link l(:button_copy), {:controller => 'projects', :action => 'add_issue', :id => @project, :copy_from => @issue}, +
  • <%= context_menu_link l(:button_copy), {:controller => 'issues', :action => 'new', :project_id => @project, :copy_from => @issue}, :class => 'icon-copy', :disabled => !@can[:copy] %>
  • <%= context_menu_link l(:button_move), {:controller => 'projects', :action => 'move_issues', :id => @project, "issue_ids[]" => @issue.id }, :class => 'icon-move', :disabled => !@can[:move] %> diff --git a/app/views/projects/add_issue.rhtml b/app/views/issues/new.rhtml similarity index 88% rename from app/views/projects/add_issue.rhtml rename to app/views/issues/new.rhtml index a68922906..4fedc6895 100644 --- a/app/views/projects/add_issue.rhtml +++ b/app/views/issues/new.rhtml @@ -1,9 +1,7 @@

    <%=l(:label_issue_new)%>: <%= @issue.tracker %>

    <% labelled_tabular_form_for :issue, @issue, - :url => {:action => 'add_issue'}, :html => {:multipart => true, :id => 'issue-form'} do |f| %> - <%= f.hidden_field :tracker_id %> <%= render :partial => 'issues/form', :locals => {:f => f} %> <%= submit_tag l(:button_create) %> <%= link_to_remote l(:label_preview), diff --git a/app/views/issues/show.rhtml b/app/views/issues/show.rhtml index 8cd44424c..7208e37be 100644 --- a/app/views/issues/show.rhtml +++ b/app/views/issues/show.rhtml @@ -3,7 +3,7 @@ <%= link_to_if_authorized l(:button_edit), {:controller => 'issues', :action => 'edit', :id => @issue}, :class => 'icon icon-edit', :accesskey => accesskey(:edit) %> <%= link_to_if_authorized l(:button_log_time), {:controller => 'timelog', :action => 'edit', :issue_id => @issue}, :class => 'icon icon-time' %> <%= watcher_tag(@issue, User.current) %> -<%= link_to_if_authorized l(:button_copy), {:controller => 'projects', :action => 'add_issue', :id => @project, :copy_from => @issue }, :class => 'icon icon-copy' %> +<%= link_to_if_authorized l(:button_copy), {:controller => 'issues', :action => 'new', :project_id => @project, :copy_from => @issue }, :class => 'icon icon-copy' %> <%= link_to_if_authorized l(:button_move), {:controller => 'projects', :action => 'move_issues', :id => @project, "issue_ids[]" => @issue.id }, :class => 'icon icon-move' %> <%= link_to_if_authorized l(:button_delete), {:controller => 'issues', :action => 'destroy', :id => @issue}, :confirm => l(:text_are_you_sure), :method => :post, :class => 'icon icon-del' %>
    • diff --git a/app/views/projects/show.rhtml b/app/views/projects/show.rhtml index 2b930e01e..ecaa9750a 100644 --- a/app/views/projects/show.rhtml +++ b/app/views/projects/show.rhtml @@ -56,7 +56,7 @@
    <% content_for :sidebar do %> - <% if authorize_for('projects', 'add_issue') && @project.trackers.any? %> + <% if authorize_for('issues', 'new') && @project.trackers.any? %>

    <%= l(:label_issue_new) %>

    <%= l(:label_tracker) %>: <%= new_issue_selector %> <% end %> diff --git a/lib/redmine.rb b/lib/redmine.rb index d0e152467..ea05d6e80 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -30,7 +30,7 @@ Redmine::AccessControl.map do |map| :versions => [:show, :status_by], :queries => :index, :reports => :issue_report}, :public => true - map.permission :add_issues, {:projects => :add_issue} + map.permission :add_issues, {:issues => :new} map.permission :edit_issues, {:projects => :bulk_edit_issues, :issues => [:edit, :update, :destroy_attachment]} map.permission :manage_issue_relations, {:issue_relations => [:new, :destroy]} diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index d60e32200..05bfc96e3 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -29,6 +29,7 @@ class IssuesControllerTest < Test::Unit::TestCase :issues, :issue_statuses, :trackers, + :projects_trackers, :issue_categories, :enabled_modules, :enumerations, @@ -126,6 +127,55 @@ class IssuesControllerTest < Test::Unit::TestCase :content => /Notes/ } } end + def test_get_new + @request.session[:user_id] = 2 + get :new, :project_id => 1, :tracker_id => 1 + assert_response :success + assert_template 'new' + end + + def test_get_new_without_tracker_id + @request.session[:user_id] = 2 + get :new, :project_id => 1 + assert_response :success + assert_template 'new' + + issue = assigns(:issue) + assert_not_nil issue + assert_equal Project.find(1).trackers.first, issue.tracker + end + + def test_update_new_form + @request.session[:user_id] = 2 + xhr :post, :new, :project_id => 1, + :issue => {:tracker_id => 2, + :subject => 'This is the test_new issue', + :description => 'This is the description', + :priority_id => 5} + assert_response :success + assert_template 'new' + end + + def test_post_new + @request.session[:user_id] = 2 + post :new, :project_id => 1, + :issue => {:tracker_id => 1, + :subject => 'This is the test_new issue', + :description => 'This is the description', + :priority_id => 5} + assert_redirected_to 'projects/ecookbook/issues' + assert Issue.find_by_subject('This is the test_new issue') + end + + def test_copy_issue + @request.session[:user_id] = 2 + get :new, :project_id => 1, :copy_from => 1 + assert_template 'new' + assert_not_nil assigns(:issue) + orig = Issue.find(1) + assert_equal orig.subject, assigns(:issue).subject + end + def test_get_edit @request.session[:user_id] = 2 get :edit, :id => 1 diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb index f19aa7748..61047079e 100644 --- a/test/functional/projects_controller_test.rb +++ b/test/functional/projects_controller_test.rb @@ -236,23 +236,4 @@ class ProjectsControllerTest < Test::Unit::TestCase assert_redirected_to 'admin/projects' assert Project.find(1).active? end - - def test_add_issue - @request.session[:user_id] = 2 - get :add_issue, :id => 1, :tracker_id => 1 - assert_response :success - assert_template 'add_issue' - post :add_issue, :id => 1, :issue => {:tracker_id => 1, :subject => 'This is the test_add_issue issue', :description => 'This is the description', :priority_id => 5} - assert_redirected_to 'projects/ecookbook/issues' - assert Issue.find_by_subject('This is the test_add_issue issue') - end - - def test_copy_issue - @request.session[:user_id] = 2 - get :add_issue, :id => 1, :copy_from => 1 - assert_template 'add_issue' - assert_not_nil assigns(:issue) - orig = Issue.find(1) - assert_equal orig.subject, assigns(:issue).subject - end end diff --git a/test/integration/issues_test.rb b/test/integration/issues_test.rb index a9d3f9c74..81d27c30f 100644 --- a/test/integration/issues_test.rb +++ b/test/integration/issues_test.rb @@ -6,11 +6,11 @@ class IssuesTest < ActionController::IntegrationTest # create an issue def test_add_issue log_user('jsmith', 'jsmith') - get "projects/add_issue/1", :tracker_id => "1" + get 'projects/1/issues/new', :tracker_id => '1' assert_response :success - assert_template "projects/add_issue" + assert_template 'issues/new' - post "projects/add_issue/1", :tracker_id => "1", + post 'projects/1/issues/new', :tracker_id => "1", :issue => { :start_date => "2006-12-26", :priority_id => "3", :subject => "new test issue", -- 2.39.5