Skip to content

Commit 000c83c

Browse files
authored
Merge pull request #310 from topcoder-platform/PM-5716
Submission confirmation email fixes
2 parents cb6f78e + 0f75f17 commit 000c83c

18 files changed

Lines changed: 1389 additions & 28 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.service.spec.ts

Lines changed: 10 additions & 1 deletion
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 () => {
@@ -1896,7 +1899,13 @@ describe('SubmissionService', () => {
18961899
challengeApiServiceMock.validateFinalFixSubmissionCreation,
18971900
).toHaveBeenCalledWith('challenge-final-fix');
18981901
expect(challengePrismaMock.$queryRaw).toHaveBeenCalled();
1899-
expect(prismaMock.submission.create).toHaveBeenCalled();
1902+
expect(prismaMock.submission.create).toHaveBeenCalledWith({
1903+
data: expect.objectContaining({
1904+
confirmationEmail: {
1905+
create: {},
1906+
},
1907+
}),
1908+
});
19001909
expect(result.type).toBe(SubmissionType.STUDIO_FINAL_FIX_SUBMISSION);
19011910
});
19021911

src/api/submission/submission.service.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2842,6 +2842,15 @@ export class SubmissionService {
28422842
}
28432843
}
28442844

2845+
/**
2846+
* Publishes the canonical submission-created event after persistence.
2847+
*
2848+
* @param submission Persisted submission fields used to build the public event payload.
2849+
* @returns A promise that resolves after Bus API accepts the event.
2850+
* @throws InternalServerErrorException when Bus API publication fails.
2851+
* Used by createSubmission to trigger downstream scanning, automation, and
2852+
* submission-confirmation consumers with stable per-submission ordering.
2853+
*/
28452854
private async publishSubmissionCreateEvent(
28462855
submission: SubmissionBusPayloadSource,
28472856
): Promise<void> {
@@ -2886,6 +2895,7 @@ export class SubmissionService {
28862895
await this.eventBusService.publish(
28872896
'submission.notification.create',
28882897
payload,
2898+
`submission:${submission.id}`,
28892899
);
28902900
this.logger.log(
28912901
`Published submission.notification.create event for submission ${submission.id}`,
@@ -3446,6 +3456,9 @@ export class SubmissionService {
34463456
type: body.type as SubmissionType,
34473457
virusScan: false,
34483458
eventRaised: false,
3459+
confirmationEmail: {
3460+
create: {},
3461+
},
34493462
},
34503463
});
34513464
this.logger.log(`Submission created with ID: ${data.id}`);
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import { HttpStatus } from '@nestjs/common';
2+
import { of } from 'rxjs';
3+
import { CommonConfig } from 'src/shared/config/common.config';
4+
import { EventBusService } from './eventBus.service';
5+
6+
describe('EventBusService', () => {
7+
it('forwards a stable Kafka key in the Bus API event envelope', async () => {
8+
const m2mService = {
9+
getM2MToken: jest.fn().mockResolvedValue('m2m-token'),
10+
};
11+
const httpService = {
12+
post: jest.fn().mockReturnValue(
13+
of({
14+
status: HttpStatus.ACCEPTED,
15+
}),
16+
),
17+
};
18+
const service = new EventBusService(
19+
m2mService as any,
20+
httpService as any,
21+
);
22+
23+
await service.publish(
24+
'submission.notification.send',
25+
{ recipients: ['member@example.com'] },
26+
'submission-confirmation:submission-1',
27+
);
28+
29+
expect(httpService.post).toHaveBeenCalledWith(
30+
CommonConfig.apis.busApiUrl,
31+
expect.objectContaining({
32+
topic: 'submission.notification.send',
33+
originator: 'review-api-v6',
34+
'mime-type': 'application/json',
35+
timestamp: expect.any(String),
36+
payload: { recipients: ['member@example.com'] },
37+
key: 'submission-confirmation:submission-1',
38+
}),
39+
{
40+
headers: {
41+
Authorization: 'Bearer m2m-token',
42+
},
43+
},
44+
);
45+
});
46+
});

src/shared/modules/global/eventBus.service.ts

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,12 @@ class EventBusMessage<T> {
1818
'mime-type': string = 'application/json';
1919
timestamp: string = new Date().toISOString();
2020
payload: T;
21+
key?: string;
2122
}
2223

23-
// event bus send email payload
24+
/**
25+
* Payload accepted by the legacy external.action.email topic.
26+
*/
2427
export class EventBusSendEmailPayload {
2528
// Template-specific variables payload. Structure depends on the sendgrid template.
2629
data: Record<string, any>;
@@ -40,10 +43,22 @@ export class EventBusService {
4043
private readonly httpService: HttpService,
4144
) {}
4245

46+
/**
47+
* Posts a message to Bus API using the review-api M2M identity.
48+
*
49+
* @param topic Event topic to publish.
50+
* @param payload Topic-specific JSON payload.
51+
* @param originator Service name recorded on the event.
52+
* @param key Optional stable Kafka partitioning and correlation key.
53+
* @returns A promise that resolves after Bus API accepts the event.
54+
* @throws InternalServerErrorException when token acquisition or Bus API publication fails.
55+
* Used by sendEmail and publish for all review-api event delivery.
56+
*/
4357
private async postMessage<T>(
4458
topic: string,
4559
payload: T,
4660
originator = 'review-api-v6',
61+
key?: string,
4762
): Promise<void> {
4863
// Get M2M token
4964
const token = await this.m2mService.getM2MToken();
@@ -52,6 +67,9 @@ export class EventBusService {
5267
msg.topic = topic;
5368
msg.originator = originator;
5469
msg.payload = payload;
70+
if (key !== undefined) {
71+
msg.key = key;
72+
}
5573
// send message to event bus
5674
const url = CommonConfig.apis.busApiUrl;
5775
try {
@@ -80,14 +98,28 @@ export class EventBusService {
8098

8199
/**
82100
* Send email message to Event bus.
83-
* @param payload send email payload
101+
*
102+
* @param payload Legacy external.action.email payload, including its template ID.
103+
* @returns A promise that resolves after Bus API accepts the event.
104+
* @throws InternalServerErrorException when Bus API publication fails.
105+
* Used by review-api features that select their SendGrid template directly.
84106
*/
85107
async sendEmail(payload: EventBusSendEmailPayload): Promise<void> {
86108
console.log(`${JSON.stringify(payload, null, 2)}`);
87109
await this.postMessage('external.action.email', payload);
88110
}
89111

90-
async publish<T>(topic: string, payload: T): Promise<void> {
91-
await this.postMessage(topic, payload);
112+
/**
113+
* Publishes a topic-specific event through Bus API.
114+
*
115+
* @param topic Event topic to publish.
116+
* @param payload Topic-specific JSON payload.
117+
* @param key Optional stable Kafka partitioning and correlation key.
118+
* @returns A promise that resolves after Bus API accepts the event.
119+
* @throws InternalServerErrorException when Bus API publication fails.
120+
* Used by submission, scan, workflow, and notification event producers.
121+
*/
122+
async publish<T>(topic: string, payload: T, key?: string): Promise<void> {
123+
await this.postMessage(topic, payload, 'review-api-v6', key);
92124
}
93125
}

0 commit comments

Comments
 (0)