Allows project to be changed from the regular issue update action (#4769, #9803).

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

8 files changed, 160 insertions, 23 deletions

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index 6433acc5f..076a3a663 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -135,7 +135,17 @@ class IssuesController < ApplicationController
   def new
     respond_to do |format|
       format.html { render :action => 'new', :layout => !request.xhr? }
-      format.js { render :partial => 'attributes' }
+      format.js {
+        render(:update) { |page|
+          if params[:project_change]
+            page.replace_html 'all_attributes', :partial => 'form'
+          else
+            page.replace_html 'attributes', :partial => 'attributes'
+          end
+          m = User.current.allowed_to?(:log_time, @issue.project) ? 'show' : 'hide'
+          page << "if ($('log_time')) {Element.#{m}('log_time');}"
+        }
+      }
     end
   end
 
@@ -274,7 +284,7 @@ private
   end
 
   def find_project
-    project_id = (params[:issue] && params[:issue][:project_id]) || params[:project_id]
+    project_id = params[:project_id] || (params[:issue] && params[:issue][:project_id])
     @project = Project.find(project_id)
   rescue ActiveRecord::RecordNotFound
     render_404
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 0427608ba..16707f8ad 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -147,7 +147,9 @@ class Issue < ActiveRecord::Base
 
     issue.init_journal(User.current, options[:notes])
 
-    issue.project = new_project
+    # Preserve previous behaviour
+    # #move_to_project doesn't change tracker automatically
+    issue.send :project=, new_project, true
     if new_tracker
       issue.tracker = new_tracker
     end
@@ -169,6 +171,16 @@ class Issue < ActiveRecord::Base
     write_attribute(:priority_id, pid)
   end
 
+  def category_id=(cid)
+    self.category = nil
+    write_attribute(:category_id, cid)
+  end
+
+  def fixed_version_id=(vid)
+    self.fixed_version = nil
+    write_attribute(:fixed_version_id, vid)
+  end
+
   def tracker_id=(tid)
     self.tracker = nil
     result = write_attribute(:tracker_id, tid)
@@ -182,11 +194,14 @@ class Issue < ActiveRecord::Base
     end
   end
 
-  def project=(project)
+  def project=(project, keep_tracker=false)
     project_was = self.project
     write_attribute(:project_id, project ? project.id : nil)
     association_instance_set('project', project)
     if project_was && project && project_was != project
+      unless keep_tracker || project.trackers.include?(tracker)
+        self.tracker = project.trackers.first
+      end
       # Reassign to the category with same name if any
       if category
         self.category = project.issue_categories.find_by_name(category.name)
@@ -229,6 +244,12 @@ class Issue < ActiveRecord::Base
     write_attribute :estimated_hours, (h.is_a?(String) ? h.to_hours : h)
   end
 
+  safe_attributes 'project_id',
+    :if => lambda {|issue, user|
+      projects = Issue.allowed_target_projects_on_move(user)
+      projects.include?(issue.project) && projects.size > 1
+    }
+
   safe_attributes 'tracker_id',
     'status_id',
     'category_id',
@@ -278,7 +299,11 @@ class Issue < ActiveRecord::Base
     attrs = delete_unsafe_attributes(attrs, user)
     return if attrs.empty?
 
-    # Tracker must be set before since new_statuses_allowed_to depends on it.
+    # Project and Tracker must be set before since new_statuses_allowed_to depends on it.
+    if p = attrs.delete('project_id')
+      self.project_id = p
+    end
+    
     if t = attrs.delete('tracker_id')
       self.tracker_id = t
     end
@@ -725,16 +750,16 @@ class Issue < ActiveRecord::Base
   # End ReportsController extraction
 
   # Returns an array of projects that current user can move issues to
-  def self.allowed_target_projects_on_move
+  def self.allowed_target_projects_on_move(user=User.current)
     projects = []
-    if User.current.admin?
+    if user.admin?
       # admin is allowed to move issues to any active (visible) project
-      projects = Project.visible.all
-    elsif User.current.logged?
+      projects = Project.visible(user).all
+    elsif user.logged?
       if Role.non_member.allowed_to?(:move_issues)
-        projects = Project.visible.all
+        projects = Project.visible(user).all
       else
-        User.current.memberships.each {|m| projects << m.project if m.roles.detect {|r| r.allowed_to?(:move_issues)}}
+        user.memberships.each {|m| projects << m.project if m.roles.detect {|r| r.allowed_to?(:move_issues)}}
       end
     end
     projects
@@ -754,7 +779,8 @@ class Issue < ActiveRecord::Base
 
     # Move subtasks
     children.each do |child|
-      child.project = project
+      # Change project and keep project
+      child.send :project=, project, true
       unless child.save
         raise ActiveRecord::Rollback
       end
diff --git a/app/views/issues/_attributes.html.erb b/app/views/issues/_attributes.html.erb
index 802c7194a..fed949e24 100644
--- a/app/views/issues/_attributes.html.erb
+++ b/app/views/issues/_attributes.html.erb
@@ -15,14 +15,14 @@
 <p><%= f.select :assigned_to_id, principals_options_for_select(@issue.assignable_users, @issue.assigned_to), :include_blank => true %></p>
 <% end %>
 
-<% if @issue.safe_attribute?('category_id') && @project.issue_categories.any? %>
-<p><%= f.select :category_id, (@project.issue_categories.collect {|c| [c.name, c.id]}), :include_blank => true %>
+<% if @issue.safe_attribute?('category_id') && @issue.project.issue_categories.any? %>
+<p><%= f.select :category_id, (@issue.project.issue_categories.collect {|c| [c.name, c.id]}), :include_blank => true %>
 <%= prompt_to_remote(image_tag('add.png', :style => 'vertical-align: middle;'),
                      l(:label_issue_category_new),
                      'issue_category[name]',
-                     {:controller => 'issue_categories', :action => 'create', :project_id => @project},
+                     {:controller => 'issue_categories', :action => 'create', :project_id => @issue.project},
                      :title => l(:label_issue_category_new),
-                     :tabindex => 199) if authorize_for('issue_categories', 'new') %></p>
+                     :tabindex => 199) if User.current.allowed_to?(:manage_categories, @issue.project) %></p>
 <% end %>
 
 <% if @issue.safe_attribute?('fixed_version_id') && @issue.assignable_versions.any? %>
