Skip to content

fix(admin): warn when inviting to an event or workshop that is not invitable - #3047

Open
costajohnt wants to merge 1 commit into
codebar:masterfrom
costajohnt:fix/warn-when-invite-target-not-invitable
Open

costajohnt wants to merge 1 commit into
codebar:masterfrom
costajohnt:fix/warn-when-invite-target-not-invitable

Conversation

@costajohnt

Copy link
Copy Markdown
Contributor

Closes #2945

Admin::EventsController#invite and Admin::WorkshopsController#invite now check invitable? before enqueueing. A non-invitable event or workshop redirects back with a warning ("The event is not invitable" / "The workshop is not invitable") and nothing is enqueued. Invitable ones behave as before.

The three InvitationManager methods drop their string returns, but each keeps a bare return unless ...invitable?. These methods run through handle_asynchronously, so that guard is the only check left when the job actually runs. Keeping it means a target that stops being invitable between the click and the job still sends nothing. Happy to drop it if you'd rather have the controller be the only check.

Other notes:

  • The workshop's virtual/in-person dispatch moved into a private send_workshop_invitations helper, unchanged, so #invite stays under the 10-line method limit with the new guard.
  • New admin.event.not_invitable key next to the existing admin.workshop.not_invitable.
  • Specs: a not-invitable case for both controllers (both fail on master), plus happy-path specs for the workshop invite, in-person and virtual. There were no workshop #invite controller specs before.

…vitable

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 codebar#2945
@mroderick

Copy link
Copy Markdown
Collaborator

Thanks, @costajohnt — this looks good to me.

Quick summary of the review (correctness, standards, testing and reliability lenses, plus independent validation of the findings):

  • The invitable? guards in Admin::EventsController#invite and Admin::WorkshopsController#invite behave as described: a non-invitable target redirects back with the warning and enqueues nothing, an invitable one is unchanged. Redirect targets, flash handling and the new admin.event.not_invitable key all check out.
  • Keeping the bare return unless ...invitable? guards in InvitationManager is the right call. The methods run through handle_asynchronously, so the controller check gates the click and the job-side guard gates the run; dropping either would leave a gap.
  • The send_workshop_invitations extraction keeps #invite under the method-length limit and changes no behaviour.
  • The new specs cover the change well: not-invitable cases for both controllers, plus the workshop happy paths (in-person and virtual) that had no coverage before.

Two small, optional notes on the new specs:

  • The warning specs assert the English copy from en.yml directly (for example 'The event is not invitable'). Asserting through the translation helper would survive copy edits; asserting the raw string also guarantees the locale keys stay wired, so this is a judgement call.
  • Nothing covers the job-side guard your PR body describes: a spec that runs the delayed job against a record that stopped being invitable between enqueue and execution would pin that behaviour down.

No blockers from my side.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

InvitationManager silently no-ops on non-invitable events/workshops while controllers flash success

2 participants