From 5fba0e2e5b89f3b3862b03f3b8c6dde977b61fba Mon Sep 17 00:00:00 2001 From: John Costa Date: Sat, 17 Oct 2026 13:07:31 -0700 Subject: [PATCH] fix(admin): warn when inviting to an event or workshop that is not invitable Admin::EventsController#invite and Admin::WorkshopsController#invite always flashed a success notice. When the event or workshop was not invitable, InvitationManager returned an error string from inside the delayed job, where nobody saw it, and sent nothing. Check invitable? in both controllers before enqueueing and show a warning instead. The InvitationManager methods drop their dead string returns but keep a bare guard, so a target that stops being invitable before the job runs still sends nothing. The workshop dispatch moves into a private helper to keep #invite within the method length limit. Fixes #2945 --- app/controllers/admin/events_controller.rb | 3 ++ app/controllers/admin/workshops_controller.rb | 19 +++++++--- app/services/invitation_manager.rb | 6 +-- config/locales/en.yml | 2 + .../admin/events_controller_spec.rb | 16 ++++++++ .../admin/workshops_controller_spec.rb | 37 +++++++++++++++++++ 6 files changed, 74 insertions(+), 9 deletions(-) diff --git a/app/controllers/admin/events_controller.rb b/app/controllers/admin/events_controller.rb index 1ac14dc93..192260ed4 100644 --- a/app/controllers/admin/events_controller.rb +++ b/app/controllers/admin/events_controller.rb @@ -43,6 +43,9 @@ def update def invite @event = Event.find_by(slug: params[:event_id]) authorize @event + unless @event.invitable? + return redirect_to admin_event_path(@event), flash: { warning: t('admin.event.not_invitable') } + end @event.chapters.each do |chapter| InvitationManager.new.send_event_emails(@event, chapter, current_user.id) diff --git a/app/controllers/admin/workshops_controller.rb b/app/controllers/admin/workshops_controller.rb index 004ea2e4f..5019790af 100644 --- a/app/controllers/admin/workshops_controller.rb +++ b/app/controllers/admin/workshops_controller.rb @@ -84,14 +84,13 @@ def attendees_emails def invite set_workshop authorize @workshop - audience = params[:for] - - if @workshop.virtual? - InvitationManager.new.send_virtual_workshop_emails(@workshop, audience, current_user.id) - else - InvitationManager.new.send_workshop_emails(@workshop, audience, current_user.id) + unless @workshop.invitable? + return redirect_to admin_workshop_path(@workshop), flash: { warning: t('admin.workshop.not_invitable') } end + audience = params[:for] + send_workshop_invitations(audience) + redirect_to admin_workshop_path(@workshop), notice: "Invitations to #{audience} are being emailed out." end @@ -162,6 +161,14 @@ def destroy_host private + def send_workshop_invitations(audience) + if @workshop.virtual? + InvitationManager.new.send_virtual_workshop_emails(@workshop, audience, current_user.id) + else + InvitationManager.new.send_workshop_emails(@workshop, audience, current_user.id) + end + end + def resolve_chapter_name_to_id chapter_value = params.dig(:workshop, :chapter_id) return if chapter_value.blank? || chapter_value.match?(/\A\d+\z/) diff --git a/app/services/invitation_manager.rb b/app/services/invitation_manager.rb index 822592283..e7348fcee 100644 --- a/app/services/invitation_manager.rb +++ b/app/services/invitation_manager.rb @@ -1,6 +1,6 @@ class InvitationManager def send_event_emails(event, chapter, initiator_id = nil) # rubocop:disable Metrics/AbcSize - return 'The event is not invitable' unless event.invitable? + return unless event.invitable? audience = event_audience(event) logger = invitation_logger(event, initiator_id, audience, chapter.id) @@ -51,7 +51,7 @@ def send_workshop_attendance_reminders(workshop) handle_asynchronously :send_workshop_attendance_reminders def send_workshop_emails(workshop, audience, initiator_id = nil) - return 'The workshop is not invitable' unless workshop.invitable? + return unless workshop.invitable? logger = invitation_logger(workshop, initiator_id, audience, workshop.chapter_id) result = start_invitation_batch(logger) @@ -70,7 +70,7 @@ def send_workshop_emails(workshop, audience, initiator_id = nil) handle_asynchronously :send_workshop_emails def send_virtual_workshop_emails(workshop, audience, initiator_id = nil) - return 'The workshop is not invitable' unless workshop.invitable? + return unless workshop.invitable? logger = invitation_logger(workshop, initiator_id, audience, workshop.chapter_id) result = start_invitation_batch(logger) diff --git a/config/locales/en.yml b/config/locales/en.yml index e96c5b323..bcd14957a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -766,6 +766,8 @@ en: members: activity: for: "for %{subject}" + event: + not_invitable: "The event is not invitable" workshop: manage_rsvps: text: "RSVP Students and Coaches to the workshop. The attendance confirmation email will be triggered and they will be removed from the waiting list." diff --git a/spec/controllers/admin/events_controller_spec.rb b/spec/controllers/admin/events_controller_spec.rb index 822a0a44d..a86dac619 100644 --- a/spec/controllers/admin/events_controller_spec.rb +++ b/spec/controllers/admin/events_controller_spec.rb @@ -31,6 +31,22 @@ expect(flash[:notice]).to eq('Invitations will be emailed out soon.') end + + context 'when the event is not invitable' do + before { event.update_column(:invitable, false) } + + it 'warns instead of claiming invitations are on their way, and enqueues nothing' do + manager = instance_spy(InvitationManager) + allow(InvitationManager).to receive(:new).and_return(manager) + + post :invite, params: { event_id: event.slug } + + expect(manager).not_to have_received(:send_event_emails) + expect(response).to redirect_to(admin_event_path(event)) + expect(flash[:warning]).to eq('The event is not invitable') + expect(flash[:notice]).to be_nil + end + end end describe 'POST #invite with unauthorised user' do diff --git a/spec/controllers/admin/workshops_controller_spec.rb b/spec/controllers/admin/workshops_controller_spec.rb index 351b6f025..deb83e10b 100644 --- a/spec/controllers/admin/workshops_controller_spec.rb +++ b/spec/controllers/admin/workshops_controller_spec.rb @@ -19,6 +19,43 @@ def assigns(symbol) controller.instance_variable_get("@#{symbol}") end + describe 'POST #invite' do + let(:manager) { instance_spy(InvitationManager) } + + before { allow(InvitationManager).to receive(:new).and_return(manager) } + + it 'enqueues the invitation emails and sets a notice' do + post :invite, params: { workshop_id: workshop.id, for: 'students' } + + expect(manager).to have_received(:send_workshop_emails).with(workshop, 'students', admin.id) + expect(response).to redirect_to(admin_workshop_path(workshop)) + expect(flash[:notice]).to eq('Invitations to students are being emailed out.') + end + + it 'enqueues the virtual invitation emails for a virtual workshop' do + virtual_workshop = Fabricate(:virtual_workshop, chapter: workshop.chapter) + + post :invite, params: { workshop_id: virtual_workshop.id, for: 'students' } + + expect(manager).to have_received(:send_virtual_workshop_emails).with(virtual_workshop, 'students', admin.id) + expect(manager).not_to have_received(:send_workshop_emails) + end + + context 'when the workshop is not invitable' do + before { workshop.update_column(:invitable, false) } + + it 'warns instead of claiming invitations are on their way, and enqueues nothing' do + post :invite, params: { workshop_id: workshop.id, for: 'students' } + + expect(manager).not_to have_received(:send_workshop_emails) + expect(manager).not_to have_received(:send_virtual_workshop_emails) + expect(response).to redirect_to(admin_workshop_path(workshop)) + expect(flash[:warning]).to eq('The workshop is not invitable') + expect(flash[:notice]).to be_nil + end + end + end + describe 'GET #show' do it 'loads the workshop attendance page with attendees' do Fabricate(:workshop_invitation, workshop:, attending: true)