Release: develop -> main#3813
Merged
Merged
Conversation
* fix(kyc): never surface DfxApproval as a user-actionable step DfxApproval is a DFX-side decision the user can never act on. KycStep.create defaults a freshly initiated step to IN_PROGRESS, and a DfxApproval step in an actionable status (NOT_STARTED/IN_PROGRESS/...) made KycInfoMapper compute processStatus=InProgress and pick it as currentStep. The client then routed it into the user-actionable lane with no UI for it -> blank/dead-end screen (RealUnitCH/app#618). Introduce KycStepNonUserActionable = [DfxApproval] and, in KycInfoMapper: - never select a non-user-actionable step as currentStep; - classify it as PendingReview (awaiting DFX) even when its raw status is in the actionable set, so it can never drive processStatus into IN_PROGRESS. Adds focused KycInfoMapper specs (red->green): DfxApproval-only InProgress now reads PendingReview and is never currentStep; a real user-actionable step still yields InProgress; OnHold behaviour unchanged. * fix(kyc): also drop an explicitly-passed DfxApproval currentStep The Fixer<->Tester loop (Tester-Inspector) found the prior change only guarded the derived (`??=`) currentStep; a caller passing `currentStep: dfxApproval` explicitly (4th arg of toDto) would still emit it. Drop a non-user-actionable step regardless of how it arrives, and cover it with a durable session-DTO regression: toDto(userData, true, [], dfxApprovalStep) -> currentStep undefined.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
…3814) * feat(kyc): add context query param to KYC status endpoint Allow the KYC endpoint to return only the steps relevant to the caller's flow. RealUnit buy-only users see LEVEL_30 steps as required and get processStatus: Completed after Ident, skipping FINANCIAL_DATA. When they later sell, the sell context surfaces the full step set. * refactor(kyc): use DTO with class-validator for context query param Replace manual validateContext in service with KycQueryDto using @IsEnum + @IsOptional decorators. Framework-level validation via the global ValidationPipe. * test(kyc): add PendingReview test for RealunitBuy with IDENT in review
* feat: add Docker + SSH deploy for DFX server migration * chore: add .dockerignore for Docker build
…3804) * fix(account-merge): dedup merge-request mail per merge event Multiple code paths (ident verify + re-check, IBAN conflict) request the same logical merge. Each minted a new AccountMerge row with a fresh correlationId, so the 60s mail-layer debounce never matched and the user received several identical merge-request mails. Dedup at the sendMergeRequest entry: if an open (!isCompleted, !isExpired) merge for the {master, slave} pair already exists, return early and reuse it instead of sending another mail. Closes #3799 * test(account-merge): cover dedup branches + log reused merges Address review follow-ups on #3799: - Log a merge-reuse entry on the dedup early-return so support keeps the audit trail of which trigger reasons hit an already-open merge. - Add tests for the not-mergeable early-return, the isCompleted filter, and the sendToSlave receiver. * fix(account-merge): scope dedup reuse to a recency window Per review: an open merge request lives until expiration (30d for IDENT/IBAN), so the dedup would also swallow a deliberate re-initiation days later — the user would never get a fresh mail. Bound the reuse to requests touched within the last few minutes so only the near-simultaneous trigger burst is deduped. --------- Co-authored-by: Joshua Krüger <josua_krueger@gmx.de>
…inated locks) (#3811) Lock the DfxApproval mapper invariants with regression tests, no production behaviour change: - KYC-terminated user + FAILED DfxApproval -> processStatus = Failed (terminated wins). - non-terminated + FAILED DfxApproval -> PendingReview (never user-actionable IN_PROGRESS). - ManualReview / OnHold DfxApproval -> PendingReview (unchanged behaviour, now guarded). - explicitly-passed non-user-actionable currentStep is dropped (no blank-screen leak). Drops the logger.warn on the currentStep guard (per review: the guard working as designed is not a bug worth logging on every DfxApproval status hit).
…endpoint (#3807) * feat(account-merge): surface MergeProcessing state on KYC status endpoint After a user confirms a merge the backend keeps processing (re-parenting, KYC follow-up, level transition, mail), but the status endpoint could not distinguish 'merge in progress' from 'idle', so the app's polling timeout surfaced as 'Fehler beim Laden'. Add a nullable processingStartedAt marker to AccountMerge set when executeMerge begins; expose it as a new KycProcessStatus.MergeProcessing value (additive — old clients treat it as a no-op) computed by KycInfoMapper from a single accountMergeRepo lookup. Closes #3802 * fix(account-merge): clear processing marker when merge fails If mergeUserData threw, processingStartedAt stayed set with isCompleted=false, so isProcessing returned true until expiration (up to 30 days) and the status endpoint was stuck on MergeProcessing. Reset the marker on failure via stopProcessing() and rethrow. * fix(account-merge): bound processing state to a timeout so a crash can't strand the client isProcessing now also requires processingStartedAt within MERGE_PROCESSING_TIMEOUT_MINUTES (10 min), and hasProcessingMerge mirrors it. A merge runs in seconds; if a pod crashes between startProcessing and complete/stopProcessing, the marker self-heals instead of pinning the status endpoint on a waiting state until the merge request expires. Addresses review follow-ups on #3802 (stale-marker cleanup / reentrancy). --------- Co-authored-by: Joshua Krüger <josua_krueger@gmx.de>
…nted KYC steps (#3805) * fix(account-merge): re-parent slave KYC steps to master at DB level mergeUserData only concatenated the slave's kycSteps onto the master in-memory. The slave rows kept userData_id = slave.id, so on the next reload of the master they vanished and already-completed steps (e.g. FINANCIAL_DATA) were re-flagged as missing. Add KycAdminService.reassignKycSteps and call it after the slave-step update loop so the FK is persisted. Closes #3800 * fix(account-merge): backfill historical merged KYC steps + log reassignment Address review on #3800: - Add a data migration that re-parents KYC steps still pointing at merged-away (slave) accounts onto their surviving master, using the completed account_merge rows as the slave->master mapping (user_data has no master FK). Looped to resolve chained merges; idempotent. - Log the affected step count in reassignKycSteps for audit trails. * fix(account-merge): fix stale-collection save that orphans re-parented KYC steps Address David May's review: drop the redundant reassignKycSteps and fix the real root cause. updateUserData loads kycSteps+users and `save(userData)` makes TypeORM's OneToManySubjectBuilder reconcile those collections — so a master loaded with a pre-merge snapshot nulls out the slave steps a merge just re-parented (2083 orphaned kyc_step rows in prod). Persist scalars/FKs only by excluding the loaded OneToMany collections from the saved object; the returned entity keeps them for the HTTP response. - remove KycAdminService.reassignKycSteps (the concat + save(master) in mergeUserData already persists the re-parent) + its call + its spec - keep the backfill migration (pairs with the fix so no new orphans appear) - add regression test: updateUserData's save() excludes kycSteps/users * chore: remove backfill migration (executed manually) Backfill of 1510 orphaned kyc_step rows was run manually against prod. 573 remain from pre-account_merge era (Nov 2023) with no resolvable master — harmless on dead Merged accounts. --------- Co-authored-by: Joshua Krüger <josua_krueger@gmx.de> Co-authored-by: David May <david.leo.may@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 1 new commit(s)
Checklist