Add and update specs

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

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

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
 

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

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