diff --git a/app/controllers/admin/workshops_controller.rb b/app/controllers/admin/workshops_controller.rb index 004ea2e4f..2d7e2119c 100644 --- a/app/controllers/admin/workshops_controller.rb +++ b/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) diff --git a/app/models/workshop.rb b/app/models/workshop.rb index 370159504..dfc749bd7 100644 --- a/app/models/workshop.rb +++ b/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 diff --git a/spec/controllers/admin/workshops_controller_spec.rb b/spec/controllers/admin/workshops_controller_spec.rb index 351b6f025..ec2fe1e67 100644 --- a/spec/controllers/admin/workshops_controller_spec.rb +++ b/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, diff --git a/spec/fabricators/workshop_fabricator.rb b/spec/fabricators/workshop_fabricator.rb index f45a64977..bde0f738f 100644 --- a/spec/fabricators/workshop_fabricator.rb +++ b/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 diff --git a/spec/models/workshop_spec.rb b/spec/models/workshop_spec.rb index c5078d522..7ca2af802 100644 --- a/spec/models/workshop_spec.rb +++ b/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