diff --git a/app/components/workflows/blankslate_component.html.erb b/app/components/workflows/blankslate_component.html.erb
index da7a8b2ae1cb..48aedf4b9bc5 100644
--- a/app/components/workflows/blankslate_component.html.erb
+++ b/app/components/workflows/blankslate_component.html.erb
@@ -32,7 +32,7 @@ See COPYRIGHT and LICENSE files for more details.
blankslate.with_heading(tag: :h2).with_content(t("admin.workflows.blankslate.title"))
blankslate.with_description_content(t("admin.workflows.blankslate.description"))
blankslate.with_primary_action(
- href: helpers.status_dialog_workflow_tab_path(@type, @tab, role_id: @role.id),
+ href: helpers.status_dialog_workflow_tab_path(@type, @tab, role_ids: @roles.map(&:id)),
scheme: :secondary,
data: { controller: "async-dialog" }
) do |button|
diff --git a/app/components/workflows/blankslate_component.rb b/app/components/workflows/blankslate_component.rb
index ed4c14ef797c..0f92b6ea2f4c 100644
--- a/app/components/workflows/blankslate_component.rb
+++ b/app/components/workflows/blankslate_component.rb
@@ -32,9 +32,9 @@ module Workflows
class BlankslateComponent < ApplicationComponent
include OpPrimer::ComponentHelpers
- def initialize(role:, type:, tab:)
+ def initialize(roles:, type:, tab:)
super
- @role = role
+ @roles = roles
@type = type
@tab = tab
end
diff --git a/app/components/workflows/page_headers/edit_component.rb b/app/components/workflows/page_headers/edit_component.rb
index fac6786a992c..794dbed58e8e 100644
--- a/app/components/workflows/page_headers/edit_component.rb
+++ b/app/components/workflows/page_headers/edit_component.rb
@@ -30,7 +30,7 @@
module Workflows::PageHeaders
class EditComponent < BaseComponent
- options :tabs, :role
+ options :tabs, :roles
def type = model
@@ -49,7 +49,7 @@ def add_action_buttons(header)
mobile_icon: :copy,
mobile_label: t(:button_copy),
size: :medium,
- href: new_workflow_copy_path(type, source_role_id: role&.id),
+ href: new_workflow_copy_path(type, source_role_id: roles&.first&.id),
aria: { label: helpers.t(:button_copy) },
title: helpers.t(:button_copy)
) do |button|
diff --git a/app/components/workflows/status_dialog_component.html.erb b/app/components/workflows/status_dialog_component.html.erb
index 3727681676f1..d9fe0c120653 100644
--- a/app/components/workflows/status_dialog_component.html.erb
+++ b/app/components/workflows/status_dialog_component.html.erb
@@ -42,7 +42,7 @@ See COPYRIGHT and LICENSE files for more details.
Workflows::StatusFormComponent.new(
all_statuses: @all_statuses,
current_statuses: @current_statuses,
- role: @role,
+ roles: @roles,
type: @type,
tab: @tab
)
diff --git a/app/components/workflows/status_dialog_component.rb b/app/components/workflows/status_dialog_component.rb
index 0f0ccde45601..2c724a580024 100644
--- a/app/components/workflows/status_dialog_component.rb
+++ b/app/components/workflows/status_dialog_component.rb
@@ -35,11 +35,11 @@ class StatusDialogComponent < ApplicationComponent
DIALOG_ID = "workflows-status-dialog"
- def initialize(all_statuses:, current_statuses:, role:, type:, tab:)
+ def initialize(all_statuses:, current_statuses:, roles:, type:, tab:)
super
@all_statuses = all_statuses
@current_statuses = current_statuses
- @role = role
+ @roles = roles
@type = type
@tab = tab
end
diff --git a/app/components/workflows/status_form_component.html.erb b/app/components/workflows/status_form_component.html.erb
index d9bf8ef27b7c..c41534cb44f5 100644
--- a/app/components/workflows/status_form_component.html.erb
+++ b/app/components/workflows/status_form_component.html.erb
@@ -29,7 +29,7 @@ See COPYRIGHT and LICENSE files for more details.
<%=
primer_form_with(
- url: helpers.confirm_statuses_workflow_tab_path(@type, @tab, role_id: @role.id),
+ url: helpers.confirm_statuses_workflow_tab_path(@type, @tab, role_ids: @roles.map(&:id)),
method: :post,
id: FORM_ID,
data: { turbo_frame: "workflow-table" }
@@ -39,7 +39,6 @@ See COPYRIGHT and LICENSE files for more details.
f,
all_statuses: @all_statuses,
current_statuses: @current_statuses,
- role: @role,
type: @type,
tab: @tab,
dialog_id:
diff --git a/app/components/workflows/status_form_component.rb b/app/components/workflows/status_form_component.rb
index 114351c64b86..8b99cfc125e6 100644
--- a/app/components/workflows/status_form_component.rb
+++ b/app/components/workflows/status_form_component.rb
@@ -32,11 +32,11 @@ module Workflows
class StatusFormComponent < ApplicationComponent
FORM_ID = "status-selection-form"
- def initialize(all_statuses:, current_statuses:, role:, type:, tab:)
+ def initialize(all_statuses:, current_statuses:, roles:, type:, tab:)
super
@all_statuses = all_statuses
@current_statuses = current_statuses
- @role = role
+ @roles = roles
@type = type
@tab = tab
end
diff --git a/app/components/workflows/status_matrix_form_component.html.erb b/app/components/workflows/status_matrix_form_component.html.erb
index 93522276a2fb..68ca0b219a19 100644
--- a/app/components/workflows/status_matrix_form_component.html.erb
+++ b/app/components/workflows/status_matrix_form_component.html.erb
@@ -32,21 +32,39 @@ See COPYRIGHT and LICENSE files for more details.
render Primer::OpenProject::SubHeader.new do |subheader|
if @type && @available_roles.any?
subheader.with_filter_component do
- render(Primer::Alpha::ActionMenu.new(select_variant: :single)) do |menu|
- menu.with_show_button(scheme: :secondary) do |button|
+ render(
+ Primer::Alpha::SelectPanel.new(
+ select_variant: :multiple,
+ fetch_strategy: :local,
+ title: t("admin.workflows.role_selector.title"),
+ data: data_attributes
+ )
+ ) do |panel|
+ panel.with_show_button(scheme: :secondary) do |button|
button.with_trailing_visual_icon(icon: :"triangle-down")
- @role ? t("admin.workflows.role_selector.label", role: @role.name) : t("admin.workflows.role_selector.no_role")
+ if @roles.many?
+ t("admin.workflows.role_selector.roles", count: @roles.size)
+ elsif @roles.one?
+ t("admin.workflows.role_selector.label", role: @roles.first.name)
+ else
+ t("admin.workflows.role_selector.no_role")
+ end
end
@available_roles.each do |available_role|
- menu.with_item(
+ panel.with_item(
label: available_role.name,
- active: available_role == @role,
- tag: :a,
- href: helpers.edit_workflow_tab_path(@type, @tab, role_id: available_role.id),
- content_arguments: { data: { "admin--workflow-checkbox-state-confirmation-trigger": "click",
- turbo_action: "advance" } }
+ active: @roles.include?(available_role),
+ item_id: available_role.id
)
end
+ panel.with_footer(show_divider: true) do
+ render(
+ Primer::Beta::Button.new(
+ scheme: :primary,
+ data: { action: "click->admin--workflow-role-select#apply" }
+ )
+ ) { t(:button_save) }
+ end
end
end
end
@@ -56,7 +74,7 @@ See COPYRIGHT and LICENSE files for more details.
scheme: :secondary,
leading_icon: :plus,
label: t("admin.workflows.status_button"),
- href: helpers.status_dialog_workflow_tab_path(@type, @tab, role_id: @role&.id, status_ids: @statuses.pluck(:id).presence),
+ href: helpers.status_dialog_workflow_tab_path(@type, @tab, role_ids: @roles.map(&:id), status_ids: @statuses.pluck(:id).presence),
data: { controller: "async-dialog" }
) do
t("admin.workflows.status_button")
@@ -67,7 +85,7 @@ See COPYRIGHT and LICENSE files for more details.
<% if @statuses.any? %>
<%= form_tag(
workflow_tab_path(@type),
- id: "workflow_form",
+ id: form_id,
method: :patch,
autocomplete: "off",
data: {
@@ -76,7 +94,9 @@ See COPYRIGHT and LICENSE files for more details.
}
) do %>
<%= hidden_field_tag "type_id", @type.id %>
- <%= hidden_field_tag "role_id", @role.id %>
+ <% @roles.each do |role| %>
+ <%= hidden_field_tag "role_ids[]", role.id %>
+ <% end %>
<%= hidden_field_tag "tab", @tab %>
<%= helpers.render_tabs helpers.workflow_tabs(@type) %>
@@ -94,7 +114,7 @@ See COPYRIGHT and LICENSE files for more details.
Primer::OpenProject::FeedbackDialog.new(
title: t("admin.workflows.leave_confirmation.title"),
data: {
- "admin--workflow-checkbox-state-target": "confirmationDialog",
+ "admin--workflow-checkbox-state-target": "confirmationDialog"
}
)
) do |dialog|
@@ -118,6 +138,6 @@ See COPYRIGHT and LICENSE files for more details.
%>
<% end %>
<% else %>
- <%= render Workflows::BlankslateComponent.new(role: @role, type: @type, tab: @tab) %>
+ <%= render Workflows::BlankslateComponent.new(roles: @roles, type: @type, tab: @tab) %>
<% end %>
<% end %>
diff --git a/app/components/workflows/status_matrix_form_component.rb b/app/components/workflows/status_matrix_form_component.rb
index 14c3ade5fdbe..5af6570d32ff 100644
--- a/app/components/workflows/status_matrix_form_component.rb
+++ b/app/components/workflows/status_matrix_form_component.rb
@@ -33,14 +33,29 @@ class StatusMatrixFormComponent < ApplicationComponent
include OpTurbo::Streamable
include OpPrimer::ComponentHelpers
- def initialize(tab:, role:, type:, available_roles:, statuses:, has_status_changes:)
+ FORM_ID = "workflow_form"
+
+ def initialize(tab:, roles:, type:, available_roles:, statuses:, has_status_changes:)
super
@tab = tab
- @role = role
+ @roles = roles
@type = type
@available_roles = available_roles
@statuses = statuses
@has_status_changes = has_status_changes
end
+
+ private
+
+ def form_id = FORM_ID
+
+ def data_attributes
+ {
+ controller: "admin--workflow-role-select",
+ "admin--workflow-role-select-base-url-value": helpers.edit_workflow_tab_path(@type, @tab),
+ "admin--workflow-role-select-current-role-ids-value": @roles.map(&:id),
+ "admin--workflow-role-select-admin--workflow-checkbox-state-outlet": "##{form_id}"
+ }
+ end
end
end
diff --git a/app/components/workflows/status_removal_danger_dialog_component.html.erb b/app/components/workflows/status_removal_danger_dialog_component.html.erb
index c2b020ff5b72..9846295eb11e 100644
--- a/app/components/workflows/status_removal_danger_dialog_component.html.erb
+++ b/app/components/workflows/status_removal_danger_dialog_component.html.erb
@@ -46,7 +46,7 @@ See COPYRIGHT and LICENSE files for more details.
# The reason this is done here is because the submit is not a DELETE, and GET form submissions
# strip url params
dialog.with_additional_details do
- concat(hidden_field_tag(:role_id, @role.id))
+ @roles.each { |role| concat(hidden_field_tag("role_ids[]", role.id)) }
@status_ids.each { |id| concat(hidden_field_tag("status_ids[]", id)) }
end
end
diff --git a/app/components/workflows/status_removal_danger_dialog_component.rb b/app/components/workflows/status_removal_danger_dialog_component.rb
index 1753687632a2..d3f926f24c36 100644
--- a/app/components/workflows/status_removal_danger_dialog_component.rb
+++ b/app/components/workflows/status_removal_danger_dialog_component.rb
@@ -35,9 +35,9 @@ class StatusRemovalDangerDialogComponent < ApplicationComponent
DIALOG_ID = "workflows-status-removal-dialog"
- def initialize(role:, type:, tab:, status_ids:, removed_count:)
+ def initialize(roles:, type:, tab:, status_ids:, removed_count:)
super
- @role = role
+ @roles = roles
@type = type
@tab = tab
@status_ids = Array(status_ids).flatten.map(&:to_i)
diff --git a/app/controllers/workflows/tabs_controller.rb b/app/controllers/workflows/tabs_controller.rb
index 94c9805c22a8..a6c2c1757870 100644
--- a/app/controllers/workflows/tabs_controller.rb
+++ b/app/controllers/workflows/tabs_controller.rb
@@ -38,27 +38,37 @@ class Workflows::TabsController < ApplicationController
before_action :set_type
before_action :set_tab
before_action :set_eligible_roles
- before_action :set_role
+ before_action :set_roles
def edit
unless turbo_frame_request?
- redirect_to edit_workflow_path(@type, role_id: params[:role_id], tab: @tab)
+ redirect_to edit_workflow_path(@type, role_ids: params[:role_ids], tab: @tab)
return
end
statuses_for_form
- if @type && @role && @statuses.any?
+ if @type && @roles.any? && @statuses.any?
workflows_for_form
end
end
- def update
- call = Workflows::BulkUpdateService
- .new(role: @role, type: @type, tab: @tab)
- .call(permitted_status_params)
+ def update # rubocop:disable Metrics/AbcSize
+ success = false
+ Workflow.transaction do
+ success = true
+ base_params = permitted_status_params
+ indeterminate = permitted_indeterminate_params
+ @roles.each do |role|
+ role_params = indeterminate.empty? ? base_params : role_specific_params(base_params, indeterminate, role)
+ result = Workflows::BulkUpdateService.new(role:, type: @type, tab: @tab)
+ .call(role_params)
+ success = false unless result.success?
+ end
+ raise ActiveRecord::Rollback unless success
+ end
- if call.success?
+ if success
render_flash_message_via_turbo_stream(
message: I18n.t(:notice_successful_update),
scheme: :success
@@ -69,7 +79,7 @@ def update
update_via_turbo_stream(
component: Workflows::StatusMatrixFormComponent.new(
tab: @tab,
- role: @role,
+ roles: @roles,
type: @type,
available_roles: @eligible_roles,
statuses:,
@@ -92,8 +102,8 @@ def status_dialog
all_statuses = Status.order(:position)
current_statuses = if params[:status_ids].present?
Status.where(id: params[:status_ids].map(&:to_i)).order(:position)
- elsif @type && @role
- statuses_for_role_and_type
+ elsif @type && @roles.any?
+ statuses_for_roles_and_type
else
Status.none
end
@@ -101,7 +111,7 @@ def status_dialog
respond_with_dialog Workflows::StatusDialogComponent.new(
all_statuses:,
current_statuses:,
- role: @role,
+ roles: @roles,
type: @type,
tab: @tab
)
@@ -114,7 +124,7 @@ def confirm_statuses # rubocop:disable Metrics/AbcSize
if removed_count > 0
respond_with_dialog Workflows::StatusRemovalDangerDialogComponent.new(
- role: @role,
+ roles: @roles,
type: @type,
tab: @tab,
status_ids: current_status_ids,
@@ -125,7 +135,7 @@ def confirm_statuses # rubocop:disable Metrics/AbcSize
update_via_turbo_stream(
component: Workflows::StatusMatrixFormComponent.new(
tab: @tab,
- role: @role,
+ roles: @roles,
type: @type,
available_roles: @eligible_roles,
statuses:,
@@ -150,8 +160,9 @@ def set_eligible_roles
@eligible_roles = Workflow.eligible_roles.order(:builtin, :position)
end
- def set_role
- @role = @eligible_roles.find(params[:role_id])
+ def set_roles
+ @roles = @eligible_roles.where(id: params[:role_ids])
+ @roles = [@eligible_roles.first] if @roles.empty?
end
def statuses_for_form
@@ -159,8 +170,8 @@ def statuses_for_form
@has_status_changes = false
@statuses = if @type && params[:status_ids].present?
statuses_from_params
- elsif @type && @role
- statuses_for_role_and_type
+ elsif @type && @roles.any?
+ statuses_for_roles_and_type
elsif @type
@type.statuses
else
@@ -170,18 +181,19 @@ def statuses_for_form
def statuses_from_params
status_ids = params[:status_ids].map(&:to_i)
- saved_ids = statuses_for_role_and_type.pluck(:id)
+ saved_ids = statuses_for_roles_and_type.pluck(:id)
@added_status_ids = status_ids - saved_ids
@has_status_changes = @added_status_ids.any? || (saved_ids - status_ids).any?
Status.where(id: status_ids).order(:position)
end
- def statuses_for_role_and_type
- @type.statuses(role: @role, tab: @tab)
+ def statuses_for_roles_and_type
+ status_ids = @roles.map { |role| @type.statuses(role:, tab: @tab).pluck(:id) }.flatten.uniq
+ Status.where(id: status_ids)
end
def workflows_for_form
- workflows = Workflow.where(role_id: @role.id, type_id: @type.id)
+ workflows = Workflow.where(role_id: @roles.map(&:id), type_id: @type.id)
@workflows = {}
@workflows["always"] = workflows.select { |w| !w.author && !w.assignee }
@workflows["author"] = workflows.select(&:author)
@@ -189,10 +201,40 @@ def workflows_for_form
end
def permitted_status_params
- return {} if params["status"].blank?
+ status_params("status")
+ end
+
+ def permitted_indeterminate_params
+ status_params("indeterminate_status")
+ end
+
+ def status_params(key)
+ return {} if params[key].blank?
- params["status"]
+ params[key]
.to_unsafe_h
- .select { |key, value| /\A\d+\z/.match?(key) && value.keys.all? { /\A\d+\z/.match?(it) } }
+ .select { |k, value| /\A\d+\z/.match?(k) && value.keys.all? { /\A\d+\z/.match?(it) } }
+ end
+
+ def role_specific_params(base_params, indeterminate, role)
+ params = base_params.deep_dup
+ indeterminate.each do |old_id, new_ids|
+ new_ids.each_key do |new_id|
+ # Restore from DB so that it isn't overwritten by indeterminate state (unchecked)
+ had_transition = Workflow.exists?(
+ role_id: role.id,
+ type_id: @type.id,
+ old_status_id: old_id.to_i,
+ new_status_id: new_id.to_i,
+ author: @tab == "author",
+ assignee: @tab == "assignee"
+ )
+ if had_transition
+ params[old_id] ||= {}
+ params[old_id][new_id] = "1"
+ end
+ end
+ end
+ params
end
end
diff --git a/app/controllers/workflows_controller.rb b/app/controllers/workflows_controller.rb
index 82b03b151186..3de82fd8d54e 100644
--- a/app/controllers/workflows_controller.rb
+++ b/app/controllers/workflows_controller.rb
@@ -38,7 +38,7 @@ class WorkflowsController < ApplicationController
before_action :find_types, only: %i[index]
before_action :find_type, only: %i[edit]
- before_action :find_optional_role, only: %i[edit]
+ before_action :find_optional_roles, only: %i[edit]
def index; end
@@ -56,16 +56,14 @@ def find_types
@types = ::Type.order(:position)
end
- def find_role
- @role = eligible_roles.find(params[:role_id])
- end
-
def find_type
@type = ::Type.find(params[:type_id])
end
- def find_optional_role
- @role = eligible_roles.find_by(id: params[:role_id]) || eligible_roles.order(:builtin, :position).first
+ def find_optional_roles
+ ordered = eligible_roles.order(:builtin, :position)
+ @roles = ordered.where(id: params[:role_ids])
+ @roles = [ordered.first] if @roles.empty?
end
def eligible_roles
diff --git a/app/forms/workflows/status_select_form.rb b/app/forms/workflows/status_select_form.rb
index cb6c4cf106f7..f650969ea3e4 100644
--- a/app/forms/workflows/status_select_form.rb
+++ b/app/forms/workflows/status_select_form.rb
@@ -30,18 +30,16 @@
module Workflows
class StatusSelectForm < ApplicationForm
- def initialize(all_statuses:, current_statuses:, role:, type:, tab:, dialog_id:)
+ def initialize(all_statuses:, current_statuses:, type:, tab:, dialog_id:)
super()
@all_statuses = all_statuses
@current_statuses = current_statuses
- @role = role
@type = type
@tab = tab
@dialog_id = dialog_id
end
form do |f|
- f.hidden(name: :role_id, value: @role.id)
f.hidden(name: :type_id, value: @type.id)
f.hidden(name: :tab, value: @tab || "always")
@current_statuses.each { |status| f.hidden(name: "original_status_ids[]", value: status.id) }
diff --git a/app/helpers/workflow_helper.rb b/app/helpers/workflow_helper.rb
index a56b291fce3d..61c7007adb09 100644
--- a/app/helpers/workflow_helper.rb
+++ b/app/helpers/workflow_helper.rb
@@ -37,7 +37,7 @@ def workflow_tabs(type)
].map do |tab|
tab.merge(
partial: "workflows/form",
- path: edit_workflow_tab_path(type, tab[:name], params.permit(:role_id)),
+ path: edit_workflow_tab_path(type, tab[:name], params.permit(role_ids: [])),
data: { "admin--workflow-checkbox-state-confirmation-trigger": "click",
turbo_frame: "workflow-table",
turbo_action: "advance" }
diff --git a/app/views/workflows/_form.html.erb b/app/views/workflows/_form.html.erb
index 95e3a7469a40..11cbe41d9449 100644
--- a/app/views/workflows/_form.html.erb
+++ b/app/views/workflows/_form.html.erb
@@ -187,9 +187,11 @@ See COPYRIGHT and LICENSE files for more details.
<% end %>
<% @statuses.each do |new_status| -%>
- <% transition_saved = workflows.any? { it.old_status_id == old_status.id && it.new_status_id == new_status.id }
- newly_added_status = @added_status_ids.include?(old_status.id) || @added_status_ids.include?(new_status.id) %>
+ <% transition_role_ids = workflows.select { it.old_status_id == old_status.id && it.new_status_id == new_status.id }.map(&:role_id).uniq
+ newly_added_status = @added_status_ids.include?(old_status.id) || @added_status_ids.include?(new_status.id)
+ some_roles = !transition_role_ids.empty? && transition_role_ids.size < @roles.size && !newly_added_status %>
|
+ <%= hidden_field_tag "indeterminate_status[#{old_status.id}][#{new_status.id}]", "1" if some_roles %>
<%=
render(Primer::BaseComponent.new(tag: :div, display: :flex, align_items: :center, mx: 1)) do
render(
@@ -198,13 +200,14 @@ See COPYRIGHT and LICENSE files for more details.
name: "status[#{old_status.id}][#{new_status.id}]",
id: "status_#{old_status.id}_#{new_status.id}", # See BUG https://github.com/primer/view_components/issues/3811
value: name,
- checked: transition_saved || newly_added_status,
+ checked: !some_roles && (transition_role_ids.any? || newly_added_status),
label: t(".matrix_checkbox_label", old_status: old_status.name, new_status: new_status.name),
visually_hide_label: true,
data: {
checkable_target: "checkbox",
old_status: old_status.id,
- new_status: new_status.id
+ new_status: new_status.id,
+ indeterminate: (true if some_roles)
}
)
)
diff --git a/app/views/workflows/edit.html.erb b/app/views/workflows/edit.html.erb
index 49db69fe7cce..b7bdacd0a47e 100644
--- a/app/views/workflows/edit.html.erb
+++ b/app/views/workflows/edit.html.erb
@@ -28,9 +28,9 @@ See COPYRIGHT and LICENSE files for more details.
++#%>
<% html_title t(:label_administration), t(:label_workflow_plural), @type.name -%>
<% content_for :content_header do %>
- <%= render Workflows::PageHeaders::EditComponent.new(@type, role: @role, tabs: workflow_tabs(@type)) %>
+ <%= render Workflows::PageHeaders::EditComponent.new(@type, roles: @roles, tabs: workflow_tabs(@type)) %>
<% end %>
-<% if @type && @role %>
- <%= turbo_frame_tag "workflow-table", src: edit_workflow_tab_path(@type, @current_tab, role_id: @role.id, status_ids: params[:status_ids]) %>
+<% if @type && @roles.any? %>
+ <%= turbo_frame_tag "workflow-table", src: edit_workflow_tab_path(@type, @current_tab, role_ids: @roles.map(&:id), status_ids: params[:status_ids]) %>
<% end %>
diff --git a/app/views/workflows/summaries/show.html.erb b/app/views/workflows/summaries/show.html.erb
index 034123f66d9f..f5ed8bcff9e4 100644
--- a/app/views/workflows/summaries/show.html.erb
+++ b/app/views/workflows/summaries/show.html.erb
@@ -61,7 +61,7 @@ See COPYRIGHT and LICENSE files for more details.
| <%= h type %> |
<% roles.each do |role, count| -%>
- <%= link_to((count > 0 ? count : content_tag(:span, "", class: "icon-close icon-context icon-button")), edit_workflow_path(type, role_id: role), title: t(:button_edit)) %>
+ <%= link_to((count > 0 ? count : content_tag(:span, "", class: "icon-close icon-context icon-button")), edit_workflow_path(type, role_ids: [role]), title: t(:button_edit)) %>
|
<% end -%>
diff --git a/app/views/workflows/tabs/edit.html.erb b/app/views/workflows/tabs/edit.html.erb
index f665ac5cd3ca..cc5624dbda93 100644
--- a/app/views/workflows/tabs/edit.html.erb
+++ b/app/views/workflows/tabs/edit.html.erb
@@ -28,8 +28,8 @@ See COPYRIGHT and LICENSE files for more details.
++#%>
<%= turbo_frame_tag "workflow-table", data: { turbo_cache: false } do %>
- <%= render Workflows::StatusMatrixFormComponent.new(tab: @tab, role: @role, type: @type, available_roles: @eligible_roles, statuses: @statuses, has_status_changes: @has_status_changes) %>
+ <%= render Workflows::StatusMatrixFormComponent.new(tab: @tab, roles: @roles, type: @type, available_roles: @eligible_roles, statuses: @statuses, has_status_changes: @has_status_changes) %>
<%= turbo_stream.replace(Workflows::PageHeaders::EditComponent.wrapper_key) do %>
- <%= render Workflows::PageHeaders::EditComponent.new(@type, role: @role, tabs: workflow_tabs(@type)) %>
+ <%= render Workflows::PageHeaders::EditComponent.new(@type, roles: @roles, tabs: workflow_tabs(@type)) %>
<% end %>
<% end %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 797f428cbfee..dd1b14da815a 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -461,8 +461,12 @@ en:
ignore: "Ignore changes"
save: "Save changes and continue"
role_selector:
+ title: "Select roles"
label: "Role: %{role}"
no_role: "Select role"
+ roles:
+ one: "%{count} role selected"
+ other: "%{count} roles selected"
blankslate:
title: "No status transitions configured"
description: "Add statuses to start configuring workflows for this role"
diff --git a/frontend/src/global_styles/primer/_overrides.sass b/frontend/src/global_styles/primer/_overrides.sass
index a34390c13e36..f9f3869f6c03 100644
--- a/frontend/src/global_styles/primer/_overrides.sass
+++ b/frontend/src/global_styles/primer/_overrides.sass
@@ -208,3 +208,13 @@ ul.SegmentedControl,
& .Banner-actions
margin: var(--base-size-8) 0 0 var(--base-size-8)
+
+// Bug in @openproject/primer-view-components: .FormControl-checkbox:indeterminate
+// has no background rule, and :indeterminate:before doesn't cancel checkmarkOut.
+.FormControl-checkbox:indeterminate
+ background: var(--control-checked-bgColor-rest, var(--color-accent-fg))
+ border-color: var(--control-checked-borderColor-rest, var(--color-accent-fg))
+
+ &:before
+ animation: none
+ clip-path: inset(0)
diff --git a/frontend/src/stimulus/controllers/dynamic/admin/workflow-checkbox-state.controller.ts b/frontend/src/stimulus/controllers/dynamic/admin/workflow-checkbox-state.controller.ts
index 3dd327642bf2..e40218e878a9 100644
--- a/frontend/src/stimulus/controllers/dynamic/admin/workflow-checkbox-state.controller.ts
+++ b/frontend/src/stimulus/controllers/dynamic/admin/workflow-checkbox-state.controller.ts
@@ -82,7 +82,10 @@ export default class WorkflowCheckboxStateController extends Controller(`input[name="${name}"]`)!.value;
}
+ private formValues(name:string):string[] {
+ return Array.from(
+ this.element.querySelectorAll(`input[name="${name}"]`),
+ ).map((el) => el.value);
+ }
+
private pushState(key:string, state:CheckboxesState) {
const savedState:SavedState = { formKey: this.formKey, checkboxes: state };
sessionStorage.setItem(key, JSON.stringify(savedState));
@@ -155,7 +166,7 @@ export default class WorkflowCheckboxStateController extends Controller {
- const onIgnoreCallback = this.onIgnoreChanges(target, event);
- this.ignoreButtonTarget.addEventListener('click', onIgnoreCallback);
-
- const onSaveCallback = this.onSaveChanges(target, event);
- this.saveButtonTarget.addEventListener('click', onSaveCallback);
-
+ private openConfirmationDialog(onIgnore:() => void, onSave:() => void) {
+ this.ignoreButtonTarget.addEventListener('click', onIgnore);
+ this.saveButtonTarget.addEventListener('click', onSave);
this.confirmationDialogTarget.addEventListener('close', () => {
- this.ignoreButtonTarget.removeEventListener('click', onIgnoreCallback);
- this.saveButtonTarget.removeEventListener('click', onSaveCallback);
+ this.ignoreButtonTarget.removeEventListener('click', onIgnore);
+ this.saveButtonTarget.removeEventListener('click', onSave);
});
-
this.confirmationDialogTarget.showModal();
+ }
+
+ private confirmThenResubmit = (target:HTMLElement, event:Event) => {
+ this.openConfirmationDialog(
+ this.onIgnoreChanges(target, event),
+ this.onSaveChanges(target, event),
+ );
};
private onIgnoreChanges = (originalTarget:HTMLElement, originalEvent:Event) => {
@@ -193,7 +206,7 @@ export default class WorkflowCheckboxStateController extends Controller params.append('role_ids[]', id));
url.search = params.toString();
turboFrame.setAttribute('src', url.toString());
@@ -251,12 +264,24 @@ export default class WorkflowCheckboxStateController extends Controller {
+ private onCheckboxChange = (event:Event) => {
+ this.removeIndeterminateMarker(event.target as HTMLInputElement);
+ this.recomputeDirtyFlag();
+ };
+
+ private recomputeDirtyFlag() {
const current = this.captureState();
const hasChanges = Object.keys(current).some((key) => current[key] !== this.initialCheckboxState[key]);
this.hasCheckboxChangesValue = hasChanges;
- };
+ }
+
+ private removeIndeterminateMarker(checkbox:HTMLInputElement):void {
+ const { oldStatus, newStatus } = checkbox.dataset;
+ this.element.querySelector(
+ `input[name="indeterminate_status[${oldStatus}][${newStatus}]"]`,
+ )?.remove();
+ }
private captureState():Record {
const checkboxes:Record = {};
@@ -273,4 +298,52 @@ export default class WorkflowCheckboxStateController extends Controller('input[type="checkbox"][data-indeterminate="true"]').forEach((cb) => {
+ cb.indeterminate = true;
+ });
+ }
+
+ //
+ // Trigger navigation with dirty-state confirmation.
+ //
+
+ navigateTo(url:string) {
+ if (this.isDirtyValue) {
+ this.confirmThenNavigate(url);
+ } else {
+ this.frameNavigateTo(url);
+ }
+ }
+
+ private confirmThenNavigate(url:string) {
+ this.openConfirmationDialog(
+ () => {
+ this.hasCheckboxChangesValue = false;
+ this.hasStatusChangesValue = false;
+ this.confirmationDialogTarget.close();
+ setTimeout(() => { this.frameNavigateTo(url); }, 0);
+ },
+ () => {
+ this.element.requestSubmit();
+ this.confirmationDialogTarget.close();
+ // Delay to allow the flash message from the form submission to appear.
+ setTimeout(() => { this.frameNavigateTo(url); }, 1000);
+ },
+ );
+ }
+
+ // This keeps the url in the /tabs/:tab/edit format consistently,
+ // rather than doing a Turbo.visit which changes the format.
+ // It also keeps history usable, similar to data-turbo-action="advance".
+ private frameNavigateTo(url:string) {
+ const turboFrame = this.element.closest('turbo-frame') as HTMLElement | null;
+ if (turboFrame) {
+ turboFrame.setAttribute('src', url);
+ history.pushState({}, '', url);
+ } else {
+ Turbo.visit(url);
+ }
+ }
}
diff --git a/frontend/src/stimulus/controllers/dynamic/admin/workflow-role-select.controller.ts b/frontend/src/stimulus/controllers/dynamic/admin/workflow-role-select.controller.ts
new file mode 100644
index 000000000000..d81f1ff94b1b
--- /dev/null
+++ b/frontend/src/stimulus/controllers/dynamic/admin/workflow-role-select.controller.ts
@@ -0,0 +1,79 @@
+/*
+ * -- copyright
+ * OpenProject is an open source project management software.
+ * Copyright (C) the OpenProject GmbH
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 3.
+ *
+ * OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+ * Copyright (C) 2006-2013 Jean-Philippe Lang
+ * Copyright (C) 2010-2013 the ChiliProject Team
+ *
+ * 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.
+ *
+ * See COPYRIGHT and LICENSE files for more details.
+ * ++
+ */
+
+import { Controller } from '@hotwired/stimulus';
+import type { SelectPanelElement } from '@primer/view-components/app/components/primer/alpha/select_panel_element';
+import WorkflowCheckboxStateController from './workflow-checkbox-state.controller';
+
+/**
+ * When the panel closes, it navigates to the workflow edit page with the selected role IDs.
+ * Delegates dirty-state confirmation to the workflow-checkbox-state controller via an outlet.
+ */
+export default class WorkflowRoleSelectController extends Controller {
+ static outlets = ['admin--workflow-checkbox-state'];
+ static values = { baseUrl: String, currentRoleIds: Array };
+
+ declare readonly adminWorkflowCheckboxStateOutlet:WorkflowCheckboxStateController;
+ declare readonly hasAdminWorkflowCheckboxStateOutlet:boolean;
+ declare baseUrlValue:string;
+ declare currentRoleIdsValue:unknown[];
+
+ apply() {
+ const panel = this.element as HTMLElement as SelectPanelElement;
+ const selectedIds = panel.items
+ .filter((item) => panel.isItemChecked(item))
+ .map((item) => item.getAttribute('data-item-id'))
+ .filter(Boolean);
+
+ // For when all roles are deselected
+ if (!selectedIds.length) {
+ this.navigateTo(this.buildUrl([]));
+ return;
+ }
+
+ if (selectedIds.slice().sort().join(',') === this.currentRoleIdsValue.slice().sort().join(',')) return;
+
+ this.navigateTo(this.buildUrl(selectedIds as string[]));
+ }
+
+ private buildUrl(roleIds:string[]):string {
+ const url = new URL(this.baseUrlValue, window.location.origin);
+ roleIds.forEach((id) => url.searchParams.append('role_ids[]', id));
+ return url.toString();
+ }
+
+ private navigateTo(url:string) {
+ if (this.hasAdminWorkflowCheckboxStateOutlet) {
+ this.adminWorkflowCheckboxStateOutlet.navigateTo(url);
+ } else {
+ Turbo.visit(url);
+ }
+ }
+}
diff --git a/spec/controllers/workflows/tabs_controller_spec.rb b/spec/controllers/workflows/tabs_controller_spec.rb
index c870c3bd936d..483c1a44baed 100644
--- a/spec/controllers/workflows/tabs_controller_spec.rb
+++ b/spec/controllers/workflows/tabs_controller_spec.rb
@@ -48,6 +48,11 @@
.with(role.id.to_s)
.and_return(role)
+ allow(role_scope)
+ .to receive(:where)
+ .with(id: [role.id.to_s])
+ .and_return([role])
+
role_scope
end
@@ -68,32 +73,58 @@
describe "#edit" do
context "when not a turbo frame request" do
- it "redirects to the parent workflow edit path" do
- get :edit,
- params: {
- role_id: role.id.to_s,
- workflow_type_id: type.id.to_s,
- tab: "always"
- }
-
- expect(response).to redirect_to(
- edit_workflow_path(type, role_id: role.id.to_s, tab: "always")
- )
+ context "with a single role" do
+ it "redirects to the parent workflow edit path" do
+ get :edit,
+ params: {
+ role_ids: [role.id.to_s],
+ workflow_type_id: type.id.to_s,
+ tab: "always"
+ }
+
+ expect(response).to redirect_to(
+ edit_workflow_path(type, role_ids: [role.id.to_s], tab: "always")
+ )
+ end
+
+ it "does not forward status_ids to the redirect" do
+ get :edit,
+ params: {
+ role_ids: [role.id.to_s],
+ workflow_type_id: type.id.to_s,
+ tab: "always",
+ status_ids: ["1", "2"]
+ }
+
+ expect(response).to redirect_to(
+ edit_workflow_path(type, role_ids: [role.id.to_s], tab: "always")
+ )
+ expect(response.location).not_to include("status_ids")
+ end
end
- it "does not forward status_ids to the redirect" do
- get :edit,
- params: {
- role_id: role.id.to_s,
- workflow_type_id: type.id.to_s,
- tab: "always",
- status_ids: ["1", "2"]
- }
-
- expect(response).to redirect_to(
- edit_workflow_path(type, role_id: role.id.to_s, tab: "always")
- )
- expect(response.location).not_to include("status_ids")
+ context "with multiple roles" do
+ let(:role2) { build_stubbed(:project_role) }
+
+ before do
+ allow(role_scope)
+ .to receive(:where)
+ .with(id: [role.id.to_s, role2.id.to_s])
+ .and_return([role, role2])
+ end
+
+ it "redirects preserving all role ids" do
+ get :edit,
+ params: {
+ role_ids: [role.id.to_s, role2.id.to_s],
+ workflow_type_id: type.id.to_s,
+ tab: "always"
+ }
+
+ expect(response).to redirect_to(
+ edit_workflow_path(type, role_ids: [role.id.to_s, role2.id.to_s], tab: "always")
+ )
+ end
end
end
end
@@ -114,7 +145,7 @@
post :confirm_statuses,
params: {
- role_id: role.id.to_s,
+ role_ids: [role.id.to_s],
workflow_type_id: type.id.to_s,
status_ids: ["1", "2"],
original_status_ids: ["1", "2"],
@@ -133,7 +164,7 @@
before do
post :confirm_statuses,
params: {
- role_id: role.id.to_s,
+ role_ids: [role.id.to_s],
workflow_type_id: type.id.to_s,
status_ids: ["1"],
original_status_ids: ["1", "2"],
@@ -152,33 +183,74 @@
describe "#update" do
let(:status_params) { { "1" => { "2" => ["always"] } } }
- let(:service) do
- instance_double(Workflows::BulkUpdateService).tap do |dbl|
- allow(Workflows::BulkUpdateService)
- .to receive(:new)
- .with(role: role, type: type, tab: "always")
- .and_return(dbl)
- end
- end
let(:call_result) { ServiceResult.success }
- let(:params) do
- {
- role_id: role.id,
- workflow_type_id: type.id,
- tab: "always",
- status: status_params
- }
- end
- before do
- allow(service).to receive(:call).with(status_params).and_return(call_result)
- allow(controller).to receive(:statuses_for_form).and_return([build_stubbed(:status)])
- post :update, params:, format: :turbo_stream
+ context "with a single role" do
+ let(:service) do
+ instance_double(Workflows::BulkUpdateService).tap do |dbl|
+ allow(Workflows::BulkUpdateService)
+ .to receive(:new)
+ .with(role: role, type: type, tab: "always")
+ .and_return(dbl)
+ end
+ end
+
+ before do
+ allow(service).to receive(:call).with(status_params).and_return(call_result)
+ allow(controller).to receive(:statuses_for_form).and_return([build_stubbed(:status)])
+ post :update,
+ params: { role_ids: [role.id.to_s], workflow_type_id: type.id, tab: "always", status: status_params },
+ format: :turbo_stream
+ end
+
+ it "calls the service and renders a flash turbo stream" do
+ expect(service).to have_received(:call).with(status_params)
+ expect(response).to have_turbo_stream action: "flash", target: "op-primer-flash-component"
+ end
end
- it "renders a flash turbo stream" do
- expect(service).to have_received(:call).with(status_params)
- expect(response).to have_turbo_stream action: "flash", target: "op-primer-flash-component"
+ context "with multiple roles" do
+ let(:role2) { build_stubbed(:project_role) }
+ let(:service1) do
+ instance_double(Workflows::BulkUpdateService).tap do |dbl|
+ allow(Workflows::BulkUpdateService)
+ .to receive(:new)
+ .with(role: role, type: type, tab: "always")
+ .and_return(dbl)
+ end
+ end
+ let(:service2) do
+ instance_double(Workflows::BulkUpdateService).tap do |dbl|
+ allow(Workflows::BulkUpdateService)
+ .to receive(:new)
+ .with(role: role2, type: type, tab: "always")
+ .and_return(dbl)
+ end
+ end
+
+ before do
+ allow(role_scope)
+ .to receive(:where)
+ .with(id: [role.id.to_s, role2.id.to_s])
+ .and_return([role, role2])
+ allow(service1).to receive(:call).with(status_params).and_return(call_result)
+ allow(service2).to receive(:call).with(status_params).and_return(call_result)
+ allow(controller).to receive(:statuses_for_form).and_return([build_stubbed(:status)])
+ post :update,
+ params: {
+ role_ids: [role.id.to_s, role2.id.to_s],
+ workflow_type_id: type.id,
+ tab: "always",
+ status: status_params
+ },
+ format: :turbo_stream
+ end
+
+ it "calls the service for each role and renders a flash turbo stream" do
+ expect(service1).to have_received(:call).with(status_params)
+ expect(service2).to have_received(:call).with(status_params)
+ expect(response).to have_turbo_stream action: "flash", target: "op-primer-flash-component"
+ end
end
end
end
diff --git a/spec/controllers/workflows_controller_spec.rb b/spec/controllers/workflows_controller_spec.rb
index 1bfd390f410b..09696f87db46 100644
--- a/spec/controllers/workflows_controller_spec.rb
+++ b/spec/controllers/workflows_controller_spec.rb
@@ -40,7 +40,7 @@
.and_return(role_scope)
allow(role_scope)
- .to receive_messages(order: role_scope, find_by: nil)
+ .to receive_messages(order: role_scope, find_by: nil, first: role)
allow(role_scope)
.to receive(:find)
@@ -52,6 +52,11 @@
.with(id: role.id.to_s)
.and_return(role)
+ allow(role_scope)
+ .to receive(:where)
+ .with(id: nil)
+ .and_return([])
+
role_scope
end
@@ -90,10 +95,6 @@
allow(Status)
.to receive(:all)
.and_return [type_status, non_type_status]
-
- allow(role_scope)
- .to receive(:order)
- .and_return([role])
end
context "without parameters" do
@@ -111,9 +112,8 @@
.to render_template :edit
end
- it "assigns the first role" do
- expect(assigns[:role])
- .to eq role
+ it "assigns @roles as the canonical collection" do
+ expect(assigns[:roles]).to contain_exactly(role)
end
it "does assign type" do
@@ -122,40 +122,59 @@
end
end
- context "with role and type params" do
+ context "with a single role param" do
before do
- get :edit, params: { role_id: role.id.to_s, type_id: type.id.to_s }
+ allow(role_scope)
+ .to receive(:where)
+ .with(id: [role.id.to_s])
+ .and_return([role])
+
+ get :edit, params: { role_ids: [role.id.to_s], type_id: type.id.to_s }
end
- it "responds with the role type statuses", :aggregate_failures do
- expect(response)
- .to have_http_status(:ok)
- expect(response)
- .to render_template :edit
- expect(assigns[:role])
- .to eq role
- expect(assigns[:type])
- .to eq type
+ it "is successful" do
+ expect(response).to have_http_status(:ok)
+ end
+
+ it "renders the edit template" do
+ expect(response).to render_template :edit
+ end
+
+ it "assigns the selected role" do
+ expect(assigns[:roles]).to contain_exactly(role)
+ end
+
+ it "assigns type" do
+ expect(assigns[:type]).to eq type
+ end
+ end
+
+ context "with multiple role params" do
+ let(:role2) { build_stubbed(:project_role) }
+
+ before do
+ allow(role_scope)
+ .to receive(:where)
+ .with(id: [role.id.to_s, role2.id.to_s])
+ .and_return([role, role2])
+
+ get :edit, params: { role_ids: [role.id.to_s, role2.id.to_s], type_id: type.id.to_s }
end
it "is successful" do
- expect(response)
- .to have_http_status(:ok)
+ expect(response).to have_http_status(:ok)
end
it "renders the edit template" do
- expect(response)
- .to render_template :edit
+ expect(response).to render_template :edit
end
- it "assigns role" do
- expect(assigns[:role])
- .to eq role
+ it "assigns all selected roles" do
+ expect(assigns[:roles]).to contain_exactly(role, role2)
end
- it "assign type" do
- expect(assigns[:type])
- .to eq type
+ it "assigns type" do
+ expect(assigns[:type]).to eq type
end
end
end
diff --git a/spec/features/roles/create_spec.rb b/spec/features/roles/create_spec.rb
index fab54f84acc4..f2a09d7cc31b 100644
--- a/spec/features/roles/create_spec.rb
+++ b/spec/features/roles/create_spec.rb
@@ -111,8 +111,11 @@
click_link type.name
end
+ new_role = Role.find_by!(name: "New role name")
click_button existing_role.name
- click_link "New role name"
+ find("[data-item-id='#{new_role.id}']").click
+ find("[data-item-id='#{existing_role.id}']").click
+ page.send_keys :escape
old_status = existing_workflow.old_status.name
new_status = existing_workflow.new_status.name
diff --git a/spec/features/workflows/edit_multi_role_spec.rb b/spec/features/workflows/edit_multi_role_spec.rb
new file mode 100644
index 000000000000..19f7871aa820
--- /dev/null
+++ b/spec/features/workflows/edit_multi_role_spec.rb
@@ -0,0 +1,333 @@
+# frozen_string_literal: true
+
+# -- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# 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.
+#
+# See COPYRIGHT and LICENSE files for more details.
+# ++
+
+require "spec_helper"
+
+RSpec.describe "Workflow edit with multiple roles", :js do
+ include Toasts::Expectations
+ include Workflows::EditHelpers
+
+ let(:role) { create(:project_role) }
+ let(:role2) { create(:project_role) }
+ let(:type) { create(:type) }
+ let(:admin) { create(:admin) }
+ let(:statuses) { (1..3).map { create(:status) } }
+
+ # workflow for 0 -> 1 for 'role'
+ let!(:role_workflow) do
+ create(:workflow, role_id: role.id, type_id: type.id,
+ old_status_id: statuses[0].id, new_status_id: statuses[1].id,
+ author: false, assignee: false)
+ end
+
+ current_user { admin }
+
+ context "when displaying checkboxes" do
+ context "when all selected roles have a transition" do
+ let!(:role2_workflow) do
+ create(:workflow, role_id: role2.id, type_id: type.id,
+ old_status_id: statuses[0].id, new_status_id: statuses[1].id,
+ author: false, assignee: false)
+ end
+
+ before { visit_workflow_edit(roles: [role, role2]) }
+
+ it "shows the checkbox as checked" do
+ expect(page).to have_field workflow_checkbox(0, 1), checked: true
+ expect(indeterminate?(workflow_checkbox(0, 1))).to be false
+ end
+ end
+
+ context "when no selected roles have a transition" do
+ before { visit_workflow_edit(roles: [role, role2]) }
+
+ it "shows the checkbox as unchecked" do
+ expect(page).to have_field workflow_checkbox(1, 0), checked: false
+ expect(indeterminate?(workflow_checkbox(1, 0))).to be false
+ end
+ end
+
+ context "when only some selected roles have a transition" do
+ before { visit_workflow_edit(roles: [role, role2]) }
+
+ it "shows the checkbox as indeterminate" do
+ expect(page).to have_field workflow_checkbox(0, 1), checked: false
+ expect(indeterminate?(workflow_checkbox(0, 1))).to be true
+ end
+
+ it "the checkbox is visible as indeterminate" do
+ expect(page).to have_field workflow_checkbox(0, 1), checked: false
+
+ expect(indeterminate?(workflow_checkbox(0, 1))).to be true
+ expect(indeterminate_visible?(workflow_checkbox(0, 1))).to be true
+ end
+ end
+
+ context "when roles have different statuses in their workflows" do
+ let!(:role2_workflow) do
+ create(:workflow, role_id: role2.id, type_id: type.id,
+ old_status_id: statuses[1].id, new_status_id: statuses[2].id,
+ author: false, assignee: false)
+ end
+
+ before { visit_workflow_edit(roles: [role, role2]) }
+
+ it "shows the union of statuses from all selected roles" do
+ expect(page).to have_field workflow_checkbox(0, 2)
+ expect(page).to have_field workflow_checkbox(1, 2)
+ end
+
+ it "pre-selects the union of statuses from all selected roles in the status dialog" do
+ within "#workflow-table" do
+ click_link "Status"
+ end
+
+ expect(page).to have_dialog("Statuses")
+ within_dialog "Statuses" do
+ expect(page).to have_css(".ng-value-label", text: statuses[0].name)
+ expect(page).to have_css(".ng-value-label", text: statuses[1].name)
+ expect(page).to have_css(".ng-value-label", text: statuses[2].name)
+ end
+ end
+ end
+
+ context "with a single role selected" do
+ before { visit_workflow_edit(roles: [role]) }
+
+ it "does not show indeterminate checkboxes" do
+ expect(page).to have_field workflow_checkbox(0, 1), checked: true
+ expect(indeterminate?(workflow_checkbox(0, 1))).to be false
+ end
+ end
+ end
+
+ context "when saving" do
+ before { visit_workflow_edit(roles: [role, role2]) }
+
+ it "adds the transition for all roles when checking an unchecked checkbox" do
+ expect_transition(role, 1, 0, exist: false)
+ expect_transition(role2, 1, 0, exist: false)
+
+ check workflow_checkbox(1, 0)
+ click_button "Save"
+ expect_flash(message: "Successful update.")
+
+ expect_transition(role, 1, 0, exist: true)
+ expect_transition(role2, 1, 0, exist: true)
+ end
+
+ it "preserves state for each role when saving an untouched indeterminate checkbox" do
+ expect_transition(role, 0, 1, exist: true)
+ expect_transition(role2, 0, 1, exist: false)
+
+ expect(page).to have_field workflow_checkbox(0, 1), checked: false
+ expect(indeterminate?(workflow_checkbox(0, 1))).to be true
+
+ click_button "Save"
+ expect_flash(message: "Successful update.")
+
+ expect_transition(role, 0, 1, exist: true)
+ expect_transition(role2, 0, 1, exist: false)
+
+ expect(indeterminate?(workflow_checkbox(0, 1))).to be true
+ end
+
+ it "adds the transition for all roles when checking an indeterminate checkbox" do
+ expect_transition(role, 0, 1, exist: true)
+ expect_transition(role2, 0, 1, exist: false)
+
+ expect(page).to have_field workflow_checkbox(0, 1), checked: false
+ expect(indeterminate?(workflow_checkbox(0, 1))).to be true
+
+ check workflow_checkbox(0, 1)
+ click_button "Save"
+ expect_flash(message: "Successful update.")
+
+ expect_transition(role, 0, 1, exist: true)
+ expect_transition(role2, 0, 1, exist: true)
+
+ expect(indeterminate?(workflow_checkbox(0, 1))).to be false
+ end
+
+ it "removes the transition from all roles when unchecking an indeterminate checkbox" do
+ expect_transition(role, 0, 1, exist: true)
+ expect_transition(role2, 0, 1, exist: false)
+
+ expect(page).to have_field workflow_checkbox(0, 1), checked: false
+ expect(indeterminate?(workflow_checkbox(0, 1))).to be true
+
+ check workflow_checkbox(0, 1)
+ uncheck workflow_checkbox(0, 1)
+ click_button "Save"
+ expect_flash(message: "Successful update.")
+
+ expect_transition(role, 0, 1, exist: false)
+ expect_transition(role2, 0, 1, exist: false)
+
+ expect(indeterminate?(workflow_checkbox(0, 1))).to be false
+ end
+
+ context "when all roles have the transition" do
+ let!(:role2_workflow) do
+ create(:workflow, role_id: role2.id, type_id: type.id,
+ old_status_id: statuses[0].id, new_status_id: statuses[1].id,
+ author: false, assignee: false)
+ end
+
+ before { visit_workflow_edit(roles: [role, role2]) }
+
+ it "removes the transition from all roles when unchecking a fully checked checkbox" do
+ expect_transition(role, 0, 1, exist: true)
+ expect_transition(role2, 0, 1, exist: true)
+
+ uncheck workflow_checkbox(0, 1)
+ click_button "Save"
+ expect_flash(message: "Successful update.")
+
+ expect_transition(role, 0, 1, exist: false)
+ expect_transition(role2, 0, 1, exist: false)
+ end
+ end
+
+ context "with multiple indeterminate checkboxes" do
+ let!(:role_workflow2) do
+ create(:workflow, role_id: role.id, type_id: type.id,
+ old_status_id: statuses[0].id, new_status_id: statuses[2].id,
+ author: false, assignee: false)
+ end
+
+ before { visit_workflow_edit(roles: [role, role2]) }
+
+ it "handles touched and untouched indeterminate checkboxes independently" do
+ # Both 0 -> 1 and 0 -> 2 are indeterminate
+ expect_transition(role, 0, 1, exist: true)
+ expect_transition(role2, 0, 1, exist: false)
+ expect_transition(role, 0, 2, exist: true)
+ expect_transition(role2, 0, 2, exist: false)
+
+ check workflow_checkbox(0, 1) # explicitly check for all roles
+
+ click_button "Save"
+ expect_flash(message: "Successful update.")
+
+ # role2 now has this workflow
+ expect_transition(role2, 0, 1, exist: true)
+
+ # 0 -> 2 stays indeterminate
+ expect_transition(role, 0, 2, exist: true)
+ expect_transition(role2, 0, 2, exist: false)
+ end
+ end
+
+ it "marks the form dirty when interacting with an indeterminate checkbox" do
+ expect(page).to have_field workflow_checkbox(0, 1), checked: false
+ expect(indeterminate?(workflow_checkbox(0, 1))).to be true
+
+ check workflow_checkbox(0, 1)
+
+ click_link "User is author"
+ expect(page).to have_dialog("Save changes before continuing?")
+ end
+
+ it "succeeds when saving with no changes to indeterminate checkboxes" do
+ expect(page).to have_field workflow_checkbox(0, 1), checked: false
+ expect(indeterminate?(workflow_checkbox(0, 1))).to be true
+
+ click_button "Save"
+ expect_flash(message: "Successful update.")
+ end
+ end
+
+ context "when deselecting all roles in the select panel" do
+ before { visit_workflow_edit(roles: [role, role2]) }
+
+ it "falls back to the first eligible role instead of leaving the page stuck" do
+ click_button "2 roles selected"
+ find("[data-item-id='#{role.id}']").click
+ find("[data-item-id='#{role2.id}']").click
+ within("select-panel") { click_button "Save" }
+
+ expect(page).to have_no_text("2 roles selected")
+ expect(page).to have_button(role.name)
+ end
+ end
+
+ context "when modifying statuses" do
+ before { visit_workflow_edit(roles: [role, role2]) }
+
+ it "preserves all selected roles after adding a status" do
+ add_status_via_dialog(statuses[2])
+
+ expect(page).to have_text("2 roles selected")
+ end
+
+ it "preserves all selected roles after removing a status via the danger dialog" do
+ remove_status_via_dialog(statuses[1])
+
+ within_dialog("Remove statuses") { click_button "Remove" }
+
+ expect(page).to have_text("2 roles selected")
+ end
+
+ it "checks all new status checkboxes as fully checked, not indeterminate, when adding a status" do
+ add_status_via_dialog(statuses[2])
+
+ [workflow_checkbox(2, 0), workflow_checkbox(0, 2), workflow_checkbox(2, 1),
+ workflow_checkbox(1, 2), workflow_checkbox(2, 2)].each do |cb|
+ expect(page).to have_field cb, checked: true
+ expect(indeterminate?(cb)).to be false
+ end
+ end
+
+ context "when no statuses are configured" do
+ let(:empty_role1) { create(:project_role) }
+ let(:empty_role2) { create(:project_role) }
+
+ before { visit_workflow_edit(roles: [empty_role1, empty_role2]) }
+
+ it "shows a blankslate" do
+ expect(page).to have_text("No status transitions configured")
+ end
+
+ it "preserves all selected roles when adding statuses from the blankslate" do
+ within "#workflow-table" do
+ all(:link, "Status").last.click
+ end
+ within_dialog "Statuses" do
+ find(".ng-arrow-wrapper").click
+ find(".ng-option", text: statuses[0].name).click
+ click_button "Apply"
+ end
+
+ expect(page).to have_text("2 roles selected")
+ end
+ end
+ end
+end
diff --git a/spec/features/workflows/edit_spec.rb b/spec/features/workflows/edit_spec.rb
index e214e19b4693..1845346ce6ff 100644
--- a/spec/features/workflows/edit_spec.rb
+++ b/spec/features/workflows/edit_spec.rb
@@ -32,6 +32,7 @@
RSpec.describe "Workflow edit", :js do
include Toasts::Expectations
+ include Workflows::EditHelpers
let(:role) { create(:project_role) }
let(:type) { create(:type) }
@@ -48,44 +49,12 @@
current_user { admin }
- def workflow_checkbox(from_index, to_index)
- "status_#{statuses[from_index].id}_#{statuses[to_index].id}"
- end
-
- def visit_workflow_edit(role: nil, tab: nil)
- params = { controller: "/workflows", action: :edit, type_id: type.id }
- params[:role_id] = role.id if role
- params[:tab] = tab if tab
- visit url_for(params)
- end
-
- def add_status_via_dialog(status)
- within "#workflow-table" do # Otherwise, click on "Statuses" menu item
- click_link "Status"
- end
- within_dialog "Statuses" do
- find(".ng-arrow-wrapper").click
- find(".ng-option", text: status.name).click
- click_button "Apply"
- end
- end
-
- def remove_status_via_dialog(status)
- within "#workflow-table" do # Otherwise, click on "Statuses" menu item
- click_link "Status"
- end
- within_dialog "Statuses" do
- find(".ng-value", text: status.name).find(".ng-value-icon").click
- click_button "Apply"
- end
- end
-
before do
visit_workflow_edit
end
it "allows adding another workflow" do
- visit_workflow_edit(role:)
+ visit_workflow_edit(roles: [role])
check workflow_checkbox(1, 0)
@@ -114,7 +83,7 @@ def remove_status_via_dialog(status)
old_status_id: statuses[0].id, new_status_id: statuses[1].id,
author: true, assignee: false)
- visit_workflow_edit(role:, tab: "author")
+ visit_workflow_edit(roles: [role], tab: "author")
within "#workflow_form_author" do
check workflow_checkbox(1, 0)
@@ -150,7 +119,7 @@ def remove_status_via_dialog(status)
old_status_id: statuses[0].id, new_status_id: statuses[1].id,
author: false, assignee: true)
- visit_workflow_edit(role:, tab: "assignee")
+ visit_workflow_edit(roles: [role], tab: "assignee")
within "#workflow_form_assignee" do
check workflow_checkbox(1, 0)
@@ -194,7 +163,7 @@ def remove_status_via_dialog(status)
end
before do
- visit_workflow_edit(role:)
+ visit_workflow_edit(roles: [role])
end
it "shows the always tab by default" do
@@ -254,9 +223,7 @@ def remove_status_via_dialog(status)
expect(page).to have_css("#workflow_form_author")
- expect(Workflow.exists?(role_id: role.id, type_id: type.id,
- old_status_id: statuses[1].id, new_status_id: statuses[0].id,
- author: false, assignee: false)).to be true
+ expect_transition(role, 1, 0, exist: true)
end
it "keeps unsaved changes and stays on the same tab when closing the dialog via 'X'" do
@@ -311,7 +278,7 @@ def remove_status_via_dialog(status)
end
before do
- visit_workflow_edit(role:)
+ visit_workflow_edit(roles: [role])
end
it "shows the matrix for the first role" do
@@ -322,8 +289,7 @@ def remove_status_via_dialog(status)
end
it "loads the matrix for a different role after switching" do
- click_button role.name
- click_link other_role.name
+ switch_role_via_panel(role, other_role)
within "#workflow_form_always" do
expect(page).to have_field workflow_checkbox(1, 2)
@@ -336,8 +302,7 @@ def remove_status_via_dialog(status)
check workflow_checkbox(1, 0)
end
- click_button role.name
- click_link other_role.name
+ switch_role_via_panel(role, other_role)
within_dialog "Save changes before continuing?" do
click_button "Ignore changes"
@@ -347,8 +312,7 @@ def remove_status_via_dialog(status)
expect(page).to have_field workflow_checkbox(1, 2)
end
- click_button other_role.name
- click_link role.name
+ switch_role_via_panel(other_role, role)
within "#workflow_form_always" do
expect(page).to have_field workflow_checkbox(1, 0), checked: false
@@ -360,8 +324,7 @@ def remove_status_via_dialog(status)
check workflow_checkbox(1, 0)
end
- click_button role.name
- click_link other_role.name
+ switch_role_via_panel(role, other_role)
within_dialog "Save changes before continuing?" do
click_button "Save changes and continue"
@@ -373,9 +336,7 @@ def remove_status_via_dialog(status)
expect(page).to have_field workflow_checkbox(1, 2)
end
- expect(Workflow.exists?(role_id: role.id, type_id: type.id,
- old_status_id: statuses[1].id, new_status_id: statuses[0].id,
- author: false, assignee: false)).to be true
+ expect_transition(role, 1, 0, exist: true)
end
it "keeps unsaved changes and stays on the same role when closing the dialog via 'X'" do
@@ -383,8 +344,7 @@ def remove_status_via_dialog(status)
check workflow_checkbox(1, 0)
end
- click_button role.name
- click_link other_role.name
+ switch_role_via_panel(role, other_role)
within_dialog "Save changes before continuing?" do
find(".close-button").click
@@ -402,8 +362,7 @@ def remove_status_via_dialog(status)
add_status_via_dialog(statuses[2])
expect(page).to have_field workflow_checkbox(0, 2)
- click_button role.name
- click_link other_role.name
+ switch_role_via_panel(role, other_role)
expect(page).to have_dialog("Save changes before continuing?")
end
@@ -417,8 +376,7 @@ def remove_status_via_dialog(status)
expect(page).to have_no_field workflow_checkbox(0, 1)
- click_button role.name
- click_link other_role.name
+ switch_role_via_panel(role, other_role)
expect(page).to have_dialog("Save changes before continuing?")
end
@@ -426,7 +384,7 @@ def remove_status_via_dialog(status)
context "when reloading the page with unsaved changes", :js do
before do
- visit_workflow_edit(role:)
+ visit_workflow_edit(roles: [role])
end
it "shows a browser confirmation when reloading with unsaved checkbox changes" do
@@ -466,7 +424,7 @@ def remove_status_via_dialog(status)
context "with status dialog", :js do
before do
- visit_workflow_edit(role:)
+ visit_workflow_edit(roles: [role])
end
it "shows only role-specific statuses in the matrix by default" do
@@ -474,7 +432,7 @@ def remove_status_via_dialog(status)
create(:workflow, role_id: other_role.id, type_id: type.id,
old_status_id: statuses[0].id, new_status_id: statuses[2].id)
- visit_workflow_edit(role:)
+ visit_workflow_edit(roles: [role])
expect(page).to have_field workflow_checkbox(0, 1)
expect(page).to have_no_field workflow_checkbox(2, 0)
@@ -573,8 +531,7 @@ def remove_status_via_dialog(status)
expect_flash(message: "Successful update.")
- expect(Workflow.exists?(role_id: role.id, type_id: type.id,
- old_status_id: statuses[0].id, new_status_id: statuses[2].id)).to be true
+ expect_transition(role, 0, 2, exist: true)
expect(page).to have_field workflow_checkbox(0, 2)
@@ -590,7 +547,7 @@ def remove_status_via_dialog(status)
expect(page).to have_field workflow_checkbox(2, 0)
- visit_workflow_edit(role:)
+ visit_workflow_edit(roles: [role])
expect(page).to have_no_field workflow_checkbox(2, 0)
end
@@ -637,7 +594,7 @@ def remove_status_via_dialog(status)
expect_flash(message: "Successful update.")
- visit_workflow_edit(role:)
+ visit_workflow_edit(roles: [role])
expect(page).to have_no_field workflow_checkbox(2, 0)
expect(page).to have_no_field workflow_checkbox(0, 2)
@@ -698,9 +655,7 @@ def remove_status_via_dialog(status)
expect_flash(message: "Successful update.")
- expect(Workflow.exists?(role_id: role.id, type_id: type.id,
- old_status_id: statuses[1].id, new_status_id: statuses[0].id,
- author: false, assignee: false)).to be true
+ expect_transition(role, 1, 0, exist: true)
expect(page).to have_dialog "Copy workflow"
end
diff --git a/spec/support/workflows/edit_helpers.rb b/spec/support/workflows/edit_helpers.rb
new file mode 100644
index 000000000000..7b30cd14bc4b
--- /dev/null
+++ b/spec/support/workflows/edit_helpers.rb
@@ -0,0 +1,93 @@
+# frozen_string_literal: true
+
+# -- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# 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.
+#
+# See COPYRIGHT and LICENSE files for more details.
+# ++
+
+module Workflows
+ module EditHelpers
+ def workflow_checkbox(from_index, to_index)
+ "status_#{statuses[from_index].id}_#{statuses[to_index].id}"
+ end
+
+ def visit_workflow_edit(roles: [], tab: nil)
+ params = {}
+ params[:role_ids] = roles.map(&:id) if roles.any?
+ params[:tab] = tab if tab
+ visit edit_workflow_path(type, **params)
+ end
+
+ def switch_role_via_panel(from_role, to_role)
+ click_button from_role.name
+ find("[data-item-id='#{to_role.id}']").click
+ find("[data-item-id='#{from_role.id}']").click
+ within("select-panel") { click_button "Save" }
+ end
+
+ def add_status_via_dialog(status)
+ within "#workflow-table" do # Otherwise, click on "Statuses" menu item
+ click_link "Status"
+ end
+ within_dialog "Statuses" do
+ find(".ng-arrow-wrapper").click
+ find(".ng-option", text: status.name).click
+ click_button "Apply"
+ end
+ end
+
+ def remove_status_via_dialog(status)
+ within "#workflow-table" do # Otherwise, click on "Statuses" menu item
+ click_link "Status"
+ end
+ within_dialog "Statuses" do
+ find(".ng-value", text: status.name).find(".ng-value-icon").click
+ click_button "Apply"
+ end
+ end
+
+ def indeterminate?(checkbox_id)
+ page.evaluate_script("document.getElementById('#{checkbox_id}')?.indeterminate ?? false")
+ end
+
+ def indeterminate_visible?(checkbox_id)
+ page.evaluate_script(<<~JS)
+ (() => {
+ const el = document.getElementById('#{checkbox_id}');
+ const bg = window.getComputedStyle(el).backgroundColor;
+ return bg !== 'rgba(0, 0, 0, 0)' && bg !== 'rgb(255, 255, 255)';
+ })()
+ JS
+ end
+
+ def expect_transition(role, from_index, to_index, exist:, author: false, assignee: false)
+ expect(Workflow.exists?(role_id: role.id, type_id: type.id,
+ old_status_id: statuses[from_index].id,
+ new_status_id: statuses[to_index].id,
+ author:, assignee:)).to be exist
+ end
+ end
+end