From ad9dd83c8fe3ada16aedaac646daf4bb9dda44ab Mon Sep 17 00:00:00 2001
From: David M <207745043+davidmillen50@users.noreply.github.com>
Date: Sun, 25 Oct 2026 13:00:27 +0000
Subject: [PATCH 01/11] chore: simplify WorkshopOrganiserAssigner and remove
 redundant chapter arg

---
 app/services/workshop_organiser_assigner.rb | 30 +++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 app/services/workshop_organiser_assigner.rb

diff --git a/app/services/workshop_organiser_assigner.rb b/app/services/workshop_organiser_assigner.rb
new file mode 100644
index 000000000..e3038a842
--- /dev/null
+++ b/app/services/workshop_organiser_assigner.rb
@@ -0,0 +1,30 @@
+class WorkshopOrganiserAssigner
+  def initialize(workshop, organiser_ids, default_to_chapter: false)
+    @workshop = workshop
+    @organiser_ids = organiser_ids
+    @default_to_chapter = default_to_chapter
+  end
+
+  def call
+    if @default_to_chapter && @organiser_ids.blank?
+      grant(@workshop.chapter.organisers.pluck(:id))
+    else
+      raise ArgumentError, 'organiser_ids is required' if @organiser_ids.nil?
+
+      desired_ids = Array(@organiser_ids).reject(&:empty?).map(&:to_s)
+      grant(desired_ids)
+      revoke_other_organisers(desired_ids)
+    end
+  end
+
+  private
+
+  def grant(organiser_ids)
+    organiser_ids.each { |id| Member.find(id).add_role(:organiser, @workshop) }
+  end
+
+  def revoke_other_organisers(desired_ids)
+    current_ids = @workshop.organisers.pluck(:id).map(&:to_s)
+    (current_ids - desired_ids).each { |id| Member.find(id).revoke(:organiser, @workshop) }
+  end
+end

From b2b98545ea2876a55682e7283eeeffcf267897d2 Mon Sep 17 00:00:00 2001
From: David M <207745043+davidmillen50@users.noreply.github.com>
Date: Sun, 25 Oct 2026 13:00:33 +0000
Subject: [PATCH 02/11] fix: raise ArgumentError when organiser_ids is nil

---
 .../workshop_organiser_assigner_spec.rb       | 70 +++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 spec/services/workshop_organiser_assigner_spec.rb

diff --git a/spec/services/workshop_organiser_assigner_spec.rb b/spec/services/workshop_organiser_assigner_spec.rb
new file mode 100644
index 000000000..2d485d408
--- /dev/null
+++ b/spec/services/workshop_organiser_assigner_spec.rb
@@ -0,0 +1,70 @@
+require 'rails_helper'
+
+RSpec.describe WorkshopOrganiserAssigner, type: :service do
+  let(:workshop) { Fabricate(:workshop) }
+  let(:chapter) { workshop.chapter }
+
+  describe '#call' do
+    context 'when organiser_ids are provided' do
+      let(:organiser) { Fabricate(:member) }
+
+      it 'grants organiser role to members in the list' do
+        expect do
+          described_class.new(workshop, [organiser.id.to_s], default_to_chapter: false).call
+          workshop.reload
+        end.to change { workshop.organisers.include?(organiser) }.from(false).to(true)
+      end
+
+      it 'revokes organiser role from members not in the list' do
+        existing_organiser = Fabricate(:member)
+        existing_organiser.add_role(:organiser, workshop)
+        workshop.reload
+        described_class.new(workshop, [organiser.id.to_s], default_to_chapter: false).call
+        workshop.reload
+        expect(workshop.organisers).not_to include(existing_organiser)
+      end
+    end
+
+    context 'when organiser_ids are blank with default_to_chapter' do
+      it 'defaults to the chapter organisers' do
+        chapter_organiser = Fabricate(:member)
+        chapter_organiser.add_role(:organiser, chapter)
+        described_class.new(workshop, [], default_to_chapter: true).call
+        workshop.reload
+        expect(workshop.organisers).to include(chapter_organiser)
+      end
+
+      it 'preserves existing workshop organisers when falling back to chapter' do
+        chapter_organiser = Fabricate(:member)
+        chapter_organiser.add_role(:organiser, chapter)
+        existing_organiser = Fabricate(:member)
+        existing_organiser.add_role(:organiser, workshop)
+        described_class.new(workshop, [], default_to_chapter: true).call
+        workshop.reload
+        expect(workshop.organisers).to include(existing_organiser)
+        expect(workshop.organisers).to include(chapter_organiser)
+      end
+    end
+
+    context 'when organiser_ids contain only blank values (update path)' do
+      it 'revokes all and does not fall back to chapter' do
+        chapter_organiser = Fabricate(:member)
+        chapter_organiser.add_role(:organiser, chapter)
+        existing_organiser = Fabricate(:member)
+        existing_organiser.add_role(:organiser, workshop)
+        described_class.new(workshop, [''], default_to_chapter: false).call
+        workshop.reload
+        expect(workshop.organisers).not_to include(existing_organiser)
+        expect(workshop.organisers).to eq([])
+      end
+    end
+
+    context 'when organiser_ids is nil' do
+      it 'raises ArgumentError' do
+        expect do
+          described_class.new(workshop, nil, default_to_chapter: false).call
+        end.to raise_error(ArgumentError, 'organiser_ids is required')
+      end
+    end
+  end
+end

