diff options
| -rw-r--r-- | app/controllers/issues_controller.rb | 14 | ||||
| -rw-r--r-- | app/models/issue.rb | 46 | ||||
| -rw-r--r-- | app/views/issues/_attributes.html.erb | 14 | ||||
| -rw-r--r-- | app/views/issues/_edit.html.erb | 2 | ||||
| -rw-r--r-- | app/views/issues/_form.html.erb | 11 | ||||
| -rw-r--r-- | test/fixtures/roles.yml | 1 | ||||
| -rw-r--r-- | test/functional/issues_controller_test.rb | 79 | ||||
| -rw-r--r-- | test/integration/api_test/issues_test.rb | 16 |
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 => ''}} |