From 88fcf484d4d3c8d591cc31d7d07790110161d923 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 29 Nov 2009 19:46:40 +0000 Subject: [PATCH] Enable tracker update on issue edit form (#2405). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@3108 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/issues_controller.rb | 15 ++++++-- app/models/issue.rb | 12 ++++++ app/views/issues/_attributes.rhtml | 34 +++++++++++++++++ app/views/issues/_form.rhtml | 46 ++++------------------- lib/redmine.rb | 4 +- test/functional/issues_controller_test.rb | 31 +++++++++++++-- test/unit/issue_test.rb | 23 ++++++++++++ 7 files changed, 118 insertions(+), 47 deletions(-) create mode 100644 app/views/issues/_attributes.rhtml diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index d0a34d7b..02be772a 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -22,7 +22,7 @@ class IssuesController < ApplicationController before_filter :find_issue, :only => [:show, :edit, :reply] before_filter :find_issues, :only => [:bulk_edit, :move, :destroy] before_filter :find_project, :only => [:new, :update_form, :preview] - before_filter :authorize, :except => [:index, :changes, :gantt, :calendar, :preview, :update_form, :context_menu] + before_filter :authorize, :except => [:index, :changes, :gantt, :calendar, :preview, :context_menu] before_filter :find_optional_project, :only => [:index, :changes, :gantt, :calendar] accept_key_auth :index, :show, :changes @@ -429,8 +429,17 @@ class IssuesController < ApplicationController end def update_form - @issue = Issue.new(params[:issue]) - render :action => :new, :layout => false + if params[:id] + @issue = @project.issues.visible.find(params[:id]) + else + @issue = Issue.new + @issue.project = @project + end + @issue.attributes = params[:issue] + @allowed_statuses = ([@issue.status] + @issue.status.find_new_statuses_allowed_to(User.current.roles_for_project(@project), @issue.tracker)).uniq + @priorities = IssuePriority.all + + render :partial => 'attributes' end def preview diff --git a/app/models/issue.rb b/app/models/issue.rb index cfc2a1bc..658c8efb 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -127,6 +127,11 @@ class Issue < ActiveRecord::Base self.priority = nil write_attribute(:priority_id, pid) end + + def tracker_id=(tid) + self.tracker = nil + write_attribute(:tracker_id, tid) + end def estimated_hours=(h) write_attribute :estimated_hours, (h.is_a?(String) ? h.to_hours : h) @@ -152,6 +157,13 @@ class Issue < ActiveRecord::Base errors.add_to_base I18n.t(:error_can_not_reopen_issue_on_closed_version) end end + + # Checks that the issue can not be added/moved to a disabled tracker + if project && (tracker_id_changed? || project_id_changed?) + unless project.trackers.include?(tracker) + errors.add :tracker_id, :inclusion + end + end end def validate_on_create diff --git a/app/views/issues/_attributes.rhtml b/app/views/issues/_attributes.rhtml new file mode 100644 index 00000000..cfa14d52 --- /dev/null +++ b/app/views/issues/_attributes.rhtml @@ -0,0 +1,34 @@ +<% fields_for :issue, @issue, :builder => TabularFormBuilder do |f| %> + +
+<% if @issue.new_record? || @allowed_statuses.any? %> +

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

+<% else %> +

<%= @issue.status.name %>

+<% end %> + +

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

+

<%= f.select :assigned_to_id, (@issue.assignable_users.collect {|m| [m.name, m.id]}), :include_blank => true %>

+<% unless @project.issue_categories.empty? %> +

<%= f.select :category_id, (@project.issue_categories.collect {|c| [c.name, c.id]}), :include_blank => true %> +<%= prompt_to_remote(l(:label_issue_category_new), + l(:label_issue_category_new), 'category[name]', + {:controller => 'projects', :action => 'add_issue_category', :id => @project}, + :class => 'small', :tabindex => 199) if authorize_for('projects', 'add_issue_category') %>

+<% end %> +<% unless @issue.assignable_versions.empty? %> +

<%= f.select :fixed_version_id, (@issue.assignable_versions.collect {|v| [v.name, v.id]}), :include_blank => true %>

+<% end %> +
+ +
+

<%= f.text_field :start_date, :size => 10 %><%= calendar_for('issue_start_date') %>

+

<%= f.text_field :due_date, :size => 10 %><%= calendar_for('issue_due_date') %>

+

<%= f.text_field :estimated_hours, :size => 3 %> <%= l(:field_hours) %>

+

<%= f.select :done_ratio, ((0..10).to_a.collect {|r| ["#{r*10} %", r*10] }) %>

+
+ +
+<%= render :partial => 'form_custom_fields' %> + +<% end %> diff --git a/app/views/issues/_form.rhtml b/app/views/issues/_form.rhtml index c61c79d6..5337b51b 100644 --- a/app/views/issues/_form.rhtml +++ b/app/views/issues/_form.rhtml @@ -1,12 +1,9 @@ -<% 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 %> -
> +

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

+<%= observe_field :issue_tracker_id, :url => { :action => :update_form, :project_id => @project, :id => @issue }, + :update => :attributes, + :with => "Form.serialize('issue-form')" %> +

<%= f.text_field :subject, :size => 80, :required => true %>

<%= f.text_area :description, :cols => 60, @@ -15,37 +12,8 @@ :class => 'wiki-edit' %>

-
-
-<% if @issue.new_record? || @allowed_statuses.any? %> -

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

-<% else %> -

<%= @issue.status.name %>

-<% end %> - -

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

-

<%= f.select :assigned_to_id, (@issue.assignable_users.collect {|m| [m.name, m.id]}), :include_blank => true %>

-<% unless @project.issue_categories.empty? %> -

<%= f.select :category_id, (@project.issue_categories.collect {|c| [c.name, c.id]}), :include_blank => true %> -<%= prompt_to_remote(l(:label_issue_category_new), - l(:label_issue_category_new), 'category[name]', - {:controller => 'projects', :action => 'add_issue_category', :id => @project}, - :class => 'small', :tabindex => 199) if authorize_for('projects', 'add_issue_category') %>

-<% end %> -<% unless @issue.assignable_versions.empty? %> -

<%= f.select :fixed_version_id, (@issue.assignable_versions.collect {|v| [v.name, v.id]}), :include_blank => true %>

-<% end %> -
- -
-

<%= f.text_field :start_date, :size => 10 %><%= calendar_for('issue_start_date') %>

-

<%= f.text_field :due_date, :size => 10 %><%= calendar_for('issue_due_date') %>

-

<%= f.text_field :estimated_hours, :size => 3 %> <%= l(:field_hours) %>

-

<%= f.select :done_ratio, ((0..10).to_a.collect {|r| ["#{r*10} %", r*10] }) %>

-
- -
-<%= render :partial => 'form_custom_fields' %> +
+ <%= render :partial => 'attributes' %>
<% if @issue.new_record? %> diff --git a/lib/redmine.rb b/lib/redmine.rb index 1b88ee10..8dbe5d0e 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -42,8 +42,8 @@ Redmine::AccessControl.map do |map| :versions => [:show, :status_by], :queries => :index, :reports => :issue_report} - map.permission :add_issues, {:issues => :new} - map.permission :edit_issues, {:issues => [:edit, :reply, :bulk_edit]} + map.permission :add_issues, {:issues => [:new, :update_form]} + map.permission :edit_issues, {:issues => [:edit, :reply, :bulk_edit, :update_form]} map.permission :manage_issue_relations, {:issue_relations => [:new, :destroy]} map.permission :add_issue_notes, {:issues => [:edit, :reply]} map.permission :edit_issue_notes, {:journals => :edit}, :require => :loggedin diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 9555db4e..4c6b2535 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -501,14 +501,20 @@ class IssuesControllerTest < ActionController::TestCase def test_update_new_form @request.session[:user_id] = 2 - xhr :post, :new, :project_id => 1, + xhr :post, :update_form, :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 + assert_template 'attributes' + + issue = assigns(:issue) + assert_kind_of Issue, issue + assert_equal 1, issue.project_id + assert_equal 2, issue.tracker_id + assert_equal 'This is the test_new issue', issue.subject + end def test_post_new @request.session[:user_id] = 2 @@ -698,6 +704,25 @@ class IssuesControllerTest < ActionController::TestCase :content => 'Urgent', :attributes => { :selected => 'selected' } } end + + def test_update_edit_form + @request.session[:user_id] = 2 + xhr :post, :update_form, :project_id => 1, + :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 'attributes' + + issue = assigns(:issue) + assert_kind_of Issue, issue + assert_equal 1, issue.id + assert_equal 1, issue.project_id + assert_equal 2, issue.tracker_id + assert_equal 'This is the test_new issue', issue.subject + end def test_reply_routing assert_routing( diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 3b0dd269..afde6c72 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -164,6 +164,29 @@ class IssueTest < ActiveSupport::TestCase assert_equal custom_value.id, issue.custom_value_for(field).id end + def test_should_update_issue_with_disabled_tracker + p = Project.find(1) + issue = Issue.find(1) + + p.trackers.delete(issue.tracker) + assert !p.trackers.include?(issue.tracker) + + issue.reload + issue.subject = 'New subject' + assert issue.save + end + + def test_should_not_set_a_disabled_tracker + p = Project.find(1) + p.trackers.delete(Tracker.find(2)) + + issue = Issue.find(1) + issue.tracker_id = 2 + issue.subject = 'New subject' + assert !issue.save + assert_not_nil issue.errors.on(:tracker_id) + end + def test_category_based_assignment issue = Issue.create(:project_id => 1, :tracker_id => 1, :author_id => 3, :status_id => 1, :priority => IssuePriority.all.first, :subject => 'Assignment test', :description => 'Assignment test', :category_id => 1) assert_equal IssueCategory.find(1).assigned_to, issue.assigned_to