Files
jobtrackingapp/docs/redesign/phase-5/p5-15/p5-15-01/saved-search-repair/README.md
T

181 lines
21 KiB
Markdown

# P5-15-01R-01 — Saved-search loading and error visibility
Source: `54a377054f6138c5645a3da293dacd84d3056237`, development branch `codex/ui-redesign-phase4`, 2026-10-09. This bounded checkpoint repairs **P5-15-01-DISCOVERY-01** only. Production remains **NO-GO**; production `main` is unchanged at `27657f31131cd95d42f0e1290853ec9c9b3959a4`.
The affected feature is **saved NAV searches in Discover (`/discover`)**, reached from the Jobs secondary navigation. It uses `SavedJobSearch` and `/api/job-discovery/saved-searches`. Jobs Table **Saved views** are a separate filter-preference domain. Neither the Table/Board query contract nor route hierarchy is changed.
## Original reproduction and root cause
The original audit recorded four genuine HTTP500s: normal/admin entry and refresh. This checkpoint reran the normal-user census **before any backend repair** and reproduced two more native saved-search GET500s against the existing disposable SQLite application. The new real-SQLite regression then failed for an empty owner, previously persisted records and a create/run/readback journey: **3 failed / 7 passed / 0 skipped**.
The exact failing operation was `OrderByDescending(x => x.UpdatedAtUtc)` inside `ListSavedSearches`, followed by `ToListAsync`. `UpdatedAtUtc` is an unconverted `DateTimeOffset`; installed EF Core SQLite raises:
> SQLite does not support expressions of type 'DateTimeOffset' in ORDER BY clauses.
This is a translation failure **before SQL execution**, not a missing account, corrupted search record, serializer exception, NAV outage or SQL statement rejected by SQLite. There is therefore no successfully executed failing SQL to reproduce. [Sanitized before/after evidence](evidence/validation.json) retains the exception, clean RED/GREEN counts, native request observations and fingerprints of raw logs/results.
| Stage | Actual behavior / authoritative boundary |
|---|---|
| Frontend | Discover requests the saved list; the old catch silently retained an empty array |
| API/authentication | Local authorization runs; the current-user service resolves the authenticated owner |
| Persistence | `Where(OwnerUserId == owner)` is retained, but the old unsupported timestamp ordering fails query translation |
| Materialization/serialization | The old query never reaches materialization or DTO serialization; native transport returns500 |
| User feedback | Old UI presented “No saved searches yet”; new UI announces the failed read and offers Retry |
Initial test-harness errors are separate from product evidence: async principal setup in a direct-controller fixture was corrected before the clean RED; validation asserts `ProblemDetails` rather than a formatter status unavailable outside MVC execution. The first new browser matrix run expected language-preference200 instead of its existing204 contract; only that fixture assertion was corrected. None of these errors was counted as a product pass or used to weaken the saved-search assertions.
## Smallest repair and compatibility
`JobDiscoveryController.ListSavedSearches` now projects owner-filtered **summary scalars and two SQL aggregate counts**. It does not materialize search-result entities. SQLite sorts the materialized **owned summaries** by `DateTimeOffset` instant; non-SQLite providers retain database ordering. Mixed-offset fixtures prove actual chronological order rather than lexical timestamp order.
Ownership filtering and result counts remain in SQL. The API already returns all of an owner's saved-search summaries; this preserves that response contract. SQLite's sort costs O(n log n) time/O(n) summary memory for that owner, rather than loading every account or every stored result. This is not client-side ownership/filter evaluation. No arbitrary limit, truncation, schema migration, timestamp rewrite, database reset or data deletion is introduced.
Existing DTO fields, descending-update semantics, cancellation, create trimming/validation, alert toggle, explicit run, result dismissal and delete/cascade behavior remain intact. No separate read-by-id endpoint or renamed search mutation is invented: the existing PATCH updates `IsActive`, not stored query/name. The real runner still uses the current service and normal persistence path.
### Provider qualification
The repository configures **SQLite and Pomelo MySQL/MariaDB**, with MariaDB11 in the existing deployment. It does **not** currently configure Npgsql/PostgreSQL. The non-SQLite branch retains server-side ordering and uses ordinary owner-filtered projections/counts; no SQLite-only schema workaround changes that branch.
**Live MariaDB and PostgreSQL integration are NOT VERIFIED.** No approved server/credential fixture was available and none was added. Source compatibility and Release compilation are evidence for the query shape, not successful execution against either server. PostgreSQL parity cannot be claimed for an unconfigured provider. Deployed provider/schema qualification remains a separate production obligation.
## Frontend states and recovery
| State | User-facing behavior |
|---|---|
| Loading | Named `status`, busy saved-search region; no successful-empty message; creation disabled |
| Loaded empty | Existing “No saved searches yet” message after a valid empty response |
| Loaded populated | Existing run/alert/delete controls and metadata; no interface redesign |
| Error / malformed response | Existing outlined MUI error Alert with localized concise wording and Retry; no internal server details; list not presented as refreshed |
| Retry | Reloads only the saved-search resource. Current role/company, municipality, discovery result set and sorting remain available |
The page validates the received list's existing DTO shape. A200 containing null, an object, or incomplete rows is a failed read, not an empty successful account. An unmounted/obsolete effect cannot publish its response. Saved rows are hidden while loading/failed, so stale data is not represented as successfully refreshed.
Save search remains disabled until a list is loaded. Review reproduced the otherwise-hidden-success risk after a failed initial read; a focused RED assertion failed before this guard and passes afterward. Retry restores creation without silently appending to an unknown list. This does not discard the user's search-name or discovery draft.
Translations:
- EN: “Loading saved searches…” / “Saved searches could not be loaded. Try again.”
- Bokmål: “Laster lagrede søk…” / “Lagrede søk kunne ikke lastes. Prøv igjen.”
Existing Retry wording and shared MUI semantics are reused. No new global resource framework, notification system or settings surface is added.
## Files changed and scope justification
| File | Bounded reason |
|---|---|
| `JobTrackerApi/Controllers/JobDiscoveryController.cs` | Owns the failed query; owner-filtered projection/counts and provider-compatible ordering |
| `JobTrackerApi.Tests/SavedJobSearchSqliteTests.cs` | Ten new real-SQLite cases, real current-user service; no EF InMemory proof for the provider defect |
| `job-tracker-ui/src/views/JobDiscoveryPage.tsx` | Owns the suppressed saved-list failure and recovery without changing discovery/query semantics |
| `job-tracker-ui/src/i18n/translations.ts` | Two new labels in both existing languages; no new terminology domain |
| `job-tracker-ui/src/job-discovery.test.tsx` | Five new behavior cases; one old fixture now includes its actual `isActive` DTO field; existing seven tests retained |
| `job-tracker-ui/e2e/saved-search-repair.spec.ts` | Three new native transport/UI contract cases, explicit provider interception and injected fault boundaries |
| This record and P5-15 acceptance/defect evidence | Current status and traceability; original failed audit evidence remains historical |
No application code outside the discovery controller/page and shared translation owner changes. No authentication/session boundary, route, Jobs Table/Board, renderer, model/schema, CI/deployment, AI configuration or dependency changes occur.
## Behavioral verification
| Requirement | Evidence / result scope |
|---|---|
| New user empty list | Real SQLite integration and native authenticated GET/UI return200 + `[]`; no provider call |
| Previously persisted records | Save seeded owned/foreign records, clear tracking and reread; exact owned IDs/counts and mixed-offset descending instants; no entities tracked |
| Create/navigation/reload | Native201 from UI, navigate Jobs→Discover, refresh and compare exact persisted DTO; reload after pause/resume and delete |
| Apply saved search | UI action restores saved query/municipality and renders the provider fixture; native SQLite controller/runner verifies actual filter inputs, run/result persistence and subsequent readback |
| Update/delete | Native pause/resume200 and delete204, readback/refresh; backend verifies result cleanup and missing/foreign404 |
| Isolation | Native second account cannot list or mutate first account's search; forged owner/id payloads remain caller-owned; six anonymous list/create/update/delete/run/dismiss requests return401 |
| Foreign persisted result | Real-SQLite fixture seeds a foreign result and verifies dismissal denial/readback. Browser foreign dismissal uses an unavailable result, so it is not the sole proof of relationship ownership |
| Failure/recovery | Jest HTTP500 and malformed payload cases; browser deliberately faults only saved-list reads and retries the **native GET**, preserving edited filters and existing one-off results |
| Table/Board | Existing full-suite ID-membership/query/alias/owner tests retained; Jobs Saved views stays separate. Saved NAV repair does not modify that domain |
### Explicit test boundaries
The browser uses the real disposable local SQLite API for list/create/toggle/delete/authentication and owner denials. Only the **owner's provider-dependent run output and one-off vacancy search** are intercepted; NAV's base URL is external and no new live-provider processing is authorized. The backend tests exercise the real runner with a deterministic `IJobDiscoverySearchService`, real SQLite and real current-user resolution. This is stronger persistence evidence than mocked repositories, but not live NAV qualification.
The existing saved-alert worker is disabled **only by the browser-run environment flag** `Workers__SavedJobSearchAlertsEnabled=false` to avoid unintended external calls. No configuration file or production behavior is changed. Scheduler-to-notification/live delivery remains outside this proof.
Reload restores the persisted search **list**. Existing Discover semantics do not automatically rerun a search or persist the current one-off input as URL state. Explicit Apply restores the saved inputs. The checkpoint does not invent automatic query execution on reload.
## Preservation reconciliation
| PRES | Existing contract → defect/change → evidence → current acceptance |
|---|---|
| **J02** | Named saved searches/run/delete/alerts/result state → list500 + silent error → scalar read repair and visible independent error/retry → ten SQLite cases, native lifecycle/denial/recovery cases plus retained alert tests → **FAIL→PARTIAL**, not whole-group PASS |
| **J01** | NAV source/search/reviewed save → list failure must not disable one-off discovery → retry retains form/results; existing discovery behavior tests remain green → **PARTIAL unchanged** |
| **J05/J07** | Table filters/saved views and Board population/status/navigation → distinct saved-search domain → no Table/Board source changes; existing full browser membership/alias tests → **PARTIAL unchanged** |
| **E07/E08/E09** | Reachability, accessible named loading/error/retry, authenticated ownership → same Discover secondary entry and safe feedback → browser navigation/roles, finite axe, native tampering/anonymous denial → **PARTIAL unchanged** |
All original57 groups remain reconciled in the [parent matrix](../../PRESERVATION_ACCEPTANCE.md). This scoped repair cannot certify scheduler delivery, mobile delete confirmation, real providers or human screen-reader acceptance. The original01 columns retain their failed observations; the successor column records current evidence.
## Fresh validation and diagnostic reconciliation
All results below are **fresh local executions**, not inherited passes. [Validation/commands/fingerprints](evidence/validation.json), [saved-search RED/GREEN/native proof](evidence/saved-search-proof.json) and [repository/build/document checks](evidence/repository-validation.json) retain provenance. Raw logs/TRX/JSON/PDF files remain under the ignored `.artifacts/p5-15-01R-01/`; portable fingerprints and finite synthetic screenshots are committed.
| Gate | Inherited / audit source | Fresh final result | Count reconciliation |
|---|---|---|---|
| Backend | 1,023 | **1,033 passed**, 0failed/0skipped | +10 actual SQLite cases described above |
| Frontend | 459 /82suites | **464 passed /82suites**, 0failed/0skipped | +5 loading/error/malformed response cases; old7 discovery cases retained |
| Ordinary browser | 117 inherited +5 audit cases:119pass/3fail | **124 passed /1 failed /125 total**, 0skipped/0expected failures | All117 inherited pass; original five audit cases now4pass/1fail; +3 new repair cases pass |
| Saved-search focused browser | No dedicated native repair cases | **3/3 passed** | These same cases are also in the125-case full run; do not add them twice |
| Separate diagnostics | 1pass/1fail | **1pass/1fail** | Preference concurrency pass; Rules draft failure unchanged; not counted inside125 |
| Focused AI | 108 | **108 passed** | Backend subset; deterministic guards, not model qualification |
| Sidecar | 49 | **49 passed** | Offline; no model/provider qualification |
| Real HTTP | 2 | **2 passed** | Production CV-review withholding and authoritative proposal boundary remain protected |
| Revision safety | 5 | **5/5 passed** | Backend subset; no revision changes |
| Standard fidelity | 16;36=36pages | **16/16;36 preview=36 PDF pages** | Same corpus and declared tolerances |
| Settings exports | 8;24=24pages | **8/8;24 preview=24 PDF pages** | Same settings corpus and declared tolerances |
| Rate-limit / ingress / PDF admission / configuration | 9/6/1/3 | **9/6/1/3 passed** | Local security/configuration evidence; not deployed F04 proof |
| Additional offline supply / evaluator / extension | 3/5/3 | **3/5/3 passed** | No live evaluation, extension deployment or provider calls |
| TypeScript / E2E TypeScript / lint / frontend production / API Release builds | Passing | **Passed** | App and test typing, global lint, both builds; API warnings/errors0 |
| Secrets / whitespace / documents /57-group reconciliation | Passing | **Passed** | [Recorded checks](evidence/repository-validation.json); original contracts and route inventory unchanged |
The complete browser command exits1 because **AUTH-01 remains an ordinary failed case**: `/analytics?months=6` loses its period query after sign-in. The separate diagnostics command exits1 because **SETTINGS-01 remains an ordinary failed case**: unsaved14→5→tab round trip returns14, persisted14, zero writes. These are **not** skipped, quarantined, expected failures or repaired here. Both original normal/admin route failures caused by DISCOVERY-01 now pass. P2-D01 remains an ordinary passing case.
An initial new browser fixture expected200 from the existing language-preference PUT, whose actual contract is204; that harness assertion was corrected before the genuine3/3 run. No saved-search failure expectation was weakened. The first post-final-change Jest run observed463pass/1fail in an unchanged CV preview “Unsaved” assertion. Its isolated rerun and subsequent **complete464-case rerun** pass. The failed report remains fingerprinted; the cause is **not established**, no CV code/test is changed and no product fix is claimed. This is an open recurrence/verification risk for later CV acceptance. An earlier full frontend run began before the last UI guard change and is retained as intermediate evidence, not the final baseline.
The fresh route census covers all32 patterns/192 entry-refresh observations and98 selected native reads, with no remaining saved-search500. Four normal/admin Discover entry-refresh GETs now return200. All entry-refresh assertions pass; login query continuity remains failed. Broader route status is therefore **31PARTIAL/1FAIL**, not32 fully accepted workflows. [Original route evidence](../ROUTE_RESULTS.md) is retained historically.
## Visual, localisation and accessibility evidence
The new browser matrix tests **EN/light1440px, EN/dark390px, Bokmål/light390px and Bokmål/dark1440px**, including loading, fault and native retry, with edited inputs and a retained one-off result. It captures13 actual screenshots and checks document width against viewport width. These are Chromium mouse/viewport checks, not physical phone/touch/software-keyboard acceptance.
| Captured state | Evidence |
|---|---|
| Persisted saved search after native reload | [EN/light desktop](evidence/saved-search-native-persisted-en-light-desktop.png) |
| Error with preserved current discovery results | [EN/light desktop](evidence/saved-search-en-light-1440-load-error.png) |
| Narrow error message and Retry | [EN/dark390px](evidence/saved-search-en-dark-390-load-error.png) |
| Native retry restores saved controls while retaining changed inputs/results | [Bokmål/light390px](evidence/saved-search-nb-light-390-native-retry.png) |
| Named pending load, no false empty state | [Bokmål/dark desktop](evidence/saved-search-nb-dark-1440-loading.png) |
The coding agent inspected the genuine desktop error, dark narrow error and Bokmål narrow retry images: controls/text reflow, current results remain present and the message has no internal details. This is **agent visual inspection**, not independent human or screen-reader acceptance.
Scoped saved-search scans: **9 axe scans,0 violations,52 incomplete color-contrast node occurrences**. The complete fresh browser run records **251 scans,0 reported violations,1,880 incomplete node occurrences**. Incompletes are unaccepted; repeated occurrences are not a count of unique defects. Named status/Alert/Retry controls have automated evidence, but spoken announcements, actual screen reader, physical touch and complete zoom/reflow remain open. Existing manual accessibility requirements are preserved.
## Current debt and stopping boundary
| Item | Disposition after this checkpoint |
|---|---|
| DISCOVERY-01 P1 | **Closed for demonstrated SQLite listing + suppressed-error/recovery defect**; no data deletion or ownership regression observed |
| AUTH-01 P2 Analytics sign-in query | **OPEN/failing**, separate bounded repair required |
| SETTINGS-01 P2 Rules draft retention | **OPEN/failing**, separate bounded repair required |
| Historical Dashboard/language preference-save500 | **OPEN**; bounded concurrency diagnosis passes again, not reproduced does not mean fixed |
| Live MariaDB/PostgreSQL/NAV; saved-alert delivery | **NOT VERIFIED**; provider/scheduler qualification remains separate; PostgreSQL not configured |
| Transient CV “Unsaved” assertion | First full attempt failed; isolated and subsequent full passes; cause unestablished, retained for later recurrence triage |
| Manual screen reader, incomplete axe, physical touch/software keyboard, broader zoom/reflow, production fonts/photos | **OPEN/NOT VERIFIED**, preserved for assigned acceptance owners |
| Board scaling/native touch; metadata-only `followMode` compose UI | **OPEN/deferred** per existing plan; no remediation |
| P5-05/P5-13/B07 | No implementation; P5-13 remains unqualified,0/366calls,0human reviews, review UI deferred, Production withholding active |
| F04, current backup/restore, production configuration/schema/secrets and tested rollback | **Release blockers remain NOT VERIFIED**; no infrastructure recovery or deployment |
The [defect register](../DEFECTS.md) retains the original three findings and now distinguishes the two currently open reproduced defects from the closed SQLite defect. J02 stays PARTIAL and the whole57 matrix is **PASS0/PARTIAL54/FAIL2/DEFERRED1/BLOCKED0/NOT VERIFIED0**. No original contract is weakened and no unrelated failure is closed by this repair.
## Checkpoints and decision
| Logical checkpoint | Commit / immediate push evidence |
|---|---|
| Real-SQLite reproducer and owner-filtered summary repair | **`bcc959904915e381ef6bfe5c236d9626021681d4`**; pushed and remote equality verified before continuing |
| Visible load/failure/retry, guarded creation, localized labels and behavior/native browser regressions | **`2753d03db643c4b90b12c62748b0e844981d48b8`**; pushed and remote equality verified before final documentation |
| Final validation, scoped proof and acceptance/defect reconciliation | This independently pushed documentation checkpoint; final hash and remote/clean-tree confirmation are reported to the owner after commit |
**P5-15-01R-01: GO for the bounded saved-search repair.** The original HTTP500 is fixed, previously persisted data and current create/reload/apply controls work at the declared native/provider boundaries, real SQLite regressions and ownership denials pass, and failed reads have safe accessible recovery. All relevant saved-search cases pass. The complete baseline remains explicitly red for the two unrelated P2 defects.
**Production: NO-GO. P5-15-02 is not started or cleared by this checkpoint.** Reconcile AUTH-01/SETTINGS-01 through separately authorized bounded work. Stop here; no Analytics/Rules repair, P5-13, B07, production merge or deployment follows automatically.