@@ -30,9 +30,9 @@
 <%= prompt_to_remote(image_tag('add.png', :style => 'vertical-align: middle;'),
                      l(:label_version_new),
                      'version[name]',
-                     {:controller => 'versions', :action => 'create', :project_id => @project},
+                     {:controller => 'versions', :action => 'create', :project_id => @issue.project},
                      :title => l(:label_version_new),
-                     :tabindex => 200) if authorize_for('versions', 'new') %>
+                     :tabindex => 200) if User.current.allowed_to?(:manage_versions, @issue.project) %>
 </p>
 <% end %>
 </div>
@@ -41,7 +41,7 @@
 <% if @issue.safe_attribute? 'parent_issue_id' %>
 <p id="parent_issue"><%= f.text_field :parent_issue_id, :size => 10 %></p>
 <div id="parent_issue_candidates" class="autocomplete"></div>
-<%= javascript_tag "observeParentIssueField('#{auto_complete_issues_path(:id => @issue, :project_id => @project) }')" %>
+<%= javascript_tag "observeParentIssueField('#{auto_complete_issues_path(:id => @issue, :project_id => @issue.project) }')" %>
 <% end %>
 
 <% if @issue.safe_attribute? 'start_date' %>
diff --git a/app/views/issues/_edit.html.erb b/app/views/issues/_edit.html.erb
index 7e1d04826..4db1a3e0f 100644
--- a/app/views/issues/_edit.html.erb
+++ b/app/views/issues/_edit.html.erb
@@ -3,7 +3,9 @@
     <div class="box">
     <% if @edit_allowed || !@allowed_statuses.empty? %>
         <fieldset class="tabular"><legend><%= l(:label_change_properties) %></legend>
+				<div id="all_attributes">
         <%= render :partial => 'form', :locals => {:f => f} %>
+				</div>
         </fieldset>
     <% end %>
     <% if User.current.allowed_to?(:log_time, @project) %>
diff --git a/app/views/issues/_form.html.erb b/app/views/issues/_form.html.erb
index e63adfbb3..fdfcfdda4 100644
--- a/app/views/issues/_form.html.erb
+++ b/app/views/issues/_form.html.erb
@@ -1,3 +1,4 @@
+<% labelled_fields_for :issue, @issue do |f| %>
 <%= call_hook(:view_issues_form_details_top, { :issue => @issue, :form => f }) %>
 
 <% if @issue.safe_attribute? 'is_private' %>
@@ -6,10 +7,15 @@
 </p>
 <% end %>
 