From 1c5e3c9073f259b9d13ed22c9c8eb52cbad4869c Mon Sep 17 00:00:00 2001
From: David M <207745043+davidmillen50@users.noreply.github.com>
Date: Sun, 25 Oct 2026 13:00:38 +0000
Subject: [PATCH 03/11] fix: strengthen organiser assignment specs to verify
 grant effects

---
 app/controllers/admin/workshops_controller.rb | 10 +---
 .../admin/workshops_controller_spec.rb        | 58 +++++++++++++++++++
 2 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/app/controllers/admin/workshops_controller.rb b/app/controllers/admin/workshops_controller.rb
index 2d7e2119c..8efe5bd0e 100644
--- a/app/controllers/admin/workshops_controller.rb
+++ b/app/controllers/admin/workshops_controller.rb
@@ -220,9 +220,7 @@ def assign_host(host_id)
   end
 
   def assign_organisers(organiser_ids)
-    organiser_ids.reject!(&:empty?)
-    grant_organiser_access(organiser_ids)
-    revoke_organiser_access(organiser_ids)
+    WorkshopOrganiserAssigner.new(@workshop, organiser_ids).call
   end
 
   def finish_creation
@@ -234,11 +232,7 @@ def finish_creation
   end
 
   def assign_organisers_or_default
-    if organiser_ids.present?
-      assign_organisers(organiser_ids)
-    else
-      grant_organiser_access(@workshop.chapter.organisers.pluck(:id))
-    end
+    WorkshopOrganiserAssigner.new(@workshop, organiser_ids, default_to_chapter: true).call
   end
 
   def host_id
diff --git a/spec/controllers/admin/workshops_controller_spec.rb b/spec/controllers/admin/workshops_controller_spec.rb
index ec2fe1e67..62c90cebe 100644
--- a/spec/controllers/admin/workshops_controller_spec.rb
+++ b/spec/controllers/admin/workshops_controller_spec.rb
@@ -297,6 +297,64 @@ def create_workshop
     end
   end
 
+  describe 'POST #create organiser assignment' do
+    let(:organiser) { Fabricate(:member) }
+
+    it 'does not raise when organiser_ids are provided' do
+      expect do
+        post :create, params: {
+          workshop: {
+            chapter_id: workshop.chapter.id,
+            local_date: (Time.zone.now + 1.week).strftime('%d/%m/%Y'),
+            local_time: '15:00', local_end_time: '17:00',
+            virtual: '1', coach_spaces: 5, student_spaces: 15,
+            host: nil, organisers: [organiser.id.to_s]
+          }
+        }
+      end.not_to raise_error
+      created = Workshop.unscoped.order(:id).last
+      expect(created.chapter).to eq(workshop.chapter)
+    end
+
+    it 'does not raise when organiser_ids are blank' do
+      expect do
+        post :create, params: {
+          workshop: {
+            chapter_id: workshop.chapter.id,
+            local_date: (Time.zone.now + 1.week).strftime('%d/%m/%Y'),
+            local_time: '15:00', local_end_time: '17:00',
+            virtual: '1', coach_spaces: 5, student_spaces: 15,
+            host: nil
+          }
+        }
+      end.not_to raise_error
+      created = Workshop.unscoped.order(:id).last
+      expect(created.chapter).to eq(workshop.chapter)
+    end
+  end
+
+  describe 'PATCH #update organiser revocation' do
+    let(:existing_organiser) { Fabricate(:member) }
+    let(:new_organiser) { Fabricate(:member) }
+
+    it 'revokes removed organisers after update' do
+      existing_organiser.add_role(:organiser, workshop)
+      patch :update, params: {
+        id: workshop.id,
+        workshop: {
+          local_date: workshop.date_and_time.strftime('%d/%m/%Y'),
+          local_time: workshop.date_and_time.strftime('%H:%M'),
+          local_end_time: (workshop.date_and_time + 2.hours).strftime('%H:%M'),
+          host: workshop.host&.id || '',
+          organisers: [new_organiser.id.to_s]
+        }
+      }
+      workshop.reload
+      expect(workshop.organisers).to include(new_organiser)
+      expect(workshop.organisers).not_to include(existing_organiser)
+    end
+  end
+
   describe 'DELETE #destroy' do
     context 'when workshop invitations have been sent' do
       before do

From dd4431632474c18b3606ee32ca43de7cf0364eb1 Mon Sep 17 00:00:00 2001
From: davidmillen50 <207745043+davidmillen50@users.noreply.github.com>
Date: Fri, 30 Oct 2026 10:09:52 +0000
Subject: [PATCH 04/11] Update
 spec/controllers/admin/workshops_controller_spec.rb

