Conversation
555b425 to
2ad7409
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends duplicate-registration detection across registration bulk status changes and payment flows by surfacing a duplicateCount in dry-run responses and blocking payment approval when duplicate registrations are present. It also updates test fixtures to avoid accidental duplicates and adds coverage for the new behavior.
Changes:
- Add
duplicateCountto bulk-action result DTOs and compute it for registration status-change dry-runs (when targetingincluded) and for payment creation. - Prevent payment approval when the payment’s registrations include duplicates (by duplicate status).
- Update seed/duplication tooling and multiple tests/fixtures to support duplicates-related scenarios and reduce unintended duplicates.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| services/121-service/test/visa-card/visa-after-fsp-change.test.ts | Update test registration phone fields to avoid duplicate detection collisions. |
| services/121-service/test/visa-card/load-visa-card-details.test.ts | Generate unique phone/WhatsApp numbers per status to avoid duplicates in test data. |
| services/121-service/test/visa-card/close-visa-card.test.ts | Add explicit unique phone numbers to test registrations. |
| services/121-service/test/visa-card/block-visa-card.test.ts | Add explicit unique phone numbers to test registrations. |
| services/121-service/test/registrations/pagination/pagination-data.ts | Adjust fixture phone/WhatsApp numbers to avoid unintended duplicates and fix mismatch. |
| services/121-service/test/registrations/duplicates/duplicate-count-in-status-change.test.ts | New integration coverage for duplicate counting in status-change dry-run responses. |
| services/121-service/test/performance/status-change-payment-in-large-program.test.ts | Add option to skip introducing duplicates during load-test data setup. |
| services/121-service/test/payment/payment-in-progress.test.ts | Adjust retry test setup to avoid creating duplicates while forcing a failed transaction. |
| services/121-service/test/payment/payment-approval-duplicates.test.ts | New integration coverage for approval blocking and dry-run duplicate reporting in payments. |
| services/121-service/test/payment/fsp-integrations/do-payment-fsp-nedbank.test.ts | Ensure seeded registrations have unique phone numbers; minor assertion formatting fix. |
| services/121-service/test/payment/fsp-integrations/do-payment-fsp-excel/do-payment-fsp-excel-export-instructions.test.ts | Align WhatsApp phone with phone number in test data. |
| services/121-service/test/payment/do-payment-retry.test.ts | Add explicit phone numbers to match WhatsApp numbers and avoid duplicates. |
| services/121-service/test/metrics/snapshots/export-list.test.ts.snap | Update snapshot to reflect fixture phone/WhatsApp changes. |
| services/121-service/test/helpers/registration.helper.ts | Extend helpers with skipIntroduceDuplicates and add dryRun query support for status change. |
| services/121-service/swagger.json | Update documented params for duplicate-registrations script endpoint. |
| services/121-service/src/scripts/services/scripts.service.ts | Add skipIntroduceDuplicates option to duplication script flow. |
| services/121-service/src/scripts/scripts.controller.ts | Accept skipIntroduceDuplicates query parameter for duplicate-registrations endpoint. |
| services/121-service/src/registration/services/registrations-bulk.service.ts | Compute duplicateCount for included status-change dry-runs; include duplicateStatus in select for payment flows. |
| services/121-service/src/registration/services/registrations-bulk.service.spec.ts | Update unit test expectations for new duplicateCount field. |
| services/121-service/src/registration/dto/bulk-action-result.dto.ts | Add duplicateCount to bulk action result DTO. |
| services/121-service/src/registration/controllers/registrations.controller.spec.ts | Update mocked bulk-action response shape to include duplicateCount. |
| services/121-service/src/payments/services/payments-management.service.ts | Compute duplicateCount for payment selections and block approval when duplicates exist. |
| services/121-service/src/payments/services/payments-management.service.spec.ts | Extend unit tests for duplicateCount reporting and adjust mocks for new approval path. |
| services/121-service/src/payments/services/payments-execution.service.ts | Include duplicateCount in dry-run response shape. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
abf7596 to
a8b93db
Compare
…rror on duplicates
…istration processes
…cates in mock data
…e payment approval tests
…nt duplicate detection
…and prevent duplicates
…queness and prevent duplicates" This reverts commit 07cfddf.
…tion in Nedbank payment tests
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
208edff to
2339fa3
Compare
| duplicateCount: 0, | ||
| }; | ||
| } | ||
|
|
There was a problem hiding this comment.
Use make a count method in this service
public async countDuplicatesForRegistrations({
registrationIds,
}: {
registrationIds: number[];
}): Promise<number> {
if (registrationIds.length === 0) {
return 0;
}
return await this.getBaseQuery()
.andWhere('registration.id IN (:...registrationIds)', {
registrationIds,
})
.andWhere('registration."duplicateStatus" = :duplicateStatus', {
duplicateStatus: DuplicateStatus.duplicate,
})
.getCount();
}
| private async throwIfPaymentHasDuplicateRegistrations({ | ||
| programId, | ||
| paymentId, | ||
| }: { | ||
| programId: number; | ||
| paymentId: number; | ||
| }): Promise<void> { | ||
| const transactionsForPayment = | ||
| await this.transactionViewScopedRepository.getByStatusOfIncludedRegistrations( | ||
| { | ||
| programId, | ||
| paymentId, | ||
| status: TransactionStatusEnum.pendingApproval, | ||
| }, | ||
| ); | ||
|
|
||
| const registrationIds = [ | ||
| ...new Set(transactionsForPayment.map((t) => t.registrationId)), | ||
| ]; | ||
| if (registrationIds.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| const duplicateRegistrations = | ||
| await this.registrationsPaginationService.getRegistrationViewsNoLimit({ | ||
| programId, | ||
| paginateQuery: { | ||
| path: '', | ||
| filter: { | ||
| duplicateStatus: DuplicateStatus.duplicate, | ||
| }, | ||
| }, | ||
| queryBuilder: this.registrationsBulkService | ||
| .getBaseQuery() | ||
| .andWhere('registration.id IN (:...registrationIds)', { | ||
| registrationIds, | ||
| }), | ||
| }); | ||
|
|
||
| if (duplicateRegistrations.length > 0) { | ||
| throw new HttpException( | ||
| `Cannot approve payment: ${duplicateRegistrations.length} registration(s) have duplicate status. Resolve duplicates before approving this payment.`, | ||
| HttpStatus.BAD_REQUEST, | ||
| ); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Made a helper method in registration bulk service
private async throwIfPaymentHasDuplicateRegistrations({
programId,
paymentId,
}: {
programId: number;
paymentId: number;
}): Promise<void> {
const transactionsForPayment =
await this.transactionViewScopedRepository.getByStatusOfIncludedRegistrations(
{
programId,
paymentId,
status: TransactionStatusEnum.pendingApproval,
},
);
const registrationIds = [
...new Set(transactionsForPayment.map((t) => t.registrationId)),
];
const duplicateCount =
await this.registrationsBulkService.countDuplicatesForRegistrations({
registrationIds,
});
if (duplicateCount > 0) {
throw new HttpException(
`Cannot approve payment: ${duplicateCount} registration(s) have duplicate status. Resolve duplicates before approving this payment.`,
HttpStatus.BAD_REQUEST,
);
}
}
| @Query('includeRegistrationEvents') includeRegistrationEvents: boolean, | ||
| @Query('skipIntroduceDuplicates') skipIntroduceDuplicates: boolean, | ||
| @Res() res, | ||
| ): Promise<void> { |
There was a problem hiding this comment.
skipIntroduceDuplicates =
skipIntroduceDuplicates !== undefined &&
skipIntroduceDuplicates.toString() === 'true';
| await this.seedMockHelper.updateDerivedData(); | ||
| await this.seedMockHelper.updateSequenceNumbers(); | ||
| await this.seedMockHelper.introduceDuplicates(); | ||
| if (String(skipIntroduceDuplicates) !== 'true') { |
There was a problem hiding this comment.
Convert it in controller as its being done in above in the same controller
if (!skipIntroduceDuplicates) {
await this.seedMockHelper.introduceDuplicates();
}
AB#43838
Describe your changes
This pull request introduces enhanced duplicate registration detection and handling throughout the payments and registrations flows, including improved reporting, validation, and test coverage. The most significant changes ensure that duplicate registrations are counted and surfaced in bulk actions and payment creation, and that payments with duplicates cannot be approved until resolved. Additional improvements allow for more flexible test data generation and dry-run support.
Checklist before requesting a code review
Portal preview-deployment
This PR does not have any preview deployments yet.