mirror of
https://github.com/redmine/redmine.git
synced 2025-11-01 19:05:51 +01:00
Makes new issue initial status settable in workflow (#5816).
git-svn-id: http://svn.redmine.org/redmine/trunk@14458 e93f8b46-1217-0410-a6f0-8f06a7374b81
This commit is contained in:
@@ -427,12 +427,12 @@ class IssuesController < ApplicationController
|
||||
@issue.author ||= User.current
|
||||
@issue.start_date ||= Date.today if Setting.default_issue_start_date_to_creation_date?
|
||||
|
||||
if attrs = params[:issue].deep_dup
|
||||
attrs = (params[:issue] || {}).deep_dup
|
||||
if action_name == 'new' && params[:was_default_status] == attrs[:status_id]
|
||||
attrs.delete(:status_id)
|
||||
end
|
||||
@issue.safe_attributes = attrs
|
||||
end
|
||||
|
||||
if @issue.project
|
||||
@issue.tracker ||= @issue.project.trackers.first
|
||||
if @issue.tracker.nil?
|
||||
@@ -446,7 +446,7 @@ class IssuesController < ApplicationController
|
||||
end
|
||||
|
||||
@priorities = IssuePriority.active
|
||||
@allowed_statuses = @issue.new_statuses_allowed_to(User.current, @issue.new_record?)
|
||||
@allowed_statuses = @issue.new_statuses_allowed_to(User.current)
|
||||
end
|
||||
|
||||
def parse_params_for_bulk_issue_attributes(params)
|
||||
|
||||
@@ -43,7 +43,9 @@ class WorkflowsController < ApplicationController
|
||||
end
|
||||
|
||||
if @trackers && @roles && @statuses.any?
|
||||
workflows = WorkflowTransition.where(:role_id => @roles.map(&:id), :tracker_id => @trackers.map(&:id))
|
||||
workflows = WorkflowTransition.
|
||||
where(:role_id => @roles.map(&:id), :tracker_id => @trackers.map(&:id)).
|
||||
preload(:old_status, :new_status)
|
||||
@workflows = {}
|
||||
@workflows['always'] = workflows.select {|w| !w.author && !w.assignee}
|
||||
@workflows['author'] = workflows.select {|w| w.author}
|
||||
|
||||
@@ -75,14 +75,14 @@ module WorkflowsHelper
|
||||
end
|
||||
|
||||
def transition_tag(workflows, old_status, new_status, name)
|
||||
w = workflows.select {|w| w.old_status_id == old_status.id && w.new_status_id == new_status.id}.size
|
||||
w = workflows.select {|w| w.old_status == old_status && w.new_status == new_status}.size
|
||||
|
||||
tag_name = "transitions[#{ old_status.id }][#{new_status.id}][#{name}]"
|
||||
tag_name = "transitions[#{ old_status.try(:id) || 0 }][#{new_status.id}][#{name}]"
|
||||
if w == 0 || w == @roles.size * @trackers.size
|
||||
|
||||
hidden_field_tag(tag_name, "0", :id => nil) +
|
||||
check_box_tag(tag_name, "1", w != 0,
|
||||
:class => "old-status-#{old_status.id} new-status-#{new_status.id}")
|
||||
:class => "old-status-#{old_status.try(:id) || 0} new-status-#{new_status.id}")
|
||||
else
|
||||
select_tag tag_name,
|
||||
options_for_select([
|
||||
|
||||
@@ -465,11 +465,15 @@ class Issue < ActiveRecord::Base
|
||||
self.tracker ||= project.trackers.first
|
||||
end
|
||||
|
||||
statuses_allowed = new_statuses_allowed_to(user)
|
||||
if (s = attrs.delete('status_id')) && safe_attribute?('status_id')
|
||||
if new_statuses_allowed_to(user).collect(&:id).include?(s.to_i)
|
||||
if statuses_allowed.collect(&:id).include?(s.to_i)
|
||||
self.status_id = s
|
||||
end
|
||||
end
|
||||
if new_record? && !statuses_allowed.include?(status)
|
||||
self.status = statuses_allowed.first || default_status
|
||||
end
|
||||
|
||||
attrs = delete_unsafe_attributes(attrs, user)
|
||||
return if attrs.empty?
|
||||
@@ -825,7 +829,7 @@ class Issue < ActiveRecord::Base
|
||||
else
|
||||
initial_status = nil
|
||||
if new_record?
|
||||
initial_status = default_status
|
||||
# nop
|
||||
elsif tracker_id_changed?
|
||||
if Tracker.where(:id => tracker_id_was, :default_status_id => status_id_was).any?
|
||||
initial_status = default_status
|
||||
@@ -843,16 +847,15 @@ class Issue < ActiveRecord::Base
|
||||
(user.id == initial_assigned_to_id || user.group_ids.include?(initial_assigned_to_id))
|
||||
|
||||
statuses = []
|
||||
if initial_status
|
||||
statuses += initial_status.find_new_statuses_allowed_to(
|
||||
statuses += IssueStatus.new_statuses_allowed(
|
||||
initial_status,
|
||||
user.admin ? Role.all.to_a : user.roles_for_project(project),
|
||||
tracker,
|
||||
author == user,
|
||||
assignee_transitions_allowed
|
||||
)
|
||||
end
|
||||
statuses << initial_status unless statuses.empty?
|
||||
statuses << default_status if include_default
|
||||
statuses << default_status if include_default || (new_record? && statuses.empty?)
|
||||
statuses = statuses.compact.uniq.sort
|
||||
if blocked?
|
||||
statuses.reject!(&:is_closed?)
|
||||
|
||||
@@ -45,28 +45,18 @@ class IssueStatus < ActiveRecord::Base
|
||||
end
|
||||
|
||||
# Returns an array of all statuses the given role can switch to
|
||||
# Uses association cache when called more than one time
|
||||
def new_statuses_allowed_to(roles, tracker, author=false, assignee=false)
|
||||
if roles && tracker
|
||||
role_ids = roles.collect(&:id)
|
||||
transitions = workflows.select do |w|
|
||||
role_ids.include?(w.role_id) &&
|
||||
w.tracker_id == tracker.id &&
|
||||
((!w.author && !w.assignee) || (author && w.author) || (assignee && w.assignee))
|
||||
end
|
||||
transitions.map(&:new_status).compact.sort
|
||||
else
|
||||
[]
|
||||
end
|
||||
self.class.new_statuses_allowed(self, roles, tracker, author, assignee)
|
||||
end
|
||||
alias :find_new_statuses_allowed_to :new_statuses_allowed_to
|
||||
|
||||
# Same thing as above but uses a database query
|
||||
# More efficient than the previous method if called just once
|
||||
def find_new_statuses_allowed_to(roles, tracker, author=false, assignee=false)
|
||||
def self.new_statuses_allowed(status, roles, tracker, author=false, assignee=false)
|
||||
if roles.present? && tracker
|
||||
status_id = status.try(:id) || 0
|
||||
|
||||
scope = IssueStatus.
|
||||
joins(:workflow_transitions_as_new_status).
|
||||
where(:workflows => {:old_status_id => id, :role_id => roles.map(&:id), :tracker_id => tracker.id})
|
||||
where(:workflows => {:old_status_id => status_id, :role_id => roles.map(&:id), :tracker_id => tracker.id})
|
||||
|
||||
unless author && assignee
|
||||
if author || assignee
|
||||
|
||||
@@ -17,6 +17,7 @@
|
||||
|
||||
class WorkflowPermission < WorkflowRule
|
||||
validates_inclusion_of :rule, :in => %w(readonly required)
|
||||
validates_presence_of :old_status
|
||||
validate :validate_field_name
|
||||
|
||||
# Returns the workflow permissions for the given trackers and roles
|
||||
|
||||
@@ -23,7 +23,7 @@ class WorkflowRule < ActiveRecord::Base
|
||||
belongs_to :old_status, :class_name => 'IssueStatus'
|
||||
belongs_to :new_status, :class_name => 'IssueStatus'
|
||||
|
||||
validates_presence_of :role, :tracker, :old_status
|
||||
validates_presence_of :role, :tracker
|
||||
attr_protected :id
|
||||
|
||||
# Copies workflows from source to targets
|
||||
|
||||
@@ -20,16 +20,17 @@
|
||||
</tr>
|
||||
</thead>
|
||||
<tbody>
|
||||
<% for old_status in @statuses %>
|
||||
<% for old_status in [nil] + @statuses %>
|
||||
<% next if old_status.nil? && name != 'always' %>
|
||||
<tr class="<%= cycle("odd", "even") %>">
|
||||
<td class="name">
|
||||
<%= link_to_function(image_tag('toggle_check.png'), "toggleCheckboxesBySelector('table.transitions-#{name} input.old-status-#{old_status.id}')",
|
||||
<%= link_to_function(image_tag('toggle_check.png'), "toggleCheckboxesBySelector('table.transitions-#{name} input.old-status-#{old_status.try(:id) || 0}')",
|
||||
:title => "#{l(:button_check_all)}/#{l(:button_uncheck_all)}") %>
|
||||
|
||||
<%= old_status.name %>
|
||||
<%= old_status ? old_status.name : content_tag('em', l(:label_issue_new)) %>
|
||||
</td>
|
||||
<% for new_status in @statuses -%>
|
||||
<% checked = workflows.detect {|w| w.old_status_id == old_status.id && w.new_status_id == new_status.id} %>
|
||||
<% checked = workflows.detect {|w| w.old_status == old_status && w.new_status == new_status} %>
|
||||
<td class="<%= checked ? 'enabled' : '' %>">
|
||||
<%= transition_tag workflows, old_status, new_status, name %>
|
||||
</td>
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
class InsertAllowedStatusesForNewIssues < ActiveRecord::Migration
|
||||
def self.up
|
||||
# Adds the default status for all trackers and roles
|
||||
sql = "INSERT INTO #{WorkflowTransition.table_name} (tracker_id, old_status_id, new_status_id, role_id, type)" +
|
||||
" SELECT t.id, 0, t.default_status_id, r.id, 'WorkflowTransition'" +
|
||||
" FROM #{Tracker.table_name} t, #{Role.table_name} r"
|
||||
WorkflowTransition.connection.execute(sql)
|
||||
|
||||
# Adds other statuses that are reachable with one transition
|
||||
# to preserve previous behaviour as default
|
||||
sql = "INSERT INTO #{WorkflowTransition.table_name} (tracker_id, old_status_id, new_status_id, role_id, type)" +
|
||||
" SELECT t.id, 0, w.new_status_id, w.role_id, 'WorkflowTransition'" +
|
||||
" FROM #{Tracker.table_name} t" +
|
||||
" JOIN #{IssueStatus.table_name} s on s.id = t.default_status_id" +
|
||||
" JOIN #{WorkflowTransition.table_name} w on w.tracker_id = t.id and w.old_status_id = s.id and w.type = 'WorkflowTransition'" +
|
||||
" WHERE w.new_status_id <> t.default_status_id"
|
||||
WorkflowTransition.connection.execute(sql)
|
||||
end
|
||||
|
||||
def self.down
|
||||
WorkflowTransition.where(:old_status_id => 0).delete_all
|
||||
end
|
||||
end
|
||||
42
test/fixtures/workflows.yml
vendored
42
test/fixtures/workflows.yml
vendored
@@ -1882,3 +1882,45 @@ WorkflowTransitions_188:
|
||||
id: 188
|
||||
tracker_id: 3
|
||||
type: WorkflowTransition
|
||||
WorkflowTransitions_271:
|
||||
new_status_id: 3
|
||||
role_id: 1
|
||||
old_status_id: 0
|
||||
id: 271
|
||||
tracker_id: 2
|
||||
type: WorkflowTransition
|
||||
WorkflowTransitions_272:
|
||||
new_status_id: 3
|
||||
role_id: 2
|
||||
old_status_id: 0
|
||||
id: 272
|
||||
tracker_id: 1
|
||||
type: WorkflowTransition
|
||||
WorkflowTransitions_273:
|
||||
new_status_id: 2
|
||||
role_id: 1
|
||||
old_status_id: 0
|
||||
id: 273
|
||||
tracker_id: 3
|
||||
type: WorkflowTransition
|
||||
WorkflowTransitions_274:
|
||||
new_status_id: 2
|
||||
role_id: 1
|
||||
old_status_id: 0
|
||||
id: 274
|
||||
tracker_id: 1
|
||||
type: WorkflowTransition
|
||||
WorkflowTransitions_275:
|
||||
new_status_id: 1
|
||||
role_id: 1
|
||||
old_status_id: 0
|
||||
id: 275
|
||||
tracker_id: 1
|
||||
type: WorkflowTransition
|
||||
WorkflowTransitions_276:
|
||||
new_status_id: 1
|
||||
role_id: 1
|
||||
old_status_id: 0
|
||||
id: 276
|
||||
tracker_id: 2
|
||||
type: WorkflowTransition
|
||||
|
||||
@@ -1602,6 +1602,37 @@ class IssuesControllerTest < ActionController::TestCase
|
||||
assert_select 'input[name=was_default_status][value="1"]'
|
||||
end
|
||||
|
||||
def test_new_should_propose_allowed_statuses
|
||||
WorkflowTransition.delete_all
|
||||
WorkflowTransition.create!(:tracker_id => 1, :role_id => 1, :old_status_id => 0, :new_status_id => 1)
|
||||
WorkflowTransition.create!(:tracker_id => 1, :role_id => 1, :old_status_id => 0, :new_status_id => 3)
|
||||
@request.session[:user_id] = 2
|
||||
|
||||
get :new, :project_id => 1
|
||||
assert_response :success
|
||||
assert_select 'select[name=?]', 'issue[status_id]' do
|
||||
assert_select 'option[value="1"]'
|
||||
assert_select 'option[value="3"]'
|
||||
assert_select 'option', 2
|
||||
assert_select 'option[value="1"][selected=selected]'
|
||||
end
|
||||
end
|
||||
|
||||
def test_new_should_propose_allowed_statuses_without_default_status_allowed
|
||||
WorkflowTransition.delete_all
|
||||
WorkflowTransition.create!(:tracker_id => 1, :role_id => 1, :old_status_id => 0, :new_status_id => 2)
|
||||
assert_equal 1, Tracker.find(1).default_status_id
|
||||
@request.session[:user_id] = 2
|
||||
|
||||
get :new, :project_id => 1
|
||||
assert_response :success
|
||||
assert_select 'select[name=?]', 'issue[status_id]' do
|
||||
assert_select 'option[value="2"]'
|
||||
assert_select 'option', 1
|
||||
assert_select 'option[value="2"][selected=selected]'
|
||||
end
|
||||
end
|
||||
|
||||
def test_get_new_with_list_custom_field
|
||||
@request.session[:user_id] = 2
|
||||
get :new, :project_id => 1, :tracker_id => 1
|
||||
@@ -1827,8 +1858,8 @@ class IssuesControllerTest < ActionController::TestCase
|
||||
def test_update_form_for_new_issue_should_propose_transitions_based_on_initial_status
|
||||
@request.session[:user_id] = 2
|
||||
WorkflowTransition.delete_all
|
||||
WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 1, :new_status_id => 2)
|
||||
WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 1, :new_status_id => 5)
|
||||
WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 0, :new_status_id => 2)
|
||||
WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 0, :new_status_id => 5)
|
||||
WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 5, :new_status_id => 4)
|
||||
|
||||
xhr :post, :new, :project_id => 1,
|
||||
@@ -1837,7 +1868,7 @@ class IssuesControllerTest < ActionController::TestCase
|
||||
:subject => 'This is an issue'}
|
||||
|
||||
assert_equal 5, assigns(:issue).status_id
|
||||
assert_equal [1,2,5], assigns(:allowed_statuses).map(&:id).sort
|
||||
assert_equal [2,5], assigns(:allowed_statuses).map(&:id).sort
|
||||
end
|
||||
|
||||
def test_update_form_with_default_status_should_ignore_submitted_status_id_if_equals
|
||||
|
||||
@@ -61,6 +61,16 @@ class WorkflowsControllerTest < ActionController::TestCase
|
||||
assert_select 'input[type=checkbox][name=?]', 'transitions[1][1][always]', 0
|
||||
end
|
||||
|
||||
def test_get_edit_should_include_allowed_statuses_for_new_issues
|
||||
WorkflowTransition.delete_all
|
||||
WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 0, :new_status_id => 1)
|
||||
|
||||
get :edit, :role_id => 1, :tracker_id => 1
|
||||
assert_response :success
|
||||
assert_select 'td', 'New issue'
|
||||
assert_select 'input[type=checkbox][name=?][value="1"][checked=checked]', 'transitions[0][1][always]'
|
||||
end
|
||||
|
||||
def test_get_edit_with_all_roles_and_all_trackers
|
||||
get :edit, :role_id => 'all', :tracker_id => 'all'
|
||||
assert_response :success
|
||||
@@ -96,6 +106,20 @@ class WorkflowsControllerTest < ActionController::TestCase
|
||||
assert_nil WorkflowTransition.where(:role_id => 2, :tracker_id => 1, :old_status_id => 5, :new_status_id => 4).first
|
||||
end
|
||||
|
||||
def test_post_edit_with_allowed_statuses_for_new_issues
|
||||
WorkflowTransition.delete_all
|
||||
|
||||
post :edit, :role_id => 2, :tracker_id => 1,
|
||||
:transitions => {
|
||||
'0' => {'1' => {'always' => '1'}, '2' => {'always' => '1'}}
|
||||
}
|
||||
assert_response 302
|
||||
|
||||
assert WorkflowTransition.where(:role_id => 2, :tracker_id => 1, :old_status_id => 0, :new_status_id => 1).any?
|
||||
assert WorkflowTransition.where(:role_id => 2, :tracker_id => 1, :old_status_id => 0, :new_status_id => 2).any?
|
||||
assert_equal 2, WorkflowTransition.where(:tracker_id => 1, :role_id => 2).count
|
||||
end
|
||||
|
||||
def test_post_edit_with_additional_transitions
|
||||
WorkflowTransition.delete_all
|
||||
|
||||
|
||||
@@ -47,13 +47,14 @@ class RoleTest < ActiveSupport::TestCase
|
||||
|
||||
def test_copy_workflows
|
||||
source = Role.find(1)
|
||||
assert_equal 90, source.workflow_rules.size
|
||||
rule_count = source.workflow_rules.count
|
||||
assert rule_count > 0
|
||||
|
||||
target = Role.new(:name => 'Target')
|
||||
assert target.save
|
||||
target.workflow_rules.copy(source)
|
||||
target.reload
|
||||
assert_equal 90, target.workflow_rules.size
|
||||
assert_equal rule_count, target.workflow_rules.size
|
||||
end
|
||||
|
||||
def test_permissions_should_be_unserialized_with_its_coder
|
||||
|
||||
@@ -30,13 +30,14 @@ class TrackerTest < ActiveSupport::TestCase
|
||||
|
||||
def test_copy_workflows
|
||||
source = Tracker.find(1)
|
||||
assert_equal 89, source.workflow_rules.size
|
||||
rules_count = source.workflow_rules.count
|
||||
assert rules_count > 0
|
||||
|
||||
target = Tracker.new(:name => 'Target', :default_status_id => 1)
|
||||
assert target.save
|
||||
target.workflow_rules.copy(source)
|
||||
target.reload
|
||||
assert_equal 89, target.workflow_rules.size
|
||||
assert_equal rules_count, target.workflow_rules.size
|
||||
end
|
||||
|
||||
def test_issue_statuses
|
||||
|
||||
Reference in New Issue
Block a user