Skip to content

refactor(settings/contact): make Support tile unconditional#588

Merged
TaprootFreak merged 3 commits into
chore/post-580-followupsfrom
refactor/support-always-visible
May 26, 2026
Merged

refactor(settings/contact): make Support tile unconditional#588
TaprootFreak merged 3 commits into
chore/post-580-followupsfrom
refactor/support-always-visible

Conversation

@TaprootFreak
Copy link
Copy Markdown
Contributor

@TaprootFreak TaprootFreak commented May 26, 2026

Re-opens #584 (closed because its base `chore/post-541-followups` was deleted after the collection-merge #571). Cherry-picked Jana's original commit (`517170a`) onto the new collection branch `chore/post-580-followups`.

Summary

The "Support kontaktieren" tile under Settings → Kontakt must always be visible — including pre-signin onboarding flows where the user has not yet registered an email. Render it unconditionally and drop the surrounding cubit/state machinery that only existed to gate this single tile.

Pair PR

Pairs with DFXswiss/api#3761 (drops `UserCapabilitiesDto.supportAvailable`).

Merge order is unconstrained — both PRs are independent-safe:

Order Backend sends App reads Crash?
App-PR alone merged `supportAvailable: true/false` still in JSON Field no longer read in `fromJson` — Dart `Map<String,dynamic>` silently ignores unknown keys No
API-PR alone merged Field gone from JSON Old app code reads `json['supportAvailable'] as bool? ?? false` → `false` → tile stays hidden (existing bug persists) No
Both merged Backend stops sending, app stops reading No

The DTO uses `as bool? ?? false` (nullable cast with default fallback) for every capability flag, so neither side is brittle to the other's deploy timing. Earlier "Merge order: API first" claim was inaccurate.

Review history

Audited via subagent during the #584 cycle (clean, no MAJORs). Main-repo mirror was needed for Visual Regression on the dfx01 self-hosted runner (same Fork-PR pattern as #585).

Credit: code-diff by Jana Rüttimann (`Blume1977`).

The "Support kontaktieren" tile must always be visible — including pre-
signin onboarding flows where the user has not yet registered an email.
Render it unconditionally and drop the surrounding cubit/state machinery
which only existed to gate this single tile.

Changes
-------

- settings_contact_page.dart: BlocProvider / BlocBuilder dropped. The
  Support OutlinedTile renders as a plain child of the existing Column,
  ahead of Telefon / E-Mail / Website. Imports trimmed accordingly.
- Cubit + State files removed:
  - lib/screens/settings_contact/cubit/settings_contact_cubit.dart
  - lib/screens/settings_contact/cubit/settings_contact_state.dart
  - test/screens/settings_contact/settings_contact_cubit_test.dart
  - test/screens/settings_contact/cubit/settings_contact_state_test.dart
- UserCapabilitiesDto.supportAvailable removed from the local DTO mirror
  to match the API contract change (paired API PR drops the field from
  UserV2Dto.capabilities). The other four capability flags
  (canEdit{Name,Mail,Phone,Address}) stay — they still gate genuine
  business decisions driven by KYC step status.
- test/screens/transaction_receipt_settings_states_test.dart loses its
  $SettingsContactState group; the other Settings/Transaction-receipt
  state checks are preserved.
- user_dto_test.dart updated to expect four flags, not five.
- settings_contact_golden_test.dart simplified — no cubit/Provider
  wrapper needed; renders SettingsContactPage directly. Existing golden
  PNG already reflects the rendered tile, no regenerate required.

Pair PR
-------

Pairs with DFXswiss/api refactor(user/realunit): drop
UserCapabilitiesDto.supportAvailable. Merge order: API first, then app.
@TaprootFreak TaprootFreak marked this pull request as ready for review May 26, 2026 14:58
@TaprootFreak TaprootFreak merged commit 21d5000 into chore/post-580-followups May 26, 2026
6 checks passed
@TaprootFreak TaprootFreak deleted the refactor/support-always-visible branch May 26, 2026 15:20
TaprootFreak added a commit that referenced this pull request May 26, 2026
PR #588 made the Support tile unconditional on `SettingsContactPage` —
but pre-signin users with no email saw cryptic backend errors when they
tried to submit a ticket. The Support API (`POST /v1/support/issue` in
`DFXswiss/api`) rejects calls when `userData.mail == null` with
`BadRequestException('Mail is missing')`. The frontend now handles this
contract cleanly: the user is always offered ticket creation, but if no
email exists yet, they are routed through an email-capture step first.

## What changes

- `SupportPage` keeps both tiles ("Ticket erfassen", "Meine Tickets")
  always visible. Only the "Ticket erfassen" tap is mail-gated; "Meine
  Tickets" remains direct (an empty list is harmless).
- New `SupportPageCubit` decides per tap: load the user, then emit one
  of three one-shot side-effect states — `NavigateToCreate` (mail
  present), `NavigateToEmailThenCreate` (mail missing), or
  `NavigationFailure(message)`. The view performs the GoRouter push
  inside `listenWhen`-filtered branches and calls `acknowledge()` to
  reset to `Idle`. Reentry guard (`if state is Navigating return`)
  blocks accidental double-taps.
