From b7c2369988ad970c2bf00078fc787094c515814d Mon Sep 17 00:00:00 2001
From: Mike Dalessio <mike@37signals.com>
Date: Sun, 30 Mar 2025 09:24:56 -0400
Subject: [PATCH] Fix setting the workflow on a collection

The previous implementation would delete all accesses if the bucket
was not all_access.

I think it's simpler to move the bucket workflow into a separate
Buckets::WorkflowsController, rather than deal with different sets of
params from different forms in the existing BucketsController.
---
 app/controllers/buckets/workflows_controller.rb  | 16 ++++++++++++++++
 app/controllers/buckets_controller.rb            |  3 +--
 app/views/buckets/edit.html.erb                  |  2 +-
 config/routes.rb                                 |  1 +
 .../buckets/workflows_controller_test.rb         | 16 ++++++++++++++++
 test/fixtures/workflow/stages.yml                | 16 ++++++++++++++++
 test/fixtures/workflows.yml                      |  4 ++++
 7 files changed, 55 insertions(+), 3 deletions(-)
 create mode 100644 app/controllers/buckets/workflows_controller.rb
 create mode 100644 test/controllers/buckets/workflows_controller_test.rb

diff --git a/app/controllers/buckets/workflows_controller.rb b/app/controllers/buckets/workflows_controller.rb
new file mode 100644
index 000000000..fb31b08d0
--- /dev/null
+++ b/app/controllers/buckets/workflows_controller.rb
@@ -0,0 +1,16 @@
+class Buckets::WorkflowsController < ApplicationController
+  include BucketScoped
+
+  before_action :set_workflow
+
+  def update
+    @bucket.update! workflow: @workflow
+
+    redirect_to bubbles_path(bucket_ids: [ @bucket ])
+  end
+
+  private
+    def set_workflow
+      @workflow = Current.account.workflows.find(params.expect(bucket: [ :workflow_id ]).require(:workflow_id))
+    end
+end
diff --git a/app/controllers/buckets_controller.rb b/app/controllers/buckets_controller.rb
index 32df270e0..63357bb92 100644
--- a/app/controllers/buckets_controller.rb
+++ b/app/controllers/buckets_controller.rb
@@ -33,8 +33,7 @@ class BucketsController < ApplicationController
     end
 
     def bucket_params
-      params.expect(bucket: [ :name, :all_access, :workflow_id ]).with_defaults(all_access: false)
-      params.require(:bucket).permit(:name, :all_access, :workflow_id)
+      params.expect(bucket: [ :name, :all_access ]).with_defaults(all_access: false)
     end
 
     def grantees
diff --git a/app/views/buckets/edit.html.erb b/app/views/buckets/edit.html.erb
index 4fa78823a..97507d54c 100644
--- a/app/views/buckets/edit.html.erb
+++ b/app/views/buckets/edit.html.erb
@@ -79,7 +79,7 @@
 <div class="panel shadow center flex flex-column gap-half margin-block-double" style="--panel-size: 55ch;">
   <strong class="txt-large">Choose a workflow</strong>
   <p class="margin-none">Use a workflow to track progress in this collection.</p>
-  <%= form_with model: @bucket, local: true, data: { controller: "form" } do |form| %>
+  <%= form_with model: @bucket, url: bucket_workflow_path(@bucket), local: true, method: :patch, data: { controller: "form" } do |form| %>
     <%= form.select :workflow_id, Current.account.workflows.map { |w| [w.name, w.id] }, { include_blank: "Choose…" }, class: "input input--select full-width", data: { action: "change->form#submit" } %>
   <% end %>
   <p><%= link_to "Manage workflows", workflows_path, class: "btn btn--plain txt-link" %></p>
diff --git a/config/routes.rb b/config/routes.rb
index a65b59db3..4468079a5 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -35,6 +35,7 @@ Rails.application.routes.draw do
   resources :buckets do
     scope module: :buckets do
       resource :subscriptions
+      resource :workflow, only: :update
     end
 
     resources :bubbles do
diff --git a/test/controllers/buckets/workflows_controller_test.rb b/test/controllers/buckets/workflows_controller_test.rb
new file mode 100644
index 000000000..6a9140425
--- /dev/null
+++ b/test/controllers/buckets/workflows_controller_test.rb
@@ -0,0 +1,16 @@
+require "test_helper"
+
+class Buckets::WorkflowsControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    sign_in_as :kevin
+  end
+
+  test "update" do
+    bucket = buckets(:writebook)
+
+    patch bucket_workflow_url(bucket), params: { bucket: { workflow_id: workflows(:on_call).id } }
+
+    assert_redirected_to bubbles_path(bucket_ids: [ bucket.id ])
+    assert_equal workflows(:on_call), bucket.reload.workflow
+  end
+end
diff --git a/test/fixtures/workflow/stages.yml b/test/fixtures/workflow/stages.yml
index 7563597fc..689d154d1 100644
--- a/test/fixtures/workflow/stages.yml
+++ b/test/fixtures/workflow/stages.yml
@@ -13,3 +13,19 @@ qa_on_hold:
 qa_review:
   name: Review
   workflow: qa
+
+on_call_inbox:
+  name: Inbox
+  workflow: on_call
+
+on_call_in_progress:
+  name: In progress
+  workflow: on_call
+
+on_call_on_hold:
+  name: Waiting on customers
+  workflow: on_call
+
+on_call_review:
+  name: Pending deploy
+  workflow: on_call
diff --git a/test/fixtures/workflows.yml b/test/fixtures/workflows.yml
index 4ac3b7570..459b9d16c 100644
--- a/test/fixtures/workflows.yml
+++ b/test/fixtures/workflows.yml
@@ -1,3 +1,7 @@
 qa:
   account: 37s
   name: QA
+
+on_call:
+  account: 37s
+  name: "On call"