test: the added line asserts that the provided organiser id reaches the created workshop, so the spec fails if the params key is dropped and the chapter fallback takes over.

Co-authored-by: Morgan Roderick <20321+mroderick@users.noreply.github.com>
---
 spec/controllers/admin/workshops_controller_spec.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/spec/controllers/admin/workshops_controller_spec.rb b/spec/controllers/admin/workshops_controller_spec.rb
index 62c90cebe..ba3a257e4 100644
--- a/spec/controllers/admin/workshops_controller_spec.rb
+++ b/spec/controllers/admin/workshops_controller_spec.rb
@@ -312,6 +312,7 @@ def create_workshop
           }
         }
       end.not_to raise_error
+      expect(Workshop.unscoped.order(:id).last.organisers).to include(organiser)
       created = Workshop.unscoped.order(:id).last
       expect(created.chapter).to eq(workshop.chapter)
     end

From abbbc7310ac1731aa84efccc91452f4d47ddbe9d Mon Sep 17 00:00:00 2001
From: David M <207745043+davidmillen50@users.noreply.github.com>
Date: Fri, 30 Oct 2026 10:52:48 +0000
Subject: [PATCH 05/11] fix: create organiser test

---
 spec/controllers/admin/workshops_controller_spec.rb | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/spec/controllers/admin/workshops_controller_spec.rb b/spec/controllers/admin/workshops_controller_spec.rb
index ba3a257e4..cd46aa479 100644
--- a/spec/controllers/admin/workshops_controller_spec.rb
+++ b/spec/controllers/admin/workshops_controller_spec.rb
@@ -307,13 +307,15 @@ def create_workshop
             chapter_id: workshop.chapter.id,
             local_date: (Time.zone.now + 1.week).strftime('%d/%m/%Y'),
             local_time: '15:00', local_end_time: '17:00',
-            virtual: '1', coach_spaces: 5, student_spaces: 15,
+            virtual: '1', slack_channel: 'workshop-channel',
+            slack_channel_link: 'https://codebar.slack.link/workshop-channel',
+            coach_spaces: 5, student_spaces: 15,
             host: nil, organisers: [organiser.id.to_s]
           }
         }
       end.not_to raise_error
-      expect(Workshop.unscoped.order(:id).last.organisers).to include(organiser)
       created = Workshop.unscoped.order(:id).last
+      expect(created.organisers).to include(organiser)
       expect(created.chapter).to eq(workshop.chapter)
     end
 

From 66305576a9c06276d3174790d83713454569d24e Mon Sep 17 00:00:00 2001
From: David M <207745043+davidmillen50@users.noreply.github.com>
Date: Thu, 12 Nov 2026 10:59:25 +0000
Subject: [PATCH 06/11] fix: handle empty workshop organiser selection

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 app/controllers/admin/workshops_controller.rb |  2 +-
 .../admin/workshops_controller_spec.rb        | 54 +++++++++++++++----
 2 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/app/controllers/admin/workshops_controller.rb b/app/controllers/admin/workshops_controller.rb
index b50d23cbf..29e6cc70f 100644
--- a/app/controllers/admin/workshops_controller.rb
+++ b/app/controllers/admin/workshops_controller.rb
@@ -261,7 +261,7 @@ def revoke_organiser_access(organiser_ids)
   end
 
   def update_workshop_details
-    assign_organisers(organiser_ids)
+    assign_organisers(organiser_ids) if params[:workshop].key?(:organisers)
     assign_host(host_id)
   end
 
diff --git a/spec/controllers/admin/workshops_controller_spec.rb b/spec/controllers/admin/workshops_controller_spec.rb
index e00259dbe..ebf1a2dc8 100644
--- a/spec/controllers/admin/workshops_controller_spec.rb
+++ b/spec/controllers/admin/workshops_controller_spec.rb
@@ -377,22 +377,49 @@ def create_workshop
     let(:existing_organiser) { Fabricate(:member) }
     let(:new_organiser) { Fabricate(:member) }
 
+    def update_workshop(organisers: :omitted)
+      submitted_organisers = organisers == :omitted ? {} : { organisers: }
+      patch :update, params: { id: workshop.id, workshop: workshop_update_attributes.merge(submitted_organisers) }
+    end
+
+    def workshop_update_attributes
+      date = workshop.date_and_time
+      {
+        local_date: date.strftime('%d/%m/%Y'),
+        local_time: date.strftime('%H:%M'),
+        local_end_time: (date + 2.hours).strftime('%H:%M'),
+        host: workshop.host&.id || ''
+      }
+    end
+
     it 'revokes removed organisers after update' do
       existing_organiser.add_role(:organiser, workshop)