- New `SupportEmailCapturePage` + `SupportEmailCaptureCubit` — a
  stand-alone form (not coupled to `KycCubit`) that calls
  `RealUnitRegistrationService.registerEmail()` and pops with `true` on
  success. The view-listener on `SupportPage` chains the
  `createTicket` push only after `pop(true)`, with a `context.mounted`
  check.
- New route: `SupportRoutes.emailCapture` → `/support/email` (child of
  `support`, between `create` and `chat`).
- Three new i18n strings (`supportEmailCaptureTitle`,
  `…Description`, `…Continue`) in EN + DE; the existing
  `registerEmailRequired` / `registerEmailInvalid` are reused for
  field-level validation. Mail regex is identical to
  `kyc_email_page.dart`.

## Tests

42 new tests, all passing locally:

- `SupportPageCubit` — initial state, both navigation branches, both
  failure paths (`ApiException` surfaces `e.message`, generic throw
  surfaces `e.toString()`), `acknowledge()`, reentry guard verified
  with `Completer` so the second `requestCreateTicket()` is a no-op.
- `SupportEmailCaptureCubit` — submit success, `ApiException`,
  generic throw.
- State surface tests (`Equatable.props`) for both cubits.
- `SupportPage` widget — both tiles always rendered, both taps,
  `Navigating` disables onTap + shows activity indicator,
  `NavigateToCreate` pushes correctly + acknowledges,
  `NavigateToEmailThenCreate` awaits `pop(true)` then pushes
  createTicket (and does NOT push on `pop(null)`),
  `NavigationFailure` shows a SnackBar with `e.message`, and a
  page-unmount test that pins no crash + no acknowledge fires when
  the view is torn down while the email-capture pop is still pending.
- `SupportEmailCapturePage` widget — empty form render, submit-button
  enable/disable based on validity, Submitting/Success/Failure
  listener branches.
- Goldens: existing `support_page_default` regenerated with
  `BlocProvider` setup (tile bytes unchanged), two new captures for
  `support_email_capture_page_default` and `…_submitting`.
