refactor: move workshop host validation to model

app/controllers/admin/workshops_controller.rb

@@ -26,9 +26,10 @@ def new
   def create
     resolve_chapter_name_to_id
     @workshop = Workshop.new(workshop_params)
+    @workshop.selected_host_id = host_id
     @workshop.created_by = current_user
     authorize(@workshop)
-    if workshop_type_valid? && @workshop.save
+    if @workshop.save
       finish_creation
     else
       flash[:warning] = @workshop.errors.full_messages; render 'new'
@@ -53,9 +54,9 @@ def show
   def update
     authorize @workshop
 
-    @workshop.assign_attributes(workshop_params)
+    @workshop.assign_attributes(workshop_params.merge(selected_host_id: host_id))
 
-    if workshop_type_valid? && @workshop.valid?
+    if @workshop.valid?
       @workshop.save
       update_workshop_details
 
@@ -258,17 +259,6 @@ def revoke_organiser_access(organiser_ids)
     end
   end
 
-  def physical_workshop_and_no_host?(workshop)
-    !workshop.virtual && host_id.empty?
-  end
-
-  def workshop_type_valid?
-    return true unless physical_workshop_and_no_host?(@workshop)
-
-    @workshop.errors.add(:host, "can't be blank")
-    false
-  end
-
   def update_workshop_details
     assign_organisers(organiser_ids)
     assign_host(host_id)

app/models/workshop.rb

@@ -6,7 +6,7 @@ class Workshop < ApplicationRecord
   include RsvpClosable
 
   attr_accessor :local_date, :local_time, :local_end_time, :rsvp_open_local_date, :rsvp_open_local_time,
-                :rsvp_close_local_date, :rsvp_close_local_time
+                :rsvp_close_local_date, :rsvp_close_local_time, :selected_host_id
 
   resourcify :permissions, role_cname: 'Permission', role_table_name: :permission
 
@@ -43,6 +43,7 @@ class Workshop < ApplicationRecord
                     if: proc { |model| model.chapter_id.present? }
   validate :rsvp_date_time_fields_must_be_paired
   validate :rsvp_close_before_workshop_start
+  validate :physical_workshop_requires_host
 
   def host
     workshop_host&.sponsor
@@ -187,4 +188,15 @@ def rsvp_close_before_workshop_start
 
     errors.add(:rsvp_close_local_date, 'must be before the workshop start time')
   end
+
+  def physical_workshop_requires_host
+    return if virtual?
+
+    host_selected = if selected_host_id.nil?
+      workshop_sponsors.any? { |link| link.host && link.sponsor.present? }
+    else
+      selected_host_id.present?
+    end
+    errors.add(:host, "can't be blank") unless host_selected
+  end
 end

spec/controllers/admin/workshops_controller_spec.rb

@@ -231,6 +231,21 @@ def assigns(symbol)
       end.not_to raise_error
     end
 
+    it 'does not create a physical workshop without a host' 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: '0',
+          host: ''
+        } }
+      end.not_to change(Workshop, :count)
+
+      expect(flash[:warning]).to include("Host can't be blank")
+    end
+
     it 'stamps created_by and records workshop.created' do
       post :create, params: { workshop: {
         chapter_id: workshop.chapter.id,

spec/fabricators/workshop_fabricator.rb

@@ -21,6 +21,8 @@
   date_and_time { Time.zone.now + 2.days }
   ends_at { |attrs| attrs[:date_and_time] + 2.hours }
   chapter
+  before_create { |workshop| workshop.selected_host_id = 'fixture-host' }
+  after_create { |workshop| workshop.selected_host_id = nil }
 end
 
 Fabricator(:workshop_auto_rsvp_in_past, from: :workshop) do

spec/models/workshop_spec.rb

@@ -12,6 +12,45 @@
   context 'with validates' do
     it { is_expected.to validate_presence_of(:chapter_id) }
 
+    describe 'physical workshop host validation' do
+      let(:workshop) { Fabricate.build(:workshop_no_sponsor, virtual: false) }
+
+      it 'requires a host for physical workshops' do
+        expect(workshop.valid?).to be(false)
+
+        expect(workshop.errors[:host]).to include("can't be blank")
+      end
+
+      it 'does not require a host for virtual workshops' do
+        workshop = Fabricate.build(:virtual_workshop)
+
+        workshop.valid?
+        expect(workshop.errors[:host]).to be_empty
+      end
+
+      it 'does not add a host error when a host is submitted' do
+        workshop.selected_host_id = '1'
+
+        workshop.valid?
+        expect(workshop.errors[:host]).to be_empty
+      end
+
+      it 'does not add a host error for an existing host without a new selection' do
+        workshop = Fabricate.build(:workshop)
+
+        workshop.valid?
+        expect(workshop.errors[:host]).to be_empty
+      end
+
+      it 'requires a host when an existing host is explicitly cleared' do
+        workshop = Fabricate.build(:workshop)
+        workshop.selected_host_id = ''
+
+        expect(workshop.valid?).to be(false)
+        expect(workshop.errors[:host]).to include("can't be blank")
+      end
+    end
+
     describe '#date_and_time' do
       it 'does not validate if chapter_id blank' do
         workshop.chapter_id = nil