From: Jean-Philippe Lang Date: Sun, 2 Nov 2014 19:45:14 +0000 (+0000) Subject: Default status per tracker (#5991). X-Git-Tag: 3.0.0~438 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=dfc594c33702a123674dcae1d6b4bfe3a2f32fd3;p=redmine.git Default status per tracker (#5991). git-svn-id: http://svn.redmine.org/redmine/trunk@13535 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 200d8b112..78f7c27ea 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -24,7 +24,6 @@ class IssuesController < ApplicationController before_filter :find_project, :only => [:new, :create, :update_form] before_filter :authorize, :except => [:index] before_filter :find_optional_project, :only => [:index] - before_filter :check_for_default_issue_status, :only => [:new, :create] before_filter :build_new_issue_from_params, :only => [:new, :create, :update_form] accept_rss_auth :index, :show accept_api_auth :index, :show, :create, :update, :destroy @@ -234,7 +233,8 @@ class IssuesController < ApplicationController target_projects ||= @projects if @copy - @available_statuses = [IssueStatus.default] + # Copied issues will get their default statuses + @available_statuses = [] else @available_statuses = @issues.map(&:new_statuses_allowed_to).reduce(:&) end @@ -425,12 +425,21 @@ class IssuesController < ApplicationController @issue = @project.issues.visible.find(params[:id]) end - @issue.safe_attributes = params[:issue] + if attrs = params[:issue].deep_dup + if params[:was_default_status] == attrs[:status_id] + attrs.delete(:status_id) + end + @issue.safe_attributes = attrs + end @issue.tracker ||= @project.trackers.first if @issue.tracker.nil? render_error l(:error_no_tracker_in_project) return false end + if @issue.status.nil? + render_error l(:error_no_default_issue_status) + return false + end @priorities = IssuePriority.active @allowed_statuses = @issue.new_statuses_allowed_to(User.current, @issue.new_record?) @@ -440,13 +449,6 @@ class IssuesController < ApplicationController end end - def check_for_default_issue_status - if IssueStatus.default.nil? - render_error l(:error_no_default_issue_status) - return false - end - end - def parse_params_for_bulk_issue_attributes(params) attributes = (params[:issue] || {}).reject {|k,v| v.blank?} attributes.keys.each {|k| attributes[k] = '' if attributes[k] == 'none'} diff --git a/app/models/issue.rb b/app/models/issue.rb index 144dffa9f..492e98687 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -162,7 +162,6 @@ class Issue < ActiveRecord::Base super if new_record? # set default values for new records only - self.status ||= IssueStatus.default self.priority ||= IssuePriority.default self.watcher_user_ids = [] end @@ -273,11 +272,19 @@ class Issue < ActiveRecord::Base issue.save ? issue : false end - def status_id=(sid) - self.status = nil - result = write_attribute(:status_id, sid) - @workflow_rule_by_attribute = nil - result + def status_id=(status_id) + if status_id.to_s != self.status_id.to_s + self.status = (status_id.present? ? IssueStatus.find_by_id(status_id) : nil) + end + self.status_id + end + + # Sets the status. + def self.status=(status) + if status != self.status + @workflow_rule_by_attribute = nil + end + association(:status).writer(status) end def priority_id=(pid) @@ -302,12 +309,24 @@ class Issue < ActiveRecord::Base self.tracker_id end + # Sets the tracker. + # This will set the status to the default status of the new tracker if: + # * the status was the default for the previous tracker + # * or if the status was not part of the new tracker statuses + # * or the status was nil def tracker=(tracker) if tracker != self.tracker + if status == default_status + self.status = nil + elsif status && tracker && !tracker.issue_status_ids.include?(status.id) + self.status = nil + end @custom_field_values = nil @workflow_rule_by_attribute = nil end association(:tracker).writer(tracker) + self.status ||= default_status + self.tracker end def project_id=(project_id) @@ -317,6 +336,14 @@ class Issue < ActiveRecord::Base self.project_id end + # Sets the project. + # Unless keep_tracker argument is set to true, this will change the tracker + # to the first tracker of the new project if the previous tracker is not part + # of the new project trackers. + # This will clear the fixed_version is it's no longer valid for the new project. + # This will clear the parent issue if it's no longer valid for the new project. + # This will set the category to the category with the same name in the new + # project if it exists, or clear it if it doesn't. def project=(project, keep_tracker=false) project_was = self.project association(:project).writer(project) @@ -339,7 +366,9 @@ class Issue < ActiveRecord::Base self.parent_issue_id = nil end @custom_field_values = nil + @workflow_rule_by_attribute = nil end + self.project end def description=(arg) @@ -776,14 +805,28 @@ class Issue < ActiveRecord::Base !relations_to.detect {|ir| ir.relation_type == 'blocks' && !ir.issue_from.closed?}.nil? end + # Returns the default status of the issue based on its tracker + # Returns nil if tracker is nil + def default_status + tracker.try(:default_status) + end + # Returns an array of statuses that user is able to apply def new_statuses_allowed_to(user=User.current, include_default=false) if new_record? && @copied_from - [IssueStatus.default, @copied_from.status].compact.uniq.sort + [default_status, @copied_from.status].compact.uniq.sort else initial_status = nil if new_record? - initial_status = IssueStatus.default + initial_status = default_status + elsif tracker_id_changed? + if Tracker.where(:id => tracker_id_was, :default_status_id => status_id_was).any? + initial_status = default_status + elsif tracker.issue_status_ids.include?(status_id_was) + initial_status = IssueStatus.find_by_id(status_id_was) + else + initial_status = default_status + end else initial_status = status_was end @@ -802,7 +845,7 @@ class Issue < ActiveRecord::Base ) end statuses << initial_status unless statuses.empty? - statuses << IssueStatus.default if include_default + statuses << default_status if include_default statuses = statuses.compact.uniq.sort if blocked? statuses.reject!(&:is_closed?) diff --git a/app/models/issue_status.rb b/app/models/issue_status.rb index 76c5ef65b..b8eb6d40e 100644 --- a/app/models/issue_status.rb +++ b/app/models/issue_status.rb @@ -22,7 +22,6 @@ class IssueStatus < ActiveRecord::Base acts_as_list before_destroy :delete_workflow_rules - after_save :update_default validates_presence_of :name validates_uniqueness_of :name @@ -33,15 +32,6 @@ class IssueStatus < ActiveRecord::Base scope :sorted, lambda { order(:position) } scope :named, lambda {|arg| where("LOWER(#{table_name}.name) = LOWER(?)", arg.to_s.strip)} - def update_default - IssueStatus.where(['id <> ?', id]).update_all({:is_default => false}) if self.is_default? - end - - # Returns the default status for new issues - def self.default - where(:is_default => true).first - end - # Update all the +Issues+ setting their done_ratio to the value of their +IssueStatus+ def self.update_issue_done_ratios if Issue.use_status_for_done_ratio? @@ -100,7 +90,11 @@ class IssueStatus < ActiveRecord::Base private def check_integrity - raise "Can't delete status" if Issue.where(:status_id => id).any? + if Issue.where(:status_id => id).any? + raise "This status is used by some issues" + elsif Tracker.where(:default_status_id => id).any? + raise "This status is used as the default status by some trackers" + end end # Deletes associated workflows diff --git a/app/models/tracker.rb b/app/models/tracker.rb index 1c867c5f1..4b86a7ecb 100644 --- a/app/models/tracker.rb +++ b/app/models/tracker.rb @@ -24,6 +24,7 @@ class Tracker < ActiveRecord::Base CORE_FIELDS_ALL = (CORE_FIELDS_UNDISABLABLE + CORE_FIELDS).freeze before_destroy :check_integrity + belongs_to :default_status, :class_name => 'IssueStatus' has_many :issues has_many :workflow_rules, :dependent => :delete_all do def copy(source_tracker) @@ -37,6 +38,7 @@ class Tracker < ActiveRecord::Base attr_protected :fields_bits + validates_presence_of :default_status validates_presence_of :name validates_uniqueness_of :name validates_length_of :name, :maximum => 30 @@ -53,14 +55,15 @@ class Tracker < ActiveRecord::Base # Returns an array of IssueStatus that are used # in the tracker's workflows def issue_statuses - if @issue_statuses - return @issue_statuses - elsif new_record? - return [] - end + @issue_statuses ||= IssueStatus.where(:id => issue_status_ids).to_a.sort + end - status_ids = WorkflowTransition.where(:tracker_id => id).uniq.pluck(:old_status_id, :new_status_id).flatten.uniq - @issue_statuses = IssueStatus.where(:id => status_ids).to_a.sort + def issue_status_ids + if new_record? + [] + else + @issue_status_ids ||= WorkflowTransition.where(:tracker_id => id).uniq.pluck(:old_status_id, :new_status_id).flatten.uniq + end end def disabled_core_fields diff --git a/app/views/issue_statuses/_form.html.erb b/app/views/issue_statuses/_form.html.erb index 37898737c..2c333a304 100644 --- a/app/views/issue_statuses/_form.html.erb +++ b/app/views/issue_statuses/_form.html.erb @@ -6,7 +6,6 @@

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

<% end %>

<%= f.check_box :is_closed %>

-

<%= f.check_box :is_default %>

<%= call_hook(:view_issue_statuses_form, :issue_status => @issue_status) %> diff --git a/app/views/issue_statuses/index.api.rsb b/app/views/issue_statuses/index.api.rsb index 8a51674d9..4f3b732d8 100644 --- a/app/views/issue_statuses/index.api.rsb +++ b/app/views/issue_statuses/index.api.rsb @@ -3,7 +3,6 @@ api.array :issue_statuses do api.issue_status do api.id status.id api.name status.name - api.is_default status.is_default api.is_closed status.is_closed end end diff --git a/app/views/issue_statuses/index.html.erb b/app/views/issue_statuses/index.html.erb index 659dd18fa..d4a2ecbb2 100644 --- a/app/views/issue_statuses/index.html.erb +++ b/app/views/issue_statuses/index.html.erb @@ -11,7 +11,6 @@ <% if Issue.use_status_for_done_ratio? %> <%=l(:field_done_ratio)%> <% end %> - <%=l(:field_is_default)%> <%=l(:field_is_closed)%> <%=l(:button_sort)%> @@ -23,7 +22,6 @@ <% if Issue.use_status_for_done_ratio? %> <%= h status.default_done_ratio %> <% end %> - <%= checked_image status.is_default? %> <%= checked_image status.is_closed? %> <%= reorder_links('issue_status', {:action => 'update', :id => status}, :put) %> diff --git a/app/views/issues/_attributes.html.erb b/app/views/issues/_attributes.html.erb index 7cae91f0c..01d262e57 100644 --- a/app/views/issues/_attributes.html.erb +++ b/app/views/issues/_attributes.html.erb @@ -5,9 +5,9 @@ <% if @issue.safe_attribute?('status_id') && @allowed_statuses.present? %>

<%= f.select :status_id, (@allowed_statuses.collect {|p| [p.name, p.id]}), {:required => true}, :onchange => "updateIssueFrom('#{escape_javascript project_issue_form_path(@project, :id => @issue, :format => 'js')}')" %>

- +<%= hidden_field_tag 'was_default_status', @issue.status_id, :id => nil if @issue.status == @issue.default_status %> <% else %> -

<%= h(@issue.status.name) %>

+

<%= @issue.status %>

<% end %> <% if @issue.safe_attribute? 'priority_id' %> diff --git a/app/views/trackers/_form.html.erb b/app/views/trackers/_form.html.erb index 4c73555f9..5a7416548 100644 --- a/app/views/trackers/_form.html.erb +++ b/app/views/trackers/_form.html.erb @@ -4,8 +4,12 @@

<%= f.text_field :name, :required => true %>

+

<%= f.select :default_status_id, + IssueStatus.sorted.map {|s| [s.name, s.id]}, + :include_blank => @tracker.default_status.nil?, + :required => true %> +

<%= f.check_box :is_in_roadmap %>

-

<% Tracker::CORE_FIELDS.each do |field| %> diff --git a/app/views/trackers/index.api.rsb b/app/views/trackers/index.api.rsb index a37c552a8..de8579124 100644 --- a/app/views/trackers/index.api.rsb +++ b/app/views/trackers/index.api.rsb @@ -3,6 +3,7 @@ api.array :trackers do api.tracker do api.id tracker.id api.name tracker.name + api.default_status(:id => tracker.default_status.id, :name => tracker.default_status.name) unless tracker.default_status.nil? end end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 3799db2bf..66de43259 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -337,6 +337,7 @@ en: field_inherit_members: Inherit members field_generate_password: Generate password field_must_change_passwd: Must change password at next logon + field_default_status: Default status setting_app_title: Application title setting_app_subtitle: Application subtitle diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 1b106ffe7..e575388e8 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -357,6 +357,7 @@ fr: field_inherit_members: Hériter les membres field_generate_password: Générer un mot de passe field_must_change_passwd: Doit changer de mot de passe à la prochaine connexion + field_default_status: Statut par défaut setting_app_title: Titre de l'application setting_app_subtitle: Sous-titre de l'application diff --git a/db/migrate/20141029181752_add_trackers_default_status_id.rb b/db/migrate/20141029181752_add_trackers_default_status_id.rb new file mode 100644 index 000000000..c0315df70 --- /dev/null +++ b/db/migrate/20141029181752_add_trackers_default_status_id.rb @@ -0,0 +1,15 @@ +class AddTrackersDefaultStatusId < ActiveRecord::Migration + def up + add_column :trackers, :default_status_id, :integer + + status_id = IssueStatus.where(:is_default => true).pluck(:id).first + status_id ||= IssueStatus.order(:position).pluck(:id).first + if status_id + Tracker.update_all :default_status_id => status_id + end + end + + def down + remove_column :trackers, :default_status_id + end +end diff --git a/db/migrate/20141029181824_remove_issue_statuses_is_default.rb b/db/migrate/20141029181824_remove_issue_statuses_is_default.rb new file mode 100644 index 000000000..67a1696c0 --- /dev/null +++ b/db/migrate/20141029181824_remove_issue_statuses_is_default.rb @@ -0,0 +1,12 @@ +class RemoveIssueStatusesIsDefault < ActiveRecord::Migration + def up + remove_column :issue_statuses, :is_default + end + + def down + add_column :issue_statuses, :is_default, :boolean, :null => false, :default => false + # Restores the first status as default + default_status_id = IssueStatus.order("position").first.pluck(:id) + IssueStatus.where(:id => default_status_id).update_all(:is_default => true) + end +end diff --git a/lib/redmine/default_data/loader.rb b/lib/redmine/default_data/loader.rb index 231a59175..9358795d5 100644 --- a/lib/redmine/default_data/loader.rb +++ b/lib/redmine/default_data/loader.rb @@ -125,18 +125,18 @@ module Redmine :browse_repository, :view_changesets] - # Trackers - Tracker.create!(:name => l(:default_tracker_bug), :is_in_chlog => true, :is_in_roadmap => false, :position => 1) - Tracker.create!(:name => l(:default_tracker_feature), :is_in_chlog => true, :is_in_roadmap => true, :position => 2) - Tracker.create!(:name => l(:default_tracker_support), :is_in_chlog => false, :is_in_roadmap => false, :position => 3) - # Issue statuses - new = IssueStatus.create!(:name => l(:default_issue_status_new), :is_closed => false, :is_default => true, :position => 1) - in_progress = IssueStatus.create!(:name => l(:default_issue_status_in_progress), :is_closed => false, :is_default => false, :position => 2) - resolved = IssueStatus.create!(:name => l(:default_issue_status_resolved), :is_closed => false, :is_default => false, :position => 3) - feedback = IssueStatus.create!(:name => l(:default_issue_status_feedback), :is_closed => false, :is_default => false, :position => 4) - closed = IssueStatus.create!(:name => l(:default_issue_status_closed), :is_closed => true, :is_default => false, :position => 5) - rejected = IssueStatus.create!(:name => l(:default_issue_status_rejected), :is_closed => true, :is_default => false, :position => 6) + new = IssueStatus.create!(:name => l(:default_issue_status_new), :is_closed => false, :position => 1) + in_progress = IssueStatus.create!(:name => l(:default_issue_status_in_progress), :is_closed => false, :position => 2) + resolved = IssueStatus.create!(:name => l(:default_issue_status_resolved), :is_closed => false, :position => 3) + feedback = IssueStatus.create!(:name => l(:default_issue_status_feedback), :is_closed => false, :position => 4) + closed = IssueStatus.create!(:name => l(:default_issue_status_closed), :is_closed => true, :position => 5) + rejected = IssueStatus.create!(:name => l(:default_issue_status_rejected), :is_closed => true, :position => 6) + + # Trackers + Tracker.create!(:name => l(:default_tracker_bug), :default_status_id => new.id, :is_in_chlog => true, :is_in_roadmap => false, :position => 1) + Tracker.create!(:name => l(:default_tracker_feature), :default_status_id => new.id, :is_in_chlog => true, :is_in_roadmap => true, :position => 2) + Tracker.create!(:name => l(:default_tracker_support), :default_status_id => new.id, :is_in_chlog => false, :is_in_roadmap => false, :position => 3) # Workflow Tracker.all.each { |t| diff --git a/lib/tasks/migrate_from_mantis.rake b/lib/tasks/migrate_from_mantis.rake index 1040dbb0a..d312238c0 100644 --- a/lib/tasks/migrate_from_mantis.rake +++ b/lib/tasks/migrate_from_mantis.rake @@ -25,15 +25,15 @@ task :migrate_from_mantis => :environment do module MantisMigrate - DEFAULT_STATUS = IssueStatus.default + new_status = IssueStatus.find_by_position(1) assigned_status = IssueStatus.find_by_position(2) resolved_status = IssueStatus.find_by_position(3) feedback_status = IssueStatus.find_by_position(4) closed_status = IssueStatus.where(:is_closed => true).first - STATUS_MAPPING = {10 => DEFAULT_STATUS, # new + STATUS_MAPPING = {10 => new_status, # new 20 => feedback_status, # feedback - 30 => DEFAULT_STATUS, # acknowledged - 40 => DEFAULT_STATUS, # confirmed + 30 => new_status, # acknowledged + 40 => new_status, # confirmed 50 => assigned_status, # assigned 80 => resolved_status, # resolved 90 => closed_status # closed @@ -317,8 +317,8 @@ task :migrate_from_mantis => :environment do i.author = User.find_by_id(users_map[bug.reporter_id]) i.category = IssueCategory.find_by_project_id_and_name(i.project_id, bug.category[0,30]) unless bug.category.blank? i.fixed_version = Version.find_by_project_id_and_name(i.project_id, bug.fixed_in_version) unless bug.fixed_in_version.blank? - i.status = STATUS_MAPPING[bug.status] || DEFAULT_STATUS i.tracker = (bug.severity == 10 ? TRACKER_FEATURE : TRACKER_BUG) + i.status = STATUS_MAPPING[bug.status] || i.status i.id = bug.id if keep_bug_ids next unless i.save issues_map[bug.id] = i.id diff --git a/lib/tasks/migrate_from_trac.rake b/lib/tasks/migrate_from_trac.rake index d1a3873f5..f3be605c1 100644 --- a/lib/tasks/migrate_from_trac.rake +++ b/lib/tasks/migrate_from_trac.rake @@ -25,12 +25,12 @@ namespace :redmine do module TracMigrate TICKET_MAP = [] - DEFAULT_STATUS = IssueStatus.default + new_status = IssueStatus.find_by_position(1) assigned_status = IssueStatus.find_by_position(2) resolved_status = IssueStatus.find_by_position(3) feedback_status = IssueStatus.find_by_position(4) closed_status = IssueStatus.where(:is_closed => true).first - STATUS_MAPPING = {'new' => DEFAULT_STATUS, + STATUS_MAPPING = {'new' => new_status, 'reopened' => feedback_status, 'assigned' => assigned_status, 'closed' => closed_status @@ -476,8 +476,8 @@ namespace :redmine do i.author = find_or_create_user(ticket.reporter) i.category = issues_category_map[ticket.component] unless ticket.component.blank? i.fixed_version = version_map[ticket.milestone] unless ticket.milestone.blank? - i.status = STATUS_MAPPING[ticket.status] || DEFAULT_STATUS i.tracker = TRACKER_MAPPING[ticket.ticket_type] || DEFAULT_TRACKER + i.status = STATUS_MAPPING[ticket.status] || i.default_status i.id = ticket.id unless Issue.exists?(ticket.id) next unless Time.fake(ticket.changetime) { i.save } TICKET_MAP[ticket.id] = i.id diff --git a/test/fixtures/issue_statuses.yml b/test/fixtures/issue_statuses.yml index ff40b1c50..56a58f20a 100644 --- a/test/fixtures/issue_statuses.yml +++ b/test/fixtures/issue_statuses.yml @@ -2,36 +2,30 @@ issue_statuses_001: id: 1 name: New - is_default: true is_closed: false position: 1 issue_statuses_002: id: 2 name: Assigned - is_default: false is_closed: false position: 2 issue_statuses_003: id: 3 name: Resolved - is_default: false is_closed: false position: 3 issue_statuses_004: name: Feedback id: 4 - is_default: false is_closed: false position: 4 issue_statuses_005: id: 5 name: Closed - is_default: false is_closed: true position: 5 issue_statuses_006: id: 6 name: Rejected - is_default: false is_closed: true position: 6 diff --git a/test/fixtures/trackers.yml b/test/fixtures/trackers.yml index 43395a2e8..c969549f6 100644 --- a/test/fixtures/trackers.yml +++ b/test/fixtures/trackers.yml @@ -3,14 +3,17 @@ trackers_001: name: Bug id: 1 is_in_chlog: true + default_status_id: 1 position: 1 trackers_002: name: Feature request id: 2 is_in_chlog: true + default_status_id: 1 position: 2 trackers_003: name: Support request id: 3 is_in_chlog: false + default_status_id: 1 position: 3 diff --git a/test/functional/issue_statuses_controller_test.rb b/test/functional/issue_statuses_controller_test.rb index 59fff8141..b8e0ea5b5 100644 --- a/test/functional/issue_statuses_controller_test.rb +++ b/test/functional/issue_statuses_controller_test.rb @@ -86,7 +86,8 @@ class IssueStatusesControllerTest < ActionController::TestCase end def test_destroy - Issue.delete_all("status_id = 1") + Issue.where(:status_id => 1).delete_all + Tracker.where(:default_status_id => 1).delete_all assert_difference 'IssueStatus.count', -1 do delete :destroy, :id => '1' @@ -96,7 +97,19 @@ class IssueStatusesControllerTest < ActionController::TestCase end def test_destroy_should_block_if_status_in_use - assert_not_nil Issue.find_by_status_id(1) + assert Issue.where(:status_id => 1).any? + Tracker.where(:default_status_id => 1).delete_all + + assert_no_difference 'IssueStatus.count' do + delete :destroy, :id => '1' + end + assert_redirected_to :action => 'index' + assert_not_nil IssueStatus.find_by_id(1) + end + + def test_destroy_should_block_if_status_in_use + Issue.where(:status_id => 1).delete_all + assert Tracker.where(:default_status_id => 1).any? assert_no_difference 'IssueStatus.count' do delete :destroy, :id => '1' diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 6b2dd7110..8ffb9c848 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -1511,6 +1511,30 @@ class IssuesControllerTest < ActionController::TestCase end end + def test_new_should_select_default_status + @request.session[:user_id] = 2 + + get :new, :project_id => 1 + assert_response :success + assert_template 'new' + assert_select 'select[name=?]', 'issue[status_id]' do + assert_select 'option[value=1][selected=selected]' + end + assert_select 'input[name=was_default_status][value=1]' + end + + def test_new_should_select_default_status + @request.session[:user_id] = 2 + + get :new, :project_id => 1 + assert_response :success + assert_template 'new' + assert_select 'select[name=?]', 'issue[status_id]' do + assert_select 'option[value=1][selected=selected]' + end + assert_select 'input[name=was_default_status][value=1]' + end + def test_get_new_with_list_custom_field @request.session[:user_id] = 2 get :new, :project_id => 1, :tracker_id => 1 @@ -1731,6 +1755,20 @@ class IssuesControllerTest < ActionController::TestCase assert_equal [1,2,5], assigns(:allowed_statuses).map(&:id).sort end + def test_update_form_with_default_status_should_ignore_submitted_status_id_if_equals + @request.session[:user_id] = 2 + tracker = Tracker.find(2) + tracker.update! :default_status_id => 2 + tracker.generate_transitions! 2, 1, :clear => true + + xhr :post, :update_form, :project_id => 1, + :issue => {:tracker_id => 2, + :status_id => 1}, + :was_default_status => 1 + + assert_equal 2, assigns(:issue).status_id + end + def test_post_create @request.session[:user_id] = 2 assert_difference 'Issue.count' do @@ -2266,13 +2304,17 @@ class IssuesControllerTest < ActionController::TestCase get :new, :project_id => 1 assert_response :success assert_template 'new' + + issue = assigns(:issue) + assert_not_nil issue.default_status + assert_select 'select[name=?]', 'issue[status_id]' do assert_select 'option', 1 - assert_select 'option[value=?]', IssueStatus.default.id.to_s + assert_select 'option[value=?]', issue.default_status.id.to_s end end - test "without workflow privilege #new should accept default status" do + test "without workflow privilege #create should accept default status" do setup_without_workflow_privilege assert_difference 'Issue.count' do post :create, :project_id => 1, @@ -2281,10 +2323,11 @@ class IssuesControllerTest < ActionController::TestCase :status_id => 1} end issue = Issue.order('id').last - assert_equal IssueStatus.default, issue.status + assert_not_nil issue.default_status + assert_equal issue.default_status, issue.status end - test "without workflow privilege #new should ignore unauthorized status" do + test "without workflow privilege #create should ignore unauthorized status" do setup_without_workflow_privilege assert_difference 'Issue.count' do post :create, :project_id => 1, @@ -2293,7 +2336,8 @@ class IssuesControllerTest < ActionController::TestCase :status_id => 3} end issue = Issue.order('id').last - assert_equal IssueStatus.default, issue.status + assert_not_nil issue.default_status + assert_equal issue.default_status, issue.status end test "without workflow privilege #update should ignore status change" do diff --git a/test/functional/trackers_controller_test.rb b/test/functional/trackers_controller_test.rb index e5246ce59..4e872926c 100644 --- a/test/functional/trackers_controller_test.rb +++ b/test/functional/trackers_controller_test.rb @@ -18,7 +18,7 @@ require File.expand_path('../../test_helper', __FILE__) class TrackersControllerTest < ActionController::TestCase - fixtures :trackers, :projects, :projects_trackers, :users, :issues, :custom_fields + fixtures :trackers, :projects, :projects_trackers, :users, :issues, :custom_fields, :issue_statuses def setup User.current = nil @@ -51,7 +51,7 @@ class TrackersControllerTest < ActionController::TestCase def test_create assert_difference 'Tracker.count' do - post :create, :tracker => { :name => 'New tracker', :project_ids => ['1', '', ''], :custom_field_ids => ['1', '6', ''] } + post :create, :tracker => { :name => 'New tracker', :default_status_id => 1, :project_ids => ['1', '', ''], :custom_field_ids => ['1', '6', ''] } end assert_redirected_to :action => 'index' tracker = Tracker.order('id DESC').first @@ -62,9 +62,9 @@ class TrackersControllerTest < ActionController::TestCase assert_equal 0, tracker.workflow_rules.count end - def create_with_disabled_core_fields + def test_create_with_disabled_core_fields assert_difference 'Tracker.count' do - post :create, :tracker => { :name => 'New tracker', :core_fields => ['assigned_to_id', 'fixed_version_id', ''] } + post :create, :tracker => { :name => 'New tracker', :default_status_id => 1, :core_fields => ['assigned_to_id', 'fixed_version_id', ''] } end assert_redirected_to :action => 'index' tracker = Tracker.order('id DESC').first @@ -74,7 +74,7 @@ class TrackersControllerTest < ActionController::TestCase def test_create_new_with_workflow_copy assert_difference 'Tracker.count' do - post :create, :tracker => { :name => 'New tracker' }, :copy_workflow_from => 1 + post :create, :tracker => { :name => 'New tracker', :default_status_id => 1 }, :copy_workflow_from => 1 end assert_redirected_to :action => 'index' tracker = Tracker.find_by_name('New tracker') @@ -164,7 +164,7 @@ class TrackersControllerTest < ActionController::TestCase end def test_destroy - tracker = Tracker.create!(:name => 'Destroyable') + tracker = Tracker.generate!(:name => 'Destroyable') assert_difference 'Tracker.count', -1 do delete :destroy, :id => tracker.id end diff --git a/test/object_helpers.rb b/test/object_helpers.rb index f3c671bf5..ca58638d9 100644 --- a/test/object_helpers.rb +++ b/test/object_helpers.rb @@ -45,11 +45,22 @@ module ObjectHelpers project end + def IssueStatus.generate!(attributes={}) + @generated_status_name ||= 'Status 0' + @generated_status_name.succ! + status = IssueStatus.new(attributes) + status.name = @generated_status_name.dup if status.name.blank? + yield status if block_given? + status.save! + status + end + def Tracker.generate!(attributes={}) @generated_tracker_name ||= 'Tracker 0' @generated_tracker_name.succ! tracker = Tracker.new(attributes) tracker.name = @generated_tracker_name.dup if tracker.name.blank? + tracker.default_status ||= IssueStatus.order('position').first || IssueStatus.generate! yield tracker if block_given? tracker.save! tracker @@ -188,6 +199,27 @@ module ObjectHelpers end end +module TrackerObjectHelpers + def generate_transitions!(*args) + options = args.last.is_a?(Hash) ? args.pop : {} + if args.size == 1 + args << args.first + end + if options[:clear] + WorkflowTransition.where(:tracker_id => id).delete_all + end + args.each_cons(2) do |old_status_id, new_status_id| + WorkflowTransition.create!( + :tracker => self, + :role_id => (options[:role_id] || 1), + :old_status_id => old_status_id, + :new_status_id => new_status_id + ) + end + end +end +Tracker.send :include, TrackerObjectHelpers + module IssueObjectHelpers def close! self.status = IssueStatus.where(:is_closed => true).first @@ -198,5 +230,4 @@ module IssueObjectHelpers Issue.generate!(attributes.merge(:parent_issue_id => self.id)) end end - Issue.send :include, IssueObjectHelpers diff --git a/test/unit/issue_status_test.rb b/test/unit/issue_status_test.rb index 387fa16a6..cb4ed90b8 100644 --- a/test/unit/issue_status_test.rb +++ b/test/unit/issue_status_test.rb @@ -28,7 +28,6 @@ class IssueStatusTest < ActiveSupport::TestCase status.name = "Test Status" assert status.save - assert !status.is_default end def test_destroy @@ -46,29 +45,6 @@ class IssueStatusTest < ActiveSupport::TestCase assert_raise(RuntimeError, "Can't delete status") { status.destroy } end - def test_default - status = IssueStatus.default - assert_kind_of IssueStatus, status - end - - def test_change_default - status = IssueStatus.find(2) - assert !status.is_default - status.is_default = true - assert status.save - status.reload - - assert_equal status, IssueStatus.default - assert !IssueStatus.find(1).is_default - end - - def test_reorder_should_not_clear_default_status - status = IssueStatus.default - status.move_to_bottom - status.reload - assert status.is_default? - end - def test_new_statuses_allowed_to WorkflowTransition.delete_all diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 834792458..46ae8bc51 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -40,11 +40,11 @@ class IssueTest < ActiveSupport::TestCase assert_nil issue.project_id assert_nil issue.tracker_id + assert_nil issue.status_id assert_nil issue.author_id assert_nil issue.assigned_to_id assert_nil issue.category_id - assert_equal IssueStatus.default, issue.status assert_equal IssuePriority.default, issue.priority end @@ -59,10 +59,9 @@ class IssueTest < ActiveSupport::TestCase end def test_create_minimal - issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 3, - :status_id => 1, :priority => IssuePriority.all.first, - :subject => 'test_create') + issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 3, :subject => 'test_create') assert issue.save + assert_equal issue.tracker.default_status, issue.status assert issue.description.nil? assert_nil issue.estimated_hours end @@ -908,7 +907,7 @@ class IssueTest < ActiveSupport::TestCase def test_copy_should_copy_status orig = Issue.find(8) - assert orig.status != IssueStatus.default + assert orig.status != orig.default_status issue = Issue.new.copy_from(orig) assert issue.save @@ -2480,4 +2479,67 @@ class IssueTest < ActiveSupport::TestCase issue.save! assert_equal false, issue.reopening? end + + def test_default_status_without_tracker_should_be_nil + issue = Issue.new + assert_nil issue.tracker + assert_nil issue.default_status + end + + def test_default_status_should_be_tracker_default_status + issue = Issue.new(:tracker_id => 1) + assert_not_nil issue.status + assert_equal issue.tracker.default_status, issue.default_status + end + + def test_initializing_with_tracker_should_set_default_status + issue = Issue.new(:tracker => Tracker.find(1)) + assert_not_nil issue.status + assert_equal issue.default_status, issue.status + end + + def test_initializing_with_tracker_id_should_set_default_status + issue = Issue.new(:tracker_id => 1) + assert_not_nil issue.status + assert_equal issue.default_status, issue.status + end + + def test_setting_tracker_should_set_default_status + issue = Issue.new + issue.tracker = Tracker.find(1) + assert_not_nil issue.status + assert_equal issue.default_status, issue.status + end + + def test_changing_tracker_should_set_default_status_if_status_was_default + WorkflowTransition.delete_all + WorkflowTransition.create! :role_id => 1, :tracker_id => 2, :old_status_id => 2, :new_status_id => 1 + Tracker.find(2).update! :default_status_id => 2 + + issue = Issue.new(:tracker_id => 1, :status_id => 1) + assert_equal IssueStatus.find(1), issue.status + issue.tracker = Tracker.find(2) + assert_equal IssueStatus.find(2), issue.status + end + + def test_changing_tracker_should_set_default_status_if_status_is_not_used_by_tracker + WorkflowTransition.delete_all + Tracker.find(2).update! :default_status_id => 2 + + issue = Issue.new(:tracker_id => 1, :status_id => 3) + assert_equal IssueStatus.find(3), issue.status + issue.tracker = Tracker.find(2) + assert_equal IssueStatus.find(2), issue.status + end + + def test_changing_tracker_should_keep_status_if_status_was_not_default_and_is_used_by_tracker + WorkflowTransition.delete_all + WorkflowTransition.create! :role_id => 1, :tracker_id => 2, :old_status_id => 2, :new_status_id => 3 + Tracker.find(2).update! :default_status_id => 2 + + issue = Issue.new(:tracker_id => 1, :status_id => 3) + assert_equal IssueStatus.find(3), issue.status + issue.tracker = Tracker.find(2) + assert_equal IssueStatus.find(3), issue.status + end end diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb index bb91ac3f7..6d250a4e2 100644 --- a/test/unit/mail_handler_test.rb +++ b/test/unit/mail_handler_test.rb @@ -412,7 +412,7 @@ class MailHandlerTest < ActiveSupport::TestCase def test_add_issue_with_japanese_keywords ja_dev = "\xe9\x96\x8b\xe7\x99\xba".force_encoding('UTF-8') - tracker = Tracker.create!(:name => ja_dev) + tracker = Tracker.generate!(:name => ja_dev) Project.find(1).trackers << tracker issue = submit_email( 'japanese_keywords_iso_2022_jp.eml', diff --git a/test/unit/tracker_test.rb b/test/unit/tracker_test.rb index 70d892b77..d9a445133 100644 --- a/test/unit/tracker_test.rb +++ b/test/unit/tracker_test.rb @@ -32,7 +32,7 @@ class TrackerTest < ActiveSupport::TestCase source = Tracker.find(1) assert_equal 89, source.workflow_rules.size - target = Tracker.new(:name => 'Target') + target = Tracker.new(:name => 'Target', :default_status_id => 1) assert target.save target.workflow_rules.copy(source) target.reload