+<% if !@issue.new_record? && @issue.safe_attribute?('project_id') %>
+<p><%= f.select :project_id, Issue.allowed_target_projects_on_move.collect {|t| [t.name, t.id]}, :required => true %></p>
+<%= observe_field :issue_project_id, :url => project_issue_form_path(@project, :id => @issue, :project_change => '1'),
+                                     :with => "Form.serialize('issue-form')" %>
+<% end %>
+
 <% if @issue.safe_attribute? 'tracker_id' %>
-<p><%= f.select :tracker_id, @project.trackers.collect {|t| [t.name, t.id]}, :required => true %></p>
+<p><%= f.select :tracker_id, @issue.project.trackers.collect {|t| [t.name, t.id]}, :required => true %></p>
 <%= observe_field :issue_tracker_id, :url => project_issue_form_path(@project, :id => @issue),
-                                     :update => :attributes,
                                      :with => "Form.serialize('issue-form')" %>
 <% end %>
 
@@ -39,3 +45,4 @@
 </div>
 
 <%= call_hook(:view_issues_form_details_bottom, { :issue => @issue, :form => f }) %>
+<% end %>
diff --git a/test/fixtures/roles.yml b/test/fixtures/roles.yml
index 57f5e6be0..0549ebe6c 100644
--- a/test/fixtures/roles.yml
+++ b/test/fixtures/roles.yml
@@ -152,7 +152,6 @@ roles_004:
     - :edit_issues
     - :manage_issue_relations
     - :add_issue_notes
-    - :move_issues
     - :save_queries
     - :view_gantt
     - :view_calendar
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index e76de084d..435ff82f0 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -721,6 +721,7 @@ class IssuesControllerTest < ActionController::TestCase
 
     assert_tag 'form', :attributes => {:id => 'issue-form'}
     assert_tag 'input', :attributes => {:name => 'issue[is_private]'}
+    assert_tag 'select', :attributes => {:name => 'issue[project_id]'}
     assert_tag 'select', :attributes => {:name => 'issue[tracker_id]'}
     assert_tag 'input', :attributes => {:name => 'issue[subject]'}
     assert_tag 'textarea', :attributes => {:name => 'issue[description]'}
@@ -748,6 +749,7 @@ class IssuesControllerTest < ActionController::TestCase
 
     assert_tag 'form', :attributes => {:id => 'issue-form'}
     assert_no_tag 'input', :attributes => {:name => 'issue[is_private]'}
+    assert_no_tag 'select', :attributes => {:name => 'issue[project_id]'}
     assert_no_tag 'select', :attributes => {:name => 'issue[tracker_id]'}
     assert_no_tag 'input', :attributes => {:name => 'issue[subject]'}
     assert_no_tag 'textarea', :attributes => {:name => 'issue[description]'}
@@ -774,6 +776,7 @@ class IssuesControllerTest < ActionController::TestCase
 
     assert_tag 'form', :attributes => {:id => 'issue-form'}
     assert_no_tag 'input', :attributes => {:name => 'issue[is_private]'}
+    assert_no_tag 'select', :attributes => {:name => 'issue[project_id]'}
     assert_no_tag 'select', :attributes => {:name => 'issue[tracker_id]'}
     assert_no_tag 'input', :attributes => {:name => 'issue[subject]'}
     assert_no_tag 'textarea', :attributes => {:name => 'issue[description]'}
@@ -1014,6 +1017,7 @@ class IssuesControllerTest < ActionController::TestCase
     assert_template 'new'
 
     assert_tag 'input', :attributes => {:name => 'issue[is_private]'}
+    assert_no_tag 'select', :attributes => {:name => 'issue[project_id]'}
     assert_tag 'select', :attributes => {:name => 'issue[tracker_id]'}
     assert_tag 'input', :attributes => {:name => 'issue[subject]'}
     assert_tag 'textarea', :attributes => {:name => 'issue[description]'}
@@ -1045,6 +1049,7 @@ class IssuesControllerTest < ActionController::TestCase
     assert_template 'new'
 
     assert_no_tag 'input', :attributes => {:name => 'issue[is_private]'}
+    assert_no_tag 'select', :attributes => {:name => 'issue[project_id]'}
     assert_tag 'select', :attributes => {:name => 'issue[tracker_id]'}
     assert_tag 'input', :attributes => {:name => 'issue[subject]'}
     assert_tag 'textarea', :attributes => {:name => 'issue[description]'}
@@ -1636,7 +1641,7 @@ class IssuesControllerTest < ActionController::TestCase
 
   def test_update_edit_form
     @request.session[:user_id] = 2
