refactor(kyc): drive KycCubit routing from API processStatus, drop local sets#494
Conversation
…g queue Companion to the seven PRs that landed today (RealUnitCH#491, RealUnitCH#492, RealUnitCH#493, RealUnitCH#494, RealUnitCH#495, RealUnitCH#496, RealUnitCH#497 + DFXswiss/api#3732, #3733). Adds: - A 'Shipped (2026-05-21)' table mapping each pair-PR to the V-IDs it closes, with the W2 pair flagged as the closure of the 2026-05-21 ident-misroute incident. - An 'Outstanding — next phase' section grouping the remainders: P0 follow-ups (V6c/V6d/V16/V20), Wave 4 (V17/V18/V14/V19 — new backend modules), and the P1/P2 long tail (V21–V27, V29–V33). Each item carries the same V-ID anchor used elsewhere so PR reviewers can cross-reference cleanly. Does not touch V<N> classifications or counts — those stay as the reviewer set them. Pure append at the end of the file.
Blume1977
force-pushed
the
refactor/kyc-cubit-api-driven
branch
from
e5e4f51 to
5536c4f
Compare
Review-Findings (Wave 2 — KycCubit API-driven Routing)Blocker1. final pending = kycStatus.kycSteps.firstWhereOrNull(
(s) => s.isRequired && _mapStepName(s.name) != null,
);
final step = pending != null ? _mapStepName(pending.name) : null;
if (step != null) {
emit(KycPending(step));
} else if (pending != null) { // toter Zweig
emit(KycUnsupportedStepFailure(pending.name));
} else {
emit(const KycCompleted()); // misroute
}Wenn der Backend 2. Unbekannter default:
return KycProcessStatus.inProgress;Wenn der API später einen neuen Wert einführt (z.B. 3. V45 ( final currentStep = kycStatus.kycSteps.firstWhere((step) => step.isCurrent);Sowohl Empfehlungen4. Dead-Data: Routen passen 5. 6. Was solide ist
Geprüft und negativ: keine versteckten |
…le erweitern V45 ist mit der `_continueKyc`-Anpassung im selben PR geschlossen wie V1/V2/V3/V5 (`docs/api-authority-plan.md` §W2.2). Audit-Eintrag aktualisiert: Datei-Anker auf `_continueKyc` (statt der inzwischen ueberholten Zeilennummer 208) und Closed-by-Hinweis ergaenzt, dass der `StateError`-Pfad aus dem `firstWhere`-on-empty mit weg ist. W2-Zeile in der `Shipped (2026-05-21)`-Tabelle erweitert: V45 in die "Closes V-IDs"-Spalte aufgenommen, damit der Audit-Diff zur PR-RealUnitCH#494 deckungsgleich ist.
|
Habe die drei Blocker uebernommen (Commits Was gefixt istB1 —
B2 — Unbekannter B3 — V45 ( Docs-Update: Neue Tests
Verifikation
PR-Body sollte unter |
…cal sets Closes the 2026-05-21 ident-misroute report and Wave 2.2 of the API-as-Decision-Authority audit. Companion to API PR DFXswiss/api#3732 (`feat/kyc-decision-fields`). The cubit used to re-implement the backend's own routing rule locally: - `_requiredStepNames` set (duplicate of `requiredKycSteps(userData)`) - `_minLevelForActions = 30` threshold (duplicate of the level check) - `actionableStatuses` / `pendingStatuses` sets (duplicate of how the backend tags step status) - A 30-line iteration over `kycSteps` that derived `KycCompleted` / `KycPending` / `_continueKyc` from filter results. After PR #3732 the API returns those decisions directly. The cubit now renders them: - `KycLevelDto.processStatus` (new field, mirrors `KycProcessStatus`) drives the top-level switch — `Completed` → `KycCompleted`, `PendingReview` → `KycPending(currentStep)`, `InProgress` → `_continueKyc`, `Failed` → `KycFailure`. - `KycStepDto.isRequired` (new field) selects which step to surface in the pending case. - `UserKycDto.canTrade` is parsed from `/v2/user` for downstream callers (buy/sell flows can render it instead of guessing from `level >= 30`). - `KycPageManager` drops the `requiredLevel` parameter (and its router plumbing) — the cubit no longer needs a threshold; `canTrade` / `processStatus` speak directly to the buy/sell question. Backwards-compat: all three new fields default to safe values (`processStatus = inProgress`, `isRequired = false`, `canTrade = false`) when the API response omits them, so a pre-#3732 backend keeps the app functional — it just falls through to `_continueKyc` for every check, which matches the old behaviour on the unhappy path. Local session gates stay local: `_legalDisclaimerAccepted` / `_registrationSignProduced` remain per-cubit-instance security gates. They do not encode business routing — they enforce a per-session ceremony on this device before any signed call. Position in the flow unchanged. Tests: - `kyc_cubit_test.dart` rewritten to drive the cubit via API fixtures (`processStatus` + `isRequired`) instead of the old level-based setup. The previous level-based + step-iteration cases collapse to five fixtures: completed, pendingReview, inProgress (→ _continueKyc), failed, and the session-gate path. The 403/TFA_REQUIRED matrix, generation-token regression, sign-gate sequencing are preserved. Verification: - `flutter analyze` — clean - `flutter test` — **1412 / 1412 passing**
… mappen Wenn der Backend einen neuen `processStatus`-Wert (z.B. `OnHold`, `Suspended`) einfuehrt oder einen Wert fehlerhaft serialisiert, hat der Client das bisher stillschweigend auf `inProgress` gemappt und im Anschluss `_continueKyc` gefeuert. Das verletzt die Foundation-Regel aus RealUnitCH#491 ("API as Decision Authority" — niemals lokale Default-Entscheidung ueber Routing) und kollidiert mit dem `Keine Fallbacks`-Pattern: jeder unbekannte Wert haette latent eine Misroute produziert ohne dass es irgendwo aufgefallen waere. `KycProcessStatusExtension.fromValue` wirft jetzt `ArgumentError`, sobald ein unbekannter String auf der Leitung ankommt. Die Pre-#3732 Backwards-Compat fuer ein **fehlendes** Feld (`null` auf der Leitung) bleibt explizit in den DTO-Parsern erhalten — das ist die dokumentierte Migration, kein lokaler Routing-Default. Tests in `kyc_level_test.dart` erweitert: Round-Trip ueber alle Enum-Werte plus drei Negativ-Faelle (`OnHold`, `Suspended`, Leerstring).
Zwei zusammenhaengende Fixes im KycCubit-Routing, beide an Wave 2.2 (`docs/api-authority-plan.md` §W2.2, `docs/api-authority-audit.md` V45) gebunden. **B1 — `PendingReview` ohne mappbaren `isRequired`-Step.** Bisher hat der `pendingReview`-Zweig die Liste mit `s.isRequired && _mapStepName(s.name) != null` gefiltert. Wenn der Backend `processStatus: PendingReview` setzte, aber kein gemappter Step als `isRequired: true` markiert war (z.B. `additionalDocuments`, `personalData`, `residencePermit`, `statutes` — alle in `_mapStepName` nicht abgedeckt), fiel die App in den `else`-Arm und emittierte `KycCompleted` → Dashboard. Das ist exakt dieselbe Klasse Misroute wie der 2026-05-21-Incident, nur in die Gegenrichtung (API: Review pending → App: Completed). Zusaetzlich war der `else if (pending != null)`-Zweig durch den Mapping-Filter in der ersten Zeile toter Code. Der Filter ist jetzt strikt auf `s.isRequired` reduziert. Fehlt ein required-Step → `KycUnsupportedStepFailure(null)`; ist der required-Step nicht gemappt → `KycUnsupportedStepFailure(pending.name)`; gemapped → `KycPending(step)`. **Nie** `KycCompleted` in diesem Zweig. `KycUnsupportedStepFailure.stepName` wurde dafuer nullable gemacht, der `KycPageManager` faellt in dem Fall auf `'-'` als Platzhalter im i18n-Template zurueck. **B3 — V45: `_continueKyc` iterierte parallel ueber `kycSteps`.** `kycStatus.kycSteps.firstWhere((step) => step.isCurrent)` war die parallele Anti-Pattern-Stelle zu V5 — die `_runCheckKyc`-Schleife. Plan und Audit binden V45 explizit an Wave 2.2 (`docs/api-authority-plan.md` §W2.2 "Delete the same `kycSteps.firstWhere(step.isCurrent)` loop in `_continueKyc` — that's V45"; `docs/api-authority-audit.md:V45` "must also be deleted in the same PR"), in der ersten Iteration des PR war das aber nicht erledigt. Sekundaerer Bug: `firstWhere` ohne `orElse` wirft `StateError`, wenn kein Step `isCurrent: true` ist — der wird vom outer `catch` zu `KycFailure(e.toString())` umgeleitet, aber die user-sichtbare Message ist dann ein nackter Stack-Trace im `kycFailureDescription`-i18n-Template. `_continueKyc` liest jetzt direkt `KycSessionDto.currentStep` — das Feld ist bereits autoritativ (wird schon fuer `urlOrToken` benutzt) und mappt 1:1 auf die `KycInfoMapper.toDto`-Decision auf dem API. Fehlt `currentStep` → `KycUnsupportedStepFailure(null)` statt `StateError`. **Tests** in `kyc_cubit_test.dart`: - `PendingReview` + kein required Step ueberhaupt → `KycUnsupportedStepFailure(null)` - `PendingReview` + required Step ohne UI-Mapping (`additionalDocuments`) → `KycUnsupportedStepFailure(additionalDocuments)` - `_continueKyc` konsumiert `KycSessionDto.currentStep` statt zu iterieren (bestehender Test umgeschrieben, plus Negativ-Fall fuer fehlendes `currentStep`) - `urlOrToken` durchgereicht aus `currentStep.session.url`
…le erweitern V45 ist mit der `_continueKyc`-Anpassung im selben PR geschlossen wie V1/V2/V3/V5 (`docs/api-authority-plan.md` §W2.2). Audit-Eintrag aktualisiert: Datei-Anker auf `_continueKyc` (statt der inzwischen ueberholten Zeilennummer 208) und Closed-by-Hinweis ergaenzt, dass der `StateError`-Pfad aus dem `firstWhere`-on-empty mit weg ist. W2-Zeile in der `Shipped (2026-05-21)`-Tabelle erweitert: V45 in die "Closes V-IDs"-Spalte aufgenommen, damit der Audit-Diff zur PR-RealUnitCH#494 deckungsgleich ist.
KycPageManager wertet das per-pushNamed extra Argument nicht mehr aus, seit das Routing API-driven aus dem KycCubit kommt. Der requiredLevel-Pass in sell_button und payment_additional_action_needed_button war damit toter Argument-Transport. Beide Aufrufer raeumen das jetzt auf.
TaprootFreak
force-pushed
the
refactor/kyc-cubit-api-driven
branch
from
897ec71 to
a489a29
Compare
|
Rebase auf aktuelles develop ( Rebase: 4 Commits sauber on top of develop neu aufgesetzt, keine Konflikte (Konflikt-Hotspot Polish-Fix (Commit
Lokal verifiziert:
PR-Body zusätzlich aktualisiert: V5 (process-status-driven routing) und V45 ( Force-push war |
…ordering in-app (#499) (#530) ## Summary Reverts #499 ("refactor(reference-data): consume /v1/legal-document, /v1/company-info, country.displayOrder"). The companion API-side PR (DFXswiss/api#3734) has been **closed unmerged** — the decision is to keep legal-document URLs, company contact info and country ordering **client-side in the app** rather than serving them from the API. `/v1/legal-document` and `/v1/company-info` do not exist in the API, so the app owns this data. ## What this restores - `legal_documents_config.dart` — the hardcoded registration-agreement PDF URLs (`_registrationAgreementPdfUrls`, de/en) and the `pdfUrls`-based `LegalDocumentConfig`. - `settings_contact_page.dart` — the hardcoded RealUnit company contact tiles (phone, email, website, imprint). - `country_field.dart` — the hardcoded `['CH','DE','IT','FR']` priority ordering. ## What this removes - `DfxLegalDocumentService`, `DfxCompanyInfoService` and their DTOs (+ tests) — the API-consumption layer added by #499. - The `Country.displayOrder` field and the `/v1/legal-document` + `/v1/company-info` wiring. ## Not affected DFXswiss/api#3732 and #3733 stay merged in the API; their app companions (#494 KYC routing, #497 capabilities) are untouched. Only the reference-data refactor is reverted. ## Verification - `flutter analyze` — No issues found. - `flutter test` — all 1504 tests pass.
2 participants
Summary
Closes the 2026-05-21 ident-misroute report. Wave 2.2 of the API-as-Decision-Authority audit (
docs/api-authority-plan.md, foundation rule in #491). Companion to API PR DFXswiss/api#3732 (feat/kyc-decision-fields).The cubit used to re-implement the backend's own routing rule locally:
_requiredStepNamesset — duplicate ofrequiredKycSteps(userData)on the API_minLevelForActions = 30threshold — duplicate of the level checkactionableStatuses/pendingStatusessets — duplicate of how the backend tags step statuskycStepsthat derivedKycCompleted/KycPending/_continueKycfrom filter results_continueKycrepeated the same manual filter overkycStepsafter a realunit registration completed (parallel code path with the same anti-pattern)That setup is exactly what misroutes a high-level user when
checkDfxApprovalre-issues their Ident step on the backend (user_data 338759: kycLevel 53 + an Outdated Ident step + a sequence-1 Ident step inInProgress→ the app sends them back toKycIdentPageeven though they didn't ask to re-verify).After API PR #3732 the backend returns those decisions directly. The cubit now renders them.
Changes
Cubit logic
KycLevelDto.processStatus(new field, mirrorsKycProcessStatuson the API) drives the top-level switch:Completed→ emitKycCompletedPendingReview→ emitKycPending(<the required step in review>)InProgress→ call_continueKyc(unchanged routing semantics, new implementation)Failed→ emitKycFailureKycStepDto.isRequired(new field) selects which step to surface in the pending case.UserKycDto.canTradeis parsed from/v2/userfor downstream callers (Wave 3 will let buy/sell flows render it directly instead of guessing fromlevel >= 30)._continueKycnow readsKycSessionDto.currentStepdirectly instead of iterating overkycStepsto findisCurrent. A missingcurrentStepsurfacesKycUnsupportedStepFailure(null)instead of leaking a bareStateErrorstack-trace into the user-facing i18n message.KycPageManagerdrops therequiredLevelparameter and its router plumbing — the cubit no longer needs a threshold.canTrade/processStatusspeak directly to the buy/sell question.The cubit body shrinks from ~95 lines of iteration logic to a five-arm switch on
processStatus. The diff removes 6 of the 10 audit-flagged violations on this file in one go.What stays local
_legalDisclaimerAccepted/_registrationSignProducedremain per-cubit-instance security gates. They do not encode business routing — they enforce a per-session ceremony on this device before any signed call. Position in the flow is unchanged from before.DTOs
KycLevelDto+KycSessionDtocarryprocessStatus, default toinProgress.KycStepDtocarriesisRequired, defaultfalse.UserKycDtocarriescanTrade, defaultfalse.KycProcessStatusenum mirrors the API enum 1:1.Backwards compatibility
All three new fields default to safe values when the API response omits them:
processStatus = inProgress→ falls through to_continueKyc, identical to the legacy behaviour on the unhappy pathisRequired = false→KycPendingfalls back toKycCompleted(no required step found)canTrade = false→ downstream callers are conservativeA pre-#3732 backend keeps the app functional; an app build that consumes the new fields and a backend that hasn't shipped them yet doesn't crash.
Tests
kyc_cubit_test.dartrewritten to drive the cubit via API fixtures (processStatus+isRequired) instead of the old level-based setup. The previous level-based + step-iteration cases collapse to five fixtures:processStatus: Completed→ emitKycCompletedprocessStatus: PendingReview+ required ident inInReview→ emitKycPending(ident)processStatus: PendingReview+ required dfxApproval inInReview→ emitKycPending(dfxApproval)processStatus: InProgress→_continueKyc→ emitKycSuccess(currentStep)processStatus: Failed→ emitKycFailureThe 403/TFA_REQUIRED matrix, generation-token regression, and sign-gate sequencing are preserved.
Verification
flutter analyze— cleanflutter test— 1435 / 1435 passingManual test plan (post-merge of #3732)
DashboardifcanTrade: true, orKycPending(ident)only ifprocessStatus: PendingReview(no more spuriousKycIdentPagefrom a re-issued Ident step)._continueKycfor each required step.canTrade: true.Closes (from audit, P0)
_requiredStepNamesset_minLevelForActions = 30+level < _requiredLevelchecksactionableStatuses/pendingStatusessets and the iteration over kycStepskycSteps(process-status-driven routing replaces the entire next-step selection algorithm)_continueKyc's parallel iteration overkycSteps; the cubit now readsKycSessionDto.currentStepdirectlyTracked in
docs/api-authority-audit.md.Follow-ups
level < 10is still in the cubit; it's a Wave-3 candidate — the API could perform this server-side once the email is set.KycStep → KycStepNamemap (_mapStepName) is still local; if the API ever exposes auiHintfield it can go too.