Skip to content

fix(activities): prevent duplicate mentor gift-card issuance and await activity dispatch - #44

Open
detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-activities-prevent-duplicate-mentor-gift-card-010407
Open

detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-activities-prevent-duplicate-mentor-gift-card-010407

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Detail bug report: View on Detail

Prevents the issueMentorGiftcard admin activity from creating and emailing duplicate Shopify discount codes on re-run, and stops runActivity from reporting success before the async work completes.

Bug

issueMentorGiftcard issues a Shopify discount code to every ACCEPTED mentor with a MATCHED project and emails each mentor their code. It had no idempotency guard:

  • The mentor-selection query (findMany) filtered only on eventId / status: 'ACCEPTED' / projects: { some: { status: 'MATCHED' } } — predicates that don't change after a mentor is gifted — so every invocation re-selected the same mentors.
  • Each run minted a fresh Shopify discount code (issueGiftcard calls discountCodeBasicCreate with a new random code) and emailed it, but wrote no DB record of issuance. Nothing distinguished "already got a code" from "didn't."
  • runActivity called the async activity without await and always returned true, 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 count of 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 await the async dispatch.

  • Schema + migration: Added a nullable Mentor.giftcardCode String? column (prisma/migrations/20260918120000_add_mentor_giftcard_code/migration.sql, TEXT NULL DEFAULT NULL). The issued code is stored for audit; null means "not yet gifted."
  • issueMentorGiftcard.ts: The findMany where now includes giftcardCode: null to exclude already-gifted mentors, while preserving the existing eventId/ACCEPTED/MATCHED filters. After a successful issueGiftcard + sendGiftcard, the mentor is stamped with prisma.mentor.update({ data: { giftcardCode: code } }). The issuance logic was extracted into an injectable issueMentorGiftcards(prisma, eventId, args, deps) core (with issueGiftcard/sendGiftcard as overridable deps) so it's testable offline.
  • activities/index.ts: runActivity is now async and awaits the activity; an dispatchActivity(name, context, args, registry) helper holds the dispatch logic. Async rejections are caught and reported as false (previously always true), so the dashboard no longer gets an instant synchronous success before the work finishes.
  • resolvers/Tasks.ts: The runActivity GraphQL mutation is now async/Promise<boolean> and awaits runActivity.
  • types/Mentor.ts: Added the giftcardCode: string | null property to satisfy the generated PrismaMentor interface, deliberately without a @Field decorator 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 compiled dist/ contains the await-based dispatch and the giftcardCode logic.
  • New offline test src/activities/tasks/issueMentorGiftcard.test.ts (run via npx tsx) — all assertions pass. It stubs Prisma/Shopify/email and verifies: the findMany filters on giftcardCode: 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; a null code from Shopify skips the email and the stamp (so a later retry can still gift that mentor); per-mentor failures are isolated; runActivity is async and awaits the activity; async rejections return false; and giftcardCode has no @Field (code not leaked over GraphQL).
  • The existing offline syncAlumniInteractions.test.ts still 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):

  • Confirmed the migration applies on Postgres 16 and adds a nullable, DEFAULT NULL column (existing rows stay "not gifted"). The repo's full prisma migrate deploy is blocked by an unrelated pre-existing migration's PG16 incompatibility, so the new migration was verified directly by dropping/re-adding the column.
  • First run: created exactly 1 Shopify discount code and 1 email for the eligible mentor and stamped its giftcardCode; a pre-stamped mentor was untouched.
  • Second run: created 0 new codes and 0 new emails; the stamp was unchanged.
  • GraphQL path: the runActivity mutation (against node dist/index.js with an admin token) blocked until the activity completed (~0.5s) and returned true with no new codes — confirming the resolver now awaits.
  • Failed-then-retry: with Shopify unreachable, issueGiftcard returned null, no email/stamp was written, and the mentor's giftcardCode stayed null; re-running against the working mock issued and stamped it.

Not verified / caveats:

  • Lint (npx eslint) could not be run: the repo's @typescript-eslint/parser@3.10.1 is incompatible with the installed typescript@5.2.2 and fails with a parser deprecation error on every file, including untouched baseline files — a pre-existing tooling issue unrelated to this change.
  • The Elasticsearch-dependentGraphQL server could not be kept up long-term in this environment (placeholder ELASTIC_URL), but the activity/resolver paths above were exercised before that unrelated crash.
  • Concurrency caveat: the await plus check-then-act giftcardCode filter prevents duplicates across separate/sequential runs and removes the synchronous false-success, but two truly concurrent invocations can both pass the findMany(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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant