Skip to content

fix(email): Exclude skipPreferences students from prefs email chain - #61

Merged
tylermenezes merged 1 commit into
mainfrom
detail/bug-fix/fix-email-exclude-skippreferences-students-from-pr-c4464c
Sep 18, 2026
Merged

tylermenezes merged 1 commit into
mainfrom
detail/bug-fix/fix-email-exclude-skippreferences-students-from-pr-c4464c

Conversation

@detail-app

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

Copy link
Copy Markdown
Contributor

Detail bug report: View on Detail

Bug

Students flagged with skipPreferences: true (designated by their partner institution for manual matching) were receiving the entire "Submit Your Project Preferences" email chain — [Action Required] Submit Your Project Preferences plus two reminders. The resolver layer hard-blocks these students from ever creating a ProjectPreference (src/resolvers/Match.ts:33,67 throws when student.skipPreferences is set), so the absence of preferences is expected for this population, not delinquent.

Root cause: the where clause in src/email/templates/studentMatchPrefs.ts, studentMatchPrefsReminder.ts, and studentMatchPrefsReminder2.ts filtered on projectPreferences: { none: {} } but did not filter on skipPreferences: false, so manually-matched students were queried as if they were delinquent in submitting preferences. They were only indirectly removed from the chain once an admin/manager attached a project (via projects: { none: {} }), a timing-dependent compensation the template cannot rely on.

Fix

Added skipPreferences: false to the where clause in all three sibling templates so manually-matched students are excluded from the entire chain, independent of operator timing:

  • src/email/templates/studentMatchPrefs.ts:17
  • src/email/templates/studentMatchPrefsReminder.ts:19
  • src/email/templates/studentMatchPrefsReminder2.ts:19

This is the system's intended exclusion signal — the only code that consults skipPreferences outside the resolvers. No schema, dispatcher, or resolver changes required.

Testing

  • Added a no-DB unit test (tests/testStudentMatchPrefsSkipPreferences.ts, node:test + node:assert/strict, matching the existing tests/testSlackReporting.ts convention). It injects an in-memory mock PrismaClient whose student.findMany re-evaluates every predicate in the where clause against seeded Student/Event/EmailSent rows, then calls the real getList of each of the three templates. It verifies flagged students are excluded from all three chain steps, flagged students with a prior chain EmailSent row do not progress to the next step, normal students are still targeted, the other exclusion paths (status, projectPreferences.none, projects.none, eventId, reminder predecessor gates) still suppress email, and the where object structurally contains skipPreferences: false.
  • Typecheck (npx tsc --skipLibCheck --noEmit) is clean.
  • Pre-existing tests/testSlackReporting.ts still passes (no regression).
  • ESLint reports only a pre-existing @typescript-eslint/typescript@5.x parser deprecation that fires identically on unmodified files; no new violation.
  • End-to-end verification (not versioned in the PR — required external services not present in CI): brought up a Postgres 14 container, applied the Prisma schema, and ran the real dispatcher (sendEmailsForGenerator in src/email/index.ts) against a seeded trigger state. Confirmed no EmailSent row is written for the flagged student for any of the three template emailIds while the normal student still receives the chain, and the dedup cap is unchanged. Also ran the real emailSend automation against a Mailpit SMTP relay and confirmed via its HTTP API that the flagged student's mailbox receives zero chain messages while the normal student's receives [Action Required] Submit Your Project Preferences. Reverting the fix causes both end-to-end runs to fail in the expected way (the flagged student receives the misdirected email and gets an EmailSent row).

Automatic Fixes PRs can be configured here.

@detail-app
detail-app Bot requested a review from tylermenezes September 18, 2026 02:55
@tylermenezes
tylermenezes merged commit abed38b into main Sep 18, 2026
1 check passed
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