-      patch :update, params: {
-        id: workshop.id,
-        workshop: {
-          local_date: workshop.date_and_time.strftime('%d/%m/%Y'),
-          local_time: workshop.date_and_time.strftime('%H:%M'),
-          local_end_time: (workshop.date_and_time + 2.hours).strftime('%H:%M'),
-          host: workshop.host&.id || '',
-          organisers: [new_organiser.id.to_s]
-        }
-      }
+      update_workshop(organisers: [new_organiser.id.to_s])
       workshop.reload
       expect(workshop.organisers).to include(new_organiser)
       expect(workshop.organisers).not_to include(existing_organiser)
     end
+
+    it 'clears organisers when the form submits only its blank sentinel' do
+      chapter_organiser = Fabricate(:member)
+      chapter_organiser.add_role(:organiser, workshop.chapter)
+      existing_organiser.add_role(:organiser, workshop)
+
+      update_workshop(organisers: [''])
+
+      expect(response).to redirect_to(admin_workshop_path(workshop))
+      expect(workshop.reload.organisers).to be_empty
+      expect(workshop.organisers).not_to include(chapter_organiser)
+    end
+
+    it 'preserves organisers when the request omits the organisers parameter' do
+      existing_organiser.add_role(:organiser, workshop)
+
+      update_workshop
+
+      expect(response).to redirect_to(admin_workshop_path(workshop))
+      expect(workshop.reload.organisers).to include(existing_organiser)
+    end
   end
 
   describe 'DELETE #destroy' do
@@ -493,6 +520,13 @@ def create_workshop
   describe 'GET #edit' do
     render_views
 
+    it 'includes a blank organiser value so deselecting all submits an empty selection' do
+      get :edit, params: { id: workshop.id }
+
+      html = Nokogiri::HTML(response.body)
+      expect(html.css('input[type="hidden"][name="workshop[organisers][]"][value=""]')).to be_present
+    end
+
     it 'pre-fills main date and time values in ISO format' do
       get :edit, params: { id: workshop.id }
 

From 427328e3a42feed67c63a4131d6262c73ae1ec99 Mon Sep 17 00:00:00 2001
From: David M <207745043+davidmillen50@users.noreply.github.com>
Date: Thu, 12 Nov 2026 11:08:43 +0000
Subject: [PATCH 07/11] fix: validate workshop organiser selections

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 app/controllers/admin/workshops_controller.rb | 41 ++++++----
 app/services/workshop_organiser_assigner.rb   | 46 +++++++++++-
 .../admin/workshops_controller_spec.rb        | 75 ++++++++++++++++++-
 .../workshop_organiser_assigner_spec.rb       | 21 ++++++
 4 files changed, 164 insertions(+), 19 deletions(-)

diff --git a/app/controllers/admin/workshops_controller.rb b/app/controllers/admin/workshops_controller.rb
index 29e6cc70f..15a8edfce 100644
--- a/app/controllers/admin/workshops_controller.rb
+++ b/app/controllers/admin/workshops_controller.rb
@@ -29,11 +29,10 @@ def create
     @workshop.selected_host_id = host_id
     @workshop.created_by = current_user
     authorize(@workshop)
-    if @workshop.save
-      finish_creation
-    else
-      flash[:warning] = @workshop.errors.full_messages; render 'new'
-    end
+    return if render_organiser_selection_error('new', default_to_chapter: true)
+    return finish_creation if @workshop.save
+
+    flash[:warning] = @workshop.errors.full_messages; render 'new'
   end
 
   def edit
@@ -55,16 +54,13 @@ def update
     authorize @workshop
 
     @workshop.assign_attributes(workshop_params.merge(selected_host_id: host_id))
+    return if params[:workshop].key?(:organisers) && render_organiser_selection_error('edit')
 
-    if @workshop.valid?
-      @workshop.save
-      update_workshop_details
+    return render_update_error unless @workshop.valid?
 
-      redirect_to admin_workshop_path(@workshop), notice: I18n.t('admin.messages.workshop.updated')
-    else
-      flash[:warning] = @workshop.errors.full_messages
-      render 'edit'
-    end
+    @workshop.save
+    update_workshop_details
+    redirect_to admin_workshop_path(@workshop), notice: I18n.t('admin.messages.workshop.updated')
   end
 
   def send_invites; end
@@ -265,6 +261,25 @@ def update_workshop_details
     assign_host(host_id)
   end
 
+  def render_organiser_selection_error(template, default_to_chapter: false)
+    error = WorkshopOrganiserAssigner.validation_error(@workshop, organiser_ids, default_to_chapter:)
+    return false unless error
+
+    render_with_organiser_error(template, error)
+    true
+  end
+
+  def render_with_organiser_error(template, error)
+    @workshop.errors.add(:base, error)
+    flash.now[:warning] = @workshop.errors.full_messages
+    render template, status: :unprocessable_content
+  end
+
+  def render_update_error
+    flash[:warning] = @workshop.errors.full_messages
+    render 'edit'
+  end
+
   def set_sponsor
     @sponsor = Sponsor.find(params[:workshop][:sponsor_ids])
   end
