Merge RB-20 — route cancel and delete through runSubmit, surface the error
CQ-002: ApplicationsStore.cancel and AdminCasesStore.delete reached the raw ApiClient and swallowed the failure in a bare catch, so a failed cancel made the row reappear with no message. Both now fold through runSubmit and expose lastError, which the two pages render. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> # Conflicts: # docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md # libs/shared/docs/behaviour-spec.mdx
This commit is contained in:
@@ -1,5 +1,6 @@
|
|||||||
import { TestBed } from '@angular/core/testing';
|
import { TestBed } from '@angular/core/testing';
|
||||||
import { describe, it, expect, vi } from 'vitest';
|
import { describe, it, expect, vi } from 'vitest';
|
||||||
|
import { SUBMIT_FAILED } from '@shared/application/submit';
|
||||||
import { ApplicationsAdapter } from '@registratie/infrastructure/applications.adapter';
|
import { ApplicationsAdapter } from '@registratie/infrastructure/applications.adapter';
|
||||||
import { AdminCasesStore } from './admin-cases.store';
|
import { AdminCasesStore } from './admin-cases.store';
|
||||||
|
|
||||||
@@ -43,7 +44,10 @@ describe('AdminCasesStore', () => {
|
|||||||
expect(s.tag === 'Success' && s.value.map((c) => c.id)).toEqual(['b']);
|
expect(s.tag === 'Success' && s.value.map((c) => c.id)).toEqual(['b']);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('rolls back the removal when the delete fails', async () => {
|
// RB-20: a failed delete must not be silent — the row rolls back AND the store
|
||||||
|
// surfaces the error the page renders. Before RB-20 this only rolled back
|
||||||
|
// (bare `catch { this.state.set(before) }`), so `lastError()` stayed null forever.
|
||||||
|
it('rolls back the removal and surfaces the error when the delete fails', async () => {
|
||||||
const deleteAny = vi.fn().mockRejectedValue(new Error('boom'));
|
const deleteAny = vi.fn().mockRejectedValue(new Error('boom'));
|
||||||
const store = setup({ listAll: () => Promise.resolve([summary('a')]), deleteAny });
|
const store = setup({ listAll: () => Promise.resolve([summary('a')]), deleteAny });
|
||||||
await store.load();
|
await store.load();
|
||||||
@@ -51,5 +55,24 @@ describe('AdminCasesStore', () => {
|
|||||||
await store.delete('a');
|
await store.delete('a');
|
||||||
const s = store.cases();
|
const s = store.cases();
|
||||||
expect(s.tag === 'Success' && s.value.map((c) => c.id)).toEqual(['a']); // reappears
|
expect(s.tag === 'Success' && s.value.map((c) => c.id)).toEqual(['a']); // reappears
|
||||||
|
expect(store.lastError()).toBe(SUBMIT_FAILED);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('clears a stale error on the next delete attempt', async () => {
|
||||||
|
const deleteAny = vi
|
||||||
|
.fn()
|
||||||
|
.mockRejectedValueOnce(new Error('boom'))
|
||||||
|
.mockResolvedValueOnce(undefined);
|
||||||
|
const store = setup({
|
||||||
|
listAll: () => Promise.resolve([summary('a'), summary('b')]),
|
||||||
|
deleteAny,
|
||||||
|
});
|
||||||
|
await store.load();
|
||||||
|
|
||||||
|
await store.delete('a');
|
||||||
|
expect(store.lastError()).toBe(SUBMIT_FAILED);
|
||||||
|
|
||||||
|
await store.delete('b');
|
||||||
|
expect(store.lastError()).toBeNull();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import { Injectable, inject, signal } from '@angular/core';
|
import { Injectable, inject, signal } from '@angular/core';
|
||||||
import { RemoteData } from '@shared/application/remote-data';
|
import { RemoteData } from '@shared/application/remote-data';
|
||||||
|
import { runSubmit, SUBMIT_FAILED } from '@shared/application/submit';
|
||||||
import { Aanvraag } from '@registratie/domain/aanvraag';
|
import { Aanvraag } from '@registratie/domain/aanvraag';
|
||||||
import {
|
import {
|
||||||
ApplicationsAdapter,
|
ApplicationsAdapter,
|
||||||
@@ -12,8 +13,9 @@ type Err = Error | undefined;
|
|||||||
* Admin view of ALL cases across owners (WP-36; `cases:manage`) — the back-office
|
* Admin view of ALL cases across owners (WP-36; `cases:manage`) — the back-office
|
||||||
* counterpart of the user-facing `ApplicationsStore`. Same shape: one root singleton
|
* counterpart of the user-facing `ApplicationsStore`. Same shape: one root singleton
|
||||||
* owns the list as a writable RemoteData signal, delete removes the row synchronously
|
* owns the list as a writable RemoteData signal, delete removes the row synchronously
|
||||||
* (optimistic) and rolls back on error. Admin delete removes any case (any owner,
|
* (optimistic), goes through `runSubmit`, and rolls back plus surfaces `lastError` on
|
||||||
* submitted or not — the server enforces the capability).
|
* failure (RB-20). Admin delete removes any case (any owner, submitted or not — the
|
||||||
|
* server enforces the capability).
|
||||||
*/
|
*/
|
||||||
@Injectable({ providedIn: 'root' })
|
@Injectable({ providedIn: 'root' })
|
||||||
export class AdminCasesStore {
|
export class AdminCasesStore {
|
||||||
@@ -22,6 +24,11 @@ export class AdminCasesStore {
|
|||||||
private state = signal<RemoteData<Err, Aanvraag[]>>({ tag: 'Loading' });
|
private state = signal<RemoteData<Err, Aanvraag[]>>({ tag: 'Loading' });
|
||||||
readonly cases = this.state.asReadonly();
|
readonly cases = this.state.asReadonly();
|
||||||
|
|
||||||
|
/** Set on a failed delete (RB-20): the optimistic removal already rolled back by
|
||||||
|
then, this is only the message for the alert the page renders above the list. */
|
||||||
|
private error = signal<string | null>(null);
|
||||||
|
readonly lastError = this.error.asReadonly();
|
||||||
|
|
||||||
/** Fetch + parse at the trust boundary, then publish as RemoteData. Keeps the
|
/** Fetch + parse at the trust boundary, then publish as RemoteData. Keeps the
|
||||||
last-good value on a resync (only shows Loading on the first load). */
|
last-good value on a resync (only shows Loading on the first load). */
|
||||||
async load() {
|
async load() {
|
||||||
@@ -42,16 +49,18 @@ export class AdminCasesStore {
|
|||||||
void this.load();
|
void this.load();
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Delete a case: drop it now (synchronous), then confirm the DELETE; roll back on error. */
|
/** Delete a case: drop it now (synchronous), then confirm the DELETE; roll back on error
|
||||||
|
AND surface it (RB-20) — a silent reappearance leaves the admin guessing why. */
|
||||||
async delete(id: string) {
|
async delete(id: string) {
|
||||||
const before = this.state();
|
const before = this.state();
|
||||||
if (before.tag === 'Success') {
|
if (before.tag === 'Success') {
|
||||||
this.state.set({ tag: 'Success', value: before.value.filter((a) => a.id !== id) });
|
this.state.set({ tag: 'Success', value: before.value.filter((a) => a.id !== id) });
|
||||||
}
|
}
|
||||||
try {
|
this.error.set(null);
|
||||||
await this.adapter.deleteAny(id);
|
const r = await runSubmit(() => this.adapter.deleteAny(id), SUBMIT_FAILED);
|
||||||
} catch {
|
if (!r.ok) {
|
||||||
this.state.set(before); // roll back: the row reappears
|
this.state.set(before); // roll back: the row reappears
|
||||||
|
this.error.set(r.error);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,77 @@
|
|||||||
|
import { TestBed } from '@angular/core/testing';
|
||||||
|
import { describe, it, expect, vi } from 'vitest';
|
||||||
|
import { SUBMIT_FAILED } from '@shared/application/submit';
|
||||||
|
import { ApplicationsAdapter } from '@registratie/infrastructure/applications.adapter';
|
||||||
|
import { ApplicationsStore } from './applications.store';
|
||||||
|
|
||||||
|
const summary = (id: string) => ({
|
||||||
|
id,
|
||||||
|
type: 'registratie',
|
||||||
|
status: { tag: 'Concept', stepIndex: 0, stepCount: 3 },
|
||||||
|
documentIds: [],
|
||||||
|
createdAt: '2026-07-23T10:00:00Z',
|
||||||
|
updatedAt: '2026-07-23T10:00:00Z',
|
||||||
|
});
|
||||||
|
|
||||||
|
function setup(adapter: Partial<ApplicationsAdapter>): ApplicationsStore {
|
||||||
|
TestBed.configureTestingModule({
|
||||||
|
providers: [{ provide: ApplicationsAdapter, useValue: adapter }],
|
||||||
|
});
|
||||||
|
// The store's own constructor kicks off `load()` (dashboard revisit refresh) —
|
||||||
|
// give every test a `list` so that initial call has something to resolve.
|
||||||
|
return TestBed.inject(ApplicationsStore);
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('ApplicationsStore', () => {
|
||||||
|
it('loads and parses the list', async () => {
|
||||||
|
const store = setup({ list: () => Promise.resolve([summary('a'), summary('b')]) });
|
||||||
|
await store.load();
|
||||||
|
const s = store.applications();
|
||||||
|
expect(s.tag).toBe('Success');
|
||||||
|
expect(s.tag === 'Success' && s.value.map((a) => a.id)).toEqual(['a', 'b']);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('cancels optimistically and confirms via the DELETE endpoint', async () => {
|
||||||
|
const cancel = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const store = setup({
|
||||||
|
list: () => Promise.resolve([summary('a'), summary('b')]),
|
||||||
|
cancel,
|
||||||
|
});
|
||||||
|
await store.load();
|
||||||
|
|
||||||
|
await store.cancel('a');
|
||||||
|
expect(cancel).toHaveBeenCalledWith('a');
|
||||||
|
const s = store.applications();
|
||||||
|
expect(s.tag === 'Success' && s.value.map((a) => a.id)).toEqual(['b']);
|
||||||
|
expect(store.lastError()).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
// RB-20: a failed cancel must not be silent — the row rolls back AND the store
|
||||||
|
// surfaces the error the page renders. Before RB-20 this only rolled back
|
||||||
|
// (bare `catch { this.state.set(before) }`), so `lastError()` stayed null forever.
|
||||||
|
it('rolls back the removal and surfaces the error when the cancel fails', async () => {
|
||||||
|
const cancel = vi.fn().mockRejectedValue(new Error('boom'));
|
||||||
|
const store = setup({ list: () => Promise.resolve([summary('a')]), cancel });
|
||||||
|
await store.load();
|
||||||
|
|
||||||
|
await store.cancel('a');
|
||||||
|
const s = store.applications();
|
||||||
|
expect(s.tag === 'Success' && s.value.map((a) => a.id)).toEqual(['a']); // reappears
|
||||||
|
expect(store.lastError()).toBe(SUBMIT_FAILED);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('clears a stale error on the next cancel attempt', async () => {
|
||||||
|
const cancel = vi
|
||||||
|
.fn()
|
||||||
|
.mockRejectedValueOnce(new Error('boom'))
|
||||||
|
.mockResolvedValueOnce(undefined);
|
||||||
|
const store = setup({ list: () => Promise.resolve([summary('a'), summary('b')]), cancel });
|
||||||
|
await store.load();
|
||||||
|
|
||||||
|
await store.cancel('a');
|
||||||
|
expect(store.lastError()).toBe(SUBMIT_FAILED);
|
||||||
|
|
||||||
|
await store.cancel('b');
|
||||||
|
expect(store.lastError()).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1,5 +1,6 @@
|
|||||||
import { Injectable, inject, signal } from '@angular/core';
|
import { Injectable, inject, signal } from '@angular/core';
|
||||||
import { RemoteData } from '@shared/application/remote-data';
|
import { RemoteData } from '@shared/application/remote-data';
|
||||||
|
import { runSubmit, SUBMIT_FAILED } from '@shared/application/submit';
|
||||||
import { Aanvraag } from '@registratie/domain/aanvraag';
|
import { Aanvraag } from '@registratie/domain/aanvraag';
|
||||||
import {
|
import {
|
||||||
ApplicationsAdapter,
|
ApplicationsAdapter,
|
||||||
@@ -15,7 +16,8 @@ type Err = Error | undefined;
|
|||||||
* the row SYNCHRONOUSLY, so the block disappears deterministically — no dependence on
|
* the row SYNCHRONOUSLY, so the block disappears deterministically — no dependence on
|
||||||
* change-detection timing, HTTP caching, or a resource `reload()`. `reload()` re-fetches
|
* change-detection timing, HTTP caching, or a resource `reload()`. `reload()` re-fetches
|
||||||
* so a page revisit reflects auto-approval (Concept → In behandeling → Goedgekeurd is
|
* so a page revisit reflects auto-approval (Concept → In behandeling → Goedgekeurd is
|
||||||
* computed server-side on read).
|
* computed server-side on read). Cancel goes through `runSubmit` and rolls back plus
|
||||||
|
* surfaces `lastError` on failure (RB-20).
|
||||||
*/
|
*/
|
||||||
@Injectable({ providedIn: 'root' })
|
@Injectable({ providedIn: 'root' })
|
||||||
export class ApplicationsStore {
|
export class ApplicationsStore {
|
||||||
@@ -24,6 +26,11 @@ export class ApplicationsStore {
|
|||||||
private state = signal<RemoteData<Err, Aanvraag[]>>({ tag: 'Loading' });
|
private state = signal<RemoteData<Err, Aanvraag[]>>({ tag: 'Loading' });
|
||||||
readonly applications = this.state.asReadonly();
|
readonly applications = this.state.asReadonly();
|
||||||
|
|
||||||
|
/** Set on a failed cancel (RB-20): the optimistic removal already rolled back by
|
||||||
|
then, this is only the message for the alert the page renders above the list. */
|
||||||
|
private error = signal<string | null>(null);
|
||||||
|
readonly lastError = this.error.asReadonly();
|
||||||
|
|
||||||
constructor() {
|
constructor() {
|
||||||
void this.load();
|
void this.load();
|
||||||
}
|
}
|
||||||
@@ -50,16 +57,19 @@ export class ApplicationsStore {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/** Cancel a Concept: drop it now (synchronous, guaranteed), then confirm the DELETE.
|
/** Cancel a Concept: drop it now (synchronous, guaranteed), then confirm the DELETE.
|
||||||
No resync — the delete succeeded, so the optimistic removal is authoritative. */
|
No resync — the delete succeeded, so the optimistic removal is authoritative. On
|
||||||
|
failure, roll back AND surface the error (RB-20) — a silent reappearance leaves the
|
||||||
|
user guessing why the block came back. */
|
||||||
async cancel(id: string) {
|
async cancel(id: string) {
|
||||||
const before = this.state();
|
const before = this.state();
|
||||||
if (before.tag === 'Success') {
|
if (before.tag === 'Success') {
|
||||||
this.state.set({ tag: 'Success', value: before.value.filter((a) => a.id !== id) });
|
this.state.set({ tag: 'Success', value: before.value.filter((a) => a.id !== id) });
|
||||||
}
|
}
|
||||||
try {
|
this.error.set(null);
|
||||||
await this.adapter.cancel(id);
|
const r = await runSubmit(() => this.adapter.cancel(id), SUBMIT_FAILED);
|
||||||
} catch {
|
if (!r.ok) {
|
||||||
this.state.set(before); // roll back: the block reappears
|
this.state.set(before); // roll back: the block reappears
|
||||||
|
this.error.set(r.error);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -42,6 +42,9 @@ import { AdminCasesStore } from '@registratie/application/admin-cases.store';
|
|||||||
} @else if (!canManage()) {
|
} @else if (!canManage()) {
|
||||||
<app-alert type="error">{{ deniedText }}</app-alert>
|
<app-alert type="error">{{ deniedText }}</app-alert>
|
||||||
} @else {
|
} @else {
|
||||||
|
@if (store.lastError(); as err) {
|
||||||
|
<app-alert type="error">{{ err }}</app-alert>
|
||||||
|
}
|
||||||
<app-async [data]="store.cases()">
|
<app-async [data]="store.cases()">
|
||||||
<ng-template appAsyncError>
|
<ng-template appAsyncError>
|
||||||
<app-alert type="error">{{ failedText }}</app-alert>
|
<app-alert type="error">{{ failedText }}</app-alert>
|
||||||
|
|||||||
@@ -51,6 +51,9 @@ import { tasksFromProfile } from '@registratie/domain/tasks';
|
|||||||
intro="Welkom in uw persoonlijke omgeving van het BIG-register. Hier ziet u uw registratie en regelt u uw zaken."
|
intro="Welkom in uw persoonlijke omgeving van het BIG-register. Hier ziet u uw registratie en regelt u uw zaken."
|
||||||
>
|
>
|
||||||
<div class="app-stack">
|
<div class="app-stack">
|
||||||
|
@if (cancelError(); as err) {
|
||||||
|
<app-alert type="error">{{ err }}</app-alert>
|
||||||
|
}
|
||||||
@if (aanvragen().length) {
|
@if (aanvragen().length) {
|
||||||
<section>
|
<section>
|
||||||
@for (a of concepten(); track a.id) {
|
@for (a of concepten(); track a.id) {
|
||||||
@@ -260,6 +263,8 @@ export class DashboardPage {
|
|||||||
protected cancelAanvraag(a: Aanvraag) {
|
protected cancelAanvraag(a: Aanvraag) {
|
||||||
void this.apps.cancel(a.id);
|
void this.apps.cancel(a.id);
|
||||||
}
|
}
|
||||||
|
/** RB-20: the message from a failed cancel, rendered above the list. */
|
||||||
|
protected cancelError = computed(() => this.apps.lastError());
|
||||||
|
|
||||||
/** Server-computed eligibility (rendered, not recomputed). */
|
/** Server-computed eligibility (rendered, not recomputed). */
|
||||||
private readonly eligible = computed(() => {
|
private readonly eligible = computed(() => {
|
||||||
|
|||||||
@@ -121,7 +121,7 @@ Every ticket tracing to a `BIO-` finding, plus every row on agent 07's authorita
|
|||||||
| **RB-17** | libs/shared/app + brief + beheer | CQRS-light | Split `runResult` (fold) from `runSubmit` (fold + idempotency mint); point the 5 reads at it | BL-007; §7 "read adapters 20 / mutations inline ~13" | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
|
| **RB-17** | libs/shared/app + brief + beheer | CQRS-light | Split `runResult` (fold) from `runSubmit` (fold + idempotency mint); point the 5 reads at it | BL-007; §7 "read adapters 20 / mutations inline ~13" | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
|
||||||
| **RB-18** | backend/Data | security | Key `IdempotencyStore` on `{SubjectId}:{idemKey}` | §7 stores "Not behind any port"; agent 02's Data note (no TTL, no reset) | S | Low | P2 | 3 | RB-17 | **SIGN-OFF** | **done** |
|
| **RB-18** | backend/Data | security | Key `IdempotencyStore` on `{SubjectId}:{idemKey}` | §7 stores "Not behind any port"; agent 02's Data note (no TTL, no reset) | S | Low | P2 | 3 | RB-17 | **SIGN-OFF** | **done** |
|
||||||
| **RB-19** | backend/Program.cs | structure | Reorder all 48 endpoints under read/write sub-banners; regroup admin-cases + org-template preview | BL-003 (940 lines, file CC 78 vs next-highest 27) | S | **High** | P2 | 4 | RB-12 | **SIGN-OFF** | open |
|
| **RB-19** | backend/Program.cs | structure | Reorder all 48 endpoints under read/write sub-banners; regroup admin-cases + org-template preview | BL-003 (940 lines, file CC 78 vs next-highest 27) | S | **High** | P2 | 4 | RB-12 | **SIGN-OFF** | open |
|
||||||
| **RB-20** | ssp/registratie | CQRS-light | `ApplicationsStore.cancel` / `AdminCasesStore.delete` through `runSubmit`; surface the error | BL-007; §7 "Command factories 3" | S | Low | P2 | 4 | — | **SIGN-OFF** | open |
|
| **RB-20** | ssp/registratie | CQRS-light | `ApplicationsStore.cancel` / `AdminCasesStore.delete` through `runSubmit`; surface the error | BL-007; §7 "Command factories 3" | S | Low | P2 | 4 | — | **SIGN-OFF** | **done** |
|
||||||
| **RB-21** | ssp/registratie | CQRS-light | Extract the read half of `createDraftSync` into `application/find-concept.ts` | §4a `createDraftSync` 143 lines — longest fn in the repo; §9 (>40) | M | Med | P2 | 4 | — | — | **done** |
|
| **RB-21** | ssp/registratie | CQRS-light | Extract the read half of `createDraftSync` into `application/find-concept.ts` | §4a `createDraftSync` 143 lines — longest fn in the repo; §9 (>40) | M | Med | P2 | 4 | — | — | **done** |
|
||||||
| **RB-22** | ssp/brief | CQRS-light | _(expand)_ `BriefStore.load()` tolerates a 404 by calling the existing `reset()` once | BL-003; §7 Backend CQRS-light row | S | Low | P2 | 4 | — | **SIGN-OFF** | open |
|
| **RB-22** | ssp/brief | CQRS-light | _(expand)_ `BriefStore.load()` tolerates a 404 by calling the existing `reset()` once | BL-003; §7 Backend CQRS-light row | S | Low | P2 | 4 | — | **SIGN-OFF** | open |
|
||||||
| **RB-23** | backend/Program.cs + Data | CQRS-light | _(contract)_ `GET /brief` 404s when absent; `GetOrCreate` → `Get` | BL-003; §7 Backend CQRS-light row | S | Med | P2 | 4 | RB-22 | **SIGN-OFF** | open |
|
| **RB-23** | backend/Program.cs + Data | CQRS-light | _(contract)_ `GET /brief` 404s when absent; `GetOrCreate` → `Get` | BL-003; §7 Backend CQRS-light row | S | Med | P2 | 4 | RB-22 | **SIGN-OFF** | open |
|
||||||
|
|||||||
@@ -0,0 +1,117 @@
|
|||||||
|
# RB-20 — route `ApplicationsStore.cancel` / `AdminCasesStore.delete` through `runSubmit`, surface the error
|
||||||
|
|
||||||
|
Status: **implemented** · 2026-08-27 · Source finding: `04-cqrs-light.md` CQ-002 ·
|
||||||
|
`00-baseline.md` BL-007 · `99-backlog.md` RB-20 · SIGN-OFF: consolidation approved
|
||||||
|
2026-08-27, HALT lifted
|
||||||
|
|
||||||
|
## What was wrong
|
||||||
|
|
||||||
|
`ApplicationsStore.cancel` and `AdminCasesStore.delete` both owned an optimistic write next
|
||||||
|
to their `RemoteData` read, and both reached `ApplicationsAdapter` directly instead of going
|
||||||
|
through `runSubmit` (the fold + Idempotency-Key mint every other mutation in the repo uses,
|
||||||
|
including `createSubmitChangeRequest` in the same folder). The failure path was a bare
|
||||||
|
`catch { this.state.set(before); }`: a failed cancel or delete rolled the row back, but the
|
||||||
|
user saw no message at all — no `ActionState`, no ProblemDetails `detail`, nothing. The
|
||||||
|
`Idempotency-Key` on the wire was also a fresh UUID per HTTP attempt (minted by
|
||||||
|
`api-client.provider.ts`'s default), not the per-logical-submit key `runSubmit` promises —
|
||||||
|
harmless today only because `Program.cs` happens to ignore the header outside the `Submit`
|
||||||
|
helper (CQ-005's note).
|
||||||
|
|
||||||
|
## What changed
|
||||||
|
|
||||||
|
CQ-002's option (a) — the smallest fix, applied identically to both stores. No new command
|
||||||
|
factory, no adapter split (CQ-002's own "Not filed" note reserves that split for option (b),
|
||||||
|
which this ticket does not take).
|
||||||
|
|
||||||
|
| File | Change |
|
||||||
|
| --------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||||
|
| `apps/ssp/src/app/registratie/application/applications.store.ts` | `cancel` now calls `runSubmit(() => this.adapter.cancel(id), SUBMIT_FAILED)`; added a private `error` signal, exposed read-only as `lastError`. On failure: roll back AND `this.error.set(r.error)`. On the next attempt, the error is cleared before the call so a stale message never survives a fresh action. |
|
||||||
|
| `apps/ssp/src/app/registratie/application/applications.store.spec.ts` (new) | 4 specs: load+parse, optimistic cancel, roll-back-and-surface-error on failure, stale-error-clears-on-next-attempt. No spec file existed for this store before RB-20. |
|
||||||
|
| `apps/ssp/src/app/registratie/application/admin-cases.store.ts` | Same shape as `applications.store.ts`: `delete` through `runSubmit`, `error`/`lastError` signal pair. |
|
||||||
|
| `apps/ssp/src/app/registratie/application/admin-cases.store.spec.ts` | Existing "rolls back … when the delete fails" spec extended to also assert `lastError()`; one new stale-error-clears spec added. |
|
||||||
|
| `apps/ssp/src/app/registratie/ui/dashboard.page.ts` | One `@if (cancelError(); as err) { <app-alert type="error">{{ err }}</app-alert> }` above the aanvragen list, mirroring `brief.page.ts`'s `lastError` rendering. `cancelError` is a `computed(() => this.apps.lastError())`. |
|
||||||
|
| `apps/ssp/src/app/registratie/ui/admin-cases.page.ts` | Same `@if (store.lastError(); as err) { <app-alert type="error">{{ err }}</app-alert> }`, placed above `<app-async>` inside the `canManage()` branch (`store` was already `protected`, so no new exposure needed). |
|
||||||
|
|
||||||
|
`applications.adapter.ts` (`cancel`, `deleteAny`) is **unchanged** — the fix is entirely in
|
||||||
|
the two stores, which now wrap the existing thin adapter calls in `runSubmit` at the call
|
||||||
|
site, exactly as `createSubmitChangeRequest` wraps `ChangeRequestAdapter.changeRequest`. The
|
||||||
|
adapter methods still return a bare `Promise<void>`; `runSubmit` is what folds that into a
|
||||||
|
`Result`.
|
||||||
|
|
||||||
|
Neither UI change introduces a new user-facing string: the rendered text is either the
|
||||||
|
existing `SUBMIT_FAILED` constant (`@@submit.failed`, already translated in
|
||||||
|
`messages.en.xlf` since RB-17) or, when the backend sends one, a ProblemDetails `detail`
|
||||||
|
string carried verbatim from the server — never a new `$localize` id. `messages.en.xlf` did
|
||||||
|
not need a new `<target>`.
|
||||||
|
|
||||||
|
## The tests, and their red failures
|
||||||
|
|
||||||
|
Both specs assert `store.lastError()` after a rejected adapter call, which only the fix can
|
||||||
|
satisfy — the old bare `catch { this.state.set(before) }` never touched an error signal, so
|
||||||
|
`lastError()` stayed `null` forever.
|
||||||
|
|
||||||
|
**Verified red without the fix** (an `Edit` undo of the store method, not `git checkout`, so
|
||||||
|
the rest of the change — imports, the other store, the UI, the specs — stayed in place):
|
||||||
|
|
||||||
|
- `applications.store.ts`: reverted `cancel` to `try { await this.adapter.cancel(id); } catch
|
||||||
|
{ this.state.set(before); }`. Reran `ng test ssp --include applications.store.spec.ts`:
|
||||||
|
2 of 4 failed —
|
||||||
|
`rolls back the removal and surfaces the error when the cancel fails` and
|
||||||
|
`clears a stale error on the next cancel attempt`, both with
|
||||||
|
`AssertionError: expected null to be 'Het indienen is niet gelukt. Probeer het later opnieuw.'`.
|
||||||
|
The other two specs (load, optimistic-cancel-success) stayed green, as expected — they
|
||||||
|
don't touch the error path. Re-applied the fix (`Edit` back to the `runSubmit` version);
|
||||||
|
reran: 4/4 green.
|
||||||
|
- `admin-cases.store.ts`: same procedure on `delete`. Reran
|
||||||
|
`ng test ssp --include admin-cases.store.spec.ts`: 2 of 4 failed with the identical
|
||||||
|
`expected null to be '...'` shape. Reverted to the fix; reran: 4/4 green.
|
||||||
|
|
||||||
|
## Judgement calls
|
||||||
|
|
||||||
|
- **Signal naming**: private backing field `error`, public readonly `lastError` — matching
|
||||||
|
the name `BriefStore`/`OrgTemplateStore` already expose for exactly this purpose (CQ-002's
|
||||||
|
own citation), rather than inventing a new name per store.
|
||||||
|
- **Error cleared at the start of each write**, not only on success, so a second cancel/delete
|
||||||
|
attempt after a failure doesn't leave a stale banner up if the retry itself is still in
|
||||||
|
flight. Covered by the "clears a stale error on the next attempt" spec in each file.
|
||||||
|
- **No `ActionState`/`SaveState` pair** (the fuller shape `BriefStore` uses for busy-state and
|
||||||
|
save-state together) — CQ-002 explicitly scoped option (a) to "one `error` signal", and
|
||||||
|
neither store needs a busy indicator: the row already disappears optimistically the instant
|
||||||
|
the click happens, so there is nothing for a spinner to cover.
|
||||||
|
- **UI placement**: one alert per page, above the list the mutated row belongs to, using the
|
||||||
|
same `@if (x(); as err) { <app-alert type="error">{{ err }}</app-alert> }` shape as
|
||||||
|
`brief.page.ts` — composition of an existing atom, no new building block (CLAUDE.md §2).
|
||||||
|
- **`applications.adapter.ts` left untouched, on purpose** — CQ-002's "Not filed" note ties
|
||||||
|
the read/write file split to option (b) only; taking option (a) means this ticket changes
|
||||||
|
no adapter code at all, matching the ticket's own framing ("(a) touches 2 files plus a UI
|
||||||
|
line each").
|
||||||
|
|
||||||
|
## Ticket accuracy
|
||||||
|
|
||||||
|
CQ-002's description matched the code as found: both stores' `cancel`/`delete` reached the
|
||||||
|
adapter directly with a bare `catch { this.state.set(before); }`, no `Result`, no error
|
||||||
|
channel — no discrepancy to flag.
|
||||||
|
|
||||||
|
## Residuals (not this ticket)
|
||||||
|
|
||||||
|
- RB-18 (key `IdempotencyStore` on `{SubjectId}:{idemKey}`) is unaffected: `cancel`/`delete`
|
||||||
|
now mint a key through `runSubmit` like every other mutation, so it lands on the same
|
||||||
|
write-only call set RB-18 already targets.
|
||||||
|
- RB-21 (extract `createDraftSync`'s read half) is a separate CQRS-light finding in the same
|
||||||
|
context, untouched by this ticket.
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
`npm run ci` (foreground, `timeout: 600000`): **green** — `✔ local CI passed`. Lint,
|
||||||
|
typecheck, `dep:check` (342 + 226 modules, 0 violations), `format:check`, `check:tokens`,
|
||||||
|
`check:seam`, tests (ssp 263/263 — 5 more than the pre-RB-20 258, from the new/extended
|
||||||
|
specs above — behandelportal 37/37, shared 138/138, beheer 23/23), `ng build --localize`
|
||||||
|
(both apps), `npm audit` (0 vulnerabilities), backend `dotnet format --verify-no-changes` +
|
||||||
|
`dotnet test --filter "Category!=Integration"` (260/260 — this filter is what keeps the
|
||||||
|
known `OpenZaakIntegrationTests.Admin_cases_…` container-dependent test, which needs a live
|
||||||
|
OpenZaak container, out of `npm run ci` entirely; it is a standing caveat, not introduced by
|
||||||
|
this change, and not exercised by this run), backend dependency audit (0 vulnerable
|
||||||
|
packages), `gen:snippets` / `gen:behaviour-spec` / `gen:api` drift checks all clean once the
|
||||||
|
regenerated `behaviour-spec.mdx` was staged alongside the code (the local gate's
|
||||||
|
`git diff --exit-code` compares the working tree to the index, so it is clean once the file
|
||||||
|
is staged — this is the documented pre-commit behaviour from RB-17's note, not a defect).
|
||||||
@@ -20,7 +20,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. 460 frontend behaviours across
|
**is** the suite, reshaped for a business reader. 465 frontend behaviours across
|
||||||
9 contexts; 237 backend behaviours across 41 test
|
9 contexts; 237 backend behaviours across 41 test
|
||||||
classes.
|
classes.
|
||||||
|
|
||||||
@@ -406,7 +406,15 @@ classes.
|
|||||||
|
|
||||||
- loads and parses the cross-owner list
|
- loads and parses the cross-owner list
|
||||||
- deletes optimistically and confirms via the admin endpoint
|
- deletes optimistically and confirms via the admin endpoint
|
||||||
- rolls back the removal when the delete fails
|
- rolls back the removal and surfaces the error when the delete fails
|
||||||
|
- clears a stale error on the next delete attempt
|
||||||
|
|
||||||
|
#### ApplicationsStore
|
||||||
|
|
||||||
|
- loads and parses the list
|
||||||
|
- cancels optimistically and confirms via the DELETE endpoint
|
||||||
|
- rolls back the removal and surfaces the error when the cancel fails
|
||||||
|
- clears a stale error on the next cancel attempt
|
||||||
|
|
||||||
#### STEPS (fixed)
|
#### STEPS (fixed)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user