From 4b94f8edb569ec66e7fb602c28cff385374cc902 Mon Sep 17 00:00:00 2001 From: Edwin van den Houdt Date: Wed, 26 Aug 2026 18:13:29 +0200 Subject: [PATCH] fix(flags): surface a failed admin toggle instead of swallowing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FeatureFlagStore.set() was try/finally with no catch. A rejected PUT /admin/flags/{key} escaped into the `void this.store.set(...)` call site as an unhandled promise rejection; the finally-block reload then snapped the control back to its old value. The admin saw a toggle that silently refused to move, with no error rendered anywhere and nothing in the state. set() now folds through the existing runSubmit helper and returns Result, reloading either way so the state still reflects the server. The page awaits it and renders the failure in an app-alert. Found by the CQRS-light pass (CQ-002/CQ-004) as one of three mutations that reach the raw ApiClient without producing a Result — the baseline's BL-007 inventory had missed all three. Co-Authored-By: Claude Opus 5 --- .../behandelportal/src/locale/messages.en.xlf | 4 ++++ apps/ssp/src/locale/messages.en.xlf | 4 ++++ libs/beheer/src/ui/feature-flags.page.ts | 14 ++++++++--- libs/shared/docs/behaviour-spec.mdx | 9 +++++++- .../src/application/feature-flags.store.ts | 23 +++++++++++++------ 5 files changed, 43 insertions(+), 11 deletions(-) diff --git a/apps/behandelportal/src/locale/messages.en.xlf b/apps/behandelportal/src/locale/messages.en.xlf index a4dddc2..6977b09 100644 --- a/apps/behandelportal/src/locale/messages.en.xlf +++ b/apps/behandelportal/src/locale/messages.en.xlf @@ -3878,6 +3878,10 @@ De functievlaggen konden niet worden geladen. The feature flags could not be loaded. + + De functievlag kon niet worden opgeslagen. + The feature flag could not be saved. + Opnieuw proberen Try again diff --git a/apps/ssp/src/locale/messages.en.xlf b/apps/ssp/src/locale/messages.en.xlf index 9860f45..fa514b7 100644 --- a/apps/ssp/src/locale/messages.en.xlf +++ b/apps/ssp/src/locale/messages.en.xlf @@ -3738,6 +3738,10 @@ De functievlaggen konden niet worden geladen. The feature flags could not be loaded. + + De functievlag kon niet worden opgeslagen. + The feature flag could not be saved. + Opnieuw proberen Try again diff --git a/libs/beheer/src/ui/feature-flags.page.ts b/libs/beheer/src/ui/feature-flags.page.ts index d97c404..f8db815 100644 --- a/libs/beheer/src/ui/feature-flags.page.ts +++ b/libs/beheer/src/ui/feature-flags.page.ts @@ -1,4 +1,4 @@ -import { Component, computed, inject } from '@angular/core'; +import { Component, computed, inject, signal } from '@angular/core'; import { PageShellComponent } from '@shared/layout/page-shell/page-shell.component'; import { AlertComponent } from '@shared/ui/alert/alert.component'; import { ButtonComponent } from '@shared/ui/button/button.component'; @@ -45,6 +45,9 @@ import { FeatureFlagStore } from '@shared/application/feature-flags.store'; } @else if (!canManage()) { {{ deniedText }} } @else { + @if (setError(); as e) { + {{ e }} + } {{ failedText }} @@ -89,8 +92,13 @@ export class FeatureFlagsPage { protected enableText = $localize`:@@flags.enable:Aanzetten`; protected disableText = $localize`:@@flags.disable:Uitzetten`; - protected toggle(key: string, enabled: boolean) { - void this.store.set(key, enabled); + /** The last failed toggle, so a rejected PUT is visible instead of silently snapping back. */ + protected setError = signal(null); + + protected async toggle(key: string, enabled: boolean) { + this.setError.set(null); + const r = await this.store.set(key, enabled); + if (!r.ok) this.setError.set(r.error); } protected reload() { void this.store.load(); diff --git a/libs/shared/docs/behaviour-spec.mdx b/libs/shared/docs/behaviour-spec.mdx index 1fce825..b0507b8 100644 --- a/libs/shared/docs/behaviour-spec.mdx +++ b/libs/shared/docs/behaviour-spec.mdx @@ -21,7 +21,7 @@ tested where._ Every bullet below is a real test name from the suite — an `it()` title (frontend) or a test method name (backend), read as a sentence. Nothing here is hand-written prose: this page **is** the suite, reshaped for a business reader. 402 frontend behaviours across -8 contexts; 217 backend behaviours across 36 test +8 contexts; 221 backend behaviours across 37 test classes. ## Frontend (by context) @@ -1049,6 +1049,13 @@ classes. - Proefbrief is admin only - Proefbrief renders the draft template with a watermark +### ProfessionsTests + +- A mapping is absent before its geldigVan +- A mapping is present on and after its geldigVan +- A closed mapping is absent from its geldigTot onwards +- ByProgram is evaluated per call not captured at type load + ### StamdataEndpointTests - Stamdata reads are admin only diff --git a/libs/shared/src/application/feature-flags.store.ts b/libs/shared/src/application/feature-flags.store.ts index 94806e2..5f57b5f 100644 --- a/libs/shared/src/application/feature-flags.store.ts +++ b/libs/shared/src/application/feature-flags.store.ts @@ -1,10 +1,14 @@ import { Injectable, computed, inject, signal } from '@angular/core'; import { RemoteData } from '@shared/application/remote-data'; +import { runSubmit } from '@shared/application/submit'; +import { Result, ok, err } from '@shared/kernel/fp'; import { FeatureFlag } from '@shared/domain/feature-flag'; import { FeatureFlagsAdapter, parseFlags } from '@shared/infrastructure/feature-flags.adapter'; type Err = Error | undefined; +const SET_FAILED = $localize`:@@flags.set.failed:De functievlag kon niet worden opgeslagen.`; + /** * Runtime feature-flag state (WP-47) — one root singleton, mirroring `AccessStore`. Loads the * resolved flag set once from `GET /flags`; `enabled(key)` gates a feature (deny-by-default: @@ -47,12 +51,17 @@ export class FeatureFlagStore { return rd.tag === 'Success' && (rd.value.find((f) => f.key === key)?.enabled ?? false); } - /** Admin toggle: persist then reload so the state reflects the server. */ - async set(key: string, enabled: boolean) { - try { - await this.adapter.set(key, enabled); - } finally { - await this.load(); - } + /** + * Admin toggle: persist, then reload so the state reflects the server either way. + * + * Returns the failure rather than throwing. The previous `try/finally` had no `catch`, + * so a rejected PUT escaped into the `void store.set(...)` call site as an unhandled + * rejection: the reload then snapped the toggle back to its old value and the admin saw + * a control that silently refused to move, with no error anywhere. + */ + async set(key: string, enabled: boolean): Promise> { + const r = await runSubmit(() => this.adapter.set(key, enabled), SET_FAILED); + await this.load(); + return r.ok ? ok(undefined) : err(r.error); } }