diff --git a/app/services/workshop_organiser_assigner.rb b/app/services/workshop_organiser_assigner.rb
index e3038a842..856ce9990 100644
--- a/app/services/workshop_organiser_assigner.rb
+++ b/app/services/workshop_organiser_assigner.rb
@@ -1,4 +1,41 @@
 class WorkshopOrganiserAssigner
+  def self.validation_error(workshop, organiser_ids, default_to_chapter: false)
+    return if default_to_chapter && default_selection?(organiser_ids)
+    return 'organiser_ids is required' if organiser_ids.nil?
+    return 'organiser_ids must be an array of ids' unless organiser_ids.is_a?(Array)
+
+    validate_ids(workshop, organiser_ids.reject(&:blank?))
+  end
+
+  def self.default_selection?(organiser_ids)
+    organiser_ids.nil? || organiser_ids == [] || (organiser_ids.is_a?(Array) && organiser_ids.all?(&:blank?))
+  end
+
+  def self.validate_ids(workshop, ids)
+    return if ids.empty?
+    return 'Select valid chapter organisers.' unless valid_ids?(ids)
+    return 'Select organisers from this chapter.' unless chapter_organisers?(workshop, ids)
+
+    nil
+  end
+
+  def self.valid_ids?(ids)
+    ids.all? { |id| valid_id?(id) }
+  end
+
+  def self.chapter_organisers?(workshop, ids)
+    eligible_ids = workshop.chapter&.organisers&.where(id: ids.map(&:to_i))&.pluck(:id).to_a.map(&:to_s)
+    (ids.map(&:to_s).uniq - eligible_ids).empty?
+  end
+
+  private_class_method :validate_ids, :valid_ids?, :chapter_organisers?
+
+  def self.valid_id?(id)
+    (id.is_a?(String) || id.is_a?(Integer)) && id.to_s.match?(/\A[1-9]\d*\z/)
+  end
+
+  private_class_method :valid_id?
+
   def initialize(workshop, organiser_ids, default_to_chapter: false)
     @workshop = workshop
     @organiser_ids = organiser_ids
@@ -6,12 +43,15 @@ def initialize(workshop, organiser_ids, default_to_chapter: false)
   end
 
   def call
-    if @default_to_chapter && @organiser_ids.blank?
+    error = self.class.validation_error(@workshop, @organiser_ids, default_to_chapter: @default_to_chapter)
+    raise ArgumentError, error if error
+
+    if @default_to_chapter && self.class.default_selection?(@organiser_ids)
       grant(@workshop.chapter.organisers.pluck(:id))
     else
-      raise ArgumentError, 'organiser_ids is required' if @organiser_ids.nil?
+      raise ArgumentError, 'organiser_ids must be an array of ids' unless @organiser_ids.is_a?(Array)
 
-      desired_ids = Array(@organiser_ids).reject(&:empty?).map(&:to_s)
+      desired_ids = @organiser_ids.reject(&:blank?).map(&:to_s)
       grant(desired_ids)
       revoke_other_organisers(desired_ids)
     end
diff --git a/spec/controllers/admin/workshops_controller_spec.rb b/spec/controllers/admin/workshops_controller_spec.rb
index ebf1a2dc8..d8a6ee47e 100644
--- a/spec/controllers/admin/workshops_controller_spec.rb
+++ b/spec/controllers/admin/workshops_controller_spec.rb
@@ -338,6 +338,7 @@ def create_workshop
     let(:organiser) { Fabricate(:member) }
 
     it 'does not raise when organiser_ids are provided' do
+      organiser.add_role(:organiser, workshop.chapter)
       expect do
         post :create, params: {
           workshop: {
@@ -363,7 +364,9 @@ def create_workshop
             chapter_id: workshop.chapter.id,
             local_date: (Time.zone.now + 1.week).strftime('%d/%m/%Y'),
             local_time: '15:00', local_end_time: '17:00',
-            virtual: '1', coach_spaces: 5, student_spaces: 15,
+            virtual: '1', slack_channel: 'workshop-channel',
+            slack_channel_link: 'https://codebar.slack.link/workshop-channel',
+            coach_spaces: 5, student_spaces: 15,
             host: nil
           }
         }
@@ -371,15 +374,56 @@ def create_workshop
       created = Workshop.unscoped.order(:id).last
       expect(created.chapter).to eq(workshop.chapter)
     end
