fix: clear scheduled_at on unschedule, return on template failures, add SES config LocalStack (testing) - #367
Open
williamchiii wants to merge 8 commits into
Open
fix: clear scheduled_at on unschedule, return on template failures, add SES config LocalStack (testing)#367williamchiii wants to merge 8 commits into
williamchiii wants to merge 8 commits into
Conversation
Resolves applicant recipient groups to deduped contact emails (excluding
is_fake seeded rows), adds raw-HTML task type and SES send methods, registers
all task types in the email worker mux, and adds SendCampaign with
draft/scheduled -> sending -> sent/failed transitions plus an admin-gated
POST /email/campaigns/{campaignId}/send endpoint.
Adds GetUserContactEmailsByRoles (role-based lookup on users, excluding is_fake rows) and a recipientRoles map, so resolveRecipients now dispatches applicant groups by application status and role groups by user role. Only interest_subscribers remains unsupported.
Introduces campaignStore and campaignMailer interfaces in the email package so the campaign service can be tested with fakes (concrete repository and email service satisfy them unchanged). Adds 8 tests covering per-recipient queueing, format routing, cross-group dedup, status transitions including sending->failed with last_error, and the guard paths. test: cover SendCampaign send pipeline Introduces campaignStore and campaignMailer interfaces in the email package so the campaign service can be tested with fakes (concrete repository and email service satisfy them unchanged). Adds 8 tests covering per-recipient queueing, format routing, cross-group dedup, status transitions including sending->failed with last_error, and the guard paths.
Adds DeleteEmailCampaign query, repo method, and a DeleteCampaign service
method guarded by canDeleteCampaign so draft, scheduled, and failed campaigns
can be removed while sent and sending ones stay on the record. Exposes
DELETE /email/campaigns/{campaignId} with OpenAPI docs.
Adds an asynq Scheduler in cmd/email_worker that sweeps every minute for campaigns whose scheduled_at has passed, failing any more than two hours late so an outage cannot trigger a surprise blast. Sends are now claimed with a conditional UPDATE, so overlapping ticks or a double-clicked Send cannot dispatch the same campaign twice. Also resolves interest_subscribers, completing all seven recipient types.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
5 tasks
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.
Description
Three small backend fixes found while building the campaign UI and verifying the
send pipeline end to end.
Unscheduling left a stale send time.
PATCH /statuswithstatus: "draft"nevercleared
scheduled_at, so an unscheduled campaign kept advertising a time it wouldnever send at. The handler cannot fix this itself —
ScheduledAtis a*time.Time,so an omitted field and an explicit
nullboth arrive as nil and it has to assume"leave it alone". Handled in the service instead, where "a draft has no send time" is
a business rule rather than a transport detail.
Not currently user-visible, since every read is gated on status: the sweep filters
status = 'scheduled', and the frontend checks the same before showing or prefillinga time. It would mislead anyone reading the table directly, or any future delivery
stats joining on the column.
Template failures panicked the worker.
SendHtmlEmaillogged parse and executefailures but did not return, so a missing or malformed template left
templateniland the next line called
.Executeon it. Both paths now return, so asynq retries anda genuinely broken template ends up archived with the reason visible. The parse error
also never said which file failed, so both log lines now carry the template path.
This affects the transactional emails (welcome, confirmation, decision), not campaigns.
Dev tooling.
.env.dev.examplewas missing every AWS/SES variable, so a freshclone could not run the email worker at all. Adds
AWS_ACCESS_KEY,AWS_ACCESS_KEY_SECRET,AWS_REGION, andEMAIL_TEMPLATE_DIRECTORY(credentialsleft blank).
Adds an optional
localstackservice that emulates SES locally. It is inert unlessAWS_ENDPOINT_URLis set, so it changes nothing by default. With it, campaigns can besent end to end without touching the real SES account or risking mail to real users.
Type of Change
Checklist
Additional Notes
returnon an error path; thescheduled_atchange is worth one asserting a draft transition setsScheduledAtDoUpdatewith a nil value, and I can add it if you'd like. Existingemail-domain tests, build, and vet are all green.
AWS_ENDPOINT_URL=http://localstack:4566in.env.dev, rundocker compose up localstack, verify the sender withawslocal ses verify-email-identity --email-address noreply@swamphacks.com,then read delivered mail from
http://localhost:4566/_aws/ses.EMAIL_TEMPLATE_DIRECTORYshould point atinternal/emailutils/templates/. Mylocal
.env.devhadinternal/email/templates/, which does not exist — worthchecking whatever is set in the deployed environments, since that path failing is
what surfaced the panic above.
.env.dev.example: theCF_*andCORE_BUCKETS_*variables.Left alone since I did not want to guess at intended values.
depend on anything here