One invitation per member per event or workshop - #2867
Draft
mroderick wants to merge 2 commits into
Draft
Conversation
Members subscribed to a chapter as both student and coach received two invitation emails, one per role, because each invitation pass matched on (event or workshop, member, role). Match on identity only and set the role on create, so the second pass finds the existing invitation and skips the member. For events with a blank audience, coach emails were labelled "Coach Invitation" instead of a general "Invitation"; label coach emails as such only when the event is actually for coaches. Fixes #2861
…l-role invite contract Applies the two validated findings from the ce-code-review run on PR #2867: - WorkshopsController#find_or_create_invitation keyed on (workshop, member, role), so a dual student+coach member self-RSVPing with the other role created a second invitation. Key on (workshop, member) and update the existing invitation to the chosen role, matching the InvitationManager identity semantics. - The dual-role specs asserted counts only; pin the surviving role (Coach for events, Student for workshops - pass order decides) and the delivered email subject so a pass reorder cannot silently flip which email dual members receive.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Members subscribed to a chapter as both student and coach received two invitation emails per event or workshop, one per role. They now get one invitation and one email. Coach emails for events are labelled "Coach Invitation" only when the event is actually for coaches.
Fixes #2861
Changes
InvitationManagernow treats event or workshop plus member as the invitation identity and applies the role only on create. The second pass (students after coaches for events, coaches after students for workshops) finds the existing invitation and skips the member instead of creating a duplicate with the other role.EventInvitationMailer#invite_coachused!event.audience, which is false for a blank audience string, so events saved through the admin form with audience untouched labelled coach emails "Coach Invitation". It now checks for'Coaches'explicitly.Review notes
find_or_create_by!with the role in the lookup hash were rewritten to assert created invitations and deliveries instead; behavior assertions are unchanged apart from the dual-role case, which previously asserted the buggy two-invitation behavior.Detail
Invitation creation for events and workshops used
find_or_create_by!(event:, member:, role:). Becauserolewas part of the lookup, a member subscribed as both student and coach was invited twice when both passes ran: once as a Coach, once as a Student, with an email each. On the reported event this affected 387 members. Workshops with audienceeveryonehad the same behavior.The fix changes the two shared creators in
InvitationManager(create_event_invitation,create_invitation) to look up by event or workshop plus member only, assigningrolein the create block.previously_new_record?still gates email sending, so the skipped member is logged as already invited.The blank-audience subject bug came from events edited via the admin form: the audience select submits an empty string when untouched, and
!''is false, producing "Coach Invitation" for a general invite. The wording now matches the audience gate insend_event_emails, which already uses.eql?comparisons.