+
+    it 'defaults to chapter organisers when the form sends only the blank sentinel' do
+      chapter_organiser = Fabricate(:member)
+      chapter_organiser.add_role(:organiser, workshop.chapter)
+
+      post :create, params: {
+        workshop: {
+          chapter_id: workshop.chapter.id,
+          local_date: (Time.zone.now + 1.week).strftime('%d/%m/%Y'),
+          local_time: '15:00', local_end_time: '17:00',
+          virtual: '1', slack_channel: 'workshop-channel',
+          slack_channel_link: 'https://codebar.slack.link/workshop-channel',
+          coach_spaces: 5, student_spaces: 15,
+          host: nil, organisers: ['']
+        }
+      }
+
+      created = Workshop.unscoped.order(:id).last
+      expect(flash[:warning]).to be_nil
+      expect(response).to redirect_to(admin_workshop_path(created))
+      expect(created.organisers).to include(chapter_organiser)
+    end
+
+    it 'rejects a scalar organiser value before creating the workshop' do
+      expect do
+        post :create, params: {
+          workshop: {
+            chapter_id: workshop.chapter.id,
+            local_date: (Time.zone.now + 1.week).strftime('%d/%m/%Y'),
+            local_time: '15:00', local_end_time: '17:00',
+            virtual: '1', slack_channel: 'workshop-channel',
+            slack_channel_link: 'https://codebar.slack.link/workshop-channel',
+            coach_spaces: 5, student_spaces: 15,
+            host: nil, organisers: 'not-an-array'
+          }
+        }
+      end.not_to change(Workshop, :count)
+
+      expect(response).to have_http_status(:unprocessable_content)
+      expect(flash[:warning]).to include('organiser_ids must be an array of ids')
+    end
   end
 
   describe 'PATCH #update organiser revocation' do
     let(:existing_organiser) { Fabricate(:member) }
     let(:new_organiser) { Fabricate(:member) }
 
-    def update_workshop(organisers: :omitted)
+    def update_workshop(organisers: :omitted, **attributes)
       submitted_organisers = organisers == :omitted ? {} : { organisers: }
-      patch :update, params: { id: workshop.id, workshop: workshop_update_attributes.merge(submitted_organisers) }
+      patch :update, params: { id: workshop.id, workshop: workshop_update_attributes.merge(attributes, submitted_organisers) }
     end
 
     def workshop_update_attributes
@@ -394,6 +438,7 @@ def workshop_update_attributes
 
     it 'revokes removed organisers after update' do
       existing_organiser.add_role(:organiser, workshop)
+      new_organiser.add_role(:organiser, workshop.chapter)
       update_workshop(organisers: [new_organiser.id.to_s])
       workshop.reload
       expect(workshop.organisers).to include(new_organiser)
@@ -420,6 +465,30 @@ def workshop_update_attributes
       expect(response).to redirect_to(admin_workshop_path(workshop))
       expect(workshop.reload.organisers).to include(existing_organiser)
     end
+
+    it 'rejects a scalar organiser value before saving workshop changes' do
+      update_workshop(organisers: new_organiser.id.to_s, description: 'Should not save')
+
+      expect(response).to have_http_status(:unprocessable_content)
+      expect(workshop.reload.description).not_to eq('Should not save')
+      expect(workshop.organisers).to be_empty
+    end
+
+    it 'rejects an unknown organiser ID before saving workshop changes' do
+      update_workshop(organisers: ['999999999'], description: 'Should not save')
+
+      expect(response).to have_http_status(:unprocessable_content)
+      expect(workshop.reload.description).not_to eq('Should not save')
+    end
+
+    it 'rejects a non-chapter organiser before saving workshop changes' do
+      outsider = Fabricate(:member)
+      update_workshop(organisers: [outsider.id.to_s], description: 'Should not save')
+
+      expect(response).to have_http_status(:unprocessable_content)
+      expect(workshop.reload.description).not_to eq('Should not save')
+      expect(workshop.organisers).to be_empty
+    end
   end
 
   describe 'DELETE #destroy' do
diff --git a/spec/services/workshop_organiser_assigner_spec.rb b/spec/services/workshop_organiser_assigner_spec.rb
index 2d485d408..7470bfea4 100644
--- a/spec/services/workshop_organiser_assigner_spec.rb
+++ b/spec/services/workshop_organiser_assigner_spec.rb
@@ -9,6 +9,7 @@
       let(:organiser) { Fabricate(:member) }
 
       it 'grants organiser role to members in the list' do
+        organiser.add_role(:organiser, chapter)
         expect do
           described_class.new(workshop, [organiser.id.to_s], default_to_chapter: false).call
           workshop.reload
@@ -17,6 +18,7 @@
 
       it 'revokes organiser role from members not in the list' do
         existing_organiser = Fabricate(:member)
+        organiser.add_role(:organiser, chapter)
         existing_organiser.add_role(:organiser, workshop)
         workshop.reload
         described_class.new(workshop, [organiser.id.to_s], default_to_chapter: false).call
@@ -34,6 +36,14 @@
         expect(workshop.organisers).to include(chapter_organiser)
       end
 
+      it 'defaults to the chapter organisers when only the form sentinel is submitted' do
+        chapter_organiser = Fabricate(:member)
+        chapter_organiser.add_role(:organiser, chapter)
+        described_class.new(workshop, [''], default_to_chapter: true).call
+
+        expect(workshop.reload.organisers).to include(chapter_organiser)
+      end
+
       it 'preserves existing workshop organisers when falling back to chapter' do
         chapter_organiser = Fabricate(:member)
         chapter_organiser.add_role(:organiser, chapter)