-    xhr :post, :new, :project_id => 1,
+    xhr :put, :new, :project_id => 1,
                              :id => 1,
                              :issue => {:tracker_id => 2,
                                         :subject => 'This is the test_new issue',
@@ -1653,6 +1658,27 @@ class IssuesControllerTest < ActionController::TestCase
     assert_equal 'This is the test_new issue', issue.subject
   end
 
+  def test_update_edit_form_with_project_change
+    @request.session[:user_id] = 2
+    xhr :put, :new, :project_id => 1,
+                             :id => 1,
+                             :project_change => '1',
+                             :issue => {:project_id => 2,
+                                        :tracker_id => 2,
+                                        :subject => 'This is the test_new issue',
+                                        :description => 'This is the description',
+                                        :priority_id => 5}
+    assert_response :success
+    assert_template 'form'
+
+    issue = assigns(:issue)
+    assert_kind_of Issue, issue
+    assert_equal 1, issue.id
+    assert_equal 2, issue.project_id
+    assert_equal 2, issue.tracker_id
+    assert_equal 'This is the test_new issue', issue.subject
+  end
+
   def test_update_using_invalid_http_verbs
     @request.session[:user_id] = 2
     subject = 'Updated by an invalid http verb'
@@ -1696,6 +1722,57 @@ class IssuesControllerTest < ActionController::TestCase
     assert mail.body.include?("Subject changed from #{old_subject} to #{new_subject}")
   end
 
+  def test_put_update_with_project_change
+    @request.session[:user_id] = 2
+    ActionMailer::Base.deliveries.clear
+
+    assert_difference('Journal.count') do
+      assert_difference('JournalDetail.count', 3) do
+        put :update, :id => 1, :issue => {:project_id => '2',
+                                         :tracker_id => '1', # no change
+                                         :priority_id => '6',
+                                         :category_id => '3'
+                                        }
+      end
+    end
+    assert_redirected_to :action => 'show', :id => '1'
+    issue = Issue.find(1)
+    assert_equal 2, issue.project_id
+    assert_equal 1, issue.tracker_id
+    assert_equal 6, issue.priority_id
+    assert_equal 3, issue.category_id
+
+    mail = ActionMailer::Base.deliveries.last
+    assert_not_nil mail
+    assert mail.subject.starts_with?("[#{issue.project.name} - #{issue.tracker.name} ##{issue.id}]")
+    assert mail.body.include?("Project changed from eCookbook to OnlineStore")
+  end
+
+  def test_put_update_with_tracker_change
+    @request.session[:user_id] = 2
+    ActionMailer::Base.deliveries.clear
+
+    assert_difference('Journal.count') do
+      assert_difference('JournalDetail.count', 2) do
+        put :update, :id => 1, :issue => {:project_id => '1',
+                                         :tracker_id => '2',
+                                         :priority_id => '6'
+                                        }
+      end
+    end
+    assert_redirected_to :action => 'show', :id => '1'
+    issue = Issue.find(1)
+    assert_equal 1, issue.project_id
+    assert_equal 2, issue.tracker_id
+    assert_equal 6, issue.priority_id
+    assert_equal 1, issue.category_id
+
+    mail = ActionMailer::Base.deliveries.last
+    assert_not_nil mail
+    assert mail.subject.starts_with?("[#{issue.project.name} - #{issue.tracker.name} ##{issue.id}]")
+    assert mail.body.include?("Tracker changed from Bug to Feature request")
+  end
+
   def test_put_update_with_custom_field_change
     @request.session[:user_id] = 2
     issue = Issue.find(1)
diff --git a/test/integration/api_test/issues_test.rb b/test/integration/api_test/issues_test.rb
index 9244ba5b5..0b7593e3f 100644
--- a/test/integration/api_test/issues_test.rb
+++ b/test/integration/api_test/issues_test.rb
@@ -455,6 +455,22 @@ class ApiTest::IssuesTest < ActionController::IntegrationTest
     end
   end
 
+  context "PUT /issues/3.xml with project change" do
+    setup do
+      @parameters = {:issue => {:project_id => 2, :subject => 'Project changed'}}
+    end
+
+    should "update project" do
+      assert_no_difference('Issue.count') do
+        put '/issues/3.xml', @parameters, credentials('jsmith')
+      end
+
+      issue = Issue.find(3)
+      assert_equal 2, issue.project_id
+      assert_equal 'Project changed', issue.subject
+    end
+  end
+
   context "PUT /issues/6.xml with failed update" do
     setup do
       @parameters = {:issue => {:subject => ''}}