From b6c794d16b47bf353a1a2dfc00e9cbd078525ee8 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Tue, 25 Feb 2014 02:54:47 +0000 Subject: [PATCH] Bulk edit workflows for multiple trackers/roles (#16164). git-svn-id: http://svn.redmine.org/redmine/trunk@12924 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/workflows_controller.rb | 103 +++++++------- app/helpers/workflows_helper.rb | 58 +++++++- app/models/workflow_permission.rb | 43 ++++-- app/models/workflow_transition.rb | 65 +++++++++ app/views/workflows/_form.html.erb | 5 +- app/views/workflows/edit.html.erb | 33 ++++- app/views/workflows/permissions.html.erb | 37 +++-- public/stylesheets/application.css | 8 +- test/functional/workflows_controller_test.rb | 136 ++++++++++--------- test/unit/workflow_transition_test.rb | 93 +++++++++++++ 10 files changed, 436 insertions(+), 145 deletions(-) create mode 100644 test/unit/workflow_transition_test.rb diff --git a/app/controllers/workflows_controller.rb b/app/controllers/workflows_controller.rb index 596462cbd..0a9600cba 100644 --- a/app/controllers/workflows_controller.rb +++ b/app/controllers/workflows_controller.rb @@ -18,40 +18,30 @@ class WorkflowsController < ApplicationController layout 'admin' - before_filter :require_admin, :find_roles, :find_trackers + before_filter :require_admin def index @workflow_counts = WorkflowTransition.count_by_tracker_and_role end def edit - @role = Role.find_by_id(params[:role_id]) if params[:role_id] - @tracker = Tracker.find_by_id(params[:tracker_id]) if params[:tracker_id] + find_trackers_roles_and_statuses_for_edit - if request.post? - WorkflowTransition.destroy_all( ["role_id=? and tracker_id=?", @role.id, @tracker.id]) - (params[:issue_status] || []).each { |status_id, transitions| - transitions.each { |new_status_id, options| - author = options.is_a?(Array) && options.include?('author') && !options.include?('always') - assignee = options.is_a?(Array) && options.include?('assignee') && !options.include?('always') - WorkflowTransition.create(:role_id => @role.id, :tracker_id => @tracker.id, :old_status_id => status_id, :new_status_id => new_status_id, :author => author, :assignee => assignee) - } - } - if @role.save - flash[:notice] = l(:notice_successful_update) - redirect_to workflows_edit_path(:role_id => @role, :tracker_id => @tracker, :used_statuses_only => params[:used_statuses_only]) - return + if request.post? && @roles && @trackers && params[:transitions] + transitions = params[:transitions].deep_dup + transitions.each do |old_status_id, transitions_by_new_status| + transitions_by_new_status.each do |new_status_id, transition_by_rule| + transition_by_rule.reject! {|rule, transition| transition == 'no_change'} + end end + WorkflowTransition.replace_transitions(@trackers, @roles, transitions) + flash[:notice] = l(:notice_successful_update) + redirect_to_referer_or workflows_edit_path + return end - @used_statuses_only = (params[:used_statuses_only] == '0' ? false : true) - if @tracker && @used_statuses_only && @tracker.issue_statuses.any? - @statuses = @tracker.issue_statuses - end - @statuses ||= IssueStatus.sorted.all - - if @tracker && @role && @statuses.any? - workflows = WorkflowTransition.where(:role_id => @role.id, :tracker_id => @tracker.id).all + if @trackers && @roles && @statuses.any? + workflows = WorkflowTransition.where(:role_id => @roles.map(&:id), :tracker_id => @trackers.map(&:id)).all @workflows = {} @workflows['always'] = workflows.select {|w| !w.author && !w.assignee} @workflows['author'] = workflows.select {|w| w.author} @@ -60,36 +50,31 @@ class WorkflowsController < ApplicationController end def permissions - @role = Role.find_by_id(params[:role_id]) if params[:role_id] - @tracker = Tracker.find_by_id(params[:tracker_id]) if params[:tracker_id] + find_trackers_roles_and_statuses_for_edit - if request.post? && @role && @tracker - WorkflowPermission.replace_permissions(@tracker, @role, params[:permissions] || {}) + if request.post? && @roles && @trackers && params[:permissions] + permissions = params[:permissions].deep_dup + permissions.each { |field, rule_by_status_id| + rule_by_status_id.reject! {|status_id, rule| rule == 'no_change'} + } + WorkflowPermission.replace_permissions(@trackers, @roles, permissions) flash[:notice] = l(:notice_successful_update) - redirect_to workflows_permissions_path(:role_id => @role, :tracker_id => @tracker, :used_statuses_only => params[:used_statuses_only]) + redirect_to_referer_or workflows_permissions_path return end - @used_statuses_only = (params[:used_statuses_only] == '0' ? false : true) - if @tracker && @used_statuses_only && @tracker.issue_statuses.any? - @statuses = @tracker.issue_statuses - end - @statuses ||= IssueStatus.sorted.all - - if @role && @tracker - @fields = (Tracker::CORE_FIELDS_ALL - @tracker.disabled_core_fields).map {|field| [field, l("field_"+field.sub(/_id$/, ''))]} - @custom_fields = @tracker.custom_fields - @permissions = WorkflowPermission. - where(:tracker_id => @tracker.id, :role_id => @role.id).inject({}) do |h, w| - h[w.old_status_id] ||= {} - h[w.old_status_id][w.field_name] = w.rule - h - end + if @roles && @trackers + @fields = (Tracker::CORE_FIELDS_ALL - @trackers.map(&:disabled_core_fields).reduce(:&)).map {|field| [field, l("field_"+field.sub(/_id$/, ''))]} + @custom_fields = @trackers.map(&:custom_fields).flatten.uniq.sort + @permissions = WorkflowPermission.rules_by_status_id(@trackers, @roles) @statuses.each {|status| @permissions[status.id] ||= {}} end end def copy + @roles = Role.sorted + @trackers = Tracker.sorted + if params[:source_tracker_id].blank? || params[:source_tracker_id] == 'any' @source_tracker = nil else @@ -119,11 +104,37 @@ class WorkflowsController < ApplicationController private + def find_trackers_roles_and_statuses_for_edit + find_roles + find_trackers + find_statuses + end + def find_roles - @roles = Role.sorted.all + ids = Array.wrap(params[:role_id]) + if ids == ['all'] + @roles = Role.sorted.all + elsif ids.present? + @roles = Role.where(:id => ids).all + end + @roles = nil if @roles.blank? end def find_trackers - @trackers = Tracker.sorted.all + ids = Array.wrap(params[:tracker_id]) + if ids == ['all'] + @trackers = Tracker.sorted.all + elsif ids.present? + @trackers = Tracker.where(:id => ids).all + end + @trackers = nil if @trackers.blank? + end + + def find_statuses + @used_statuses_only = (params[:used_statuses_only] == '0' ? false : true) + if @trackers && @used_statuses_only + @statuses = @trackers.map(&:issue_statuses).flatten.uniq.sort.presence + end + @statuses ||= IssueStatus.sorted.all end end diff --git a/app/helpers/workflows_helper.rb b/app/helpers/workflows_helper.rb index bfeacd821..185488e36 100644 --- a/app/helpers/workflows_helper.rb +++ b/app/helpers/workflows_helper.rb @@ -18,24 +18,74 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. module WorkflowsHelper + def options_for_workflow_select(name, objects, selected, options={}) + option_tags = ''.html_safe + multiple = false + if selected + if selected.size == objects.size + selected = 'all' + else + selected = selected.map(&:id) + if selected.size > 1 + multiple = true + end + end + else + selected = objects.first.try(:id) + end + all_tag_options = {:value => 'all', :selected => (selected == 'all')} + if multiple + all_tag_options.merge!(:style => "display:none;") + end + option_tags << content_tag('option', 'All', all_tag_options) + option_tags << options_from_collection_for_select(objects, "id", "name", selected) + select_tag name, option_tags, {:multiple => multiple}.merge(options) + end + def field_required?(field) field.is_a?(CustomField) ? field.is_required? : %w(project_id tracker_id subject priority_id is_private).include?(field) end - def field_permission_tag(permissions, status, field, role) + def field_permission_tag(permissions, status, field, roles) name = field.is_a?(CustomField) ? field.id.to_s : field options = [["", ""], [l(:label_readonly), "readonly"]] options << [l(:label_required), "required"] unless field_required?(field) html_options = {} - selected = permissions[status.id][name] + + if perm = permissions[status.id][name] + if perm.uniq.size > 1 || perm.size < @roles.size * @trackers.size + options << [l(:label_no_change_option), "no_change"] + selected = 'no_change' + else + selected = perm.first + end + end + + hidden = field.is_a?(CustomField) && + !field.visible? && + !roles.detect {|role| role.custom_fields.to_a.include?(field)} - hidden = field.is_a?(CustomField) && !field.visible? && !role.custom_fields.to_a.include?(field) if hidden options[0][0] = l(:label_hidden) selected = '' html_options[:disabled] = true end - select_tag("permissions[#{name}][#{status.id}]", options_for_select(options, selected), html_options) + select_tag("permissions[#{status.id}][#{name}]", options_for_select(options, selected), html_options) + 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 + + tag_name = "transitions[#{ old_status.id }][#{new_status.id}][#{name}]" + if w == 0 || w == @roles.size * @trackers.size + + hidden_field_tag(tag_name, "0") + + check_box_tag(tag_name, "1", w != 0, + :class => "old-status-#{old_status.id} new-status-#{new_status.id}") + else + select_tag tag_name, + options_for_select([["Oui", "1"], ["Non", "0"], [l(:label_no_change_option), "no_change"]], "no_change") + end end end diff --git a/app/models/workflow_permission.rb b/app/models/workflow_permission.rb index 0338a3ea1..bf4430ec5 100644 --- a/app/models/workflow_permission.rb +++ b/app/models/workflow_permission.rb @@ -19,20 +19,43 @@ class WorkflowPermission < WorkflowRule validates_inclusion_of :rule, :in => %w(readonly required) validate :validate_field_name - # Replaces the workflow permissions for the given tracker and role + # Returns the workflow permissions for the given trackers and roles + # grouped by status_id # # Example: - # WorkflowPermission.replace_permissions role, tracker, {'due_date' => {'1' => 'readonly', '2' => 'required'}} - def self.replace_permissions(tracker, role, permissions) - destroy_all(:tracker_id => tracker.id, :role_id => role.id) + # WorkflowPermission.rules_by_status_id trackers, roles + # # => {1 => {'start_date' => 'required', 'due_date' => 'readonly'}} + def self.rules_by_status_id(trackers, roles) + WorkflowPermission.where(:tracker_id => trackers.map(&:id), :role_id => roles.map(&:id)).inject({}) do |h, w| + h[w.old_status_id] ||= {} + h[w.old_status_id][w.field_name] ||= [] + h[w.old_status_id][w.field_name] << w.rule + h + end + end - permissions.each { |field, rule_by_status_id| - rule_by_status_id.each { |status_id, rule| - if rule.present? - WorkflowPermission.create(:role_id => role.id, :tracker_id => tracker.id, :old_status_id => status_id, :field_name => field, :rule => rule) - end + # Replaces the workflow permissions for the given trackers and roles + # + # Example: + # WorkflowPermission.replace_permissions trackers, roles, {'1' => {'start_date' => 'required', 'due_date' => 'readonly'}} + def self.replace_permissions(trackers, roles, permissions) + trackers = Array.wrap trackers + roles = Array.wrap roles + + transaction do + permissions.each { |status_id, rule_by_field| + rule_by_field.each { |field, rule| + destroy_all(:tracker_id => trackers.map(&:id), :role_id => roles.map(&:id), :old_status_id => status_id, :field_name => field) + if rule.present? + trackers.each do |tracker| + roles.each do |role| + WorkflowPermission.create(:role_id => role.id, :tracker_id => tracker.id, :old_status_id => status_id, :field_name => field, :rule => rule) + end + end + end + } } - } + end end protected diff --git a/app/models/workflow_transition.rb b/app/models/workflow_transition.rb index 35d9f26a4..fb0c31c80 100644 --- a/app/models/workflow_transition.rb +++ b/app/models/workflow_transition.rb @@ -36,4 +36,69 @@ class WorkflowTransition < WorkflowRule result end + + def self.replace_transitions(trackers, roles, transitions) + trackers = Array.wrap trackers + roles = Array.wrap roles + + transaction do + records = WorkflowTransition.where(:tracker_id => trackers.map(&:id), :role_id => roles.map(&:id)).all + + transitions.each do |old_status_id, transitions_by_new_status| + transitions_by_new_status.each do |new_status_id, transition_by_rule| + transition_by_rule.each do |rule, transition| + trackers.each do |tracker| + roles.each do |role| + w = records.select {|r| + r.old_status_id == old_status_id.to_i && + r.new_status_id == new_status_id.to_i && + r.tracker_id == tracker.id && + r.role_id == role.id && + !r.destroyed? + } + + if rule == 'always' + w = w.select {|r| !r.author && !r.assignee} + else + w = w.select {|r| r.author || r.assignee} + end + if w.size > 1 + w[1..-1].each(&:destroy) + end + w = w.first + + if transition == "1" || transition == true + unless w + w = WorkflowTransition.new(:old_status_id => old_status_id, :new_status_id => new_status_id, :tracker_id => tracker.id, :role_id => role.id) + records << w + end + w.author = true if rule == "author" + w.assignee = true if rule == "assignee" + w.save if w.changed? + elsif w + if rule == 'always' + w.destroy + elsif rule == 'author' + if w.assignee + w.author = false + w.save if w.changed? + else + w.destroy + end + elsif rule == 'assignee' + if w.author + w.assignee = false + w.save if w.changed? + else + w.destroy + end + end + end + end + end + end + end + end + end + end end diff --git a/app/views/workflows/_form.html.erb b/app/views/workflows/_form.html.erb index 19d94d60c..9c5ef62fb 100644 --- a/app/views/workflows/_form.html.erb +++ b/app/views/workflows/_form.html.erb @@ -1,4 +1,4 @@ - +
<% end -%> diff --git a/app/views/workflows/edit.html.erb b/app/views/workflows/edit.html.erb index 8c02653c5..19b409357 100644 --- a/app/views/workflows/edit.html.erb +++ b/app/views/workflows/edit.html.erb @@ -4,8 +4,8 @@
@@ -14,10 +14,14 @@ <%= form_tag({}, :method => 'get') do %>

+ <%= options_for_workflow_select 'role_id[]', Role.sorted, @roles, :id => 'role_id', :class => 'expandable' %> + + <%= image_tag 'bullet_toggle_plus.png' %> + <%= options_for_workflow_select 'tracker_id[]', Tracker.sorted, @trackers, :id => 'tracker_id', :class => 'expandable' %> + + <%= image_tag 'bullet_toggle_plus.png' %> <%= submit_tag l(:button_edit), :name => nil %> @@ -27,10 +31,10 @@

<% end %> -<% if @tracker && @role && @statuses.any? %> +<% if @trackers && @roles && @statuses.any? %> <%= form_tag({}, :id => 'workflow_form' ) do %> - <%= hidden_field_tag 'tracker_id', @tracker.id %> - <%= hidden_field_tag 'role_id', @role.id %> + <%= @trackers.map {|tracker| hidden_field_tag 'tracker_id[]', tracker.id}.join.html_safe %> + <%= @roles.map {|role| hidden_field_tag 'role_id[]', role.id}.join.html_safe %> <%= hidden_field_tag 'used_statuses_only', params[:used_statuses_only] %>
<%= render :partial => 'form', :locals => {:name => 'always', :workflows => @workflows['always']} %> @@ -54,3 +58,18 @@ <%= submit_tag l(:button_save) %> <% end %> <% end %> + +<%= javascript_tag do %> +$("a[data-expands]").click(function(e){ + e.preventDefault(); + var target = $($(this).attr("data-expands")); + if (target.attr("multiple")) { + target.attr("multiple", false); + target.find("option[value=all]").show(); + } else { + target.attr("multiple", true); + target.find("option[value=all]").attr("selected", false).hide(); + } +}); + +<% end %> diff --git a/app/views/workflows/permissions.html.erb b/app/views/workflows/permissions.html.erb index 11212e6b8..54b8304ce 100644 --- a/app/views/workflows/permissions.html.erb +++ b/app/views/workflows/permissions.html.erb @@ -4,8 +4,8 @@
    -
  • <%= link_to l(:label_status_transitions), {:action => 'edit', :role_id => @role, :tracker_id => @tracker} %>
    • -
  • <%= link_to l(:label_fields_permissions), {:action => 'permissions', :role_id => @role, :tracker_id => @tracker}, :class => 'selected' %>
    • +
  • <%= link_to l(:label_status_transitions), workflows_edit_path(:role_id => @roles, :tracker_id => @trackers) %>
    • +
  • <%= link_to l(:label_fields_permissions), workflows_permissions_path(:role_id => @roles, :tracker_id => @trackers), :class => 'selected' %>
@@ -14,10 +14,14 @@ <%= form_tag({}, :method => 'get') do %>

+ <%= options_for_workflow_select 'role_id[]', Role.sorted, @roles, :id => 'role_id', :class => 'expandable' %> + + <%= image_tag 'bullet_toggle_plus.png' %> + <%= options_for_workflow_select 'tracker_id[]', Tracker.sorted, @trackers, :id => 'tracker_id', :class => 'expandable' %> + + <%= image_tag 'bullet_toggle_plus.png' %> <%= submit_tag l(:button_edit), :name => nil %> @@ -26,13 +30,13 @@

<% end %> -<% if @tracker && @role && @statuses.any? %> +<% if @trackers && @roles && @statuses.any? %> <%= form_tag({}, :id => 'workflow_form' ) do %> - <%= hidden_field_tag 'tracker_id', @tracker.id %> - <%= hidden_field_tag 'role_id', @role.id %> + <%= @trackers.map {|tracker| hidden_field_tag 'tracker_id[]', tracker.id}.join.html_safe %> + <%= @roles.map {|role| hidden_field_tag 'role_id[]', role.id}.join.html_safe %> <%= hidden_field_tag 'used_statuses_only', params[:used_statuses_only] %>
-
@@ -31,8 +31,7 @@ <% for new_status in @statuses -%> <% checked = workflows.detect {|w| w.old_status_id == old_status.id && w.new_status_id == new_status.id} %> - <%= check_box_tag "issue_status[#{ old_status.id }][#{new_status.id}][]", name, checked, - :class => "old-status-#{old_status.id} new-status-#{new_status.id}" %> + <%= transition_tag workflows, old_status, new_status, name %>
+
<% end -%> @@ -82,7 +86,7 @@ <% for status in @statuses -%> <% end -%> @@ -103,4 +107,17 @@ $("a.repeat-value").click(function(e){ var selected = td.find("select").find(":selected").val(); td.nextAll('td').find("select").val(selected); }); + +$("a[data-expands]").click(function(e){ + e.preventDefault(); + var target = $($(this).attr("data-expands")); + if (target.attr("multiple")) { + target.attr("multiple", false); + target.find("option[value=all]").show(); + } else { + target.attr("multiple", true); + target.find("option[value=all]").attr("selected", false).hide(); + } +}); + <% end %> diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css index cb5ebe371..3fdef01e3 100644 --- a/public/stylesheets/application.css +++ b/public/stylesheets/application.css @@ -254,6 +254,8 @@ table.boards td.last-message {text-align:left;font-size:80%;} table.messages td.last_message {text-align:left;} +#query_form_content {font-size:90%;} + table.query-columns { border-collapse: collapse; border: 0; @@ -340,7 +342,7 @@ div.issue table.attributes td {width:28%;} #issue_tree td.checkbox, #relations td.checkbox {display:none;} #relations td.buttons {padding:0;} -fieldset.collapsible { border-width: 1px 0 0 0; font-size: 0.9em; } +fieldset.collapsible {border-width: 1px 0 0 0;} fieldset.collapsible>legend { padding-left: 16px; background: url(../images/arrow_expanded.png) no-repeat 0% 40%; cursor:pointer; } fieldset.collapsible.collapsed>legend { background-image: url(../images/arrow_collapsed.png); } @@ -453,10 +455,12 @@ ul.properties li span {font-style:italic;} #workflow_copy_form select { width: 200px; } table.transitions td.enabled {background: #bfb;} -table.fields_permissions select {font-size:90%} +#workflow_form table select {font-size:90%; max-width:100px;} table.fields_permissions td.readonly {background:#ddd;} table.fields_permissions td.required {background:#d88;} +select.expandable {vertical-align:top;} + textarea#custom_field_possible_values {width: 95%; resize:vertical} textarea#custom_field_default_value {width: 95%; resize:vertical} diff --git a/test/functional/workflows_controller_test.rb b/test/functional/workflows_controller_test.rb index 09cfc12bf..ea00cbc77 100644 --- a/test/functional/workflows_controller_test.rb +++ b/test/functional/workflows_controller_test.rb @@ -39,8 +39,6 @@ class WorkflowsControllerTest < ActionController::TestCase get :edit assert_response :success assert_template 'edit' - assert_not_nil assigns(:roles) - assert_not_nil assigns(:trackers) end def test_get_edit_with_role_and_tracker @@ -57,18 +55,11 @@ class WorkflowsControllerTest < ActionController::TestCase assert_equal [2, 3, 5], assigns(:statuses).collect(&:id) # allowed transitions - assert_tag :tag => 'input', :attributes => { :type => 'checkbox', - :name => 'issue_status[3][5][]', - :value => 'always', - :checked => 'checked' } + assert_select 'input[type=checkbox][name=?][value=1][checked=checked]', 'transitions[3][5][always]' # not allowed - assert_tag :tag => 'input', :attributes => { :type => 'checkbox', - :name => 'issue_status[3][2][]', - :value => 'always', - :checked => nil } + assert_select 'input[type=checkbox][name=?][value=1]:not([checked=checked])', 'transitions[3][2][always]' # unused - assert_no_tag :tag => 'input', :attributes => { :type => 'checkbox', - :name => 'issue_status[1][1][]' } + assert_select 'input[type=checkbox][name=?]', 'transitions[1][1][always]', 0 end def test_get_edit_with_role_and_tracker_and_all_statuses @@ -81,19 +72,18 @@ class WorkflowsControllerTest < ActionController::TestCase assert_not_nil assigns(:statuses) assert_equal IssueStatus.count, assigns(:statuses).size - assert_tag :tag => 'input', :attributes => { :type => 'checkbox', - :name => 'issue_status[1][1][]', - :value => 'always', - :checked => nil } + assert_select 'input[type=checkbox][name=?]', 'transitions[1][1][always]' end def test_post_edit + WorkflowTransition.delete_all + post :edit, :role_id => 2, :tracker_id => 1, - :issue_status => { - '4' => {'5' => ['always']}, - '3' => {'1' => ['always'], '2' => ['always']} + :transitions => { + '4' => {'5' => {'always' => '1'}}, + '3' => {'1' => {'always' => '1'}, '2' => {'always' => '1'}} } - assert_redirected_to '/workflows/edit?role_id=2&tracker_id=1' + assert_response 302 assert_equal 3, WorkflowTransition.where(:tracker_id => 1, :role_id => 2).count assert_not_nil WorkflowTransition.where(:role_id => 2, :tracker_id => 1, :old_status_id => 3, :new_status_id => 2).first @@ -101,12 +91,16 @@ class WorkflowsControllerTest < ActionController::TestCase end def test_post_edit_with_additional_transitions + WorkflowTransition.delete_all + post :edit, :role_id => 2, :tracker_id => 1, - :issue_status => { - '4' => {'5' => ['always']}, - '3' => {'1' => ['author'], '2' => ['assignee'], '4' => ['author', 'assignee']} + :transitions => { + '4' => {'5' => {'always' => '1', 'author' => '0', 'assignee' => '0'}}, + '3' => {'1' => {'always' => '0', 'author' => '1', 'assignee' => '0'}, + '2' => {'always' => '0', 'author' => '0', 'assignee' => '1'}, + '4' => {'always' => '0', 'author' => '1', 'assignee' => '1'}} } - assert_redirected_to '/workflows/edit?role_id=2&tracker_id=1' + assert_response 302 assert_equal 4, WorkflowTransition.where(:tracker_id => 1, :role_id => 2).count @@ -124,20 +118,11 @@ class WorkflowsControllerTest < ActionController::TestCase assert w.assignee end - def test_clear_workflow - assert WorkflowTransition.where(:role_id => 1, :tracker_id => 2).count > 0 - - post :edit, :role_id => 1, :tracker_id => 2 - assert_equal 0, WorkflowTransition.where(:role_id => 1, :tracker_id => 2).count - end - def test_get_permissions get :permissions assert_response :success assert_template 'permissions' - assert_not_nil assigns(:roles) - assert_not_nil assigns(:trackers) end def test_get_permissions_with_role_and_tracker @@ -150,11 +135,11 @@ class WorkflowsControllerTest < ActionController::TestCase assert_response :success assert_template 'permissions' - assert_select 'input[name=role_id][value=1]' - assert_select 'input[name=tracker_id][value=2]' + assert_select 'input[name=?][value=1]', 'role_id[]' + assert_select 'input[name=?][value=2]', 'tracker_id[]' # Required field - assert_select 'select[name=?]', 'permissions[assigned_to_id][2]' do + assert_select 'select[name=?]', 'permissions[2][assigned_to_id]' do assert_select 'option[value=]' assert_select 'option[value=][selected=selected]', 0 assert_select 'option[value=readonly]', :text => 'Read-only' @@ -164,7 +149,7 @@ class WorkflowsControllerTest < ActionController::TestCase end # Read-only field - assert_select 'select[name=?]', 'permissions[fixed_version_id][3]' do + assert_select 'select[name=?]', 'permissions[3][fixed_version_id]' do assert_select 'option[value=]' assert_select 'option[value=][selected=selected]', 0 assert_select 'option[value=readonly]', :text => 'Read-only' @@ -174,7 +159,7 @@ class WorkflowsControllerTest < ActionController::TestCase end # Other field - assert_select 'select[name=?]', 'permissions[due_date][3]' do + assert_select 'select[name=?]', 'permissions[3][due_date]' do assert_select 'option[value=]' assert_select 'option[value=][selected=selected]', 0 assert_select 'option[value=readonly]', :text => 'Read-only' @@ -193,7 +178,7 @@ class WorkflowsControllerTest < ActionController::TestCase # Custom field that is always required # The default option is "(Required)" - assert_select 'select[name=?]', "permissions[#{cf.id}][3]" do + assert_select 'select[name=?]', "permissions[3][#{cf.id}]" do assert_select 'option[value=]' assert_select 'option[value=readonly]', :text => 'Read-only' assert_select 'option[value=required]', 0 @@ -209,15 +194,56 @@ class WorkflowsControllerTest < ActionController::TestCase assert_response :success assert_template 'permissions' - assert_select 'select[name=?]:not(.disabled)', "permissions[#{cf1.id}][1]" - assert_select 'select[name=?]:not(.disabled)', "permissions[#{cf3.id}][1]" + assert_select 'select[name=?]:not(.disabled)', "permissions[1][#{cf1.id}]" + assert_select 'select[name=?]:not(.disabled)', "permissions[1][#{cf3.id}]" - assert_select 'select[name=?][disabled=disabled]', "permissions[#{cf2.id}][1]" do + assert_select 'select[name=?][disabled=disabled]', "permissions[1][#{cf2.id}]" do assert_select 'option[value=][selected=selected]', :text => 'Hidden' end end - def test_get_permissions_with_role_and_tracker_and_all_statuses + def test_get_permissions_with_missing_permissions_for_roles_should_default_to_no_change + WorkflowPermission.delete_all + WorkflowPermission.create!(:role_id => 1, :tracker_id => 2, :old_status_id => 1, :field_name => 'assigned_to_id', :rule => 'required') + + get :permissions, :role_id => [1, 2], :tracker_id => 2 + assert_response :success + + assert_select 'select[name=?]', 'permissions[1][assigned_to_id]' do + assert_select 'option[selected]', 1 + assert_select 'option[selected][value=no_change]' + end + end + + def test_get_permissions_with_different_permissions_for_roles_should_default_to_no_change + WorkflowPermission.delete_all + WorkflowPermission.create!(:role_id => 1, :tracker_id => 2, :old_status_id => 1, :field_name => 'assigned_to_id', :rule => 'required') + WorkflowPermission.create!(:role_id => 2, :tracker_id => 2, :old_status_id => 1, :field_name => 'assigned_to_id', :rule => 'readonly') + + get :permissions, :role_id => [1, 2], :tracker_id => 2 + assert_response :success + + assert_select 'select[name=?]', 'permissions[1][assigned_to_id]' do + assert_select 'option[selected]', 1 + assert_select 'option[selected][value=no_change]' + end + end + + def test_get_permissions_with_same_permissions_for_roles_should_default_to_permission + WorkflowPermission.delete_all + WorkflowPermission.create!(:role_id => 1, :tracker_id => 2, :old_status_id => 1, :field_name => 'assigned_to_id', :rule => 'required') + WorkflowPermission.create!(:role_id => 2, :tracker_id => 2, :old_status_id => 1, :field_name => 'assigned_to_id', :rule => 'required') + + get :permissions, :role_id => [1, 2], :tracker_id => 2 + assert_response :success + + assert_select 'select[name=?]', 'permissions[1][assigned_to_id]' do + assert_select 'option[selected]', 1 + assert_select 'option[selected][value=required]' + end + end + + def test_get_permissions_with_role_and_tracker_and_all_statuses_should_show_all_statuses WorkflowTransition.delete_all get :permissions, :role_id => 1, :tracker_id => 2, :used_statuses_only => '0' @@ -229,11 +255,11 @@ class WorkflowsControllerTest < ActionController::TestCase WorkflowPermission.delete_all post :permissions, :role_id => 1, :tracker_id => 2, :permissions => { - 'assigned_to_id' => {'1' => '', '2' => 'readonly', '3' => ''}, - 'fixed_version_id' => {'1' => 'required', '2' => 'readonly', '3' => ''}, - 'due_date' => {'1' => '', '2' => '', '3' => ''}, + '1' => {'assigned_to_id' => '', 'fixed_version_id' => 'required', 'due_date' => ''}, + '2' => {'assigned_to_id' => 'readonly', 'fixed_version_id' => 'readonly', 'due_date' => ''}, + '3' => {'assigned_to_id' => '', 'fixed_version_id' => '', 'due_date' => ''} } - assert_redirected_to '/workflows/permissions?role_id=1&tracker_id=2' + assert_response 302 workflows = WorkflowPermission.all assert_equal 3, workflows.size @@ -246,22 +272,6 @@ class WorkflowsControllerTest < ActionController::TestCase assert workflows.detect {|wf| wf.old_status_id == 2 && wf.field_name == 'fixed_version_id' && wf.rule == 'readonly'} end - def test_post_permissions_should_clear_permissions - WorkflowPermission.delete_all - WorkflowPermission.create!(:role_id => 1, :tracker_id => 2, :old_status_id => 2, :field_name => 'assigned_to_id', :rule => 'required') - WorkflowPermission.create!(:role_id => 1, :tracker_id => 2, :old_status_id => 2, :field_name => 'fixed_version_id', :rule => 'required') - wf1 = WorkflowPermission.create!(:role_id => 1, :tracker_id => 3, :old_status_id => 2, :field_name => 'fixed_version_id', :rule => 'required') - wf2 = WorkflowPermission.create!(:role_id => 2, :tracker_id => 2, :old_status_id => 3, :field_name => 'fixed_version_id', :rule => 'readonly') - - post :permissions, :role_id => 1, :tracker_id => 2 - assert_redirected_to '/workflows/permissions?role_id=1&tracker_id=2' - - workflows = WorkflowPermission.all - assert_equal 2, workflows.size - assert wf1.reload - assert wf2.reload - end - def test_get_copy get :copy assert_response :success diff --git a/test/unit/workflow_transition_test.rb b/test/unit/workflow_transition_test.rb new file mode 100644 index 000000000..0b5e1ab1e --- /dev/null +++ b/test/unit/workflow_transition_test.rb @@ -0,0 +1,93 @@ +# Redmine - project management software +# Copyright (C) 2006-2014 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +require File.expand_path('../../test_helper', __FILE__) + +class WorkflowTransitionTest < ActiveSupport::TestCase + fixtures :roles, :trackers, :issue_statuses + + def setup + WorkflowTransition.delete_all + end + + def test_replace_transitions_should_create_enabled_transitions + w = WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 1, :new_status_id => 2) + + transitions = {'1' => { + '2' => {'always' => '1'}, + '3' => {'always' => '1'} + }} + assert_difference 'WorkflowTransition.count' do + WorkflowTransition.replace_transitions(Tracker.find(1), Role.find(1), transitions) + end + assert WorkflowTransition.where(:role_id => 1, :tracker_id => 1, :old_status_id => 1, :new_status_id => 3).exists? + end + + def test_replace_transitions_should_delete_disabled_transitions + w1 = WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 1, :new_status_id => 2) + w2 = WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 1, :new_status_id => 3) + + transitions = {'1' => { + '2' => {'always' => '0'}, + '3' => {'always' => '1'} + }} + assert_difference 'WorkflowTransition.count', -1 do + WorkflowTransition.replace_transitions(Tracker.find(1), Role.find(1), transitions) + end + assert !WorkflowTransition.exists?(w1.id) + end + + def test_replace_transitions_should_create_enabled_additional_transitions + transitions = {'1' => { + '2' => {'always' => '0', 'assignee' => '0', 'author' => '1'} + }} + assert_difference 'WorkflowTransition.count' do + WorkflowTransition.replace_transitions(Tracker.find(1), Role.find(1), transitions) + end + w = WorkflowTransition.where(:role_id => 1, :tracker_id => 1, :old_status_id => 1, :new_status_id => 2).first + assert w + assert_equal false, w.assignee + assert_equal true, w.author + end + + def test_replace_transitions_should_delete_disabled_additional_transitions + w = WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 1, :new_status_id => 2, :assignee => true) + + transitions = {'1' => { + '2' => {'always' => '0', 'assignee' => '0', 'author' => '0'} + }} + assert_difference 'WorkflowTransition.count', -1 do + WorkflowTransition.replace_transitions(Tracker.find(1), Role.find(1), transitions) + end + assert !WorkflowTransition.exists?(w.id) + end + + def test_replace_transitions_should_update_additional_transitions + WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, :old_status_id => 1, :new_status_id => 2, :assignee => true) + + transitions = {'1' => { + '2' => {'always' => '0', 'assignee' => '0', 'author' => '1'} + }} + assert_no_difference 'WorkflowTransition.count' do + WorkflowTransition.replace_transitions(Tracker.find(1), Role.find(1), transitions) + end + w = WorkflowTransition.where(:role_id => 1, :tracker_id => 1, :old_status_id => 1, :new_status_id => 2).first + assert w + assert_equal false, w.assignee + assert_equal true, w.author + end +end
@@ -62,7 +66,7 @@ <% for status in @statuses -%> - <%= field_permission_tag(@permissions, status, field, @role) %> + <%= field_permission_tag(@permissions, status, field, @roles) %> <% unless status == @statuses.last %>»<% end %> - <%= field_permission_tag(@permissions, status, field, @role) %> + <%= field_permission_tag(@permissions, status, field, @roles) %> <% unless status == @statuses.last %>»<% end %>