From 4c7d03bcad0dc5b3641aa1c4c3ca8de4503e9bef Mon Sep 17 00:00:00 2001 From: Mir Bhatia Date: Thu, 19 Mar 2026 12:32:48 +0100 Subject: [PATCH] Add and update specs --- spec/controllers/workflows_controller_spec.rb | 91 ++-- spec/features/roles/create_spec.rb | 4 +- spec/features/types/crud_spec.rb | 3 - spec/features/workflows/edit_spec.rb | 429 +++++++++++++++--- spec/models/type_spec.rb | 50 ++ 5 files changed, 474 insertions(+), 103 deletions(-) diff --git a/spec/controllers/workflows_controller_spec.rb b/spec/controllers/workflows_controller_spec.rb index d78bed0ba57..5afe075ede3 100644 --- a/spec/controllers/workflows_controller_spec.rb +++ b/spec/controllers/workflows_controller_spec.rb @@ -153,9 +153,9 @@ RSpec.describe WorkflowsController do .to render_template :edit end - it "does not assign role" do + it "assigns the first role" do expect(assigns[:role]) - .to be_nil + .to eq role end it "does assign type" do @@ -169,12 +169,12 @@ RSpec.describe WorkflowsController do end end - context "with role and type params, and only used statuses" do + context "with role and type params" do before do - get :edit, params: { role_id: role.id.to_s, type_id: type.id.to_s, used_statuses_only: "1" } + get :edit, params: { role_id: role.id.to_s, type_id: type.id.to_s } end - it "responds with the used statuses", :aggregate_failures do + it "responds with the role type statuses", :aggregate_failures do expect(response) .to have_http_status(:ok) expect(response) @@ -188,33 +188,6 @@ RSpec.describe WorkflowsController do expect(assigns[:statuses]) .to eq type.statuses end - end - - context "with role and type params, and no statuses filter" do - before do - get :edit, params: { role_id: role.id.to_s, type_id: type.id.to_s } - end - - it "responds with all 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 - expect(assigns[:roles]) - .to eq [role] - expect(assigns[:statuses]) - .to eq Status.all - end - end - - context "with role and type params and with all statuses" do - before do - get :edit, params: { role_id: role.id.to_s, type_id: type.id.to_s, used_statuses_only: "0" } - end it "is successful" do expect(response) @@ -243,7 +216,59 @@ RSpec.describe WorkflowsController do it "assigns statuses" do expect(assigns[:statuses]) - .to eq Status.all + .to eq type.statuses + end + end + end + + describe "#confirm_statuses" do + before do + allow(controller) + .to receive(:respond_with_dialog) + .and_call_original + allow(role_scope) + .to receive(:order) + .and_return([role]) + end + + context "when no statuses were removed" do + before do + post :confirm_statuses, + params: { + role_id: role.id.to_s, + type_id: type.id.to_s, + status_ids: ["1", "2"], + original_status_ids: ["1", "2"], + tab: "always" + }, + as: :turbo_stream + end + + it "redirects to the edit page" do + expect(response) + .to redirect_to(edit_workflow_path(type.id, role_id: role.id.to_s, tab: "always", status_ids: [1, 2])) + + expect(response).to have_http_status(:see_other) + end + end + + context "when statuses were removed" do + before do + post :confirm_statuses, + params: { + role_id: role.id.to_s, + type_id: type.id.to_s, + status_ids: ["1"], + original_status_ids: ["1", "2"], + tab: "always" + }, + as: :turbo_stream + end + + it "responds with the danger dialog" do + expect(controller) + .to have_received(:respond_with_dialog) + .with(an_instance_of(Workflows::StatusRemovalDangerDialogComponent)) end end end diff --git a/spec/features/roles/create_spec.rb b/spec/features/roles/create_spec.rb index ccd7a474405..adebd7b178b 100644 --- a/spec/features/roles/create_spec.rb +++ b/spec/features/roles/create_spec.rb @@ -109,8 +109,8 @@ RSpec.describe "Role creation", :js do visit(url_for(controller: :workflows, action: :index, only_path: true)) click_link type.name - select "New role name", from: "Role" - click_button "Edit" + click_button existing_role.name + click_link "New role name" old_status = existing_workflow.old_status.name new_status = existing_workflow.new_status.name diff --git a/spec/features/types/crud_spec.rb b/spec/features/types/crud_spec.rb index d7d57cfd24d..48666b0049f 100644 --- a/spec/features/types/crud_spec.rb +++ b/spec/features/types/crud_spec.rb @@ -72,9 +72,6 @@ RSpec.describe "Types", :js do visit(url_for(controller: :workflows, action: :index, only_path: true)) click_on "A new type" - select existing_role.name, from: "Role" - click_on "Edit" - from_id = existing_workflow.old_status_id to_id = existing_workflow.new_status_id diff --git a/spec/features/workflows/edit_spec.rb b/spec/features/workflows/edit_spec.rb index 3cc7e827693..11075f8e721 100644 --- a/spec/features/workflows/edit_spec.rb +++ b/spec/features/workflows/edit_spec.rb @@ -48,54 +48,72 @@ RSpec.describe "Workflow edit" do 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) + click_link "Status" + 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) + click_link "Status" + within_dialog "Statuses" do + find(".ng-value", text: status.name).find(".ng-value-icon").click + click_button "Apply" + end + end + before do - visit url_for(controller: "/workflows", action: :edit, type_id: type.id) + visit_workflow_edit end it "allows adding another workflow" do - click_button "Edit" + visit_workflow_edit(role:) - within "#workflow_form_always" do - check "status_#{statuses[1].id}_#{statuses[2].id}" - end + check workflow_checkbox(1, 0) click_button "Save" expect_flash(message: "Successful update.") - within "#workflow_form_always" do - expect(page) - .to have_field "status_#{statuses[0].id}_#{statuses[1].id}", checked: true - expect(page) - .to have_field "status_#{statuses[1].id}_#{statuses[2].id}", checked: true + expect(page) + .to have_field workflow_checkbox(0, 1), checked: true + expect(page) + .to have_field workflow_checkbox(1, 0), checked: true - expect(page) - .to have_field "status_#{statuses[0].id}_#{statuses[2].id}", checked: false - expect(page) - .to have_field "status_#{statuses[1].id}_#{statuses[0].id}", checked: false - expect(page) - .to have_field "status_#{statuses[2].id}_#{statuses[0].id}", checked: false - expect(page) - .to have_field "status_#{statuses[2].id}_#{statuses[1].id}", checked: false + expect(Workflow.where(type_id: type.id, role_id: role.id).count).to be 2 - expect(Workflow.where(type_id: type.id, role_id: role.id).count).to be 2 + w = Workflow.where(role_id: role.id, type_id: type.id, old_status_id: statuses[0].id, new_status_id: statuses[1].id).first + assert !w.author + assert !w.assignee - w = Workflow.where(role_id: role.id, type_id: type.id, old_status_id: statuses[0].id, new_status_id: statuses[1].id).first - assert !w.author - assert !w.assignee - - w = Workflow.where(role_id: role.id, type_id: type.id, old_status_id: statuses[1].id, new_status_id: statuses[2].id).first - assert !w.author - assert !w.assignee - end + w = Workflow.where(role_id: role.id, type_id: type.id, old_status_id: statuses[1].id, new_status_id: statuses[0].id).first + assert !w.author + assert !w.assignee end it "allows editing the workflow when the user is author" do - click_link "User is author" - click_button "Edit" + create(:workflow, role_id: role.id, type_id: type.id, + old_status_id: statuses[0].id, new_status_id: statuses[1].id, + author: true, assignee: false) + + visit_workflow_edit(role:, tab: "author") within "#workflow_form_author" do - check "status_#{statuses[2].id}_#{statuses[1].id}" + check workflow_checkbox(1, 0) end click_button "Save" @@ -104,39 +122,34 @@ RSpec.describe "Workflow edit" do within "#workflow_form_author" do expect(page) - .to have_field "status_#{statuses[2].id}_#{statuses[1].id}", checked: true + .to have_field workflow_checkbox(0, 1), checked: true + expect(page) + .to have_field workflow_checkbox(1, 0), checked: true - expect(page) - .to have_field "status_#{statuses[0].id}_#{statuses[1].id}", checked: false - expect(page) - .to have_field "status_#{statuses[1].id}_#{statuses[2].id}", checked: false - expect(page) - .to have_field "status_#{statuses[0].id}_#{statuses[2].id}", checked: false - expect(page) - .to have_field "status_#{statuses[1].id}_#{statuses[0].id}", checked: false - expect(page) - .to have_field "status_#{statuses[2].id}_#{statuses[0].id}", checked: false - - expect(Workflow.where(type_id: type.id, role_id: role.id).count).to be 2 + expect(Workflow.where(type_id: type.id, role_id: role.id, author: true).count).to be 2 # the newly added Workflow - w = Workflow.where(role_id: role.id, type_id: type.id, old_status_id: statuses[2].id, new_status_id: statuses[1].id).first + w = Workflow.where(role_id: role.id, type_id: type.id, old_status_id: statuses[1].id, new_status_id: statuses[0].id).first assert w.author assert !w.assignee - # The already existing Workflow - w = Workflow.where(role_id: role.id, type_id: type.id, old_status_id: statuses[0].id, new_status_id: statuses[1].id).first + # The always workflow is unchanged + w = Workflow.where(role_id: role.id, type_id: type.id, old_status_id: statuses[0].id, new_status_id: statuses[1].id, + author: false).first assert !w.author assert !w.assignee end end it "allows editing the workflow when the user is assignee" do - click_link "User is assignee" - click_button "Edit" + 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: true) + + visit_workflow_edit(role:, tab: "assignee") within "#workflow_form_assignee" do - check "status_#{statuses[2].id}_#{statuses[0].id}" + check workflow_checkbox(1, 0) end click_button "Save" @@ -145,33 +158,129 @@ RSpec.describe "Workflow edit" do within "#workflow_form_assignee" do expect(page) - .to have_field "status_#{statuses[2].id}_#{statuses[0].id}", checked: true + .to have_field workflow_checkbox(0, 1), checked: true + expect(page) + .to have_field workflow_checkbox(1, 0), checked: true - expect(page) - .to have_field "status_#{statuses[0].id}_#{statuses[1].id}", checked: false - expect(page) - .to have_field "status_#{statuses[1].id}_#{statuses[2].id}", checked: false - expect(page) - .to have_field "status_#{statuses[0].id}_#{statuses[2].id}", checked: false - expect(page) - .to have_field "status_#{statuses[1].id}_#{statuses[0].id}", checked: false - expect(page) - .to have_field "status_#{statuses[2].id}_#{statuses[1].id}", checked: false - - expect(Workflow.where(type_id: type.id, role_id: role.id).count).to be 2 + expect(Workflow.where(type_id: type.id, role_id: role.id, assignee: true).count).to be 2 # the newly added Workflow - w = Workflow.where(role_id: role.id, type_id: type.id, old_status_id: statuses[2].id, new_status_id: statuses[0].id).first + w = Workflow.where(role_id: role.id, type_id: type.id, old_status_id: statuses[1].id, new_status_id: statuses[0].id).first assert !w.author assert w.assignee - # The already existing Workflow - w = Workflow.where(role_id: role.id, type_id: type.id, old_status_id: statuses[0].id, new_status_id: statuses[1].id).first + # The always workflow is unchanged + w = Workflow.where(role_id: role.id, type_id: type.id, old_status_id: statuses[0].id, new_status_id: statuses[1].id, + assignee: false).first assert !w.author assert !w.assignee end end + context "when switching tabs", :js do + let!(:author_workflow) do + create(:workflow, role_id: role.id, type_id: type.id, + old_status_id: statuses[1].id, new_status_id: statuses[2].id, + author: true, assignee: false) + end + let!(:assignee_workflow) 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: true) + end + + before do + visit_workflow_edit(role:) + end + + it "shows the always tab by default" do + within "#workflow_form_always" do + expect(page).to have_field workflow_checkbox(0, 1), checked: true + end + end + + it "shows the author matrix when switching to the author tab" do + click_link "User is author" + + within "#workflow_form_author" do + expect(page).to have_field workflow_checkbox(1, 2), checked: true + expect(page).to have_no_field workflow_checkbox(0, 1) + end + end + + it "shows the assignee matrix when switching to the assignee tab" do + click_link "User is assignee" + + within "#workflow_form_assignee" do + expect(page).to have_field workflow_checkbox(0, 2), checked: true + expect(page).to have_no_field workflow_checkbox(0, 1) + end + end + + it "loses unsaved checkbox changes when switching tabs" do + within "#workflow_form_always" do + check workflow_checkbox(1, 0) + end + + click_link "User is author" + click_link "Default transitions" + + within "#workflow_form_always" do + expect(page).to have_field workflow_checkbox(1, 0), checked: false + end + end + end + + context "when switching roles", :js do + let(:other_role) { create(:project_role) } + let!(:other_workflow) do + create(:workflow, role_id: other_role.id, type_id: type.id, + old_status_id: statuses[1].id, new_status_id: statuses[2].id, + author: false, assignee: false) + end + + before do + visit_workflow_edit(role:) + end + + it "shows the matrix for the first role" do + within "#workflow_form_always" do + expect(page).to have_field workflow_checkbox(0, 1) + expect(page).to have_no_field workflow_checkbox(1, 2) + end + end + + it "loads the matrix for a different role after switching" do + click_button role.name + click_link other_role.name + + within "#workflow_form_always" do + expect(page).to have_field workflow_checkbox(1, 2) + expect(page).to have_no_field workflow_checkbox(0, 1) + end + end + + it "loses unsaved checkbox changes when switching roles" do + within "#workflow_form_always" do + check workflow_checkbox(1, 0) + end + + click_button role.name + click_link other_role.name + + within "#workflow_form_always" do + expect(page).to have_field workflow_checkbox(1, 2) + end + + click_button other_role.name + click_link role.name + + within "#workflow_form_always" do + expect(page).to have_field workflow_checkbox(1, 0), checked: false + end + end + end + it "allows navigating to Workflow summary page" do within ".PageHeader-actions" do click_on "Summary" @@ -189,4 +298,194 @@ RSpec.describe "Workflow edit" do expect(page).to have_heading "Copy workflow" expect(page).to have_current_path(copy_workflows_path) end + + context "with status dialog", :js do + before do + visit_workflow_edit(role:) + end + + it "shows only role-specific statuses in the matrix by default" do + other_role = create(:project_role) + 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:) + + expect(page).to have_field workflow_checkbox(0, 1) + expect(page).to have_no_field workflow_checkbox(2, 0) + expect(page).to have_no_field workflow_checkbox(0, 2) + end + + it "pre selects the current role statuses in the dialog" do + click_link "Status" + + 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_no_css(".ng-value-label", text: statuses[2].name) + end + end + + it "adds a new status via the dialog and shows it in the workflow table" do + expect(page).to have_no_field workflow_checkbox(2, 0) + expect(page).to have_no_field workflow_checkbox(0, 2) + + add_status_via_dialog(statuses[2]) + + expect(page).to have_field workflow_checkbox(2, 0) + expect(page).to have_field workflow_checkbox(0, 2) + end + + it "checks all new checkboxes when adding a new status" do + add_status_via_dialog(statuses[2]) + + expect(page).to have_field workflow_checkbox(2, 0), checked: true + expect(page).to have_field workflow_checkbox(0, 2), checked: true + expect(page).to have_field workflow_checkbox(2, 1), checked: true + expect(page).to have_field workflow_checkbox(1, 2), checked: true + expect(page).to have_field workflow_checkbox(2, 2), checked: true + end + + it "removes the status after confirming the danger dialog" do + expect(page).to have_field workflow_checkbox(0, 1) + + remove_status_via_dialog(statuses[1]) + + expect(page).to have_dialog("Remove statuses?") + + within_dialog "Remove statuses?" do + expect(page).to have_text("Remove 1 status?") + + click_button "Remove" + end + + expect(page).to have_no_field workflow_checkbox(0, 1) + end + + it "cancels the status dialog without changing the matrix" do + expect(page).to have_no_field workflow_checkbox(2, 0) + + click_link "Status" + + within_dialog "Statuses" do + find(".ng-arrow-wrapper").click + find(".ng-option", text: statuses[2].name).click + click_button "Cancel" + end + + expect(page).to have_no_dialog("Statuses") + + expect(page).to have_no_field workflow_checkbox(2, 0) + end + + it "cancels the removal danger dialog and keeps the status in the matrix" do + expect(page).to have_field workflow_checkbox(0, 1) + + remove_status_via_dialog(statuses[1]) + + within_dialog "Remove statuses?" do + click_button "Cancel" + end + + expect(page).to have_no_dialog("Remove statuses?") + + expect(page).to have_field workflow_checkbox(0, 1) + end + + it "saves after adding a status via the dialog and persists it in the database" do + expect(page).to have_no_field workflow_checkbox(2, 0) + + add_status_via_dialog(statuses[2]) + + expect(page).to have_field workflow_checkbox(2, 0) + + click_button "Save" + + 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(page).to have_field workflow_checkbox(0, 2) + + page.driver.refresh + + expect(page).to have_field workflow_checkbox(2, 0) + end + + it "reverts applied statuses when navigating away without saving" do + expect(page).to have_no_field workflow_checkbox(2, 0) + + add_status_via_dialog(statuses[2]) + + expect(page).to have_field workflow_checkbox(2, 0) + + visit_workflow_edit(role:) + + expect(page).to have_no_field workflow_checkbox(2, 0) + end + + it "preserves existing checkbox changes after adding a new status via the dialog" do + expect(page).to have_field workflow_checkbox(0, 1), checked: true + uncheck workflow_checkbox(0, 1) + expect(page).to have_field workflow_checkbox(0, 1), checked: false + + add_status_via_dialog(statuses[2]) + + expect(page).to have_field workflow_checkbox(0, 1), checked: false + expect(page).to have_field workflow_checkbox(2, 0) + end + + it "preserves existing checkbox changes after removing an unrelated status" do + add_status_via_dialog(statuses[2]) + + expect(page).to have_field workflow_checkbox(0, 1), checked: true + uncheck workflow_checkbox(0, 1) + expect(page).to have_field workflow_checkbox(0, 1), checked: false + + remove_status_via_dialog(statuses[2]) + + within_dialog "Remove statuses?" do + click_button "Remove" + end + + expect(page).to have_field workflow_checkbox(0, 1), checked: false + end + + it "removes an unused status on save" do + add_status_via_dialog(statuses[2]) + + expect(page).to have_field workflow_checkbox(2, 0) + + uncheck workflow_checkbox(2, 0) + uncheck workflow_checkbox(0, 2) + uncheck workflow_checkbox(2, 1) + uncheck workflow_checkbox(1, 2) + uncheck workflow_checkbox(2, 2) + + click_button "Save" + + expect_flash(message: "Successful update.") + + visit_workflow_edit(role:) + + expect(page).to have_no_field workflow_checkbox(2, 0) + expect(page).to have_no_field workflow_checkbox(0, 2) + expect(page).to have_no_field workflow_checkbox(2, 1) + expect(page).to have_no_field workflow_checkbox(1, 2) + expect(page).to have_no_field workflow_checkbox(2, 2) + end + + it "shows a blankslate when no statuses are configured" do + uncheck workflow_checkbox(0, 1) + + click_button "Save" + + expect_flash(message: "Successful update.") + + expect(page).to have_text("No status transitions configured") + expect(page).to have_text("Add statuses to start configuring workflows for this role") + end + end end diff --git a/spec/models/type_spec.rb b/spec/models/type_spec.rb index 583be22b8d9..703747fa2f5 100644 --- a/spec/models/type_spec.rb +++ b/spec/models/type_spec.rb @@ -126,6 +126,56 @@ RSpec.describe Type do expect(subject.pluck(:id)).to contain_exactly(default_status.id, statuses[0].id, statuses[1].id) end end + + context "with role filter" do + let(:other_role) { create(:project_role) } + let(:other_statuses) { (1..2).map { create(:status) } } + let!(:other_workflow) do + create(:workflow, role_id: other_role.id, + type_id: type.id, + old_status_id: other_statuses[0].id, + new_status_id: other_statuses[1].id) + end + + subject { type.statuses(role:) } + + it "returns only statuses for the given role" do + expect(subject.pluck(:id)).to contain_exactly(statuses[0].id, statuses[1].id) + end + end + + context "with tab filter" do + let(:author_statuses) { (1..2).map { create(:status) } } + let(:assignee_statuses) { (1..2).map { create(:status) } } + let!(:author_workflow) do + create(:workflow, role_id: role.id, + type_id: type.id, + old_status_id: author_statuses[0].id, + new_status_id: author_statuses[1].id, + author: true, + assignee: false) + end + let!(:assignee_workflow) do + create(:workflow, role_id: role.id, + type_id: type.id, + old_status_id: assignee_statuses[0].id, + new_status_id: assignee_statuses[1].id, + author: false, + assignee: true) + end + + it "returns only always statuses for the always tab" do + expect(type.statuses(tab: "always").pluck(:id)).to contain_exactly(statuses[0].id, statuses[1].id) + end + + it "returns only author statuses for the author tab" do + expect(type.statuses(tab: "author").pluck(:id)).to contain_exactly(author_statuses[0].id, author_statuses[1].id) + end + + it "returns only assignee statuses for the assignee tab" do + expect(type.statuses(tab: "assignee").pluck(:id)).to contain_exactly(assignee_statuses[0].id, assignee_statuses[1].id) + end + end end end