@@ -66,5 +76,16 @@
         end.to raise_error(ArgumentError, 'organiser_ids is required')
       end
     end
+
+    context 'when organiser_ids contain a non-chapter organiser' do
+      it 'rejects the selection before changing workshop roles' do
+        organiser = Fabricate(:member)
+
+        expect do
+          described_class.new(workshop, [organiser.id.to_s]).call
+        end.to raise_error(ArgumentError, 'Select organisers from this chapter.')
+        expect(workshop.reload.organisers).to be_empty
+      end
+    end
   end
 end

From 0095afb64dda40f6e57960de4a803e4d7f5a538e Mon Sep 17 00:00:00 2001
From: David M <207745043+davidmillen50@users.noreply.github.com>
Date: Thu, 12 Nov 2026 11:19:35 +0000
Subject: [PATCH 08/11] fix: make organiser assignment atomic

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 app/services/workshop_organiser_assigner.rb   | 35 +++++++++++++------
 .../workshop_organiser_assigner_spec.rb       | 26 ++++++++++++++
 2 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/app/services/workshop_organiser_assigner.rb b/app/services/workshop_organiser_assigner.rb
index 856ce9990..2d64f2f7e 100644
--- a/app/services/workshop_organiser_assigner.rb
+++ b/app/services/workshop_organiser_assigner.rb
@@ -46,21 +46,34 @@ def call
     error = self.class.validation_error(@workshop, @organiser_ids, default_to_chapter: @default_to_chapter)
     raise ArgumentError, error if error
 
-    if @default_to_chapter && self.class.default_selection?(@organiser_ids)
-      grant(@workshop.chapter.organisers.pluck(:id))
-    else
-      raise ArgumentError, 'organiser_ids must be an array of ids' unless @organiser_ids.is_a?(Array)
-
-      desired_ids = @organiser_ids.reject(&:blank?).map(&:to_s)
-      grant(desired_ids)
-      revoke_other_organisers(desired_ids)
-    end
+    default_selection = @default_to_chapter && self.class.default_selection?(@organiser_ids)
+    Member.transaction { assign_roles(default_selection) }
   end
 
   private
 
-  def grant(organiser_ids)
-    organiser_ids.each { |id| Member.find(id).add_role(:organiser, @workshop) }
+  def assign_roles(default_selection)
+    desired_ids = organiser_ids_for(default_selection)
+    grant(resolve_members(desired_ids))
+    revoke_other_organisers(desired_ids) unless default_selection
+  end
+
+  def organiser_ids_for(default_selection)
+    return @workshop.chapter.organisers.pluck(:id).map(&:to_s) if default_selection
+
+    @organiser_ids.reject(&:blank?).map(&:to_s)
+  end
+
+  def resolve_members(desired_ids)
+    members = Member.where(id: desired_ids.map(&:to_i)).index_by { |member| member.id.to_s }
+    missing_ids = desired_ids.uniq - members.keys
+    raise ArgumentError, "Unknown organiser IDs: #{missing_ids.join(', ')}" if missing_ids.any?
+
+    members.values
+  end
+
+  def grant(organisers)
+    organisers.each { |organiser| organiser.add_role(:organiser, @workshop) }
   end
 
   def revoke_other_organisers(desired_ids)
diff --git a/spec/services/workshop_organiser_assigner_spec.rb b/spec/services/workshop_organiser_assigner_spec.rb
index 7470bfea4..b53375515 100644
--- a/spec/services/workshop_organiser_assigner_spec.rb
+++ b/spec/services/workshop_organiser_assigner_spec.rb
@@ -87,5 +87,31 @@
         expect(workshop.reload.organisers).to be_empty
       end
     end
+
+    context 'when organiser_ids include a stale member ID' do
+      it 'rejects all requested IDs before granting any roles' do
+        chapter_organiser = Fabricate(:member)
+        chapter_organiser.add_role(:organiser, chapter)
+        allow(described_class).to receive(:validation_error).and_return(nil)
+
+        expect do
+          described_class.new(workshop, [chapter_organiser.id.to_s, '999999999']).call
+        end.to raise_error(ArgumentError, 'Unknown organiser IDs: 999999999')
+        expect(workshop.reload.organisers).to be_empty
+      end
+    end
+
+    it 'rolls back grants if a later roster change fails' do
+      chapter_organiser = Fabricate(:member)
+      chapter_organiser.add_role(:organiser, chapter)
+      existing_organiser = Fabricate(:member)
+      existing_organiser.add_role(:organiser, workshop)
+      assigner = described_class.new(workshop, [chapter_organiser.id.to_s])
+      allow(assigner).to receive(:revoke_other_organisers).and_raise(StandardError, 'revoke failed')
+
+      expect { assigner.call }.to raise_error(StandardError, 'revoke failed')
+      expect(workshop.reload.organisers).to include(existing_organiser)
+      expect(workshop.organisers).not_to include(chapter_organiser)
+    end
   end
 end

