Repository navigation
Conversation
Add pins for two unpinned behaviours: - WaitingList.next_spot filters by role, so an older Coach entry must not be returned when a Student seat frees up (model) and must not be promoted when a Student cancels (controller). - The reject flow sends the promoted member the waiting-list variant of the attending email; assert the delivery, the promoted copy's text, and that exactly one email goes out.
WorkshopInvitationController#reject set `attending: false` but left the member's WaitingList row in place, so a cancelling member's own entry could be returned by `WaitingList.next_spot` for the seat they just freed: the flow destroyed that entry, re-set the freshly cancelled invitation to attending and emailed the member they are attending again. Destroy the cancelling member's entry before looking up the next spot, so a rejection cannot list the member back in and the promotion goes to the real next entry for the role.
The reject endpoint authenticates by the invitation token in the URL, like accept and update. Its rejection form must survive a browser withholding the session cookie (e.g. Safari/WebKit ITP on cross-site navigation), so CSRF enforcement raises InvalidAuthenticityToken before the token can do its job. Add reject to the skip_forgery_protection list, matching accept, update and WaitingListsController, and pin the token-only, no-session request with a spec.
WaitingListsController#destroy called destroy on WaitingList.find_by(...), which is nil when the entry no longer exists — reachable by replaying a stale "Remove from the waiting list" link (double-click, or a bookmarked token URL after the entry was consumed), a 500 that Rollbar hears. Guard the lookup and redirect with a notice when the entry is gone, and skip the waiting_list.left activity record.
This branch has not been deployed
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
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.
Closes three pre-existing gaps in the workshop waiting-list flow and pins two previously unpinned behaviours. Work done from
master(before thefeat/post-close-rsvp-waitlistrename, so the tested entry point isWaitingList.next_spot; the branch'spromote_nextkeeps this filtering).Cancelling while waitlisted left a stale entry.
WorkshopInvitationController#rejectsetattending: falsebut never destroyed the member's ownWaitingListrow. Worse, the promotion lookup could pick the cancelling member's own entry for the seat they had just freed: that entry was destroyed, the freshly cancelled invitation was set back toattending: true, and the member was emailed a confirmation they had already declined. The entry is now destroyed before the next spot is looked up, so promotion goes to the real next entry for the role.Destroy of a gone entry 500'd.
WaitingListsController#destroycalleddestroyonfind_by(...), which is nil when the entry has already been consumed — reachable by replaying a stale "Remove from the waiting list" link. It now redirects with a notice instead.Reject needed a session.
rejectis token-authenticated likeacceptandupdate, but was not in theskip_forgery_protectionlist, so a browser withholding the session cookie gotInvalidAuthenticityToken. It is now skipped for the same reason the other token-authenticated actions are.New pins: cross-role promotion (an older Coach entry is never picked when a Student seat frees up), the promotion email (delivery, its waiting-list body copy, and that exactly one email goes out), and the no-session token-only reject request.
Review notes
Suggested focus:
app/controllers/workshop_invitation_controller.rb): destroying the cancelling member's entry before computing the next spot is what prevents the self-promotion; the three new specs in its context pin entry removal, no re-acceptance, and promotion of the entry behind.rejectwidens what a cross-site POST can invoke. The authenticator is the invitation token, which only email recipients hold; the reasoning mirrorsaccept,updateandWaitingListsController(PR fix: skip CSRF protection for feedback form submission #2641, Rollbar Security warning in Firefox #535).feat/post-close-rsvp-waitlist: the closed-waitlist view guard, the destroy gate matrix (custom close vs 3.5h freeze), admin removal between freeze and start,promote_nextupdate-failure path, concurrency, and the promotion/freeze boundary. The admin-removal email assertion also waits for that branch.