Skip to content

Commit 768dbfa

Browse files
committed
Merge remote-tracking branch 'origin/develop' into PM-5747
# Conflicts: # src/api/submission/submission.controller.ts # src/api/submission/submission.service.ts
2 parents 44476de + 52bcd7e commit 768dbfa

19 files changed

Lines changed: 1405 additions & 134 deletions

docs/KAFKA_SETUP.md

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,12 @@ docker exec -it kafka kafka-console-consumer --topic avscan.action.scan --from-b
109109
}
110110
```
111111

112-
2. Register the handler in the `src/shared/modules/kafka/handlers/registered-handlers.config.ts` config handlers array.
113-
3. The handler will automatically be registered and start consuming messages.
112+
2. Add the handler to
113+
`src/shared/modules/kafka/handlers/registered-handlers.config.ts` so the
114+
handler catalog remains complete.
115+
3. Add the handler class to `GlobalProvidersModule.providers`. Nest must
116+
instantiate the provider for its `onModuleInit` registration to run.
117+
4. The registered topic is included when the Kafka consumer starts.
114118

115119
### Dead Letter Queue (DLQ) Support
116120

@@ -127,9 +131,13 @@ The application includes a robust Dead Letter Queue implementation for handling
127131

128132
2. **Retry Mechanism**:
129133

130-
- Failed messages are automatically retried up to the configured maximum number of retries
134+
- Failed messages are automatically retried up to the configured maximum
135+
number of retries, whether or not DLQ publication is enabled
131136
- Retry count is tracked per message using a unique key based on topic, partition, and offset
132137
- Exponential backoff is applied between retries
138+
- `KAFKA_DLQ_ENABLED` controls whether an event is copied to a DLQ after the
139+
retry budget is exhausted; exhausted messages are logged and committed
140+
when DLQ publication is disabled
133141

134142
3. **DLQ Processing**:
135143

@@ -156,6 +164,12 @@ The application includes a robust Dead Letter Queue implementation for handling
156164

157165
- The service uses `@platformatic/kafka` 2.8.0 for broker failover and consumer group recovery fixes.
158166
- Platformatic Kafka 2.x raises the aggregate consumer Fetch limit to 50 MiB. The service deliberately retains the previous 10 MiB `maxBytes` limit to avoid increasing its per-consumer memory envelope.
167+
- Streams use committed-offset mode so restarts resume the consumer group's
168+
last successful offset. A topic without a committed offset starts at latest,
169+
which avoids replaying historical events when a handler is first deployed.
170+
- Submission confirmation additionally persists a request in the same database
171+
operation as each normal member submission. Its scheduled recovery pass does
172+
not depend on Kafka redelivery, so a missed source event remains recoverable.
159173
- Terminal consumer or producer client errors and offset commit timeouts mark Kafka health as `reconnecting` and start the shared reconnect lifecycle. A successful reconnect returns health to `ready`; exhausted attempts mark it as `failed` with the last failure reason.
160174

161175
### Environment Variables

docs/MANUAL_UPLOAD_FLOW.md

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,21 @@ sequenceDiagram
4040
participant S3DMZ as S3 DMZ bucket
4141
participant AV as AV scan pipeline
4242
participant Kafka as Kafka
43+
participant Email as email-service-v6
44+
participant Member
4345
participant AI as AI workflow queue
4446
participant ReviewDB as review DB
4547
4648
Client->>ReviewAPI: POST /submissions/manual-upload
4749
ReviewAPI->>ReviewAPI: Validate auth, file, submitter, challenge, phase window
4850
ReviewAPI->>S3DMZ: Upload file to DMZ
49-
ReviewAPI->>ReviewDB: Create submission row
51+
ReviewAPI->>ReviewDB: Create submission + confirmation request atomically
5052
ReviewAPI->>Kafka: Publish avscan.action.scan
53+
ReviewAPI->>Kafka: Publish submission.notification.create
54+
Kafka->>ReviewAPI: submission.notification.create
55+
ReviewAPI->>Kafka: Publish submission.notification.send
56+
Kafka->>Email: submission.notification.send
57+
Email-->>Member: Submission confirmation
5158
AV->>Kafka: Publish submission.scan.complete
5259
Kafka->>ReviewAPI: submission.scan.complete
5360
ReviewAPI->>ReviewDB: Update submission URL + virusScan=true
@@ -127,6 +134,11 @@ The created submission row is initially stored with:
127134
- `eventRaised = false`
128135
- `url = <DMZ URL>`
129136

137+
The same database operation also creates one
138+
`submissionConfirmationEmail` request with no `publishedAt` timestamp. This
139+
request is the durable recovery source for the member receipt; historical and
140+
validation-only submissions are not automatically added to it.
141+
130142
At this point the file is not yet considered clean.
131143

132144
### 6. Events emitted immediately after creation
@@ -147,6 +159,55 @@ The AV scan event includes:
147159
- quarantine destination bucket
148160
- callback topic `submission.scan.complete`
149161

162+
### 7. Submission receipt confirmation
163+
164+
The Kafka handler for `submission.notification.create` asks the durable
165+
confirmation dispatcher to claim the related request. The dispatcher re-reads
166+
the persisted submission instead of trusting member or challenge fields in the
167+
event. It then:
168+
169+
- resolves the submitter email and handle from member storage
170+
- resolves the challenge title from challenge storage
171+
- publishes `submission.notification.send` with Kafka key
172+
`submission-confirmation:{submissionId}`
173+
- records the request's `publishedAt` only after Bus API accepts that event
174+
175+
A once-per-minute recovery pass selects unpublished requests. A five-minute
176+
database lease with an opaque per-claim token prevents the Kafka handler,
177+
multiple ECS tasks, and the recovery pass from concurrently publishing the same
178+
request. A stale worker cannot release a newer worker's lease. If review-api
179+
stops after the submission is stored but before
180+
`submission.notification.create` reaches Kafka, the recovery pass still
181+
publishes the confirmation from the durable request.
182+
183+
The email-ready payload uses the `v3` dynamic-template shape:
184+
185+
```json
186+
{
187+
"recipients": ["member@example.com"],
188+
"version": "v3",
189+
"data": {
190+
"submitter": { "handle": "memberHandle" },
191+
"challenge": { "challengeTitle": "Challenge title" },
192+
"submission": {
193+
"id": "submission-id",
194+
"challengeId": "challenge-id"
195+
}
196+
}
197+
}
198+
```
199+
200+
The email service selects its configured template from the
201+
`submission.notification.send` topic, so review-api does not hardcode a
202+
SendGrid template ID. A populated request `publishedAt` suppresses ordinary
203+
replay after successful publication. It records Bus acceptance, not a SendGrid
204+
delivery receipt; a process failure after Bus acceptance but before the request
205+
is marked can still produce at-least-once delivery. Transient enrichment or
206+
publication failures release the lease and become eligible for scheduled
207+
recovery after five minutes, preventing permanently bad records from starving
208+
new requests. Keep `KAFKA_DLQ_ENABLED=true` to retain exhausted source events
209+
for operational inspection and replay.
210+
150211
## What Happens After AV Scan
151212

152213
When `submission.scan.complete` is received:
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
-- Persist one recoverable confirmation request for each new member submission.
2+
-- The table is intentionally empty after migration; historical submissions are
3+
-- not backfilled or emailed without an explicit operational replay.
4+
CREATE TABLE "submissionConfirmationEmail" (
5+
"submissionId" VARCHAR(14) NOT NULL,
6+
"processingStartedAt" TIMESTAMP(3),
7+
"processingToken" VARCHAR(36),
8+
"publishedAt" TIMESTAMP(3),
9+
"nextAttemptAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,
10+
"attemptCount" INTEGER NOT NULL DEFAULT 0,
11+
"lastError" TEXT,
12+
"createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,
13+
"updatedAt" TIMESTAMP(3) NOT NULL,
14+
15+
CONSTRAINT "submissionConfirmationEmail_pkey" PRIMARY KEY ("submissionId")
16+
);
17+
18+
CREATE INDEX "submissionConfirmationEmail_pending_idx"
19+
ON "submissionConfirmationEmail"("publishedAt", "nextAttemptAt", "processingStartedAt", "createdAt");
20+
21+
ALTER TABLE "submissionConfirmationEmail"
22+
ADD CONSTRAINT "submissionConfirmationEmail_submissionId_fkey"
23+
FOREIGN KEY ("submissionId") REFERENCES "submission"("id")
24+
ON DELETE CASCADE ON UPDATE CASCADE;

prisma/schema.prisma

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,7 @@ model submission {
447447
aiReviewDecisions aiReviewDecision[]
448448
auditLogs reviewAudit[]
449449
accessAudits submissionAccessAudit[]
450+
confirmationEmail submissionConfirmationEmail?
450451
451452
@@index([memberId])
452453
@@index([challengeId])
@@ -455,6 +456,23 @@ model submission {
455456
@@index([submittedDate])
456457
}
457458

459+
/// Durable submission-confirmation request created atomically with a member submission.
460+
model submissionConfirmationEmail {
461+
submissionId String @id @db.VarChar(14)
462+
processingStartedAt DateTime?
463+
processingToken String? @db.VarChar(36)
464+
publishedAt DateTime?
465+
nextAttemptAt DateTime @default(now())
466+
attemptCount Int @default(0)
467+
lastError String? @db.Text
468+
createdAt DateTime @default(now())
469+
updatedAt DateTime @updatedAt
470+
471+
submission submission @relation(fields: [submissionId], references: [id], onDelete: Cascade)
472+
473+
@@index([publishedAt, nextAttemptAt, processingStartedAt, createdAt], map: "submissionConfirmationEmail_pending_idx")
474+
}
475+
458476
enum ReviewOpportunityStatus {
459477
OPEN
460478
CLOSED

src/api/submission/submission.controller.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ export class SubmissionController {
405405
@ApiOperation({
406406
summary: 'Download the submission',
407407
description:
408-
'Roles: Copilot, Admin, User, Reviewer. After challenge completion, the exact metadata value allowAllRegistrantsToDownloadWinningSubmissions=true lets every registered Submitter download only an exact final winning submission and denies non-winners without legacy fallback. Other values require passing-submission eligibility, except non-Design First2Finish challenges retain legacy submitter eligibility. Design challenges also require submissionsViewable. | Scopes: read:submission',
408+
'Roles: Copilot, Admin, User, Reviewer. After challenge completion, the exact metadata value allowAllRegistrantsToDownloadWinningSubmissions=true lets every registered Submitter download only an exact final winning submission and denies non-winners without legacy fallback. Other values require passing-submission eligibility, except non-Design First2Finish challenges retain legacy submitter eligibility. | Scopes: read:submission',
409409
})
410410
@ApiParam({
411411
name: 'submissionId',

src/api/submission/submission.service.spec.ts

Lines changed: 15 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,9 @@ describe('SubmissionService', () => {
377377
virusScan: true,
378378
}),
379379
});
380+
expect(
381+
prisma.submission.create.mock.calls[0][0].data,
382+
).not.toHaveProperty('confirmationEmail');
380383
});
381384

382385
it('rejects validation upload requests without file contents', async () => {
@@ -1269,7 +1272,7 @@ describe('SubmissionService', () => {
12691272
expect(s3Send).not.toHaveBeenCalled();
12701273
});
12711274

1272-
it('lets the Design visibility gate block registrants even when the new flag is enabled', async () => {
1275+
it('allows all Design registrants to download winner submissions when legacy visibility is disabled', async () => {
12731276
resourceApiService.getMemberResourcesRoles.mockResolvedValue([
12741277
{ roleName: 'Submitter' },
12751278
]);
@@ -1282,66 +1285,6 @@ describe('SubmissionService', () => {
12821285
},
12831286
winners: [{ userId: 'owner-user', placement: 1 }],
12841287
});
1285-
prismaMock.submission.findFirst.mockResolvedValue({ id: 'passing-sub' });
1286-
1287-
await expect(
1288-
service.getSubmissionFileStream(
1289-
{
1290-
userId: 'registered-user',
1291-
isMachine: false,
1292-
roles: [],
1293-
} as any,
1294-
'sub-123',
1295-
),
1296-
).rejects.toBeInstanceOf(ForbiddenException);
1297-
1298-
expect(prismaMock.submission.findFirst).not.toHaveBeenCalled();
1299-
expect(s3Send).not.toHaveBeenCalled();
1300-
});
1301-
1302-
it('applies the Design visibility gate to legacy Design tracks', async () => {
1303-
resourceApiService.getMemberResourcesRoles.mockResolvedValue([
1304-
{ roleName: 'Submitter' },
1305-
]);
1306-
challengeApiServiceMock.getChallengeDetail.mockResolvedValue({
1307-
status: ChallengeStatus.COMPLETED,
1308-
track: 'Legacy',
1309-
legacy: { track: 'DESIGN' },
1310-
metadata: {
1311-
submissionsViewable: 'false',
1312-
allowAllRegistrantsToDownloadWinningSubmissions: 'true',
1313-
},
1314-
winners: [{ userId: 'owner-user', placement: 1 }],
1315-
});
1316-
1317-
await expect(
1318-
service.getSubmissionFileStream(
1319-
{
1320-
userId: 'registered-user',
1321-
isMachine: false,
1322-
roles: [],
1323-
} as any,
1324-
'sub-123',
1325-
),
1326-
).rejects.toBeInstanceOf(ForbiddenException);
1327-
1328-
expect(prismaMock.submission.findFirst).not.toHaveBeenCalled();
1329-
expect(s3Send).not.toHaveBeenCalled();
1330-
});
1331-
1332-
it('allows all Design registrants to download winner submissions when both gates are enabled', async () => {
1333-
resourceApiService.getMemberResourcesRoles.mockResolvedValue([
1334-
{ roleName: 'Submitter' },
1335-
]);
1336-
challengeApiServiceMock.getChallengeDetail.mockResolvedValue({
1337-
status: ChallengeStatus.COMPLETED,
1338-
track: 'Design',
1339-
metadata: {
1340-
submissionsViewable: 'true',
1341-
allowAllRegistrantsToDownloadWinningSubmissions: 'true',
1342-
},
1343-
winners: [{ userId: 'owner-user', placement: 1 }],
1344-
});
13451288
prismaMock.challengeResult.findUnique.mockResolvedValue({
13461289
submissionId: 'sub-123',
13471290
userId: 'owner-user',
@@ -1362,15 +1305,15 @@ describe('SubmissionService', () => {
13621305
expect(s3Send).toHaveBeenCalledTimes(2);
13631306
});
13641307

1365-
it('uses passing-submitter eligibility for viewable Design challenges when the new flag is disabled', async () => {
1308+
it('uses passing-submitter eligibility for Design challenges when legacy visibility is disabled', async () => {
13661309
resourceApiService.getMemberResourcesRoles.mockResolvedValue([
13671310
{ roleName: 'Submitter' },
13681311
]);
13691312
challengeApiServiceMock.getChallengeDetail.mockResolvedValue({
13701313
status: ChallengeStatus.COMPLETED,
13711314
track: 'Design',
13721315
metadata: {
1373-
submissionsViewable: 'true',
1316+
submissionsViewable: 'false',
13741317
allowAllRegistrantsToDownloadWinningSubmissions: 'false',
13751318
},
13761319
winners: [{ userId: 'owner-user', placement: 1 }],
@@ -1391,15 +1334,15 @@ describe('SubmissionService', () => {
13911334
expect(s3Send).toHaveBeenCalledTimes(2);
13921335
});
13931336

1394-
it('denies a non-passing submitter for a viewable Design challenge when the new flag is disabled', async () => {
1337+
it('denies a non-passing Design submitter when the new flag is disabled regardless of legacy visibility', async () => {
13951338
resourceApiService.getMemberResourcesRoles.mockResolvedValue([
13961339
{ roleName: 'Submitter' },
13971340
]);
13981341
challengeApiServiceMock.getChallengeDetail.mockResolvedValue({
13991342
status: ChallengeStatus.COMPLETED,
14001343
track: 'Design',
14011344
metadata: {
1402-
submissionsViewable: 'true',
1345+
submissionsViewable: 'false',
14031346
allowAllRegistrantsToDownloadWinningSubmissions: 'false',
14041347
},
14051348
winners: [{ userId: 'owner-user', placement: 1 }],
@@ -1942,7 +1885,13 @@ describe('SubmissionService', () => {
19421885
challengeApiServiceMock.validateFinalFixSubmissionCreation,
19431886
).toHaveBeenCalledWith('challenge-final-fix');
19441887
expect(challengePrismaMock.$queryRaw).toHaveBeenCalled();
1945-
expect(prismaMock.submission.create).toHaveBeenCalled();
1888+
expect(prismaMock.submission.create).toHaveBeenCalledWith({
1889+
data: expect.objectContaining({
1890+
confirmationEmail: {
1891+
create: {},
1892+
},
1893+
}),
1894+
});
19461895
expect(result.type).toBe(SubmissionType.STUDIO_FINAL_FIX_SUBMISSION);
19471896
});
19481897

0 commit comments

Comments
 (0)