fix(activities): prevent duplicate mentor gift-card issuance and await activity dispatch - #44
Open
detail-app[bot] wants to merge 1 commit into
Conversation
…t activity dispatch
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.
Detail bug report: View on Detail
Prevents the
issueMentorGiftcardadmin activity from creating and emailing duplicate Shopify discount codes on re-run, and stopsrunActivityfrom reporting success before the async work completes.Bug
issueMentorGiftcardissues a Shopify discount code to every ACCEPTED mentor with a MATCHED project and emails each mentor their code. It had no idempotency guard:findMany) filtered only oneventId/status: 'ACCEPTED'/projects: { some: { status: 'MATCHED' } }— predicates that don't change after a mentor is gifted — so every invocation re-selected the same mentors.issueGiftcardcallsdiscountCodeBasicCreatewith a new random code) and emailed it, but wrote no DB record of issuance. Nothing distinguished "already got a code" from "didn't."runActivitycalled the async activity withoutawaitand always returnedtrue, so the resolver reported success to the dashboard the instant the call was dispatched — before any code was created — making accidental retries and double-clicks likely with no UI signal to discourage them.Net effect: each redundant run created
initialValue × already-gifted mentor countof new discount liability plus duplicate emails, with zero DB trace (the evidence only appears on the Shopify side, after the fact).Fix
Record issuance at issue time and filter on it; also
awaitthe async dispatch.Mentor.giftcardCode String?column (prisma/migrations/20260918120000_add_mentor_giftcard_code/migration.sql,TEXT NULL DEFAULT NULL). The issued code is stored for audit;nullmeans "not yet gifted."issueMentorGiftcard.ts: ThefindManywherenow includesgiftcardCode: nullto exclude already-gifted mentors, while preserving the existingeventId/ACCEPTED/MATCHEDfilters. After a successfulissueGiftcard+sendGiftcard, the mentor is stamped withprisma.mentor.update({ data: { giftcardCode: code } }). The issuance logic was extracted into an injectableissueMentorGiftcards(prisma, eventId, args, deps)core (withissueGiftcard/sendGiftcardas overridable deps) so it's testable offline.activities/index.ts:runActivityis nowasyncandawaits the activity; andispatchActivity(name, context, args, registry)helper holds the dispatch logic. Async rejections are caught and reported asfalse(previously alwaystrue), so the dashboard no longer gets an instant synchronous success before the work finishes.resolvers/Tasks.ts: TherunActivityGraphQL mutation is nowasync/Promise<boolean>andawaitsrunActivity.types/Mentor.ts: Added thegiftcardCode: string | nullproperty to satisfy the generatedPrismaMentorinterface, deliberately without a@Fielddecorator so the redeemable code is not exposed over GraphQL (it stays a DB-only audit value).Testing
Unit tests, typecheck, and build all pass:
npx tsc --skipLibCheck --noEmit— exit 0.npm run build— exit 0; the compileddist/contains the await-based dispatch and thegiftcardCodelogic.src/activities/tasks/issueMentorGiftcard.test.ts(run vianpx tsx) — all assertions pass. It stubs Prisma/Shopify/email and verifies: thefindManyfilters ongiftcardCode: null; a first run issues+emails+stamps each mentor; a re-run issues zero codes and zero emails; a mixed cohort only gifts not-yet-gifted mentors; anullcode from Shopify skips the email and the stamp (so a later retry can still gift that mentor); per-mentor failures are isolated;runActivityis async and awaits the activity; async rejections returnfalse; andgiftcardCodehas no@Field(code not leaked over GraphQL).syncAlumniInteractions.test.tsstill passes (no regression).End-to-end verification (against a real Postgres 16 DB and local Shopify HTTPS / SMTP mocks, since no sandbox credentials were available):
DEFAULT NULLcolumn (existing rows stay "not gifted"). The repo's fullprisma migrate deployis blocked by an unrelated pre-existing migration's PG16 incompatibility, so the new migration was verified directly by dropping/re-adding the column.giftcardCode; a pre-stamped mentor was untouched.runActivitymutation (againstnode dist/index.jswith an admin token) blocked until the activity completed (~0.5s) and returnedtruewith no new codes — confirming the resolver now awaits.issueGiftcardreturned null, no email/stamp was written, and the mentor'sgiftcardCodestayed null; re-running against the working mock issued and stamped it.Not verified / caveats:
npx eslint) could not be run: the repo's@typescript-eslint/parser@3.10.1is incompatible with the installedtypescript@5.2.2and fails with a parser deprecation error on every file, including untouched baseline files — a pre-existing tooling issue unrelated to this change.ELASTIC_URL), but the activity/resolver paths above were exercised before that unrelated crash.awaitplus check-then-actgiftcardCodefilter prevents duplicates across separate/sequential runs and removes the synchronous false-success, but two truly concurrent invocations can both pass thefindMany(giftcardCode: null)before either stamps — a concurrent double-click was observed to create 2 codes/2 emails for 1 mentor. Strict once-only under true concurrency would require a DB row lock (SELECT ... FOR UPDATE) or a unique constraint on issuance; that hardening is left as a follow-up since the load-bearing fix (no re-issuance across runs, no instant false-success) is in place.Automatic Fixes PRs can be configured here.