From 121bc44cc504b3086c35d40f984d75d978c59fb0 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 8 Jan 2012 11:16:54 +0000 Subject: [PATCH] Use the regular issue form to copy a single issue. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@8552 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/issues_controller.rb | 2 +- app/models/issue.rb | 11 +++- app/views/context_menus/issues.html.erb | 9 ++-- app/views/issues/_action_menu.html.erb | 3 +- app/views/issues/_form.html.erb | 2 +- app/views/issues/new.html.erb | 3 ++ .../context_menus_controller_test.rb | 5 +- test/functional/issues_controller_test.rb | 52 ++++++++++++++++++- test/unit/issue_test.rb | 1 + 9 files changed, 74 insertions(+), 14 deletions(-) diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index ad1d0a012..43ed6387e 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -158,7 +158,7 @@ class IssuesController < ApplicationController format.html { render_attachment_warning_if_needed(@issue) flash[:notice] = l(:notice_issue_successful_create, :id => "##{@issue.id}") - redirect_to(params[:continue] ? { :action => 'new', :project_id => @project, :issue => {:tracker_id => @issue.tracker, :parent_issue_id => @issue.parent_issue_id}.reject {|k,v| v.nil?} } : + redirect_to(params[:continue] ? { :action => 'new', :project_id => @issue.project, :issue => {:tracker_id => @issue.tracker, :parent_issue_id => @issue.parent_issue_id}.reject {|k,v| v.nil?} } : { :action => 'show', :id => @issue }) } format.api { render :action => 'show', :status => :created, :location => issue_url(@issue) } diff --git a/app/models/issue.rb b/app/models/issue.rb index 4e4deb5db..1259361da 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -127,12 +127,14 @@ class Issue < ActiveRecord::Base (project && tracker) ? (project.all_issue_custom_fields & tracker.custom_fields.all) : [] end + # Copies attributes from another issue, arg can be an id or an Issue def copy_from(arg) issue = arg.is_a?(Issue) ? arg : Issue.visible.find(arg) self.attributes = issue.attributes.dup.except("id", "root_id", "parent_id", "lft", "rgt", "created_on", "updated_on") self.custom_field_values = issue.custom_field_values.inject({}) {|h,v| h[v.custom_field_id] = v.value; h} self.status = issue.status self.author = User.current + @copied_from = issue self end @@ -143,6 +145,11 @@ class Issue < ActiveRecord::Base copy end + # Returns true if the issue is a copy + def copy? + @copied_from.present? + end + # Moves/copies an issue to a new project and tracker # Returns the moved/copied issue on success, false on failure def move_to_project(new_project, new_tracker=nil, options={}) @@ -255,7 +262,9 @@ class Issue < ActiveRecord::Base safe_attributes 'project_id', :if => lambda {|issue, user| - if user.allowed_to?(:move_issues, issue.project) + if issue.new_record? + issue.copy? + elsif user.allowed_to?(:move_issues, issue.project) projects = Issue.allowed_target_projects_on_move(user) projects.include?(issue.project) && projects.size > 1 end diff --git a/app/views/context_menus/issues.html.erb b/app/views/context_menus/issues.html.erb index 628433cb7..ce418af58 100644 --- a/app/views/context_menus/issues.html.erb +++ b/app/views/context_menus/issues.html.erb @@ -107,11 +107,12 @@ <% end %> <% if @issue.present? %> -
  • <%= context_menu_link l(:button_duplicate), {:controller => 'issues', :action => 'new', :project_id => @project, :copy_from => @issue}, - :class => 'icon-duplicate', :disabled => !@can[:copy] %>
    • -<% end %> +
  • <%= context_menu_link l(:button_copy), {:controller => 'issues', :action => 'new', :project_id => @project, :copy_from => @issue}, + :class => 'icon-copy', :disabled => !@can[:copy] %>
    • +<% else %>
  • <%= context_menu_link l(:button_copy), {:controller => 'issues', :action => 'bulk_edit', :ids => @issues.collect(&:id), :copy => '1'}, - :class => 'icon-copy', :disabled => !@can[:move] %>
    • + :class => 'icon-copy', :disabled => !@can[:move] %> +<% end %>
  • <%= context_menu_link l(:button_delete), issues_path(:ids => @issues.collect(&:id), :back_url => @back), :method => :delete, :confirm => issues_destroy_confirmation_message(@issues), :class => 'icon-del', :disabled => !@can[:delete] %>
    • diff --git a/app/views/issues/_action_menu.html.erb b/app/views/issues/_action_menu.html.erb index d4a6c6552..c643934d1 100644 --- a/app/views/issues/_action_menu.html.erb +++ b/app/views/issues/_action_menu.html.erb @@ -2,7 +2,6 @@ <%= link_to_if_authorized(l(:button_update), {:controller => 'issues', :action => 'edit', :id => @issue }, :onclick => 'showAndScrollTo("update", "notes"); return false;', :class => 'icon icon-edit', :accesskey => accesskey(:edit)) %> <%= link_to_if_authorized l(:button_log_time), {:controller => 'timelog', :action => 'new', :issue_id => @issue}, :class => 'icon icon-time-add' %> <%= watcher_tag(@issue, User.current) %> -<%= link_to_if_authorized l(:button_duplicate), {:controller => 'issues', :action => 'new', :project_id => @project, :copy_from => @issue }, :class => 'icon icon-duplicate' %> -<%= link_to_if_authorized l(:button_copy), {:controller => 'issues', :action => 'bulk_edit', :id => @issue, :copy => '1'}, :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 l(:button_delete), issue_path(@issue), :confirm => issues_destroy_confirmation_message(@issue), :method => :delete, :class => 'icon icon-del' if User.current.allowed_to?(:delete_issues, @project) %> diff --git a/app/views/issues/_form.html.erb b/app/views/issues/_form.html.erb index 4aab69a64..19b5f08eb 100644 --- a/app/views/issues/_form.html.erb +++ b/app/views/issues/_form.html.erb @@ -7,7 +7,7 @@

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

    <%= f.select :project_id, project_tree_options_for_select(Issue.allowed_target_projects_on_move, :selected => @issue.project), :required => true %>

    <%= observe_field :issue_project_id, :url => project_issue_form_path(@project, :id => @issue, :project_change => '1'), :with => "Form.serialize('issue-form')" %> diff --git a/app/views/issues/new.html.erb b/app/views/issues/new.html.erb index 615263b23..e7a6bcaca 100644 --- a/app/views/issues/new.html.erb +++ b/app/views/issues/new.html.erb @@ -5,8 +5,11 @@ <% 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] %>
    +
    <%= render :partial => 'issues/form', :locals => {:f => f} %> +

    <%= label_tag('attachments[1][file]', l(:label_attachment_plural))%><%= render :partial => 'attachments/form' %>

    diff --git a/test/functional/context_menus_controller_test.rb b/test/functional/context_menus_controller_test.rb index 7a188560b..426ca67b5 100644 --- a/test/functional/context_menus_controller_test.rb +++ b/test/functional/context_menus_controller_test.rb @@ -43,11 +43,8 @@ class ContextMenusControllerTest < ActionController::TestCase assert_tag :tag => 'a', :content => 'Dave Lopper', :attributes => { :href => '/issues/bulk_update?ids%5B%5D=1&issue%5Bassigned_to_id%5D=3', :class => '' } - assert_tag :tag => 'a', :content => 'Duplicate', - :attributes => { :href => '/projects/ecookbook/issues/1/copy', - :class => 'icon-duplicate' } assert_tag :tag => 'a', :content => 'Copy', - :attributes => { :href => '/issues/bulk_edit?copy=1&ids%5B%5D=1', + :attributes => { :href => '/projects/ecookbook/issues/1/copy', :class => 'icon-copy' } assert_no_tag :tag => 'a', :content => 'Move' assert_tag :tag => 'a', :content => 'Delete', diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 9b09d7f6b..5879e666b 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -1571,13 +1571,63 @@ class IssuesControllerTest < ActionController::TestCase end end - def test_copy_issue + def test_new_as_copy @request.session[:user_id] = 2 get :new, :project_id => 1, :copy_from => 1 + + assert_response :success assert_template 'new' + assert_not_nil assigns(:issue) orig = Issue.find(1) + assert_equal 1, assigns(:issue).project_id assert_equal orig.subject, assigns(:issue).subject + assert assigns(:issue).copy? + + assert_tag 'form', :attributes => {:id => 'issue-form', :action => '/projects/ecookbook/issues'} + assert_tag 'select', :attributes => {:name => 'issue[project_id]'} + assert_tag 'select', :attributes => {:name => 'issue[project_id]'}, + :child => {:tag => 'option', :attributes => {:value => '1', :selected => 'selected'}, :content => 'eCookbook'} + assert_tag 'select', :attributes => {:name => 'issue[project_id]'}, + :child => {:tag => 'option', :attributes => {:value => '2', :selected => nil}, :content => 'OnlineStore'} + assert_tag 'input', :attributes => {:name => 'copy_from', :value => '1'} + end + + def test_create_as_copy_on_different_project + @request.session[:user_id] = 2 + assert_difference 'Issue.count' do + post :create, :project_id => 1, :copy_from => 1, + :issue => {:project_id => '2', :tracker_id => '3', :status_id => '1', :subject => 'Copy'} + + assert_not_nil assigns(:issue) + assert assigns(:issue).copy? + end + issue = Issue.first(:order => 'id DESC') + assert_redirected_to "/issues/#{issue.id}" + + assert_equal 2, issue.project_id + assert_equal 3, issue.tracker_id + assert_equal 'Copy', issue.subject + end + + def test_create_as_copy_with_failure + @request.session[:user_id] = 2 + post :create, :project_id => 1, :copy_from => 1, + :issue => {:project_id => '2', :tracker_id => '3', :status_id => '1', :subject => ''} + + assert_response :success + assert_template 'new' + + assert_not_nil assigns(:issue) + assert assigns(:issue).copy? + + assert_tag 'form', :attributes => {:id => 'issue-form', :action => '/projects/ecookbook/issues'} + assert_tag 'select', :attributes => {:name => 'issue[project_id]'} + assert_tag 'select', :attributes => {:name => 'issue[project_id]'}, + :child => {:tag => 'option', :attributes => {:value => '1', :selected => nil}, :content => 'eCookbook'} + assert_tag 'select', :attributes => {:name => 'issue[project_id]'}, + :child => {:tag => 'option', :attributes => {:value => '2', :selected => 'selected'}, :content => 'OnlineStore'} + assert_tag 'input', :attributes => {:name => 'copy_from', :value => '1'} end def test_get_edit diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 6d6748999..0e49a7fe0 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -388,6 +388,7 @@ class IssueTest < ActiveSupport::TestCase def test_copy issue = Issue.new.copy_from(1) + assert issue.copy? assert issue.save issue.reload orig = Issue.find(1)