From 88045ee4878ab88f98f14e32a38f187df1a5948e Mon Sep 17 00:00:00 2001
From: David M <207745043+davidmillen50@users.noreply.github.com>
Date: Thu, 12 Nov 2026 11:24:13 +0000
Subject: [PATCH 09/11] fix: address organiser review feedback

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 spec/controllers/admin/workshops_controller_spec.rb | 5 ++++-
 spec/services/workshop_organiser_assigner_spec.rb   | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/spec/controllers/admin/workshops_controller_spec.rb b/spec/controllers/admin/workshops_controller_spec.rb
index d8a6ee47e..f52997cc8 100644
--- a/spec/controllers/admin/workshops_controller_spec.rb
+++ b/spec/controllers/admin/workshops_controller_spec.rb
@@ -358,6 +358,9 @@ def create_workshop
     end
 
     it 'does not raise when organiser_ids are blank' do
+      chapter_organiser = Fabricate(:member)
+      chapter_organiser.add_role(:organiser, workshop.chapter)
+
       expect do
         post :create, params: {
           workshop: {
@@ -372,7 +375,7 @@ def create_workshop
         }
       end.not_to raise_error
       created = Workshop.unscoped.order(:id).last
-      expect(created.chapter).to eq(workshop.chapter)
+      expect(created.reload.organisers).to include(chapter_organiser)
     end
 
     it 'defaults to chapter organisers when the form sends only the blank sentinel' do
diff --git a/spec/services/workshop_organiser_assigner_spec.rb b/spec/services/workshop_organiser_assigner_spec.rb
index b53375515..2eec7d86f 100644
--- a/spec/services/workshop_organiser_assigner_spec.rb
+++ b/spec/services/workshop_organiser_assigner_spec.rb
@@ -1,6 +1,6 @@
 require 'rails_helper'
 
-RSpec.describe WorkshopOrganiserAssigner, type: :service do
+RSpec.describe WorkshopOrganiserAssigner do
   let(:workshop) { Fabricate(:workshop) }
   let(:chapter) { workshop.chapter }
 

From adb986ed9df15002bf41b23bdda48d6c1082a4a2 Mon Sep 17 00:00:00 2001
From: David M <207745043+davidmillen50@users.noreply.github.com>
Date: Thu, 12 Nov 2026 11:34:20 +0000
Subject: [PATCH 10/11] fix: remove obsolete workshop organiser helpers

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 app/controllers/admin/workshops_controller.rb | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/app/controllers/admin/workshops_controller.rb b/app/controllers/admin/workshops_controller.rb
index 15a8edfce..a33be6e50 100644
--- a/app/controllers/admin/workshops_controller.rb
+++ b/app/controllers/admin/workshops_controller.rb
@@ -246,16 +246,6 @@ def organiser_ids
     params[:workshop][:organisers]
   end
 
-  def grant_organiser_access(organiser_ids)
-    organiser_ids.each { |id| Member.find(id).add_role(:organiser, @workshop) }
-  end
-
-  def revoke_organiser_access(organiser_ids)
-    (@workshop.organisers.pluck(:id).map(&:to_s) - organiser_ids).each do |id|
-      Member.find(id).revoke(:organiser, @workshop)
-    end
-  end
-
   def update_workshop_details
     assign_organisers(organiser_ids) if params[:workshop].key?(:organisers)
     assign_host(host_id)

From 99960583b7ae2b94b437df7c5a751372edd10904 Mon Sep 17 00:00:00 2001
From: David M <207745043+davidmillen50@users.noreply.github.com>
Date: Thu, 12 Nov 2026 11:53:36 +0000
Subject: [PATCH 11/11] fix: cover blank scalar organiser submissions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 spec/controllers/admin/workshops_controller_spec.rb | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/spec/controllers/admin/workshops_controller_spec.rb b/spec/controllers/admin/workshops_controller_spec.rb
index f52997cc8..01c77c73d 100644
--- a/spec/controllers/admin/workshops_controller_spec.rb
+++ b/spec/controllers/admin/workshops_controller_spec.rb
@@ -469,12 +469,13 @@ def workshop_update_attributes
       expect(workshop.reload.organisers).to include(existing_organiser)
     end
 
-    it 'rejects a scalar organiser value before saving workshop changes' do
-      update_workshop(organisers: new_organiser.id.to_s, description: 'Should not save')
+    it 'rejects a blank scalar organiser value before saving workshop changes' do
+      existing_organiser.add_role(:organiser, workshop)
+      update_workshop(organisers: '', description: 'Should not save')
 
       expect(response).to have_http_status(:unprocessable_content)
       expect(workshop.reload.description).not_to eq('Should not save')
-      expect(workshop.organisers).to be_empty
+      expect(workshop.organisers).to include(existing_organiser)
     end
 
     it 'rejects an unknown organiser ID before saving workshop changes' do