From f812eb4d709d253d9ac1c8f50717c06b5c6de5a4 Mon Sep 17 00:00:00 2001 From: joshuakrueger-dfx Date: Mon, 27 Jul 2026 17:58:14 +0200 Subject: [PATCH 1/3] fix(kyc): report submission completeness for personal and financial data The financial and personal data submit endpoints stored incomplete drafts and returned HTTP 200 without advancing the step, leaving clients unable to tell that anything was missing. Both endpoints now return the completeness state and the missing field keys. Also declares the previously undocumented question conditions in the API contract, so clients can determine which questions apply. --- .../generic/kyc/controllers/kyc.controller.ts | 10 +- .../kyc/dto/output/kyc-financial-out.dto.ts | 16 ++ .../generic/kyc/dto/output/kyc-info.dto.ts | 16 ++ .../services/__tests__/kyc.service.spec.ts | 211 ++++++++++++++++++ .../__tests__/financial.service.spec.ts | 143 ++++++++++++ .../services/integration/financial.service.ts | 30 ++- .../generic/kyc/services/kyc.service.ts | 17 +- .../user/models/user-data/user-data.entity.ts | 6 +- 8 files changed, 428 insertions(+), 21 deletions(-) create mode 100644 src/subdomains/generic/kyc/services/integration/__tests__/financial.service.spec.ts diff --git a/src/subdomains/generic/kyc/controllers/kyc.controller.ts b/src/subdomains/generic/kyc/controllers/kyc.controller.ts index 67acea4473..25f23520ef 100644 --- a/src/subdomains/generic/kyc/controllers/kyc.controller.ts +++ b/src/subdomains/generic/kyc/controllers/kyc.controller.ts @@ -63,7 +63,7 @@ import { Start2faDto } from '../dto/input/start-2fa.dto'; import { Verify2faDto } from '../dto/input/verify-2fa.dto'; import { FileType, KycFileDataDto } from '../dto/kyc-file.dto'; import { KycFinancialOutData } from '../dto/output/kyc-financial-out.dto'; -import { KycLevelDto, KycSessionDto, KycStepBase } from '../dto/output/kyc-info.dto'; +import { KycLevelDto, KycSessionDto, KycStepBase, KycStepSubmitDto } from '../dto/output/kyc-info.dto'; import { MergedDto } from '../dto/output/kyc-merged.dto'; import { Setup2faDto } from '../dto/output/setup-2fa.dto'; import { SumSubWebhookResult } from '../dto/sum-sub.dto'; @@ -204,13 +204,13 @@ export class KycController { } @Put('data/personal/:id') - @ApiOkResponse({ type: KycStepBase }) + @ApiOkResponse({ type: KycStepSubmitDto }) @ApiUnauthorizedResponse(MergedResponse) async updatePersonalData( @Headers(CodeHeaderName) code: string, @Param('id') id: string, @Body() data: KycPersonalData, - ): Promise { + ): Promise { return this.kycService.updatePersonalData(code, +id, data); } @@ -438,7 +438,7 @@ export class KycController { } @Put('data/financial/:id') - @ApiOkResponse({ type: KycStepBase }) + @ApiOkResponse({ type: KycStepSubmitDto }) @ApiUnauthorizedResponse(MergedResponse) @ApiForbiddenResponse(TfaResponse) async updateFinancialData( @@ -446,7 +446,7 @@ export class KycController { @RealIP() ip: string, @Param('id') id: string, @Body() data: KycFinancialInData, - ): Promise { + ): Promise { return this.kycService.updateFinancialData(code, ip, +id, data); } diff --git a/src/subdomains/generic/kyc/dto/output/kyc-financial-out.dto.ts b/src/subdomains/generic/kyc/dto/output/kyc-financial-out.dto.ts index e6da6c4af3..8a05cf3df5 100644 --- a/src/subdomains/generic/kyc/dto/output/kyc-financial-out.dto.ts +++ b/src/subdomains/generic/kyc/dto/output/kyc-financial-out.dto.ts @@ -10,6 +10,14 @@ export class KycFinancialOption { text: string; } +export class KycFinancialCondition { + @ApiProperty({ description: 'Question key this condition depends on' }) + question: string; + + @ApiProperty({ description: 'Response value that makes the dependent question applicable' }) + response: string; +} + export class KycFinancialQuestion { @ApiProperty({ description: 'Question key' }) key: string; @@ -25,6 +33,14 @@ export class KycFinancialQuestion { @ApiPropertyOptional({ description: 'Response options', type: KycFinancialOption, isArray: true }) options?: KycFinancialOption[]; + + @ApiPropertyOptional({ + description: + 'Conditions under which this question applies. If set, the question is only required when at least one condition matches the given responses.', + type: KycFinancialCondition, + isArray: true, + }) + conditions?: KycFinancialCondition[]; } export class KycFinancialOutData extends KycFinancialInData { diff --git a/src/subdomains/generic/kyc/dto/output/kyc-info.dto.ts b/src/subdomains/generic/kyc/dto/output/kyc-info.dto.ts index 593d87f83c..6b83b5f907 100644 --- a/src/subdomains/generic/kyc/dto/output/kyc-info.dto.ts +++ b/src/subdomains/generic/kyc/dto/output/kyc-info.dto.ts @@ -65,6 +65,22 @@ export class KycStepBase { sequenceNumber: number; } +export class KycStepSubmitDto extends KycStepBase { + @ApiProperty({ + description: + 'Whether the submission fulfills all required fields and advanced the step (e.g. completed or internal review). Draft data is still stored when false.', + }) + complete: boolean; + + @ApiProperty({ + description: + 'Keys of missing required fields when complete is false; empty when complete is true. Financial: unanswered applicable question keys; personal data: missing requiredKycFields names.', + type: String, + isArray: true, + }) + missingFields: string[]; +} + export class KycStepDto extends KycStepBase { @ApiProperty() isCurrent: boolean; diff --git a/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts b/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts index 34604aa837..ff3c1a8d47 100644 --- a/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts +++ b/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts @@ -7,6 +7,7 @@ import { UserRole } from 'src/shared/auth/user-role.enum'; import { createCustomCountry } from 'src/shared/models/country/__mocks__/country.entity.mock'; import { Country } from 'src/shared/models/country/country.entity'; import { DfxLogger } from 'src/shared/services/dfx-logger'; +import { AccountType } from '../../../user/models/user-data/account-type.enum'; import { createCustomUserData } from '../../../user/models/user-data/__mocks__/user-data.entity.mock'; import { UserData } from '../../../user/models/user-data/user-data.entity'; import { RiskStatus, UserDataStatus } from '../../../user/models/user-data/user-data.enum'; @@ -14,6 +15,7 @@ import { UserDataService } from '../../../user/models/user-data/user-data.servic import { UserStatus } from '../../../user/models/user/user.enum'; import { IdentDocument } from '../../dto/ident.dto'; import { FileType, KycFileBlob } from '../../dto/kyc-file.dto'; +import { KycStepStatus } from '../../dto/output/kyc-info.dto'; import { KycFile } from '../../entities/kyc-file.entity'; import { KycStep } from '../../entities/kyc-step.entity'; import { ContentType } from '../../enums/content-type.enum'; @@ -574,3 +576,212 @@ describe('KycService checkDfxApproval duplicate-key recovery', () => { expect(kycStepRepo.update).not.toHaveBeenCalled(); }); }); + +// updatePersonalData / updateFinancialData return completeness feedback so clients know when a +// draft submit did not advance the step (and which required fields are still missing). +describe('KycService submit completeness feedback', () => { + let service: KycService; + let userDataService: jest.Mocked; + let kycStepRepo: jest.Mocked; + + const personalFinancialComplete = [ + { key: 'tnc', value: 'accept' }, + { key: 'own_funds', value: 'accept' }, + { key: 'source_of_funds', value: 'employment_income' }, + { key: 'occupation', value: 'employed' }, + { key: 'occupation_description', value: 'Software engineer' }, + { key: 'sector', value: 'it' }, + { key: 'risky_business', value: 'no_risky_business' }, + { key: 'income', value: '50k' }, + { key: 'assets', value: '50k' }, + { key: 'notification_of_changes', value: 'accept' }, + ]; + + beforeEach(() => { + userDataService = createMock(); + kycStepRepo = createMock(); + kycStepRepo.update.mockResolvedValue(undefined as never); + + service = Object.create(KycService.prototype); + (service as any).userDataService = userDataService; + (service as any).kycStepRepo = kycStepRepo; + jest.spyOn(service as any, 'createStepLog').mockResolvedValue(undefined); + jest.spyOn(service as any, 'updateProgress').mockImplementation(async (user: UserData) => user); + jest.spyOn(service as any, 'verify2fa').mockResolvedValue(undefined); + }); + + function financialStep(): KycStep { + return Object.assign(new KycStep(), { + id: 11, + name: KycStepName.FINANCIAL_DATA, + status: ReviewStatus.IN_PROGRESS, + sequenceNumber: 0, + }); + } + + function personalStep(): KycStep { + return Object.assign(new KycStep(), { + id: 12, + name: KycStepName.PERSONAL_DATA, + status: ReviewStatus.IN_PROGRESS, + sequenceNumber: 0, + }); + } + + function userWithPendingStep(step: KycStep, overrides: Partial = {}): UserData { + const user = createCustomUserData({ + kycHash: 'hash', + accountType: AccountType.PERSONAL, + kycSteps: [step], + ...overrides, + }); + return user; + } + + describe('updateFinancialData', () => { + it('returns complete=true with empty missingFields and moves the step to internal review', async () => { + const kycStep = financialStep(); + const user = userWithPendingStep(kycStep); + jest.spyOn(service as any, 'getUser').mockResolvedValue(user); + + const result = await service.updateFinancialData('hash', '1.2.3.4', 11, { responses: personalFinancialComplete }); + + expect(result.complete).toBe(true); + expect(result.missingFields).toEqual([]); + expect(kycStep.status).toBe(ReviewStatus.INTERNAL_REVIEW); + expect(result.status).toBe(KycStepStatus.IN_REVIEW); + expect(kycStepRepo.update).toHaveBeenCalledTimes(2); + expect(service['createStepLog']).toHaveBeenCalled(); + }); + + it('returns complete=false with missing keys, keeps step out of review, still saves responses', async () => { + const kycStep = financialStep(); + const user = userWithPendingStep(kycStep); + jest.spyOn(service as any, 'getUser').mockResolvedValue(user); + + const partialResponses = [ + { key: 'tnc', value: 'accept' }, + { key: 'own_funds', value: 'accept' }, + ]; + + const result = await service.updateFinancialData('hash', '1.2.3.4', 11, { responses: partialResponses }); + + expect(result.complete).toBe(false); + expect(result.missingFields).toEqual( + expect.arrayContaining([ + 'source_of_funds', + 'occupation', + 'sector', + 'risky_business', + 'income', + 'assets', + 'notification_of_changes', + ]), + ); + expect(result.missingFields).not.toContain('occupation_description'); + expect(kycStep.status).not.toBe(ReviewStatus.INTERNAL_REVIEW); + expect(kycStep.status).not.toBe(ReviewStatus.COMPLETED); + expect(kycStepRepo.update).toHaveBeenCalledTimes(1); + expect(kycStep.getResult()).toEqual(partialResponses); + expect(service['createStepLog']).not.toHaveBeenCalled(); + }); + + it('does not list conditional questions when their condition is not met', async () => { + const kycStep = financialStep(); + const user = userWithPendingStep(kycStep); + jest.spyOn(service as any, 'getUser').mockResolvedValue(user); + + // sector=it / risky_business=no → sector_description and risky_business_description not applicable + const result = await service.updateFinancialData('hash', '1.2.3.4', 11, { + responses: personalFinancialComplete, + }); + + expect(result.complete).toBe(true); + expect(result.missingFields).toEqual([]); + expect(result.missingFields).not.toContain('sector_description'); + expect(result.missingFields).not.toContain('risky_business_description'); + }); + + it('lists conditional questions when their condition is met and they are unanswered', async () => { + const kycStep = financialStep(); + const user = userWithPendingStep(kycStep); + jest.spyOn(service as any, 'getUser').mockResolvedValue(user); + + const responses = [ + { key: 'tnc', value: 'accept' }, + { key: 'own_funds', value: 'accept' }, + { key: 'source_of_funds', value: 'employment_income' }, + { key: 'occupation', value: 'self_employed' }, + // occupation_description missing + { key: 'sector', value: 'other' }, + // sector_description missing + { key: 'risky_business', value: 'yes_risky_business' }, + // risky_business_description missing + { key: 'income', value: '50k' }, + { key: 'assets', value: '50k' }, + { key: 'notification_of_changes', value: 'accept' }, + ]; + + const result = await service.updateFinancialData('hash', '1.2.3.4', 11, { responses }); + + expect(result.complete).toBe(false); + expect(result.missingFields).toEqual( + expect.arrayContaining(['occupation_description', 'sector_description', 'risky_business_description']), + ); + expect(kycStep.status).not.toBe(ReviewStatus.INTERNAL_REVIEW); + expect(kycStepRepo.update).toHaveBeenCalledTimes(1); + }); + }); + + describe('updatePersonalData', () => { + it('returns complete=false with missing required field names when data is incomplete', async () => { + const kycStep = personalStep(); + const incompleteUser = userWithPendingStep(kycStep, { + firstname: undefined, + surname: undefined, + phone: undefined, + street: 'Street', + location: 'City', + zip: '8000', + mail: 'a@b.com', + accountType: AccountType.PERSONAL, + }); + jest.spyOn(service as any, 'getUser').mockResolvedValue(incompleteUser); + userDataService.updatePersonalData.mockResolvedValue(incompleteUser); + + const result = await service.updatePersonalData('hash', 12, {} as any); + + expect(result.complete).toBe(false); + expect(result.missingFields).toEqual(expect.arrayContaining(['firstname', 'surname', 'phone'])); + expect(kycStep.status).toBe(ReviewStatus.IN_PROGRESS); + expect(kycStepRepo.update).not.toHaveBeenCalled(); + expect(service['createStepLog']).not.toHaveBeenCalled(); + }); + + it('returns complete=true and completes the step when personal data is complete', async () => { + const kycStep = personalStep(); + const completeUser = userWithPendingStep(kycStep, { + firstname: 'Ada', + surname: 'Lovelace', + phone: '+41 79 000 00 00', + street: 'Street', + location: 'City', + zip: '8000', + mail: 'a@b.com', + accountType: AccountType.PERSONAL, + country: createCustomCountry({ id: 1 }), + }); + jest.spyOn(service as any, 'getUser').mockResolvedValue(completeUser); + userDataService.updatePersonalData.mockResolvedValue(completeUser); + + const result = await service.updatePersonalData('hash', 12, {} as any); + + expect(result.complete).toBe(true); + expect(result.missingFields).toEqual([]); + expect(kycStep.status).toBe(ReviewStatus.COMPLETED); + expect(result.status).toBe(KycStepStatus.COMPLETED); + expect(kycStepRepo.update).toHaveBeenCalledTimes(1); + expect(service['createStepLog']).toHaveBeenCalled(); + }); + }); +}); diff --git a/src/subdomains/generic/kyc/services/integration/__tests__/financial.service.spec.ts b/src/subdomains/generic/kyc/services/integration/__tests__/financial.service.spec.ts new file mode 100644 index 0000000000..08319a744c --- /dev/null +++ b/src/subdomains/generic/kyc/services/integration/__tests__/financial.service.spec.ts @@ -0,0 +1,143 @@ +import { BadRequestException } from '@nestjs/common'; +import { AccountType } from 'src/subdomains/generic/user/models/user-data/account-type.enum'; +import { KycFinancialResponse } from '../../../dto/input/kyc-financial-in.dto'; +import { FinancialService } from '../financial.service'; + +describe('FinancialService', () => { + const personalCompleteResponses = (): KycFinancialResponse[] => [ + { key: 'tnc', value: 'accept' }, + { key: 'own_funds', value: 'accept' }, + { key: 'source_of_funds', value: 'employment_income' }, + { key: 'occupation', value: 'employed' }, + { key: 'occupation_description', value: 'Software engineer' }, + { key: 'sector', value: 'it' }, + { key: 'risky_business', value: 'no_risky_business' }, + { key: 'income', value: '50k' }, + { key: 'assets', value: '50k' }, + { key: 'notification_of_changes', value: 'accept' }, + ]; + + describe('getMissingFields', () => { + it('returns empty when all applicable personal questions are answered', () => { + expect(FinancialService.getMissingFields(personalCompleteResponses(), AccountType.PERSONAL)).toEqual([]); + expect(FinancialService.isComplete(personalCompleteResponses(), AccountType.PERSONAL)).toBe(true); + }); + + it('lists unanswered applicable question keys', () => { + const responses: KycFinancialResponse[] = [ + { key: 'tnc', value: 'accept' }, + { key: 'own_funds', value: 'accept' }, + ]; + + const missing = FinancialService.getMissingFields(responses, AccountType.PERSONAL); + + expect(missing).toEqual( + expect.arrayContaining([ + 'source_of_funds', + 'occupation', + 'sector', + 'risky_business', + 'income', + 'assets', + 'notification_of_changes', + ]), + ); + expect(missing).not.toContain('occupation_description'); + expect(missing).not.toContain('sector_description'); + expect(missing).not.toContain('risky_business_description'); + expect(FinancialService.isComplete(responses, AccountType.PERSONAL)).toBe(false); + }); + + it('does not report a conditional question when its condition is not met', () => { + const responses: KycFinancialResponse[] = [ + { key: 'tnc', value: 'accept' }, + { key: 'own_funds', value: 'accept' }, + { key: 'source_of_funds', value: 'employment_income' }, + { key: 'occupation', value: 'employed' }, + // occupation_description required (condition met) — omit on purpose below when testing other fields + { key: 'occupation_description', value: 'Engineer' }, + { key: 'sector', value: 'it' }, // sector_description only when sector=other + { key: 'risky_business', value: 'no_risky_business' }, // description only when yes + { key: 'income', value: '50k' }, + { key: 'assets', value: '50k' }, + { key: 'notification_of_changes', value: 'accept' }, + ]; + + const missing = FinancialService.getMissingFields(responses, AccountType.PERSONAL); + + expect(missing).toEqual([]); + expect(missing).not.toContain('sector_description'); + expect(missing).not.toContain('risky_business_description'); + }); + + it('reports a conditional question when its condition is met and it is unanswered', () => { + const responses: KycFinancialResponse[] = [ + { key: 'tnc', value: 'accept' }, + { key: 'own_funds', value: 'accept' }, + { key: 'source_of_funds', value: 'employment_income' }, + { key: 'occupation', value: 'employed' }, + // occupation_description required but missing + { key: 'sector', value: 'other' }, + // sector_description required but missing + { key: 'risky_business', value: 'yes_risky_business' }, + // risky_business_description required but missing + { key: 'income', value: '50k' }, + { key: 'assets', value: '50k' }, + { key: 'notification_of_changes', value: 'accept' }, + ]; + + const missing = FinancialService.getMissingFields(responses, AccountType.PERSONAL); + + expect(missing).toEqual( + expect.arrayContaining(['occupation_description', 'sector_description', 'risky_business_description']), + ); + expect(FinancialService.isComplete(responses, AccountType.PERSONAL)).toBe(false); + }); + + it('throws on duplicate response keys', () => { + expect(() => + FinancialService.getMissingFields( + [ + { key: 'tnc', value: 'accept' }, + { key: 'tnc', value: 'accept' }, + ], + AccountType.PERSONAL, + ), + ).toThrow(BadRequestException); + }); + + it('throws on invalid option values when no earlier required answer is missing', () => { + expect(() => + FinancialService.getMissingFields( + [ + { key: 'tnc', value: 'accept' }, + { key: 'own_funds', value: 'accept' }, + { key: 'source_of_funds', value: 'not_a_valid_option' }, + ], + AccountType.PERSONAL, + ), + ).toThrow(BadRequestException); + }); + + it('does not throw for a later invalid option when an earlier required answer is missing', () => { + // tnc unanswered first; source_of_funds carries an invalid option later in the catalog. + // Pre-refactor every() short-circuited on the first missing field and never reached option checks. + const responses: KycFinancialResponse[] = [ + { key: 'own_funds', value: 'accept' }, + { key: 'source_of_funds', value: 'not_a_valid_option' }, + ]; + + expect(() => FinancialService.getMissingFields(responses, AccountType.PERSONAL)).not.toThrow(); + + const missing = FinancialService.getMissingFields(responses, AccountType.PERSONAL); + expect(missing).toContain('tnc'); + expect(missing).toEqual(expect.arrayContaining(['tnc', 'occupation', 'sector', 'income', 'assets'])); + }); + + it('still throws when the first answered question with options has an invalid value', () => { + expect(() => + FinancialService.getMissingFields([{ key: 'tnc', value: 'not_a_valid_option' }], AccountType.PERSONAL), + ).toThrow(BadRequestException); + }); + }); +}); diff --git a/src/subdomains/generic/kyc/services/integration/financial.service.ts b/src/subdomains/generic/kyc/services/integration/financial.service.ts index 47cc2dcfc1..4efb4372bb 100644 --- a/src/subdomains/generic/kyc/services/integration/financial.service.ts +++ b/src/subdomains/generic/kyc/services/integration/financial.service.ts @@ -25,25 +25,39 @@ export class FinancialService { })); } - static isComplete(responses: KycFinancialResponse[], accountType: AccountType): boolean { + static getMissingFields(responses: KycFinancialResponse[], accountType: AccountType): string[] { const hasDuplicates = new Set(responses.map((r) => r.key)).size !== responses.length; if (hasDuplicates) throw new BadRequestException('Duplicate response keys found'); - return getFinancialQuestions(accountType).every((q) => { + const missingFields: string[] = []; + + for (const q of getFinancialQuestions(accountType)) { if ( q.conditions?.length && - !q.conditions?.some((c) => responses.some((r) => r.key === c.question && r.value === c.response)) - ) - return true; + !q.conditions.some((c) => responses.some((r) => r.key === c.question && r.value === c.response)) + ) { + continue; + } const response = responses.find((r) => r.key === q.key); - if (!response?.value) return false; + if (!response?.value) { + if (!missingFields.includes(q.key)) missingFields.push(q.key); + continue; + } + + // Match pre-refactor every() short-circuit: once a required answer is missing, do not + // validate option values on later questions (draft submits must stay HTTP 200). + if (missingFields.length > 0) continue; const responseValues = q.type === QuestionType.MULTIPLE_CHOICE ? response.value.split(',') : [response.value]; if (q.options && !responseValues.every((o) => q.options.includes(o))) throw new BadRequestException(`Response '${response.value}' for question '${q.key}' is not valid`); + } - return true; - }); + return missingFields; + } + + static isComplete(responses: KycFinancialResponse[], accountType: AccountType): boolean { + return this.getMissingFields(responses, accountType).length === 0; } } diff --git a/src/subdomains/generic/kyc/services/kyc.service.ts b/src/subdomains/generic/kyc/services/kyc.service.ts index 4fbe642f36..d56fe36d39 100644 --- a/src/subdomains/generic/kyc/services/kyc.service.ts +++ b/src/subdomains/generic/kyc/services/kyc.service.ts @@ -72,7 +72,7 @@ import { KycFileMapper } from '../dto/mapper/kyc-file.mapper'; import { KycInfoMapper } from '../dto/mapper/kyc-info.mapper'; import { KycStepMapper } from '../dto/mapper/kyc-step.mapper'; import { KycFinancialOutData } from '../dto/output/kyc-financial-out.dto'; -import { KycLevelDto, KycSessionDto, KycStepBase } from '../dto/output/kyc-info.dto'; +import { KycLevelDto, KycSessionDto, KycStepBase, KycStepSubmitDto } from '../dto/output/kyc-info.dto'; import { SumSubBlockLabels, SumSubRejectionLabels, @@ -620,20 +620,22 @@ export class KycService { return KycStepMapper.toStepBase(kycStep); } - async updatePersonalData(kycHash: string, stepId: number, data: KycPersonalData): Promise { + async updatePersonalData(kycHash: string, stepId: number, data: KycPersonalData): Promise { let user = await this.getUser(kycHash); const kycStep = user.getPendingStepOrThrow(stepId, KycStepName.PERSONAL_DATA); user = await this.userDataService.updatePersonalData(user, data); - if (user.isDataComplete) { + const missingFields = user.missingKycFields; + const complete = missingFields.length === 0; + if (complete) { await this.kycStepRepo.update(...kycStep.complete(data)); await this.createStepLog(user, kycStep); } await this.updateProgress(user, false); - return KycStepMapper.toStepBase(kycStep); + return { ...KycStepMapper.toStepBase(kycStep), complete, missingFields }; } async updateKycStep( @@ -883,7 +885,7 @@ export class KycService { ip: string, stepId: number, data: KycFinancialInData, - ): Promise { + ): Promise { const user = await this.getUser(kycHash); const kycStep = user.getPendingStepOrThrow(stepId, KycStepName.FINANCIAL_DATA); @@ -891,7 +893,8 @@ export class KycService { await this.kycStepRepo.update(...kycStep.update(undefined, data.responses)); - const complete = FinancialService.isComplete(data.responses, user.accountType); + const missingFields = FinancialService.getMissingFields(data.responses, user.accountType); + const complete = missingFields.length === 0; if (complete) { await this.kycStepRepo.update(...kycStep.internalReview()); await this.createStepLog(user, kycStep); @@ -899,7 +902,7 @@ export class KycService { await this.updateProgress(user, false); - return KycStepMapper.toStepBase(kycStep); + return { ...KycStepMapper.toStepBase(kycStep), complete, missingFields }; } async updatePaymentData(kycHash: string, stepId: number, data: PaymentDataDto): Promise { diff --git a/src/subdomains/generic/user/models/user-data/user-data.entity.ts b/src/subdomains/generic/user/models/user-data/user-data.entity.ts index 822cfc2740..987ea2a594 100644 --- a/src/subdomains/generic/user/models/user-data/user-data.entity.ts +++ b/src/subdomains/generic/user/models/user-data/user-data.entity.ts @@ -864,8 +864,12 @@ export class UserData extends IEntity { ); } + get missingKycFields(): string[] { + return this.requiredKycFields.filter((f) => !this[f]); + } + get isDataComplete(): boolean { - return this.requiredKycFields.every((f) => this[f]); + return this.missingKycFields.length === 0; } get requiredInvoiceFields(): string[] { From 41994e69109e73e50a662f6da0efbf5df19e68b2 Mon Sep 17 00:00:00 2001 From: joshuakrueger-dfx Date: Tue, 28 Jul 2026 10:42:38 +0200 Subject: [PATCH 2/3] fix(kyc): preserve step state and use request field names in submit response The financial draft path passed an undefined status into KycStep.update, which cleared status and sequenceNumber on the returned entity, so the response omitted two required fields. The personal data endpoint reported entity field names that do not exist in the request contract; they are now mapped to the request field paths, and the mail field is omitted because it cannot be set through this endpoint. --- .../personal-data-missing-fields.spec.ts | 41 ++++++ .../generic/kyc/dto/mapper/kyc-step.mapper.ts | 11 ++ .../mapper/personal-data-missing-fields.ts | 27 ++++ .../generic/kyc/dto/output/kyc-info.dto.ts | 2 +- .../services/__tests__/kyc.service.spec.ts | 124 +++++++++++++++--- .../__tests__/financial.service.spec.ts | 73 +++++++++-- .../generic/kyc/services/kyc.service.ts | 11 +- 7 files changed, 254 insertions(+), 35 deletions(-) create mode 100644 src/subdomains/generic/kyc/dto/mapper/__tests__/personal-data-missing-fields.spec.ts create mode 100644 src/subdomains/generic/kyc/dto/mapper/personal-data-missing-fields.ts diff --git a/src/subdomains/generic/kyc/dto/mapper/__tests__/personal-data-missing-fields.spec.ts b/src/subdomains/generic/kyc/dto/mapper/__tests__/personal-data-missing-fields.spec.ts new file mode 100644 index 0000000000..1e0d81f9d5 --- /dev/null +++ b/src/subdomains/generic/kyc/dto/mapper/__tests__/personal-data-missing-fields.spec.ts @@ -0,0 +1,41 @@ +import { toPersonalDataMissingFields } from '../personal-data-missing-fields'; + +describe('toPersonalDataMissingFields', () => { + it('maps entity names to KycPersonalData request paths', () => { + expect( + toPersonalDataMissingFields([ + 'firstname', + 'surname', + 'street', + 'location', + 'zip', + 'country', + 'organizationStreet', + 'organizationLocation', + 'organizationZip', + 'organizationCountry', + 'organizationName', + 'phone', + 'accountType', + ]), + ).toEqual([ + 'firstName', + 'lastName', + 'address.street', + 'address.city', + 'address.zip', + 'address.country', + 'organizationAddress.street', + 'organizationAddress.city', + 'organizationAddress.zip', + 'organizationAddress.country', + 'organizationName', + 'phone', + 'accountType', + ]); + }); + + it('omits mail because it is not settable on the personal-data endpoint', () => { + expect(toPersonalDataMissingFields(['mail', 'firstname', 'phone'])).toEqual(['firstName', 'phone']); + }); +}); diff --git a/src/subdomains/generic/kyc/dto/mapper/kyc-step.mapper.ts b/src/subdomains/generic/kyc/dto/mapper/kyc-step.mapper.ts index e31ed18ffe..4715641372 100644 --- a/src/subdomains/generic/kyc/dto/mapper/kyc-step.mapper.ts +++ b/src/subdomains/generic/kyc/dto/mapper/kyc-step.mapper.ts @@ -8,6 +8,7 @@ import { KycStepDto, KycStepReason, KycStepSessionDto, + KycStepSubmitDto, } from '../output/kyc-info.dto'; export class KycStepMapper { @@ -30,6 +31,16 @@ export class KycStepMapper { return Object.assign(new KycStepSessionDto(), dto); } + static toStepSubmit(kycStep: KycStep, complete: boolean, missingFields: string[]): KycStepSubmitDto { + const dto: KycStepSubmitDto = { + ...KycStepMapper.toStepBase(kycStep), + complete, + missingFields, + }; + + return Object.assign(new KycStepSubmitDto(), dto); + } + static toStepBase(kycStep: KycStep): KycStepBase { return { name: kycStep.name, diff --git a/src/subdomains/generic/kyc/dto/mapper/personal-data-missing-fields.ts b/src/subdomains/generic/kyc/dto/mapper/personal-data-missing-fields.ts new file mode 100644 index 0000000000..fefc2b6a06 --- /dev/null +++ b/src/subdomains/generic/kyc/dto/mapper/personal-data-missing-fields.ts @@ -0,0 +1,27 @@ +/** + * Maps UserData entity field names to KycPersonalData request paths for submit feedback. + * `mail` is intentionally omitted: it is not settable on the personal-data endpoint + * (ContactData step owns it), so reporting it as missing is not actionable for the client. + */ +const PERSONAL_DATA_ENTITY_TO_REQUEST: Readonly> = { + accountType: 'accountType', + firstname: 'firstName', + surname: 'lastName', + phone: 'phone', + street: 'address.street', + location: 'address.city', + zip: 'address.zip', + country: 'address.country', + organizationName: 'organizationName', + organizationStreet: 'organizationAddress.street', + organizationLocation: 'organizationAddress.city', + organizationZip: 'organizationAddress.zip', + organizationCountry: 'organizationAddress.country', +}; + +export function toPersonalDataMissingFields(entityFields: string[]): string[] { + return entityFields + .filter((f) => f !== 'mail') + .map((f) => PERSONAL_DATA_ENTITY_TO_REQUEST[f]) + .filter((path): path is string => path != null); +} diff --git a/src/subdomains/generic/kyc/dto/output/kyc-info.dto.ts b/src/subdomains/generic/kyc/dto/output/kyc-info.dto.ts index 6b83b5f907..1a5b7f6b86 100644 --- a/src/subdomains/generic/kyc/dto/output/kyc-info.dto.ts +++ b/src/subdomains/generic/kyc/dto/output/kyc-info.dto.ts @@ -74,7 +74,7 @@ export class KycStepSubmitDto extends KycStepBase { @ApiProperty({ description: - 'Keys of missing required fields when complete is false; empty when complete is true. Financial: unanswered applicable question keys; personal data: missing requiredKycFields names.', + 'Request field paths still missing when complete is false; empty when complete is true. Financial: unanswered applicable question keys. Personal data: KycPersonalData paths (e.g. firstName, address.street, organizationAddress.city). mail is never listed on personal data (ContactData step).', type: String, isArray: true, }) diff --git a/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts b/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts index ff3c1a8d47..39eb82656d 100644 --- a/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts +++ b/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts @@ -650,11 +650,12 @@ describe('KycService submit completeness feedback', () => { expect(result.missingFields).toEqual([]); expect(kycStep.status).toBe(ReviewStatus.INTERNAL_REVIEW); expect(result.status).toBe(KycStepStatus.IN_REVIEW); + expect(result.sequenceNumber).toBe(0); expect(kycStepRepo.update).toHaveBeenCalledTimes(2); expect(service['createStepLog']).toHaveBeenCalled(); }); - it('returns complete=false with missing keys, keeps step out of review, still saves responses', async () => { + it('returns complete=false with missing keys, keeps status/sequenceNumber, still saves responses', async () => { const kycStep = financialStep(); const user = userWithPendingStep(kycStep); jest.spyOn(service as any, 'getUser').mockResolvedValue(user); @@ -679,8 +680,10 @@ describe('KycService submit completeness feedback', () => { ]), ); expect(result.missingFields).not.toContain('occupation_description'); - expect(kycStep.status).not.toBe(ReviewStatus.INTERNAL_REVIEW); - expect(kycStep.status).not.toBe(ReviewStatus.COMPLETED); + expect(kycStep.status).toBe(ReviewStatus.IN_PROGRESS); + expect(result.status).toBe(KycStepStatus.IN_PROGRESS); + expect(result.sequenceNumber).toBe(0); + expect(kycStep.sequenceNumber).toBe(0); expect(kycStepRepo.update).toHaveBeenCalledTimes(1); expect(kycStep.getResult()).toEqual(partialResponses); expect(service['createStepLog']).not.toHaveBeenCalled(); @@ -691,15 +694,16 @@ describe('KycService submit completeness feedback', () => { const user = userWithPendingStep(kycStep); jest.spyOn(service as any, 'getUser').mockResolvedValue(user); - // sector=it / risky_business=no → sector_description and risky_business_description not applicable - const result = await service.updateFinancialData('hash', '1.2.3.4', 11, { - responses: personalFinancialComplete, - }); + // Partial submit: income/assets missing; sector=it / risky_business=no → conditionals N/A + const responses = personalFinancialComplete.filter((r) => r.key !== 'income' && r.key !== 'assets'); - expect(result.complete).toBe(true); - expect(result.missingFields).toEqual([]); + const result = await service.updateFinancialData('hash', '1.2.3.4', 11, { responses }); + + expect(result.complete).toBe(false); + expect(result.missingFields).toEqual(expect.arrayContaining(['income', 'assets'])); expect(result.missingFields).not.toContain('sector_description'); expect(result.missingFields).not.toContain('risky_business_description'); + expect(result.status).toBe(KycStepStatus.IN_PROGRESS); }); it('lists conditional questions when their condition is met and they are unanswered', async () => { @@ -712,11 +716,8 @@ describe('KycService submit completeness feedback', () => { { key: 'own_funds', value: 'accept' }, { key: 'source_of_funds', value: 'employment_income' }, { key: 'occupation', value: 'self_employed' }, - // occupation_description missing { key: 'sector', value: 'other' }, - // sector_description missing { key: 'risky_business', value: 'yes_risky_business' }, - // risky_business_description missing { key: 'income', value: '50k' }, { key: 'assets', value: '50k' }, { key: 'notification_of_changes', value: 'accept' }, @@ -728,36 +729,105 @@ describe('KycService submit completeness feedback', () => { expect(result.missingFields).toEqual( expect.arrayContaining(['occupation_description', 'sector_description', 'risky_business_description']), ); - expect(kycStep.status).not.toBe(ReviewStatus.INTERNAL_REVIEW); + expect(kycStep.status).toBe(ReviewStatus.IN_PROGRESS); + expect(result.status).toBe(KycStepStatus.IN_PROGRESS); expect(kycStepRepo.update).toHaveBeenCalledTimes(1); }); }); describe('updatePersonalData', () => { - it('returns complete=false with missing required field names when data is incomplete', async () => { + it('returns complete=false with KycPersonalData request paths when organization fields are missing', async () => { + // ORGANIZATION submit with natural-person fields set (as ValidationPipe requires) but org + // address/name still empty — mirrors a partial org KYC payload after updatePersonalData. const kycStep = personalStep(); const incompleteUser = userWithPendingStep(kycStep, { - firstname: undefined, - surname: undefined, - phone: undefined, + accountType: AccountType.ORGANIZATION, + firstname: 'Ada', + surname: 'Lovelace', + phone: '+41 79 000 00 00', street: 'Street', location: 'City', zip: '8000', mail: 'a@b.com', - accountType: AccountType.PERSONAL, + country: createCustomCountry({ id: 1 }), + organizationName: undefined, + organizationStreet: undefined, + organizationLocation: undefined, + organizationZip: undefined, + organizationCountry: undefined, }); jest.spyOn(service as any, 'getUser').mockResolvedValue(incompleteUser); userDataService.updatePersonalData.mockResolvedValue(incompleteUser); - const result = await service.updatePersonalData('hash', 12, {} as any); + const result = await service.updatePersonalData('hash', 12, { + accountType: AccountType.ORGANIZATION, + firstName: 'Ada', + lastName: 'Lovelace', + phone: '+41 79 000 00 00', + address: { + street: 'Street', + city: 'City', + zip: '8000', + country: createCustomCountry({ id: 1 }), + }, + } as any); expect(result.complete).toBe(false); - expect(result.missingFields).toEqual(expect.arrayContaining(['firstname', 'surname', 'phone'])); + expect(result.missingFields).toEqual( + expect.arrayContaining([ + 'organizationName', + 'organizationAddress.street', + 'organizationAddress.city', + 'organizationAddress.zip', + 'organizationAddress.country', + ]), + ); + expect(result.missingFields).not.toContain('mail'); + expect(result.missingFields).not.toContain('firstname'); + expect(result.missingFields).not.toContain('surname'); + expect(result.status).toBe(KycStepStatus.IN_PROGRESS); + expect(result.sequenceNumber).toBe(0); expect(kycStep.status).toBe(ReviewStatus.IN_PROGRESS); expect(kycStepRepo.update).not.toHaveBeenCalled(); expect(service['createStepLog']).not.toHaveBeenCalled(); }); + it('omits mail from missingFields even when entity mail is unset', async () => { + // mail is required for isDataComplete but not settable on this endpoint (ContactData). + const kycStep = personalStep(); + const userWithoutMail = userWithPendingStep(kycStep, { + accountType: AccountType.PERSONAL, + firstname: 'Ada', + surname: 'Lovelace', + phone: '+41 79 000 00 00', + street: 'Street', + location: 'City', + zip: '8000', + mail: undefined, + country: createCustomCountry({ id: 1 }), + }); + jest.spyOn(service as any, 'getUser').mockResolvedValue(userWithoutMail); + userDataService.updatePersonalData.mockResolvedValue(userWithoutMail); + + const result = await service.updatePersonalData('hash', 12, { + accountType: AccountType.PERSONAL, + firstName: 'Ada', + lastName: 'Lovelace', + phone: '+41 79 000 00 00', + address: { + street: 'Street', + city: 'City', + zip: '8000', + country: createCustomCountry({ id: 1 }), + }, + } as any); + + expect(result.complete).toBe(false); + expect(result.missingFields).toEqual([]); + expect(result.missingFields).not.toContain('mail'); + expect(kycStepRepo.update).not.toHaveBeenCalled(); + }); + it('returns complete=true and completes the step when personal data is complete', async () => { const kycStep = personalStep(); const completeUser = userWithPendingStep(kycStep, { @@ -774,12 +844,24 @@ describe('KycService submit completeness feedback', () => { jest.spyOn(service as any, 'getUser').mockResolvedValue(completeUser); userDataService.updatePersonalData.mockResolvedValue(completeUser); - const result = await service.updatePersonalData('hash', 12, {} as any); + const result = await service.updatePersonalData('hash', 12, { + accountType: AccountType.PERSONAL, + firstName: 'Ada', + lastName: 'Lovelace', + phone: '+41 79 000 00 00', + address: { + street: 'Street', + city: 'City', + zip: '8000', + country: createCustomCountry({ id: 1 }), + }, + } as any); expect(result.complete).toBe(true); expect(result.missingFields).toEqual([]); expect(kycStep.status).toBe(ReviewStatus.COMPLETED); expect(result.status).toBe(KycStepStatus.COMPLETED); + expect(result.sequenceNumber).toBe(0); expect(kycStepRepo.update).toHaveBeenCalledTimes(1); expect(service['createStepLog']).toHaveBeenCalled(); }); diff --git a/src/subdomains/generic/kyc/services/integration/__tests__/financial.service.spec.ts b/src/subdomains/generic/kyc/services/integration/__tests__/financial.service.spec.ts index 08319a744c..5aa35f0342 100644 --- a/src/subdomains/generic/kyc/services/integration/__tests__/financial.service.spec.ts +++ b/src/subdomains/generic/kyc/services/integration/__tests__/financial.service.spec.ts @@ -17,6 +17,17 @@ describe('FinancialService', () => { { key: 'notification_of_changes', value: 'accept' }, ]; + const organizationCompleteResponses = (): KycFinancialResponse[] => [ + { key: 'tnc', value: 'accept' }, + { key: 'own_funds', value: 'accept' }, + { key: 'source_of_funds', value: 'operations' }, + { key: 'sector', value: 'it' }, + { key: 'risky_business', value: 'no_risky_business' }, + { key: 'income', value: '50k' }, + { key: 'assets', value: '50k' }, + { key: 'notification_of_changes', value: 'accept' }, + ]; + describe('getMissingFields', () => { it('returns empty when all applicable personal questions are answered', () => { expect(FinancialService.getMissingFields(personalCompleteResponses(), AccountType.PERSONAL)).toEqual([]); @@ -49,23 +60,22 @@ describe('FinancialService', () => { }); it('does not report a conditional question when its condition is not met', () => { + // sector=it / risky_business=no → descriptions not applicable; omit income so the list is non-empty const responses: KycFinancialResponse[] = [ { key: 'tnc', value: 'accept' }, { key: 'own_funds', value: 'accept' }, { key: 'source_of_funds', value: 'employment_income' }, { key: 'occupation', value: 'employed' }, - // occupation_description required (condition met) — omit on purpose below when testing other fields { key: 'occupation_description', value: 'Engineer' }, - { key: 'sector', value: 'it' }, // sector_description only when sector=other - { key: 'risky_business', value: 'no_risky_business' }, // description only when yes - { key: 'income', value: '50k' }, + { key: 'sector', value: 'it' }, + { key: 'risky_business', value: 'no_risky_business' }, { key: 'assets', value: '50k' }, { key: 'notification_of_changes', value: 'accept' }, ]; const missing = FinancialService.getMissingFields(responses, AccountType.PERSONAL); - expect(missing).toEqual([]); + expect(missing).toContain('income'); expect(missing).not.toContain('sector_description'); expect(missing).not.toContain('risky_business_description'); }); @@ -76,11 +86,8 @@ describe('FinancialService', () => { { key: 'own_funds', value: 'accept' }, { key: 'source_of_funds', value: 'employment_income' }, { key: 'occupation', value: 'employed' }, - // occupation_description required but missing { key: 'sector', value: 'other' }, - // sector_description required but missing { key: 'risky_business', value: 'yes_risky_business' }, - // risky_business_description required but missing { key: 'income', value: '50k' }, { key: 'assets', value: '50k' }, { key: 'notification_of_changes', value: 'accept' }, @@ -94,6 +101,56 @@ describe('FinancialService', () => { expect(FinancialService.isComplete(responses, AccountType.PERSONAL)).toBe(false); }); + it('returns empty for a complete organization questionnaire', () => { + expect(FinancialService.getMissingFields(organizationCompleteResponses(), AccountType.ORGANIZATION)).toEqual([]); + expect(FinancialService.isComplete(organizationCompleteResponses(), AccountType.ORGANIZATION)).toBe(true); + }); + + it('lists missing organization keys without personal-only occupation questions', () => { + const responses: KycFinancialResponse[] = [ + { key: 'tnc', value: 'accept' }, + { key: 'own_funds', value: 'accept' }, + ]; + + const missing = FinancialService.getMissingFields(responses, AccountType.ORGANIZATION); + + expect(missing).toEqual( + expect.arrayContaining([ + 'source_of_funds', + 'sector', + 'risky_business', + 'income', + 'assets', + 'notification_of_changes', + ]), + ); + expect(missing).not.toContain('occupation'); + expect(missing).not.toContain('occupation_description'); + expect(FinancialService.isComplete(responses, AccountType.ORGANIZATION)).toBe(false); + }); + + it('accepts a valid multi-value multiple-choice answer', () => { + const responses = personalCompleteResponses().map((r) => + r.key === 'source_of_funds' ? { key: 'source_of_funds', value: 'employment_income,pension' } : r, + ); + + expect(FinancialService.getMissingFields(responses, AccountType.PERSONAL)).toEqual([]); + expect(FinancialService.isComplete(responses, AccountType.PERSONAL)).toBe(true); + }); + + it('throws when a multi-value multiple-choice answer contains an invalid option', () => { + expect(() => + FinancialService.getMissingFields( + [ + { key: 'tnc', value: 'accept' }, + { key: 'own_funds', value: 'accept' }, + { key: 'source_of_funds', value: 'employment_income,not_a_valid_option' }, + ], + AccountType.PERSONAL, + ), + ).toThrow(BadRequestException); + }); + it('throws on duplicate response keys', () => { expect(() => FinancialService.getMissingFields( diff --git a/src/subdomains/generic/kyc/services/kyc.service.ts b/src/subdomains/generic/kyc/services/kyc.service.ts index d56fe36d39..e7a83a95a4 100644 --- a/src/subdomains/generic/kyc/services/kyc.service.ts +++ b/src/subdomains/generic/kyc/services/kyc.service.ts @@ -71,6 +71,7 @@ import { FileSubType, FileType, KycFileBlob, KycFileDataDto } from '../dto/kyc-f import { KycFileMapper } from '../dto/mapper/kyc-file.mapper'; import { KycInfoMapper } from '../dto/mapper/kyc-info.mapper'; import { KycStepMapper } from '../dto/mapper/kyc-step.mapper'; +import { toPersonalDataMissingFields } from '../dto/mapper/personal-data-missing-fields'; import { KycFinancialOutData } from '../dto/output/kyc-financial-out.dto'; import { KycLevelDto, KycSessionDto, KycStepBase, KycStepSubmitDto } from '../dto/output/kyc-info.dto'; import { @@ -626,8 +627,8 @@ export class KycService { user = await this.userDataService.updatePersonalData(user, data); - const missingFields = user.missingKycFields; - const complete = missingFields.length === 0; + const complete = user.isDataComplete; + const missingFields = toPersonalDataMissingFields(user.missingKycFields); if (complete) { await this.kycStepRepo.update(...kycStep.complete(data)); await this.createStepLog(user, kycStep); @@ -635,7 +636,7 @@ export class KycService { await this.updateProgress(user, false); - return { ...KycStepMapper.toStepBase(kycStep), complete, missingFields }; + return KycStepMapper.toStepSubmit(kycStep, complete, missingFields); } async updateKycStep( @@ -891,7 +892,7 @@ export class KycService { await this.verify2fa(user, ip); - await this.kycStepRepo.update(...kycStep.update(undefined, data.responses)); + await this.kycStepRepo.update(...kycStep.update(kycStep.status, data.responses, undefined, kycStep.sequenceNumber)); const missingFields = FinancialService.getMissingFields(data.responses, user.accountType); const complete = missingFields.length === 0; @@ -902,7 +903,7 @@ export class KycService { await this.updateProgress(user, false); - return { ...KycStepMapper.toStepBase(kycStep), complete, missingFields }; + return KycStepMapper.toStepSubmit(kycStep, complete, missingFields); } async updatePaymentData(kycHash: string, stepId: number, data: PaymentDataDto): Promise { From bab9e569c8bc5004720a7167310bd3a0d427cd1c Mon Sep 17 00:00:00 2001 From: joshuakrueger-dfx Date: Tue, 28 Jul 2026 11:28:59 +0200 Subject: [PATCH 3/3] fix(kyc): keep omitted step fields untouched in KycStep.update Object.assign copied undefined values, so calling update without a status or sequence number cleared both on the in-memory entity. Callers that only wanted to store a result therefore returned responses without two required fields, and the previous call-site workaround wrote a possibly stale status back to the database. Dropping undefined values at the source keeps the write behaviour unchanged and fixes the file upload path as well. --- .../personal-data-missing-fields.spec.ts | 17 +++++++- .../mapper/personal-data-missing-fields.ts | 41 +++++++++++++++---- .../generic/kyc/dto/output/kyc-info.dto.ts | 2 +- .../__tests__/kyc-step.entity.spec.ts | 39 ++++++++++++++++++ .../generic/kyc/entities/kyc-step.entity.ts | 16 +++++--- .../services/__tests__/kyc.service.spec.ts | 36 ++++++++++++++++ .../generic/kyc/services/kyc.service.ts | 2 +- .../user/models/user-data/user-data.entity.ts | 36 ++++++++++++---- 8 files changed, 164 insertions(+), 25 deletions(-) create mode 100644 src/subdomains/generic/kyc/entities/__tests__/kyc-step.entity.spec.ts diff --git a/src/subdomains/generic/kyc/dto/mapper/__tests__/personal-data-missing-fields.spec.ts b/src/subdomains/generic/kyc/dto/mapper/__tests__/personal-data-missing-fields.spec.ts index 1e0d81f9d5..2e320bb272 100644 --- a/src/subdomains/generic/kyc/dto/mapper/__tests__/personal-data-missing-fields.spec.ts +++ b/src/subdomains/generic/kyc/dto/mapper/__tests__/personal-data-missing-fields.spec.ts @@ -1,3 +1,4 @@ +import { DfxLogger } from 'src/shared/services/dfx-logger'; import { toPersonalDataMissingFields } from '../personal-data-missing-fields'; describe('toPersonalDataMissingFields', () => { @@ -35,7 +36,21 @@ describe('toPersonalDataMissingFields', () => { ]); }); - it('omits mail because it is not settable on the personal-data endpoint', () => { + it('omits mail as a known non-reportable field without treating it as unknown', () => { + const errorSpy = jest.spyOn(DfxLogger.prototype, 'error').mockImplementation(); + expect(toPersonalDataMissingFields(['mail', 'firstname', 'phone'])).toEqual(['firstName', 'phone']); + expect(errorSpy).not.toHaveBeenCalled(); + + errorSpy.mockRestore(); + }); + + it('logs unknown entity fields instead of silently dropping them', () => { + const errorSpy = jest.spyOn(DfxLogger.prototype, 'error').mockImplementation(); + + expect(toPersonalDataMissingFields(['notARealField', 'phone'])).toEqual(['phone']); + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('notARealField')); + + errorSpy.mockRestore(); }); }); diff --git a/src/subdomains/generic/kyc/dto/mapper/personal-data-missing-fields.ts b/src/subdomains/generic/kyc/dto/mapper/personal-data-missing-fields.ts index fefc2b6a06..0436ca6ecd 100644 --- a/src/subdomains/generic/kyc/dto/mapper/personal-data-missing-fields.ts +++ b/src/subdomains/generic/kyc/dto/mapper/personal-data-missing-fields.ts @@ -1,13 +1,22 @@ +import { DfxLogger } from 'src/shared/services/dfx-logger'; +import { RequiredKycField } from 'src/subdomains/generic/user/models/user-data/user-data.entity'; + +const logger = new DfxLogger('PersonalDataMissingFields'); + /** * Maps UserData entity field names to KycPersonalData request paths for submit feedback. - * `mail` is intentionally omitted: it is not settable on the personal-data endpoint - * (ContactData step owns it), so reporting it as missing is not actionable for the client. + * `string` = request path; `null` = known required field that is not reportable here + * (e.g. mail is owned by the ContactData step). + * + * Typed as Record over RequiredKycField so adding a field to requiredKycFields breaks the build + * until a mapping entry is added. */ -const PERSONAL_DATA_ENTITY_TO_REQUEST: Readonly> = { +const PERSONAL_DATA_ENTITY_TO_REQUEST: Record = { accountType: 'accountType', + mail: null, + phone: 'phone', firstname: 'firstName', surname: 'lastName', - phone: 'phone', street: 'address.street', location: 'address.city', zip: 'address.zip', @@ -19,9 +28,23 @@ const PERSONAL_DATA_ENTITY_TO_REQUEST: Readonly> = { organizationCountry: 'organizationAddress.country', }; -export function toPersonalDataMissingFields(entityFields: string[]): string[] { - return entityFields - .filter((f) => f !== 'mail') - .map((f) => PERSONAL_DATA_ENTITY_TO_REQUEST[f]) - .filter((path): path is string => path != null); +function isRequiredKycField(field: string): field is RequiredKycField { + return Object.prototype.hasOwnProperty.call(PERSONAL_DATA_ENTITY_TO_REQUEST, field); +} + +export function toPersonalDataMissingFields(entityFields: readonly string[]): string[] { + const paths: string[] = []; + + for (const field of entityFields) { + if (!isRequiredKycField(field)) { + logger.error(`Unknown KYC field in personal-data completeness mapping: ${field}`); + continue; + } + + const path = PERSONAL_DATA_ENTITY_TO_REQUEST[field]; + // null = known but not reportable on this endpoint (e.g. mail → ContactData) + if (path != null) paths.push(path); + } + + return paths; } diff --git a/src/subdomains/generic/kyc/dto/output/kyc-info.dto.ts b/src/subdomains/generic/kyc/dto/output/kyc-info.dto.ts index 1a5b7f6b86..a1d5239b76 100644 --- a/src/subdomains/generic/kyc/dto/output/kyc-info.dto.ts +++ b/src/subdomains/generic/kyc/dto/output/kyc-info.dto.ts @@ -74,7 +74,7 @@ export class KycStepSubmitDto extends KycStepBase { @ApiProperty({ description: - 'Request field paths still missing when complete is false; empty when complete is true. Financial: unanswered applicable question keys. Personal data: KycPersonalData paths (e.g. firstName, address.street, organizationAddress.city). mail is never listed on personal data (ContactData step).', + 'Request field paths still missing when complete is false; empty when complete is true. Financial: unanswered applicable question keys. Personal data: KycPersonalData paths (e.g. firstName, address.street, organizationAddress.city). The list may be empty while complete is false when the blocking field belongs to another KYC step (e.g. mail via ContactData).', type: String, isArray: true, }) diff --git a/src/subdomains/generic/kyc/entities/__tests__/kyc-step.entity.spec.ts b/src/subdomains/generic/kyc/entities/__tests__/kyc-step.entity.spec.ts new file mode 100644 index 0000000000..7f629178c5 --- /dev/null +++ b/src/subdomains/generic/kyc/entities/__tests__/kyc-step.entity.spec.ts @@ -0,0 +1,39 @@ +import { KycStepName } from '../../enums/kyc-step-name.enum'; +import { ReviewStatus } from '../../enums/review-status.enum'; +import { KycStep } from '../kyc-step.entity'; + +describe('KycStep', () => { + describe('update', () => { + it('does not clear status or sequenceNumber when they are omitted (undefined)', () => { + const step = Object.assign(new KycStep(), { + id: 1, + name: KycStepName.FINANCIAL_DATA, + status: ReviewStatus.IN_PROGRESS, + sequenceNumber: 7, + }); + + const [, update] = step.update(undefined, [{ key: 'tnc', value: 'accept' }]); + + expect(step.status).toBe(ReviewStatus.IN_PROGRESS); + expect(step.sequenceNumber).toBe(7); + expect(step.getResult()).toEqual([{ key: 'tnc', value: 'accept' }]); + expect(update).not.toHaveProperty('status'); + expect(update).not.toHaveProperty('sequenceNumber'); + expect(update).toHaveProperty('result'); + }); + + it('still applies an explicit status change', () => { + const step = Object.assign(new KycStep(), { + id: 1, + name: KycStepName.FINANCIAL_DATA, + status: ReviewStatus.IN_PROGRESS, + sequenceNumber: 2, + }); + + step.update(ReviewStatus.OUTDATED, undefined, undefined, 3); + + expect(step.status).toBe(ReviewStatus.OUTDATED); + expect(step.sequenceNumber).toBe(3); + }); + }); +}); diff --git a/src/subdomains/generic/kyc/entities/kyc-step.entity.ts b/src/subdomains/generic/kyc/entities/kyc-step.entity.ts index 8f96031946..3b911ff71a 100644 --- a/src/subdomains/generic/kyc/entities/kyc-step.entity.ts +++ b/src/subdomains/generic/kyc/entities/kyc-step.entity.ts @@ -219,12 +219,16 @@ export class KycStep extends IEntity { comment?: string, sequenceNumber?: number, ): UpdateResult { - const update: Partial = { - status, - result: this.setResult(result), - comment: this.addComment(comment), - sequenceNumber, - }; + // Drop undefined so callers can omit status/sequenceNumber without wiping in-memory or DB values + // (Object.assign copies undefined; TypeORM skips it on write, the entity would still lose the field). + const update = Object.fromEntries( + Object.entries({ + status, + result: this.setResult(result), + comment: this.addComment(comment), + sequenceNumber, + }).filter(([, v]) => v !== undefined), + ) as Partial; Object.assign(this, update); diff --git a/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts b/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts index 39eb82656d..8cd26004f0 100644 --- a/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts +++ b/src/subdomains/generic/kyc/services/__tests__/kyc.service.spec.ts @@ -866,4 +866,40 @@ describe('KycService submit completeness feedback', () => { expect(service['createStepLog']).toHaveBeenCalled(); }); }); + + describe('updateFileData', () => { + it('preserves sequenceNumber in the response when only status and result are updated', async () => { + // OWNER_DIRECTORY is in KycStepIdentRequiredForReview; with high kycLevel the step goes MANUAL_REVIEW + // without changing sequenceNumber — previously update() wiped sequenceNumber when omitted. + const kycStep = Object.assign(new KycStep(), { + id: 13, + name: KycStepName.OWNER_DIRECTORY, + status: ReviewStatus.IN_PROGRESS, + sequenceNumber: 3, + userData: createCustomUserData({ kycLevel: 50 }), + }); + const user = userWithPendingStep(kycStep); + jest.spyOn(service as any, 'getUser').mockResolvedValue(user); + + const documentService = createMock(); + documentService.uploadUserFile.mockResolvedValue({ url: 'https://example.com/file.pdf' } as never); + (service as any).documentService = documentService; + + const result = await service.updateFileData( + 'hash', + 13, + KycStepName.OWNER_DIRECTORY, + { + file: 'data:application/pdf;base64,eA==', + fileName: 'register.pdf', + } as any, + FileType.STOCK_REGISTER, + ); + + expect(kycStep.sequenceNumber).toBe(3); + expect(result.sequenceNumber).toBe(3); + expect(result.status).toBe(KycStepStatus.IN_REVIEW); + expect(kycStep.status).toBe(ReviewStatus.MANUAL_REVIEW); + }); + }); }); diff --git a/src/subdomains/generic/kyc/services/kyc.service.ts b/src/subdomains/generic/kyc/services/kyc.service.ts index e7a83a95a4..76ae1e599e 100644 --- a/src/subdomains/generic/kyc/services/kyc.service.ts +++ b/src/subdomains/generic/kyc/services/kyc.service.ts @@ -892,7 +892,7 @@ export class KycService { await this.verify2fa(user, ip); - await this.kycStepRepo.update(...kycStep.update(kycStep.status, data.responses, undefined, kycStep.sequenceNumber)); + await this.kycStepRepo.update(...kycStep.update(undefined, data.responses)); const missingFields = FinancialService.getMissingFields(data.responses, user.accountType); const complete = missingFields.length === 0; diff --git a/src/subdomains/generic/user/models/user-data/user-data.entity.ts b/src/subdomains/generic/user/models/user-data/user-data.entity.ts index 987ea2a594..1c569e614c 100644 --- a/src/subdomains/generic/user/models/user-data/user-data.entity.ts +++ b/src/subdomains/generic/user/models/user-data/user-data.entity.ts @@ -50,6 +50,32 @@ import { UserDataStatus, } from './user-data.enum'; +/** Base required KYC fields for every account type (entity property names). */ +export const PersonalRequiredKycFields = [ + 'accountType', + 'mail', + 'phone', + 'firstname', + 'surname', + 'street', + 'location', + 'zip', + 'country', +] as const; + +/** Extra required fields for non-personal accounts. */ +export const OrganizationRequiredKycFields = [ + 'organizationName', + 'organizationStreet', + 'organizationLocation', + 'organizationZip', + 'organizationCountry', +] as const; + +export type RequiredKycField = + | (typeof PersonalRequiredKycFields)[number] + | (typeof OrganizationRequiredKycFields)[number]; + @Entity() @Index( (userData: UserData) => [userData.identDocumentId, userData.nationality, userData.accountType, userData.kycType], @@ -856,15 +882,11 @@ export class UserData extends IEntity { } } - get requiredKycFields(): string[] { - return ['accountType', 'mail', 'phone', 'firstname', 'surname', 'street', 'location', 'zip', 'country'].concat( - this.isPersonalAccount - ? [] - : ['organizationName', 'organizationStreet', 'organizationLocation', 'organizationZip', 'organizationCountry'], - ); + get requiredKycFields(): RequiredKycField[] { + return [...PersonalRequiredKycFields, ...(this.isPersonalAccount ? [] : OrganizationRequiredKycFields)]; } - get missingKycFields(): string[] { + get missingKycFields(): RequiredKycField[] { return this.requiredKycFields.filter((f) => !this[f]); }