mirror of
https://github.com/azaion/ui.git
synced 2026-06-21 10:41:10 +00:00
[AZ-512] Admin class inline edit form + PATCH wiring (cy4 batch 16)
Implements the cycle-3-deferred AZ-512 task under the user-authorized
Option B path (MSW-stubbed; live deploy gates at Step 16 on AZ-513).
Code:
- src/features/admin/AdminPage.tsx — inline edit affordance:
editingId/editForm state, handleStartEdit/Cancel/Update, Enter+Escape
keyboard handling, colspan row swap when editing, pencil (✎) button
per row. Full-body PATCH (Risk 2). Single editingId enforces the
one-row-at-a-time invariant (Risk 3). Disabled buttons during the
in-flight PATCH (Risk 4). Inline role="alert" on validation/server
errors (no alert() per Finding B4 anti-pattern).
- src/i18n/{en,ua}.json — `admin.classes` flat → nested with `title`
+ 6 new keys (edit, save, cancel, nameRequired,
maxSizeMustBePositive, updateFailed). Parity gate FT-P-22 PASS.
Test infrastructure:
- tests/msw/handlers/admin.ts — PATCH /api/admin/classes/:id
partial-merge handler.
- tests/admin_class_edit.test.tsx — 12 tests covering AC-1..AC-6
+ AC-8 (AC-7 satisfied by static FT-P-22 gate).
- tests/destructive_ux.test.tsx — adjacent-hygiene selector fix
at 3 call sites: the new ✎ button moved the first-button
position; targeting × explicitly preserves the existing
it.fails()/control semantics.
Docs:
- _docs/02_document/components/08_admin/description.md — recorded
edit affordance + PATCH wiring + AZ-513 cross-workspace note.
- _docs/03_implementation/batch_16_cycle4_report.md
- _docs/03_implementation/implementation_report_admin_class_edit_cycle4.md
- _docs/02_tasks/todo → done — AZ-512 archived.
Quality gates: 32 files / 243 tests / 13 quarantined skips PASS;
all 35 static checks PASS (FT-P-22/23, STC-ARCH-01/02, STC-SEC*,
banned-deps incl. SEC1B/C/D).
Cross-workspace dependency: admin/ AZ-513 (POST + PATCH + DELETE
/classes routes) NOT yet shipped. Step 11 (Run Tests) passes on
stubs; Step 16 (Deploy) holds until AZ-513 lands live. Leftover
record at _docs/_process_leftovers/2026-05-13_az-512-admin-
classes-prereq.md stays open.
Discovered pre-existing bug (NOT bundled): tests/msw/handlers/
admin.ts returns paginate(seedUsers) for GET /api/admin/users,
but AdminPage consumes as flat User[] → users.map crash. Test
files use the same flat-array workaround
destructive_ux.test.tsx documented. Flagged in batch + impl
reports for separate triage.
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -0,0 +1,186 @@
|
||||
# Admin: edit existing detection class (inline form + PATCH wiring)
|
||||
|
||||
> **STATUS (2026-05-13, cycle 4 close)**: **DONE in UI** via user-authorized **Option B** path. Implementation lives in cycle 4 batch 16 — see `_docs/03_implementation/batch_16_cycle4_report.md` and `_docs/03_implementation/implementation_report_admin_class_edit_cycle4.md`. 12 vitest tests pass (8/8 ACs covered); all static gates pass. **Live deploy gates at Step 16 on AZ-513** (admin/ workspace must ship `POST | PATCH | DELETE /classes` and deploy before UI prod cutover). Leftover record `_docs/_process_leftovers/2026-05-13_az-512-admin-classes-prereq.md` stays open until that point.
|
||||
|
||||
**Task**: AZ-512_admin_edit_detection_class
|
||||
**Name**: Admin — edit existing detection class
|
||||
**Description**: Re-introduce the "edit detection class" affordance the WPF→React port lost. Wire an inline edit form on each Detection Class row in the Admin page, calling `PATCH /api/admin/classes/{id}` with the editable fields, refreshing classes via the existing read endpoint. Closes Architecture Vision principle **P12** ("admin can edit existing detection classes — add + edit + delete is the full CRUD surface").
|
||||
**Complexity**: 3 points
|
||||
**Dependencies**: None in the UI workspace. Cross-workspace hard prerequisite: `admin/` sibling service must expose `PATCH /api/admin/classes/{id}` — verification step BLOCKS implementation if absent (see Risks).
|
||||
**Component**: 08_admin (primary)
|
||||
**Tracker**: AZ-512
|
||||
**Epic**: AZ-509
|
||||
|
||||
## Problem
|
||||
|
||||
`AdminPage.tsx` today supports only two of the three CRUD operations for detection classes:
|
||||
|
||||
- **Add** — `handleAddClass` POSTs `endpoints.admin.classes()` with `{ name, shortName, color, maxSizeM }`.
|
||||
- **Delete** — `handleDeleteClass(id)` DELETEs `endpoints.admin.class(id)`.
|
||||
- **Edit** — **missing**. Operators wanting to fix a typo in a class name, recolour a class, or adjust its `maxSizeM` must delete the class (orphaning every detection that references it) and recreate it. That's a destructive workaround for a routine maintenance action.
|
||||
|
||||
This was confirmed as a user-visible gap during Step 4.5 (Architecture Vision finalisation, 2026-05-10): Vision principle **P12** was elevated to a binding constraint expressly because the verification log (`_docs/02_document/04_verification_log.md` F10) showed the modern UI was a regression vs the legacy WPF page, which supported in-place edit. The principle has been on the books since but no cycle has scheduled the work.
|
||||
|
||||
The endpoint builder `endpoints.admin.class(id)` already exists (used today by DELETE) and matches the conventional PATCH target for an item-by-id mutation. The `api.patch()` helper exists in `api/client.ts`. The piece that doesn't exist (or isn't verified to exist) is the backend route handler.
|
||||
|
||||
## Outcome
|
||||
|
||||
- An admin user looking at the Detection Classes table can click any row (or a per-row pencil affordance) and see the row swap to an inline edit form populated with the current values.
|
||||
- Edits to `name`, `shortName`, `color`, and `maxSizeM` are sent via `PATCH /api/admin/classes/{id}`; on 200 the row re-renders with the updated values; on 4xx/5xx an inline error message appears next to the form.
|
||||
- A Cancel button on the form discards local edits and reverts the row.
|
||||
- Validation: `name` is required; `maxSizeM` is a positive number; `color` is a hex string from the standard color input.
|
||||
- All new user-visible strings are added to both `en.json` and `ua.json` per principle P6.
|
||||
- Closes P12. `_docs/02_document/04_verification_log.md` F10 moves to RESOLVED.
|
||||
- No regression in add or delete; no change to the rest of the Admin page (users, aircrafts, AI/GPS settings).
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
|
||||
- `src/features/admin/AdminPage.tsx`:
|
||||
- Add `editingId: number | null` and `editForm: { name, shortName, color, maxSizeM }` state.
|
||||
- Add row-click (or pencil-icon click) handler that sets `editingId` and seeds `editForm` from the current row.
|
||||
- Replace the read-only row markup with the editable form markup when `c.id === editingId`.
|
||||
- Add `handleUpdateClass()` that calls `api.patch(endpoints.admin.class(c.id), editForm)`, on success re-fetches classes from `endpoints.annotations.classes()` (mirrors `handleAddClass`'s refresh pattern), clears `editingId`, surfaces errors inline (no `alert()`).
|
||||
- Add `handleCancelEdit()` that clears `editingId` and `editForm`.
|
||||
- Wire keyboard convenience: `Enter` in the form submits; `Escape` cancels.
|
||||
- New i18n strings in `en.json` + `ua.json` under `admin.classes.*`: `edit` (button/title), `save`, `cancel`, `nameRequired`, `maxSizeMustBePositive`, `updateFailed`.
|
||||
- Update `_docs/02_document/components/08_admin/description.md` to record the new affordance (one paragraph in the relevant section).
|
||||
|
||||
### Excluded
|
||||
|
||||
- Fixing the missing ConfirmDialog on class **DELETE** (Finding B4 — separate task; do NOT bundle even though the same file is being touched. Scope discipline.).
|
||||
- Editing `photoMode` for an existing class — `photoMode` is a class-creation property today; mutating it after creation has cross-detection implications (`yoloId = classId + photoModeOffset`) that need backend rules; out of scope.
|
||||
- Bulk edit / multi-select edit — single-row edit only.
|
||||
- Renaming the underlying API endpoint or changing its wire shape.
|
||||
- Adding edit affordances to **users** or **aircrafts** in this page — separate concerns.
|
||||
- Refactoring `AdminPage.tsx` to extract per-section components — Step 8 refactor candidate, not this task.
|
||||
|
||||
## Cross-Workspace Verification (BLOCKING gate)
|
||||
|
||||
Before implementing the form, the implementer MUST verify the backend endpoint exists:
|
||||
|
||||
1. Read `../admin/` source (or the service's OpenAPI/Swagger surface) to confirm `PATCH /api/admin/classes/{id}` is routed and accepts `{ name?, shortName?, color?, maxSizeM? }`.
|
||||
2. If the endpoint exists → proceed with implementation per the AC below.
|
||||
3. If the endpoint is missing → **STOP**. Surface to the user via Choose A/B/C/D:
|
||||
- **A**: File a hard-prerequisite ticket on the `admin/` workspace, pause AZ-512 until that lands.
|
||||
- **B**: Implement only the UI form, mock-stubbed against MSW in tests, mark the cycle's Step 11 (Run Tests) as "blocked on admin/ PATCH" and ship a draft PR for review.
|
||||
- **C**: Drop AZ-512 from cycle 3, defer to a future cycle once `admin/` work is scheduled.
|
||||
|
||||
Do not invent a workaround that bypasses the missing endpoint.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: Edit affordance is visible on every class row**
|
||||
Given the Admin page is loaded for an admin user
|
||||
When the Detection Classes table renders
|
||||
Then each row displays an edit affordance (pencil icon or click-to-edit cue) alongside the existing delete affordance.
|
||||
|
||||
**AC-2: Clicking edit opens the inline form pre-populated**
|
||||
Given a class row is in read-only state
|
||||
When the user activates its edit affordance
|
||||
Then the row replaces its read-only cells with editable `name`, `shortName`, `color`, `maxSizeM` inputs; the inputs are seeded with the row's current values; Save and Cancel buttons are visible; no other row enters edit mode simultaneously.
|
||||
|
||||
**AC-3: Save sends PATCH and refreshes the list**
|
||||
Given the inline form has valid edits
|
||||
When the user clicks Save (or presses Enter inside the form)
|
||||
Then exactly one `PATCH /api/admin/classes/{id}` request is made with body `{ name, shortName, color, maxSizeM }`; on 200 the classes list re-fetches and the row re-renders in read-only state with the new values; the form closes.
|
||||
|
||||
**AC-4: Cancel discards edits**
|
||||
Given the inline form has unsaved edits
|
||||
When the user clicks Cancel (or presses Escape inside the form)
|
||||
Then no network request is made; the form closes; the row reverts to its previous read-only values.
|
||||
|
||||
**AC-5: Validation prevents invalid submits**
|
||||
Given the inline form has `name === ''` OR `maxSizeM <= 0` OR `maxSizeM` is non-numeric
|
||||
When the user clicks Save
|
||||
Then NO network request is made; an inline error message appears next to the offending field with the appropriate i18n key (`admin.classes.nameRequired` / `admin.classes.maxSizeMustBePositive`); focus moves to the offending field.
|
||||
|
||||
**AC-6: Backend error is surfaced**
|
||||
Given the PATCH request fails with 4xx or 5xx
|
||||
When the response is handled
|
||||
Then an inline error message appears under the form using the `admin.classes.updateFailed` i18n key; the form stays open with the user's edits intact; no alert() is used (Finding B4 anti-pattern).
|
||||
|
||||
**AC-7: i18n parity**
|
||||
Given the en.json and ua.json bundles after the task lands
|
||||
When the AZ-465 i18n parity test runs
|
||||
Then every new admin.classes.* key exists in both bundles with non-empty values; t() coverage is preserved.
|
||||
|
||||
**AC-8: Existing add + delete behaviour is unchanged**
|
||||
Given the Admin page after the task lands
|
||||
When an admin user adds a new class or deletes an existing class
|
||||
Then the network requests and UI behaviour are byte-identical to today (regression guard).
|
||||
|
||||
## Non-Functional Requirements
|
||||
|
||||
**Performance**: editing a row triggers exactly two requests in the success path — `PATCH` then `GET classes` (the existing refresh pattern). No additional polling, no debounced auto-save.
|
||||
|
||||
**Compatibility**: the wire contract is additive — `PATCH /api/admin/classes/{id}` accepting `{ name?, shortName?, color?, maxSizeM? }` is the assumed shape. If the live endpoint requires every field, the form's `editForm` already carries every field (seeded from the row), so the request body is always complete — no compatibility variance.
|
||||
|
||||
**Accessibility**: the inline form must be keyboard-navigable; Tab moves between inputs; Enter submits; Escape cancels. The edit affordance must have an accessible name (`aria-label={t('admin.classes.edit')}`) when implemented as an icon-only button.
|
||||
|
||||
## Unit Tests
|
||||
|
||||
| AC Ref | What to Test | Required Outcome |
|
||||
|--------|--------------|------------------|
|
||||
| AC-2 | Click the edit affordance on row N | row N renders the inline form with seeded values; other rows unchanged |
|
||||
| AC-3 | Submit valid form | one PATCH call to `/api/admin/classes/{id}` with the expected body; row re-renders with new values |
|
||||
| AC-3 | Submit via Enter key | same as Save button |
|
||||
| AC-4 | Click Cancel | no network call; row reverts |
|
||||
| AC-4 | Press Escape in form | same as Cancel button |
|
||||
| AC-5 | Empty name, click Save | no PATCH; inline error visible |
|
||||
| AC-5 | Negative maxSizeM, click Save | no PATCH; inline error visible |
|
||||
| AC-6 | PATCH returns 500 | form stays open; inline error visible; no alert() |
|
||||
| AC-7 | i18n keys exist in both bundles | passes the existing AZ-465 parity assertion |
|
||||
| AC-8 | Add + delete unchanged | full re-run of the existing AdminPage tests |
|
||||
|
||||
## Blackbox Tests
|
||||
|
||||
| AC Ref | Initial Data/Conditions | What to Test | Expected Behavior | NFR References |
|
||||
|--------|------------------------|--------------|-------------------|----------------|
|
||||
| AC-2 + AC-3 | Logged in as admin; classes table has ≥ 3 rows | Click edit on row 2; change name; Save | DevTools shows one PATCH; row 2's name updates in place | Performance |
|
||||
| AC-4 | Same | Click edit on row 2; change name; Cancel | No PATCH; row 2 unchanged | — |
|
||||
| AC-5 | Same | Click edit on row 2; clear name; Save | No PATCH; inline error visible next to name input | — |
|
||||
| AC-6 | Same; backend stubbed to return 500 on PATCH | Click edit on row 2; change name; Save | Inline error visible; form stays open | Reliability |
|
||||
| AC-7 | Switch language between en and ua | Click edit on any row | Form labels + error messages render in the active language | — |
|
||||
|
||||
## Constraints
|
||||
|
||||
- Use the existing `endpoints.admin.class(id)` builder. Do not introduce a new endpoint helper for PATCH — the URL is the same as DELETE and that's the wire-contract single-source-of-truth invariant established by AZ-486.
|
||||
- Use the existing `api.patch()` helper. Do not call `fetch()` directly.
|
||||
- Render the inline form **inside the same `<tr>`** as the row being edited — do NOT open a modal or a side drawer. The legacy WPF behaviour (per `_docs/legacy/wpf-era.md` §10 and `_docs/ui_design/`) is in-row inline edit.
|
||||
- Every new visible string MUST exist in both `en.json` and `ua.json` (P6 enforcement); the AZ-465 i18n parity test will fail otherwise.
|
||||
- Do not use `alert()` or `window.confirm()` for errors (Finding B4 anti-pattern); inline messages only.
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
**Risk 1: Backend endpoint does not exist** *(highest)*
|
||||
- *Risk*: `PATCH /api/admin/classes/{id}` may not be implemented in `../admin/`; the form would 404 in production.
|
||||
- *Mitigation*: The Cross-Workspace Verification gate above is BLOCKING. The implementer must verify before writing the form. If missing, the gate's Choose A/B/C/D forces a decision; we do not paper over with a stub.
|
||||
|
||||
**Risk 2: PATCH semantics — full body vs partial body**
|
||||
- *Risk*: The backend may treat PATCH as full-body (replace, like PUT) rather than partial (merge). If so, an undocumented absent field could be silently nulled.
|
||||
- *Mitigation*: Always send the complete `editForm` (every field from the seeded row). This is the safer default regardless of backend semantics. Document the decision in the implementation report.
|
||||
|
||||
**Risk 3: Two rows in edit mode simultaneously**
|
||||
- *Risk*: Subtle UI bug — clicking "edit" on row 3 while row 2 is still in edit mode could leave both open if state is per-row.
|
||||
- *Mitigation*: Use a single `editingId: number | null` state (NOT per-row) so opening one row's editor automatically closes any other. AC-2 explicitly asserts this.
|
||||
|
||||
**Risk 4: Cancel after partial save (network in-flight)**
|
||||
- *Risk*: User clicks Save, then Cancel before the PATCH resolves. Race condition between server-side success and client-side cancel.
|
||||
- *Mitigation*: Disable the form (or at least Save + Cancel buttons) while a PATCH is in flight, with a spinner indicator. The 200 response always wins — the form closes; no further action on Cancel.
|
||||
|
||||
**Risk 5: i18n drift introduced by missed keys**
|
||||
- *Risk*: A new error string in en.json without the matching ua.json key breaks AZ-465's parity test.
|
||||
- *Mitigation*: Add all six new keys to BOTH bundles in the same commit. Run `bun run test tests/i18n_parity.test.ts` (or whatever the AZ-465 test path is) locally before marking the task done.
|
||||
|
||||
## References
|
||||
|
||||
- `_docs/02_document/architecture.md` — Architecture Vision principle P12.
|
||||
- `_docs/02_document/04_verification_log.md` — F10 (Class edit affordance missing).
|
||||
- `_docs/02_document/components/08_admin/description.md` — current Admin page surface.
|
||||
- `src/features/admin/AdminPage.tsx` — implementation target.
|
||||
- `src/api/endpoints.ts:30` — `endpoints.admin.class(id)` (existing PATCH/DELETE target).
|
||||
- `src/api/client.ts:106` — `api.patch()` helper.
|
||||
- `_docs/02_tasks/done/AZ-466_test_destructive_ux.md` — Finding B4 / no-alert anti-pattern enforced via `<DestructiveButton>` and static check.
|
||||
- `_docs/02_tasks/done/AZ-465_test_i18n.md` — i18n parity test that protects AC-7.
|
||||
Reference in New Issue
Block a user