fix(flags): surface a failed admin toggle instead of swallowing it
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<string, void>, 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 <noreply@anthropic.com>
This commit is contained in:
@@ -3878,6 +3878,10 @@
|
|||||||
<source>De functievlaggen konden niet worden geladen.</source>
|
<source>De functievlaggen konden niet worden geladen.</source>
|
||||||
<target datatype="html">The feature flags could not be loaded.</target>
|
<target datatype="html">The feature flags could not be loaded.</target>
|
||||||
</trans-unit>
|
</trans-unit>
|
||||||
|
<trans-unit id="flags.set.failed" datatype="html">
|
||||||
|
<source>De functievlag kon niet worden opgeslagen.</source>
|
||||||
|
<target datatype="html">The feature flag could not be saved.</target>
|
||||||
|
</trans-unit>
|
||||||
<trans-unit id="flags.retry" datatype="html">
|
<trans-unit id="flags.retry" datatype="html">
|
||||||
<source>Opnieuw proberen</source>
|
<source>Opnieuw proberen</source>
|
||||||
<target datatype="html">Try again</target>
|
<target datatype="html">Try again</target>
|
||||||
|
|||||||
@@ -3738,6 +3738,10 @@
|
|||||||
<source>De functievlaggen konden niet worden geladen.</source>
|
<source>De functievlaggen konden niet worden geladen.</source>
|
||||||
<target datatype="html">The feature flags could not be loaded.</target>
|
<target datatype="html">The feature flags could not be loaded.</target>
|
||||||
</trans-unit>
|
</trans-unit>
|
||||||
|
<trans-unit id="flags.set.failed" datatype="html">
|
||||||
|
<source>De functievlag kon niet worden opgeslagen.</source>
|
||||||
|
<target datatype="html">The feature flag could not be saved.</target>
|
||||||
|
</trans-unit>
|
||||||
<trans-unit id="flags.retry" datatype="html">
|
<trans-unit id="flags.retry" datatype="html">
|
||||||
<source>Opnieuw proberen</source>
|
<source>Opnieuw proberen</source>
|
||||||
<target datatype="html">Try again</target>
|
<target datatype="html">Try again</target>
|
||||||
|
|||||||
@@ -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 { PageShellComponent } from '@shared/layout/page-shell/page-shell.component';
|
||||||
import { AlertComponent } from '@shared/ui/alert/alert.component';
|
import { AlertComponent } from '@shared/ui/alert/alert.component';
|
||||||
import { ButtonComponent } from '@shared/ui/button/button.component';
|
import { ButtonComponent } from '@shared/ui/button/button.component';
|
||||||
@@ -45,6 +45,9 @@ import { FeatureFlagStore } from '@shared/application/feature-flags.store';
|
|||||||
} @else if (!canManage()) {
|
} @else if (!canManage()) {
|
||||||
<app-alert type="error">{{ deniedText }}</app-alert>
|
<app-alert type="error">{{ deniedText }}</app-alert>
|
||||||
} @else {
|
} @else {
|
||||||
|
@if (setError(); as e) {
|
||||||
|
<app-alert type="error">{{ e }}</app-alert>
|
||||||
|
}
|
||||||
<app-async [data]="store.flags()">
|
<app-async [data]="store.flags()">
|
||||||
<ng-template appAsyncError>
|
<ng-template appAsyncError>
|
||||||
<app-alert type="error">{{ failedText }}</app-alert>
|
<app-alert type="error">{{ failedText }}</app-alert>
|
||||||
@@ -89,8 +92,13 @@ export class FeatureFlagsPage {
|
|||||||
protected enableText = $localize`:@@flags.enable:Aanzetten`;
|
protected enableText = $localize`:@@flags.enable:Aanzetten`;
|
||||||
protected disableText = $localize`:@@flags.disable:Uitzetten`;
|
protected disableText = $localize`:@@flags.disable:Uitzetten`;
|
||||||
|
|
||||||
protected toggle(key: string, enabled: boolean) {
|
/** The last failed toggle, so a rejected PUT is visible instead of silently snapping back. */
|
||||||
void this.store.set(key, enabled);
|
protected setError = signal<string | null>(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() {
|
protected reload() {
|
||||||
void this.store.load();
|
void this.store.load();
|
||||||
|
|||||||
@@ -21,7 +21,7 @@ tested where._
|
|||||||
Every bullet below is a real test name from the suite — an `it()` title (frontend) or a test
|
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
|
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
|
**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.
|
classes.
|
||||||
|
|
||||||
## Frontend (by context)
|
## Frontend (by context)
|
||||||
@@ -1049,6 +1049,13 @@ classes.
|
|||||||
- Proefbrief is admin only
|
- Proefbrief is admin only
|
||||||
- Proefbrief renders the draft template with a watermark
|
- 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
|
### StamdataEndpointTests
|
||||||
|
|
||||||
- Stamdata reads are admin only
|
- Stamdata reads are admin only
|
||||||
|
|||||||
@@ -1,10 +1,14 @@
|
|||||||
import { Injectable, computed, inject, signal } from '@angular/core';
|
import { Injectable, computed, inject, signal } from '@angular/core';
|
||||||
import { RemoteData } from '@shared/application/remote-data';
|
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 { FeatureFlag } from '@shared/domain/feature-flag';
|
||||||
import { FeatureFlagsAdapter, parseFlags } from '@shared/infrastructure/feature-flags.adapter';
|
import { FeatureFlagsAdapter, parseFlags } from '@shared/infrastructure/feature-flags.adapter';
|
||||||
|
|
||||||
type Err = Error | undefined;
|
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
|
* 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:
|
* 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);
|
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) {
|
* Admin toggle: persist, then reload so the state reflects the server either way.
|
||||||
try {
|
*
|
||||||
await this.adapter.set(key, enabled);
|
* Returns the failure rather than throwing. The previous `try/finally` had no `catch`,
|
||||||
} finally {
|
* so a rejected PUT escaped into the `void store.set(...)` call site as an unhandled
|
||||||
await this.load();
|
* 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<Result<string, void>> {
|
||||||
|
const r = await runSubmit(() => this.adapter.set(key, enabled), SET_FAILED);
|
||||||
|
await this.load();
|
||||||
|
return r.ok ? ok(undefined) : err(r.error);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user