Skip to content

Commit d5df5db

Browse files
carhartlewisclaude
andcommitted
fix(background-checks): fix 13 bugs across billing, webhooks, custom uploads, and UI
P1: Upload file before marking custom background check as completed, scope refund to identity API failures only, reconcile state on duplicate webhook events, and verify checkout session status before processing. P2: Remove unnecessary buffer copy in raw body parsing, track SWR loading state for custom attachments, handle numeric-string epoch timestamps, include device task in totals while loading, add 30s timeout to Identity API calls, assert 402 status in payment test, accept $0 prices in billing, and validate whitespace-only employee names via DTO transform. P3: Deduplicate BackgroundCheckStatus type definition. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f2f59e2 commit d5df5db

14 files changed

Lines changed: 131 additions & 73 deletions

apps/api/src/background-checks/background-check-billing.service.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ export class BackgroundCheckBillingService {
7878
expand: ['setup_intent'],
7979
});
8080

81+
if (session.status !== 'complete') {
82+
throw new BadRequestException('Checkout session is not complete.');
83+
}
84+
8185
const stripeCustomerId = this.extractStripeId(session.customer);
8286
if (!stripeCustomerId) {
8387
throw new BadRequestException('Checkout session is missing a customer.');
@@ -192,7 +196,7 @@ export class BackgroundCheckBillingService {
192196

193197
const stripe = this.stripeService.getClient();
194198
const price = await stripe.prices.retrieve(priceId);
195-
if (!price.unit_amount) {
199+
if (price.unit_amount === null || price.unit_amount === undefined) {
196200
throw new BadRequestException('Background check price has no unit amount.');
197201
}
198202

apps/api/src/background-checks/background-check-custom.service.spec.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@ jest.mock('@db', () => ({
77
background_check: 'background_check',
88
},
99
BackgroundCheckStatus: {
10+
invited: 'invited',
1011
completed: 'completed',
1112
},
1213
db: {
1314
backgroundCheckRequest: {
1415
findUnique: jest.fn(),
1516
upsert: jest.fn(),
17+
update: jest.fn(),
1618
},
1719
member: {
1820
findFirst: jest.fn(),
@@ -31,7 +33,7 @@ describe('BackgroundCheckCustomService', () => {
3133
jest.clearAllMocks();
3234
});
3335

34-
it('creates a completed background check and stores the uploaded report as an attachment', async () => {
36+
it('creates a background check record, uploads the file, then marks completed', async () => {
3537
const uploadAttachment = jest.fn().mockResolvedValue({
3638
id: 'att_1',
3739
name: 'report.pdf',
@@ -53,8 +55,14 @@ describe('BackgroundCheckCustomService', () => {
5355
mockedDb.backgroundCheckRequest.upsert,
5456
).mockResolvedValueOnce({
5557
id: 'bcr_1',
56-
status: BackgroundCheckStatus.completed,
58+
status: 'invited',
5759
} as Awaited<ReturnType<typeof db.backgroundCheckRequest.upsert>>);
60+
mockAsync<Awaited<ReturnType<typeof db.backgroundCheckRequest.update>>>(
61+
mockedDb.backgroundCheckRequest.update,
62+
).mockResolvedValueOnce({
63+
id: 'bcr_1',
64+
status: BackgroundCheckStatus.completed,
65+
} as Awaited<ReturnType<typeof db.backgroundCheckRequest.update>>);
5866

5967
await service.attachForMember({
6068
organizationId: 'org_1',
@@ -67,15 +75,17 @@ describe('BackgroundCheckCustomService', () => {
6775
userId: 'usr_1',
6876
});
6977

78+
// Record is created with non-completed status first
7079
expect(mockedDb.backgroundCheckRequest.upsert).toHaveBeenCalledWith(
7180
expect.objectContaining({
7281
create: expect.objectContaining({
7382
employeeName: 'Ada Lovelace',
7483
employeeEmail: 'ada@work.example',
75-
status: BackgroundCheckStatus.completed,
84+
status: 'invited',
7685
}),
7786
}),
7887
);
88+
// Upload happens before marking completed
7989
expect(uploadAttachment).toHaveBeenCalledWith(
8090
'org_1',
8191
'bcr_1',
@@ -86,6 +96,15 @@ describe('BackgroundCheckCustomService', () => {
8696
}),
8797
'usr_1',
8898
);
99+
// Only marked completed after successful upload
100+
expect(mockedDb.backgroundCheckRequest.update).toHaveBeenCalledWith(
101+
expect.objectContaining({
102+
where: { id: 'bcr_1' },
103+
data: expect.objectContaining({
104+
status: BackgroundCheckStatus.completed,
105+
}),
106+
}),
107+
);
89108
});
90109

91110
it('returns custom attachments for an existing background check', async () => {

apps/api/src/background-checks/background-check-custom.service.ts

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,28 +57,29 @@ export class BackgroundCheckCustomService {
5757
throw new NotFoundException('Member not found.');
5858
}
5959

60+
const resolvedName = employeeName?.trim() || member.user.name || member.user.email;
61+
const resolvedEmail = employeeEmail?.trim().toLowerCase() || member.user.email;
62+
63+
// Create/update the record without marking it completed yet
6064
const record = await db.backgroundCheckRequest.upsert({
6165
where: { organizationId_memberId: { organizationId, memberId } },
6266
create: {
6367
organizationId,
6468
memberId,
65-
employeeName: employeeName?.trim() || member.user.name || member.user.email,
66-
employeeEmail: employeeEmail?.trim().toLowerCase() || member.user.email,
69+
employeeName: resolvedName,
70+
employeeEmail: resolvedEmail,
6771
requesterNotes,
68-
status: BackgroundCheckStatus.completed,
72+
status: BackgroundCheckStatus.invited,
6973
lastSyncedAt: new Date(),
70-
reportSyncedAt: new Date(),
7174
},
7275
update: {
73-
employeeName: employeeName?.trim() || member.user.name || member.user.email,
74-
employeeEmail: employeeEmail?.trim().toLowerCase() || member.user.email,
76+
employeeName: resolvedName,
77+
employeeEmail: resolvedEmail,
7578
requesterNotes,
76-
status: BackgroundCheckStatus.completed,
77-
lastSyncedAt: new Date(),
78-
reportSyncedAt: new Date(),
7979
},
8080
});
8181

82+
// Upload first — if this fails the record stays non-completed
8283
await this.attachmentsService.uploadAttachment(
8384
organizationId,
8485
record.id,
@@ -87,6 +88,14 @@ export class BackgroundCheckCustomService {
8788
userId,
8889
);
8990

90-
return record;
91+
// Mark completed only after a successful upload
92+
return db.backgroundCheckRequest.update({
93+
where: { id: record.id },
94+
data: {
95+
status: BackgroundCheckStatus.completed,
96+
lastSyncedAt: new Date(),
97+
reportSyncedAt: new Date(),
98+
},
99+
});
91100
}
92101
}

apps/api/src/background-checks/background-check-identity.client.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,10 @@ export class BackgroundCheckIdentityClient {
9090

9191
private async fetchIdentity(url: string, init: RequestInit): Promise<Response> {
9292
try {
93-
return await fetch(url, init);
93+
return await fetch(url, {
94+
...init,
95+
signal: init.signal ?? AbortSignal.timeout(30_000),
96+
});
9497
} catch (error) {
9598
this.logger.error('Identity background check network request failed', {
9699
url,

apps/api/src/background-checks/background-check-payment.service.spec.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { HttpException } from '@nestjs/common';
1+
import { HttpException, HttpStatus } from '@nestjs/common';
22
import { db } from '@db';
33
import { StripeService } from '../stripe/stripe.service';
44
import { BackgroundCheckBillingService } from './background-check-billing.service';
@@ -34,7 +34,11 @@ describe('BackgroundCheckPaymentService', () => {
3434

3535
await expect(
3636
service.charge({ organizationId: 'org_1', memberId: 'mem_1' }),
37-
).rejects.toBeInstanceOf(HttpException);
37+
).rejects.toThrow(
38+
expect.objectContaining({
39+
status: HttpStatus.PAYMENT_REQUIRED,
40+
}),
41+
);
3842
});
3943

4044
it('charges Stripe with payment-method scoped idempotency key', async () => {

apps/api/src/background-checks/background-check-webhook.service.spec.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ describe('BackgroundChecksService webhooks', () => {
240240
);
241241
});
242242

243-
it('dedupes webhook events', async () => {
243+
it('reconciles state even on duplicate webhook events', async () => {
244244
const payload = webhookPayload();
245245
const rawBody = JSON.stringify(payload);
246246
const timestamp = String(Date.now());
@@ -259,8 +259,11 @@ describe('BackgroundChecksService webhooks', () => {
259259
clientVersion: 'test',
260260
}),
261261
);
262+
const identityClient = {
263+
getBackgroundCheck: jest.fn().mockResolvedValue({ status: 'completed_with_flags' }),
264+
};
262265
const service = new BackgroundChecksService(
263-
{} as unknown as BackgroundCheckIdentityClient,
266+
identityClient as unknown as BackgroundCheckIdentityClient,
264267
{} as unknown as BackgroundCheckPaymentService,
265268
);
266269

@@ -273,6 +276,13 @@ describe('BackgroundChecksService webhooks', () => {
273276
});
274277

275278
expect(result).toEqual({ ok: true, duplicate: true });
276-
expect(mockedDb.backgroundCheckRequest.update).not.toHaveBeenCalled();
279+
// State reconciliation still happens on duplicates
280+
expect(mockedDb.backgroundCheckRequest.update).toHaveBeenCalledWith(
281+
expect.objectContaining({
282+
data: expect.objectContaining({
283+
status: 'completed_with_flags',
284+
}),
285+
}),
286+
);
277287
});
278288
});

apps/api/src/background-checks/background-checks.controller.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export class PeopleBackgroundChecksController {
5656
return this.backgroundChecksService.requestForMember({
5757
organizationId,
5858
memberId,
59-
employeeName: body.employeeName.trim(),
59+
employeeName: body.employeeName,
6060
employeeEmail: body.employeeEmail.trim().toLowerCase(),
6161
requesterNotes: body.requesterNotes?.trim() || undefined,
6262
requesterEmail: authContext.userEmail ?? 'api-key@trycomp.ai',

apps/api/src/background-checks/background-checks.service.ts

Lines changed: 40 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -97,47 +97,17 @@ export class BackgroundChecksService {
9797
},
9898
});
9999

100+
let identityResult;
100101
try {
101-
const identityResult = await this.identityClient.createBackgroundCheck({
102+
identityResult = await this.identityClient.createBackgroundCheck({
102103
organizationId,
103104
memberId,
104105
employeeName,
105106
employeeEmail,
106107
requesterEmail,
107108
});
108-
109-
return db.backgroundCheckRequest.upsert({
110-
where: { organizationId_memberId: { organizationId, memberId } },
111-
create: {
112-
organizationId,
113-
memberId,
114-
employeeName,
115-
employeeEmail,
116-
requesterNotes,
117-
identityBackgroundCheckId: identityResult.id,
118-
candidateUrl: identityResult.candidateUrl ?? null,
119-
status: identityResult.status,
120-
stripePaymentIntentId: payment.paymentIntentId,
121-
stripePaymentStatus: payment.status,
122-
stripeAmountCents: payment.amount,
123-
stripeCurrency: payment.currency,
124-
lastSyncedAt: new Date(),
125-
},
126-
update: {
127-
employeeName,
128-
employeeEmail,
129-
requesterNotes,
130-
identityBackgroundCheckId: identityResult.id,
131-
candidateUrl: identityResult.candidateUrl ?? null,
132-
status: identityResult.status,
133-
stripePaymentIntentId: payment.paymentIntentId,
134-
stripePaymentStatus: payment.status,
135-
stripeAmountCents: payment.amount,
136-
stripeCurrency: payment.currency,
137-
lastSyncedAt: new Date(),
138-
},
139-
});
140109
} catch (error) {
110+
// Only refund when the identity API call itself fails — not on DB errors
141111
const refundId = await this.paymentService.refund({
142112
organizationId,
143113
memberId,
@@ -169,6 +139,38 @@ export class BackgroundChecksService {
169139

170140
throw error;
171141
}
142+
143+
return db.backgroundCheckRequest.upsert({
144+
where: { organizationId_memberId: { organizationId, memberId } },
145+
create: {
146+
organizationId,
147+
memberId,
148+
employeeName,
149+
employeeEmail,
150+
requesterNotes,
151+
identityBackgroundCheckId: identityResult.id,
152+
candidateUrl: identityResult.candidateUrl ?? null,
153+
status: identityResult.status,
154+
stripePaymentIntentId: payment.paymentIntentId,
155+
stripePaymentStatus: payment.status,
156+
stripeAmountCents: payment.amount,
157+
stripeCurrency: payment.currency,
158+
lastSyncedAt: new Date(),
159+
},
160+
update: {
161+
employeeName,
162+
employeeEmail,
163+
requesterNotes,
164+
identityBackgroundCheckId: identityResult.id,
165+
candidateUrl: identityResult.candidateUrl ?? null,
166+
status: identityResult.status,
167+
stripePaymentIntentId: payment.paymentIntentId,
168+
stripePaymentStatus: payment.status,
169+
stripeAmountCents: payment.amount,
170+
stripeCurrency: payment.currency,
171+
lastSyncedAt: new Date(),
172+
},
173+
});
172174
}
173175

174176
async getById({
@@ -231,6 +233,7 @@ export class BackgroundChecksService {
231233
throw new NotFoundException('Background check request not found.');
232234
}
233235

236+
let isDuplicate = false;
234237
try {
235238
await db.backgroundCheckWebhookEvent.create({
236239
data: {
@@ -243,9 +246,10 @@ export class BackgroundChecksService {
243246
});
244247
} catch (error) {
245248
if (this.isUniqueConstraintError(error)) {
246-
return { ok: true, duplicate: true };
249+
isDuplicate = true;
250+
} else {
251+
throw error;
247252
}
248-
throw error;
249253
}
250254

251255
const reportSnapshot = await fetchCompletedReportSnapshot({
@@ -278,7 +282,7 @@ export class BackgroundChecksService {
278282
},
279283
});
280284

281-
return { ok: true };
285+
return { ok: true, ...(isDuplicate ? { duplicate: true } : {}) };
282286
}
283287

284288
private isUniqueConstraintError(error: unknown): boolean {

apps/api/src/background-checks/dto/request-background-check.dto.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1-
import { IsEmail, IsOptional, IsString, MaxLength, MinLength } from 'class-validator';
1+
import { IsEmail, IsNotEmpty, IsOptional, IsString, MaxLength } from 'class-validator';
2+
import { Transform } from 'class-transformer';
23

34
export class RequestBackgroundCheckDto {
45
@IsString()
5-
@MinLength(1)
6+
@Transform(({ value }) => (typeof value === 'string' ? value.trim() : value))
7+
@IsNotEmpty({ message: 'employeeName must not be empty' })
68
employeeName: string;
79

810
@IsEmail()

apps/api/src/main.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ async function bootstrap(): Promise<void> {
100100
const jsonParserWithRaw = express.json({
101101
limit: '150mb',
102102
verify: (req, _res, buf) => {
103-
(req as express.Request).rawBody = Buffer.from(buf);
103+
(req as express.Request).rawBody = buf;
104104
},
105105
});
106106
const jsonParser = express.json({ limit: '150mb' });

0 commit comments

Comments
 (0)