TaprootFreak added a commit that referenced this pull request May 28, 2026
…lessons (#593)

Companion to
[DFXswiss/api#3773](DFXswiss/api#3773). The
David-review back-and-forth on the Wave-3 reset produced a concrete set
of rules that should bind every future capability consumption in this
repo — this PR writes them down so the next contributor doesn't have to
rediscover them.

## Three docs updated

### 1. \`CONTRIBUTING.md\` — new sub-section "Consuming API capabilities
— eight rules"

Lives inside the existing "API as Decision Authority — CRITICAL"
section. Covers:

1. Read the capability shape, don't reconstruct it
2. Tile/button visibility for discoverable actions is unconditional
3. Map prerequisite types to UI components, not to business rules
4. Legacy backend tolerance — capability optional, sane fallback
5. No reactive 400-handling for what a capability could pre-tell
6. Pair-PR discipline
7. Tests pin the contract, not the implementation
8. Push back on capability shape that's over-engineered

Cross-references the API-side mirror in
[\`DFXswiss/api:CONTRIBUTING.md\`](https://github.com/DFXswiss/api/blob/develop/CONTRIBUTING.md).

### 2. \`docs/api-authority-plan.md\` — Wave-3 lessons-learned block

Documents the **six-PR** history of V9 (a single capability flag):

| PR | Direction | Outcome |
|---|---|---|
| [api#3733](DFXswiss/api#3733) | API:
\`+supportAvailable: bool\` | merged |
| [app#497](#497) | App:
consume \`supportAvailable\` bool | merged |
| [app#588](#588) | App:
unconditional Support tile | merged |
| [api#3761](DFXswiss/api#3761) | API:
\`-supportAvailable: bool\` | merged |
| [api#3767](DFXswiss/api#3767) | API:
ActionCapability tree (4 DTOs, 170 LOC) | **closed without merge** |
| [api#3772](DFXswiss/api#3772) | API:
\`createSupportTicket: { available, missingPrerequisite? }\` (91 LOC) |
merged |

Plus three "what we'd do differently" points and forward guidance for
Waves 4 and 5.

### 3. \`docs/api-authority-audit.md\` — V9 closed

Marked as closed by api#3772 with a link back to the lessons-learned
section (because the linear "closed by W3" mapping in the audit table
doesn't capture the actual non-linear history).

## Why now

Cyrill asked for these rules to be prominently documented in both repos
and in cross-repo working notes so the pattern doesn't drift on the next
capability. The API side gets the mirror PR
([DFXswiss/api#3773](DFXswiss/api#3773)).

## Verification

- Pure documentation — no code changes
- Markdown renders cleanly (verified)
- Cross-references resolve

## Companion PRs

- API rules:
[DFXswiss/api#3773](DFXswiss/api#3773)
- App pair-PR consuming the first capability following this pattern
(\`createSupportTicket\`): opening shortly on
\`refactor/consume-support-capability\`.
TaprootFreak added a commit that referenced this pull request May 28, 2026
)

Companion app PR to
[DFXswiss/api#3772](DFXswiss/api#3772) (merged
2026-05-26). **Closes V9** in
[\`docs/api-authority-audit.md\`](docs/api-authority-audit.md).

The Support tile now reads the new
\`user.capabilities.createSupportTicket\` field for its tap decision. No
more local \`mail != null\` reconstruction; the rule lives on the
backend, the app maps a typed enum value to a UI step.

## Architecture

Per the eight consumer rules in [\`CONTRIBUTING.md\`](CONTRIBUTING.md) →
"Consuming API capabilities — eight rules" (documented in
[#593](#593)):

| State | Tap action |
|---|---|
| \`capability == null\` (legacy backend pre-#3772) | Direct push to
Support — API is the authority |
| \`capability.available == true\` | Direct push to Support |
| \`capability.available == false, missingPrerequisite == email\` | Push
email capture page; on \`pop(true)\` re-init the cubit and push Support
if the refreshed capability is now available (or null — symmetric to
branch 1) |
| \`missingPrerequisite == unknown\` or \`null\` | Defensive direct push
— let the API render the error |

\`MissingPrerequisite\` is an **open enum** with \`email\` +
\`unknown\`. Additive backend values degrade to \`unknown\` so a future
prerequisite type never breaks \`/v2/user\` parsing for unrelated
callers (KYC, settings, etc.).

## What changes

### \`lib/\`
- \`packages/service/dfx/models/user/dto/user_dto.dart\` —
\`UserCapabilitiesDto.createSupportTicket\` optional field +
\`CreateSupportTicketCapabilityDto\` + open enum
\`MissingPrerequisite\`.
- \`screens/settings_contact/settings_contact_page.dart\` —
BlocProvider-wrapped; Support tile \`onTap\` dispatches through
\`_onSupportTap\` (4 branches above). Tile layout unchanged.
- \`screens/settings_contact/cubit/...\` — new \`SettingsContactCubit\`
+ state (\`part of\` pattern, States extend Equatable).
- \`screens/support/cubits/support_email_capture/...\` — new cubit +
state for the standalone email capture flow.
- \`screens/support/subpages/support_email_capture_page.dart\` —
standalone page (no KYC coupling) calling
\`RealUnitRegistrationService.registerEmail\`. \`mergeRequested\` status
surfaces a dedicated message — the multi-step verification flow is
deliberately not dragged into this minimal page.
- \`setup/routing/routes/support_routes.dart\` +
\`setup/routing/router_config.dart\` — new
\`SupportRoutes.emailCapture\` under \`/support/email\`.
- \`assets/languages/strings_{en,de}.arb\` — 4 new keys (alphabetically
sorted, both languages).
- \`lib/generated/i18n.dart\` — regenerated via \`dart run
tool/generate_localization.dart\`.

### \`test/\`

| File | Tests |
|---|---|
| \`packages/service/dfx/models/user/dto/user_dto_test.dart\` | +14
cases for createSupportTicket parsing, incl. \`unknown\` degradation and
JSON-null handling |
| \`screens/settings_contact/cubit/settings_contact_cubit_test.dart\` |
6 cases |
| \`screens/settings_contact/cubit/settings_contact_state_test.dart\` |
8 cases |
| \`screens/settings_contact/settings_contact_page_test.dart\` | 15
widget tests covering tile visibility + 11 routing branches incl.
\`unknown\` and pop(null\|false) |
|
\`screens/support/cubits/support_email_capture/support_email_capture_cubit_test.dart\`
| 5 cases (success, mergeRequested, ApiException, generic throw) |
|
\`screens/support/cubits/support_email_capture/support_email_capture_state_test.dart\`
| 7 Equatable cases |
| \`screens/support/subpages/support_email_capture_page_test.dart\` | 9
widget tests |
| \`goldens/screens/support/support_email_capture_golden_test.dart\` | 2
goldens (default + submitting) |
| \`goldens/screens/settings_contact/settings_contact_golden_test.dart\`
| Re-baselined for BlocProvider wrap; visual surface unchanged |

## Local verification

- \`dart format\` clean on all touched files
- \`dart analyze lib/ test/\` — no issues found
- \`flutter test\` on touched scope → **140/140 green**

## Review history

Implemented + reviewed via internal subagent loop, three iterations:
1. First implementation off \`develop\` → had to rebase onto
\`chore/post-580-followups\` (PR #588 base mismatch).
2. Reviewer found 2 SHOULD-FIX (Branch-1 asymmetry with \`?? false\`
violating the no-fallback rule; \`MissingPrerequisite.fromString\` throw
breaking unrelated \`/v2/user\` callers on additive backend changes).
Both addressed: explicit null-check symmetry, open-enum \`unknown\`
degradation.
3. Final reviewer pass found a \`dart format\` issue on the enum block —
fixed; added an explicit \`unknown\`-routing widget test as a
NICE-TO-HAVE.

## Targeting \`chore/post-580-followups\`

Per request — this is a post-#580 follow-up that consumes a new API
capability rather than introducing one in isolation. PR base set
accordingly.

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants