fix: blast only when send flips on, not on every save - #5
Merged
Merged
Conversation
The afterChange hook fired whenever `send` was truthy, which is not the same question as "was a send just requested". A blast that fails leaves the flag set — the reset only runs after a successful send — so every later save of that event re-attempted it: an editor fixing a typo, the events-site cron writing remindersSent, Wack Hacker appending a photo to images[]. That last one is how this surfaced. The bot attached a Discord photo to an event, the hook re-fired on a blast nobody asked for, our mail provider rejected its key, and Payload answered the bot's write with the mail error's 401 — for a document that had already saved. The bot read that as "the CMS refused me" and told an organizer their photos had not been filed while they sat on the event. Today the re-fire is harmless only because the mail key is invalid. It stops being harmless the moment mail works again, which is the change queued up behind this one: the next save of any event still carrying a checked `send` would mail every RSVP. Create is treated separately because Payload's `previousDoc` on a create is not a prior state, so comparing against it there would suppress the first send. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AHAyCUvMbU4jG9bg5EuC3C
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR updates the events collection afterChange hook to send reminder blasts only when an editor (or API client) newly requests a send by flipping the send checkbox from false to true, rather than whenever send remains truthy across subsequent saves. This prevents unintended re-sends caused by later, unrelated updates (e.g., bot image uploads or cron-driven field updates) while keeping the existing “send then reset send to false” workflow intact.
Changes:
- Add
previousDoc+operationawareness to the EventsafterChangehook. - Gate
sendEventBlastbehind a “just requested” (false → true) transition check (withcreatehandled explicitly). - Keep internal updates (that set
send: false/sentAt) usingcontext.skipEmailSendto avoid re-triggering.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
rayhanadev
added a commit
to purduehackers/wack-hacker
that referenced
this pull request
Sep 1, 2026
An organizer opened an image drop, posted a photo, and was told the CMS would not let the bot add it to the event. The photo was on the event. The bot had attached it and then reported the opposite, which sent someone hunting through the CMS for a permission problem that did not exist. Payload runs a collection's `afterChange` hooks after the document is committed. The events collection blasts a reminder email in one of those hooks, the CMS's mail key was rejected, and the whole request answered 401 — describing a write that had already saved. `isPermissionDenied` read 401 as "access denied", because a status code is all it looked at. A status is not evidence about a write. Both `attachImages` and `detachImages` now re-read the event when the write reports failure and treat a change that is actually there as the success it was. Only a failure that survives that check is reported, and the thread notice quotes what the CMS said rather than naming a cause it cannot know. The attach failure is also always reported to telemetry now. The old code emitted nothing on that path, which is why answering "what did the CMS actually say" needed the CMS's own runtime logs rather than ours. Requires purduehackers/cms#5, which stops the blast hook re-firing on saves that never asked for it — the condition that produced the misleading status here. Claude-Session: https://claude.ai/code/session_01AHAyCUvMbU4jG9bg5EuC3C Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Events.afterChangefired wheneversendwas truthy, which is not the same question as "was a send just requested". A blast that fails leaves the flag set — the reset only runs after a successful send — so every later save of that event re-attempted it: an editor fixing a typo, the events-site cron writingremindersSent, Wack Hacker appending a photo toimages[].How this surfaced
The bot attached a Discord photo to an event via
PATCH /api/events/193. The hook re-fired a blast nobody asked for, Resend rejected the CMS's key, and Payload answered the bot's write with the mail error's status:afterChangeruns after the document commits, so that 401 described a write that had already saved. The bot read it as a refusal and told an organizer their photos had not been filed — while they sat on the event. (The bot side now verifies before believing a failing status: purduehackers/wack-hacker#162.)Why it matters now
The re-fire is harmless today only because the mail key is invalid. It stops being harmless the moment mail works again — the next save of any event still carrying a checked
sendwould mail every RSVP. That makes this worth landing ahead of the provider switch, not alongside it.The change
Create is treated separately because Payload's
previousDocon a create is not a prior state — comparing against it there would suppress the first send. Re-sending is now asked for by re-checking the box, not by touching the event.Verified with
tsc --noEmitandprettier --check.🤖 Generated with Claude Code
https://claude.ai/code/session_01AHAyCUvMbU4jG9bg5EuC3C