fix(background-check): persist exemption reason + justification on member#2863
Merged
Conversation
…mber "Confirm exemption" was failing for every customer with a "Failed to confirm exemption" toast. Root cause: apps/api/src/main.ts sets `forbidNonWhitelisted: true` on the global ValidationPipe. The V1 frontend sends `backgroundCheckExempt` together with `backgroundCheckExemptReason` + `backgroundCheckExemptJustification`, but the latter two were never declared on UpdatePeopleDto — so the PATCH /v1/people/:id request was rejected with 400 before the service ever ran. This fix: 1. Adds nullable columns `backgroundCheckExemptReason` (varchar) and `backgroundCheckExemptJustification` (text) to the Member model (named prisma migrate dev migration — not hand-written SQL). 2. Whitelists both fields on UpdatePeopleDto as optional strings with sensible length caps (100 / 2000 chars). 3. Persists both fields on member.update; clears them to null when backgroundCheckExempt is set to false so a future re-exemption starts from a clean state. Audit log retains the prior values from the original exempt-true request via AuditLogInterceptor. 4. Adds 4 unit tests in member-queries.spec.ts covering: persist on exempt=true, clear on exempt=false, clear overrides incoming stale reason on exempt=false, untouched when patch omits exempt. Pre-existing typecheck failures in unrelated specs and a pre-existing people.service.spec runtime error were verified to also occur on the baseline (origin/main) before this change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
tofikwest
changed the title
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
The Prisma schema change in this branch added two non-optional (nullable) columns to Member — backgroundCheckExemptReason + backgroundCheckExemptJustification. The strict-typed test mock createMockMember returns Member, so the mock had to include both keys or the spread of overrides would surface them as undefined and fail the Vercel build: Type 'undefined' is not assignable to type 'string | null'. Initializing both to null in the default mock matches the DB default. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
🎉 This PR is included in version 3.56.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
2 participants
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.
Summary
Customer reported "Mark as exempt" toast: "Failed to confirm exemption". Every customer is hard-blocked on the V1 exempt flow.
Root cause
apps/api/src/main.ts:137-139setsforbidNonWhitelisted: trueon the globalValidationPipe. The V1 frontend (already on main, PR #2839) sends:```ts
PATCH /v1/people/:id body: {
backgroundCheckExempt: true,
backgroundCheckExemptReason: 'other',
backgroundCheckExemptJustification: 'Founder',
}
```
UpdatePeopleDtoonly declaresbackgroundCheckExempt. The two new fields aren't whitelisted → NestJS rejects with 400 Bad Request before the service runs → client toast.I flagged this exact follow-up in PR #2839's description but called it "backend ignores them". I was wrong —
forbidNonWhitelistedactively rejects, not silently drops. Customer-blocking.What this PR does
Member:backgroundCheckExemptReason(varchar)backgroundCheckExemptJustification(text)20260515221108_add_background_check_exemption_reason_justificationgenerated viabunx prisma migrate dev --name …(per CLAUDE.md, not hand-written SQL). AdditiveALTER TABLE— no defaults required, no rewrite, no downtime.UpdatePeopleDtowith length caps (100 / 2000 chars).MemberQueries.updateMember) — persists both fields onmember.update. WhenbackgroundCheckExempt === false, clears both tonullso a future re-exemption starts clean. TheAuditLogInterceptoralready captures the request body for@RequirePermissionendpoints, so historical reason/justification live in the audit log from the original exempt-true request.member-queries.spec.tswith 4 cases:exempt=trueexempt=falseexempt=false(defensive)exemptleaves both fields untouchedWhy "clear on un-exempt"
The audit log retains the original reason/justification from when the member was exempted (request body captured by
AuditLogInterceptor). The columns on theMemberrow are current-state fields, not history. Un-exempting → no current reason → null. Re-exempting later means the admin must supply fresh reason + justification.No frontend change
The V1 frontend already sends these fields; it just needs the API to stop rejecting them. Deploy the API and the customer is unblocked.
Verification
Test plan
🤖 Generated with Claude Code