mirror of
https://github.com/azaion/ui.git
synced 2026-06-21 09:41:11 +00:00
[AZ-447] autodev Steps 1-4 baseline: docs, tests, refactor specs
Captures the full output of autodev existing-code Phase A through Step 4 (Code Testability Revision) for the Azaion UI workspace: - Step 1 Document: _docs/02_document/ (FINAL_report, architecture, glossary, components/, modules/, diagrams/, system-flows, module-layout) plus _docs/00_problem/ + _docs/01_solution/ + _docs/legacy/ + _docs/how_to_test + README. - Step 2 Architecture Baseline: architecture_compliance_baseline.md. - Step 3 Test Spec: _docs/02_document/tests/ (environment, test-data, blackbox/performance/resilience/security/ resource-limit tests, traceability-matrix), enum_spec_snapshot, expected_results/results_report.md (98 rows), plus the run-tests.sh + run-performance-tests.sh runners. - Step 4 Code Testability Revision: 01-testability-refactoring/ run dir (list-of-changes C01-C07, deferred_to_refactor, analysis/research_findings + refactoring_roadmap) and the 7 child task specs AZ-448..AZ-454 under _docs/02_tasks/todo/ plus _dependencies_table.md. - _docs/_autodev_state.md pins the cursor at Step 4 / refactor Phase 4 entry so /autodev resumes cleanly. Epic AZ-447 (UI testability gates) tracks the 7 child tasks that will land in subsequent commits. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Oleksandr Bezdieniezhnykh
@@ -0,0 +1,18 @@
|
||||
# Task Dependencies
|
||||
|
||||
| Task | Name | Epic | Complexity | Depends on |
|
||||
|------|------|------|-----------|------------|
|
||||
| AZ-448 | C01 — Externalize OWM API key | AZ-447 | 2 | None |
|
||||
| AZ-449 | C02 — Externalize OWM base URL | AZ-447 | 2 | AZ-448 (same file) |
|
||||
| AZ-450 | C03 — Externalize map tile URLs | AZ-447 | 2 | None |
|
||||
| AZ-451 | C04 — Bundle Leaflet marker icon locally | AZ-447 | 2 | None |
|
||||
| AZ-452 | C05 — `getApiBase()` accessor | AZ-447 | 3 | None |
|
||||
| AZ-453 | C06 — `navigateToLoginImpl()` accessor | AZ-447 | 2 | None |
|
||||
| AZ-454 | C07 — Document `setToken/getToken` | AZ-447 | 1 | None |
|
||||
|
||||
## Notes
|
||||
|
||||
- Epic AZ-447 is the umbrella for the autodev existing-code Step 4 testability run (`01-testability-refactoring`).
|
||||
- AZ-448 and AZ-449 share `src/features/flights/flightPlanUtils.ts` and should land in one commit to avoid a mid-state where the URL still hardcodes a base while the key is externalized.
|
||||
- Total: 14 complexity points across 7 tasks.
|
||||
- Every task fits the existing-code flow Step 4 allowed-change list (externalize hardcoded URLs/credentials, wrap globals in thin accessors, comment-only documentation). Deferred items are in `_docs/04_refactoring/01-testability-refactoring/deferred_to_refactor.md`.
|
||||
@@ -0,0 +1,86 @@
|
||||
# Externalize OpenWeatherMap API key
|
||||
|
||||
**Task**: AZ-448_refactor_owm_api_key
|
||||
**Name**: Externalize OWM API key to VITE_OWM_API_KEY
|
||||
**Description**: Move the hardcoded OpenWeatherMap API key from `flightPlanUtils.ts` into a Vite-driven env var so it can be redacted in source and overridden in the test profiles.
|
||||
**Complexity**: 2 points
|
||||
**Dependencies**: None
|
||||
**Component**: `src/features/flights/flightPlanUtils.ts`, `.env.example`, `src/vite-env.d.ts`
|
||||
**Tracker**: AZ-448
|
||||
**Epic**: AZ-447
|
||||
|
||||
## Problem
|
||||
|
||||
`src/features/flights/flightPlanUtils.ts:60` inlines `const apiKey = '335799082893fad97fa36118b131f919'`. The literal:
|
||||
|
||||
- Violates restriction O6 (no hardcoded credentials) and AC-O6 (`acceptance_criteria.md`).
|
||||
- Blocks the security static check NFT-SEC-09 (results_report.md row 63 — quarantined until this fix lands).
|
||||
- Prevents the e2e profile's `owm-stub` from intercepting OWM calls deterministically.
|
||||
|
||||
## Outcome
|
||||
|
||||
- Source tree has no literal OWM key string after the change.
|
||||
- The fast and e2e test profiles can supply test-time keys via env vars without touching source.
|
||||
- NFT-SEC-09 source-string check (`scripts/run-tests.sh --static-only`) passes.
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
- Reading `import.meta.env.VITE_OWM_API_KEY` at call time inside `getWeatherData`.
|
||||
- Adding `.env.example` (workspace-root) documenting the variable.
|
||||
- Extending the `ImportMetaEnv` type in `src/vite-env.d.ts`.
|
||||
|
||||
### Excluded
|
||||
- Changing the `getWeatherData(lat, lon)` signature or callers.
|
||||
- Reworking the weather-fetch error handling — only the key sourcing changes.
|
||||
- Touching the OWM base URL (that is C02 / AZ-449).
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: Key sourcing**
|
||||
Given the SPA is built with `VITE_OWM_API_KEY=<key>` in the environment,
|
||||
When `getWeatherData(lat, lon)` is invoked,
|
||||
Then the resulting HTTP request URL contains `appid=<key>`.
|
||||
|
||||
**AC-2: No literal in source**
|
||||
Given a `grep -r "335799082893fad97fa36118b131f919" src/` after the change,
|
||||
When the grep runs,
|
||||
Then the literal key produces no matches.
|
||||
|
||||
**AC-3: Missing-env tolerance**
|
||||
Given the SPA is built with `VITE_OWM_API_KEY` unset,
|
||||
When `getWeatherData(lat, lon)` is invoked,
|
||||
Then it returns `null` (matches the existing try/catch fallback) and does NOT throw.
|
||||
|
||||
**AC-4: Type declaration**
|
||||
Given a TypeScript build (`bun run build`),
|
||||
When the env interface is resolved,
|
||||
Then `ImportMetaEnv` in `src/vite-env.d.ts` declares `readonly VITE_OWM_API_KEY: string | undefined`.
|
||||
|
||||
## Non-Functional Requirements
|
||||
|
||||
**Compatibility**
|
||||
- Build tooling pinned to Vite 6 (S3) — `import.meta.env` is built-in; no new dependency.
|
||||
|
||||
**Security**
|
||||
- `.env.example` MUST use a placeholder (`<your-key>`) — never check in the production key.
|
||||
|
||||
## Blackbox Tests
|
||||
|
||||
| AC Ref | Initial Data / Conditions | What to Test | Expected Behavior | NFR References |
|
||||
|--------|--------------------------|--------------|-------------------|----------------|
|
||||
| AC-1 | `VITE_OWM_API_KEY=test-key-123` set at build; `getWeatherData(0, 0)` called | Outbound fetch URL contains `appid=test-key-123` | Pass | — |
|
||||
| AC-2 | Repo state after task lands | `grep -r "335799082893fad97fa36118b131f919" src/` | No matches | Security |
|
||||
| AC-3 | `VITE_OWM_API_KEY` undefined at build; `getWeatherData(0, 0)` called | Function returns null without throwing | Pass | — |
|
||||
| AC-4 | Repo state after task lands | `src/vite-env.d.ts` extends `ImportMetaEnv` with the field | Declaration present | Compat |
|
||||
|
||||
## Constraints
|
||||
|
||||
- Adhere to S3 (Vite 6) and the existing project tooling — no new dependency, no `process.env` polyfill.
|
||||
- Adhere to AC-N4 (no signature library) — env-driven config only.
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
**Risk 1: Build-time leakage**
|
||||
- *Risk*: Vite inlines `import.meta.env.VITE_*` at build time, so a build run with the production key embedded into `dist/` leaks the key into the bundle.
|
||||
- *Mitigation*: This is a known Vite behavior, identical to today's hardcoded key. Long-term fix is to proxy OWM through the suite's `loader/` (per E10) — out of scope for this testability task; flagged for Phase B.
|
||||
@@ -0,0 +1,66 @@
|
||||
# Externalize OpenWeatherMap base URL
|
||||
|
||||
**Task**: AZ-449_refactor_owm_base_url
|
||||
**Name**: Externalize OWM base URL to VITE_OWM_BASE_URL
|
||||
**Description**: Move the hardcoded `https://api.openweathermap.org/data/2.5/weather` URL into a Vite env var so the e2e profile's `owm-stub` can intercept OWM calls without code edits.
|
||||
**Complexity**: 2 points
|
||||
**Dependencies**: AZ-448_refactor_owm_api_key (same file)
|
||||
**Component**: `src/features/flights/flightPlanUtils.ts`, `.env.example`, `src/vite-env.d.ts`
|
||||
**Tracker**: AZ-449
|
||||
**Epic**: AZ-447
|
||||
|
||||
## Problem
|
||||
|
||||
The OWM endpoint is hardcoded at `flightPlanUtils.ts:60`. The e2e test profile spins up an `owm-stub:8081` service to keep tests deterministic and avoid rate-limiting, but the stub cannot intercept calls when the base URL is baked into source.
|
||||
|
||||
## Outcome
|
||||
|
||||
- The base URL is resolvable from `import.meta.env.VITE_OWM_BASE_URL` at call time.
|
||||
- Default behavior (unset env var) preserves today's production endpoint.
|
||||
- The e2e profile sets `VITE_OWM_BASE_URL=http://owm-stub:8081/data/2.5` at build time and weather calls hit the stub.
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
- Reading `VITE_OWM_BASE_URL` (default `https://api.openweathermap.org/data/2.5`) inside `getWeatherData`.
|
||||
- Adding the entry to `.env.example` and `src/vite-env.d.ts`.
|
||||
|
||||
### Excluded
|
||||
- Any change to error handling, caching, or response shape.
|
||||
- Touching the API key (AZ-448 covers it).
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: Default endpoint preserved**
|
||||
Given `VITE_OWM_BASE_URL` is unset at build time,
|
||||
When `getWeatherData(lat, lon)` is invoked,
|
||||
Then the fetch target hostname is `api.openweathermap.org`.
|
||||
|
||||
**AC-2: Override honored**
|
||||
Given `VITE_OWM_BASE_URL=http://owm-stub:8081/data/2.5` at build time,
|
||||
When `getWeatherData(lat, lon)` is invoked,
|
||||
Then the fetch target URL begins with `http://owm-stub:8081/data/2.5/weather?lat=...`.
|
||||
|
||||
**AC-3: Type declaration**
|
||||
Given a TypeScript build,
|
||||
When the env interface resolves,
|
||||
Then `ImportMetaEnv` declares `readonly VITE_OWM_BASE_URL: string | undefined`.
|
||||
|
||||
## Blackbox Tests
|
||||
|
||||
| AC Ref | Initial Data / Conditions | What to Test | Expected Behavior | NFR References |
|
||||
|--------|--------------------------|--------------|-------------------|----------------|
|
||||
| AC-1 | env unset; `getWeatherData(0, 0)` called | Outbound URL host | `api.openweathermap.org` | Compat |
|
||||
| AC-2 | env set to stub; `getWeatherData(0, 0)` called | Outbound URL prefix | Matches stub URL | E2E determinism |
|
||||
| AC-3 | Build run | `vite-env.d.ts` declaration present | Pass | Compat |
|
||||
|
||||
## Constraints
|
||||
|
||||
- Land in one commit with AZ-448 to keep the file's mid-state coherent.
|
||||
- Must not break the (unset-env) production behavior.
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
**Risk 1: Trailing-slash drift**
|
||||
- *Risk*: A consumer might set the env to `https://host/` and the path concatenation produces `//weather`.
|
||||
- *Mitigation*: Normalize by trimming a trailing slash before concatenation; document the rule in `.env.example`.
|
||||
@@ -0,0 +1,80 @@
|
||||
# Externalize map tile URLs
|
||||
|
||||
**Task**: AZ-450_refactor_tile_urls
|
||||
**Name**: Externalize OSM + Esri tile URLs
|
||||
**Description**: Move the hardcoded OSM + Esri-satellite tile URLs into Vite env vars so the e2e profile's `tile-stub` can serve tiles deterministically and the air-gap test (AC-N3 / NFT-RES-03) does not trigger external network calls.
|
||||
**Complexity**: 2 points
|
||||
**Dependencies**: None
|
||||
**Component**: `src/features/flights/types.ts`, `.env.example`, `src/vite-env.d.ts`
|
||||
**Tracker**: AZ-450
|
||||
**Epic**: AZ-447
|
||||
|
||||
## Problem
|
||||
|
||||
`src/features/flights/types.ts:56-57` inlines:
|
||||
|
||||
- `classic: 'https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png'`
|
||||
- `satellite: 'https://server.arcgisonline.com/ArcGIS/rest/services/World_Imagery/MapServer/tile/{z}/{y}/{x}'`
|
||||
|
||||
These URLs:
|
||||
|
||||
- Break the e2e profile's `tile-stub:8082` intercept (Leaflet bypasses any local override path).
|
||||
- Make AC-N3 / NFT-RES-03 (offline boot) flap because Leaflet attempts external tile fetches on the map mount.
|
||||
- Violate restriction E1 (air-gap-friendly bundle).
|
||||
|
||||
## Outcome
|
||||
|
||||
- Both tile URLs are resolvable from `import.meta.env.VITE_OSM_TILE_URL` and `VITE_ESRI_TILE_URL`.
|
||||
- Defaults preserve today's production URLs.
|
||||
- The e2e profile redirects tile traffic to `tile-stub` without source edits.
|
||||
- Call sites in `FlightMap.tsx` (and any other consumer of `TILE_URLS`) are unchanged.
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
- `TILE_URLS` re-exported from `types.ts` as a const computed from `import.meta.env`.
|
||||
- `.env.example` and `src/vite-env.d.ts` updated.
|
||||
|
||||
### Excluded
|
||||
- Any Leaflet config change in `FlightMap.tsx`.
|
||||
- Adding new tile providers.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: Default tiles preserved**
|
||||
Given env vars unset,
|
||||
When `FlightMap.tsx` mounts with `mapType='classic'`,
|
||||
Then the Leaflet `TileLayer.url` resolves to `https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png`.
|
||||
|
||||
**AC-2: Override honored — classic**
|
||||
Given `VITE_OSM_TILE_URL=http://tile-stub:8082/{z}/{x}/{y}.png` at build time,
|
||||
When `FlightMap.tsx` mounts,
|
||||
Then the Leaflet `TileLayer.url` resolves to the stub URL.
|
||||
|
||||
**AC-3: Override honored — satellite**
|
||||
Given `VITE_ESRI_TILE_URL=http://tile-stub:8082/sat/{z}/{y}/{x}` at build time,
|
||||
When `mapType='satellite'`,
|
||||
Then the Leaflet `TileLayer.url` resolves to the stub URL.
|
||||
|
||||
**AC-4: Type declarations**
|
||||
Given a TypeScript build,
|
||||
Then `ImportMetaEnv` declares both fields as `string | undefined`.
|
||||
|
||||
## Blackbox Tests
|
||||
|
||||
| AC Ref | Initial Data / Conditions | What to Test | Expected Behavior | NFR References |
|
||||
|--------|--------------------------|--------------|-------------------|----------------|
|
||||
| AC-1 | env unset; FlightMap mounted | TileLayer URL (DOM `src` of the first tile request) | OSM URL | Compat |
|
||||
| AC-2/3 | env set; FlightMap mounted | TileLayer URL captured at network boundary | Stub URL | E2E determinism |
|
||||
| AC-4 | Build run | `vite-env.d.ts` declarations present | Pass | Compat |
|
||||
|
||||
## Constraints
|
||||
|
||||
- Leaflet's TileLayer API surface MUST NOT change.
|
||||
- `TILE_URLS.classic | TILE_URLS.satellite` accessor pattern preserved.
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
**Risk 1: Computed-at-module-load timing**
|
||||
- *Risk*: `import.meta.env` is statically replaced at build time; module-load timing in tests using Vite's dev server should match.
|
||||
- *Mitigation*: Standard Vite pattern — no special handling needed. Smoke-test by toggling the env var across two builds.
|
||||
@@ -0,0 +1,70 @@
|
||||
# Bundle the Leaflet marker icon locally
|
||||
|
||||
**Task**: AZ-451_refactor_leaflet_marker_icon
|
||||
**Name**: Replace `unpkg.com` marker icon URL with a Vite-bundled asset
|
||||
**Description**: Stop loading the Leaflet marker icon from `unpkg.com` at runtime. Import the PNG from the already-installed `leaflet` package as a Vite asset so the bundle is air-gap-friendly and the version matches the pinned dependency.
|
||||
**Complexity**: 2 points
|
||||
**Dependencies**: None
|
||||
**Component**: `src/features/flights/mapIcons.ts`, bundled asset under `dist/assets/`
|
||||
**Tracker**: AZ-451
|
||||
**Epic**: AZ-447
|
||||
|
||||
## Problem
|
||||
|
||||
`src/features/flights/mapIcons.ts:18` sets `iconUrl: 'https://unpkg.com/leaflet@1.7.1/dist/images/marker-icon.png'`. This:
|
||||
|
||||
- Breaks AC-N3 / NFT-RES-03 / restriction E1 (air-gap-friendly bundle) — the app fetches from `unpkg.com` on every map mount.
|
||||
- Pins `leaflet@1.7.1` in a URL while `package.json` pins `leaflet@^1.9.4` — a silent version mismatch.
|
||||
- Cannot be intercepted by the e2e `tile-stub` (different host).
|
||||
|
||||
## Outcome
|
||||
|
||||
- No external CDN URL remains in `src/`.
|
||||
- The marker icon renders from a bundled asset path produced by Vite (e.g., `/assets/marker-icon-<hash>.png`).
|
||||
- The icon version matches the pinned `leaflet` package version.
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
- `import markerIcon from 'leaflet/dist/images/marker-icon.png'` in `mapIcons.ts`.
|
||||
- Updating `iconUrl` to use the imported asset.
|
||||
|
||||
### Excluded
|
||||
- Any change to icon size, anchor, or color logic in `mapIcons.ts`.
|
||||
- Adding new marker variants.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: No external CDN**
|
||||
Given the repo after the change,
|
||||
When `grep -r "unpkg.com" src/` runs,
|
||||
Then it produces no matches.
|
||||
|
||||
**AC-2: Asset bundled by Vite**
|
||||
Given a `bun run build`,
|
||||
When the `dist/` directory is inspected,
|
||||
Then a file matching `dist/assets/marker-icon-*.png` exists and `dist/index.html` or a chunk references it.
|
||||
|
||||
**AC-3: Marker still renders**
|
||||
Given a manual smoke against the running dev server,
|
||||
When the `FlightMap` displays a waypoint,
|
||||
Then the marker icon is visible at the expected position (no broken-image placeholder).
|
||||
|
||||
## Blackbox Tests
|
||||
|
||||
| AC Ref | Initial Data / Conditions | What to Test | Expected Behavior | NFR References |
|
||||
|--------|--------------------------|--------------|-------------------|----------------|
|
||||
| AC-1 | Repo state after task lands | `grep -r unpkg.com src/` | No matches | Security / air-gap |
|
||||
| AC-2 | `dist/` after build | `find dist/assets -name 'marker-icon-*.png'` | Exactly one match | Compat |
|
||||
| AC-3 | Dev server running | Visual smoke | Marker visible | UX |
|
||||
|
||||
## Constraints
|
||||
|
||||
- Adhere to S9 (Leaflet 1.9.4) — use the pinned package's assets.
|
||||
- The `customIcon` export shape MUST NOT change (callers depend on the `L.divIcon | L.icon` instance).
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
**Risk 1: Type for the PNG import**
|
||||
- *Risk*: TypeScript may complain about `import markerIcon from '...png'` without a module declaration.
|
||||
- *Mitigation*: Vite's default `client.d.ts` (already referenced via `tsconfig.json` "types") provides the declaration. If TS still complains, extend `src/vite-env.d.ts` with the asset declaration.
|
||||
@@ -0,0 +1,85 @@
|
||||
# Add a getApiBase() accessor
|
||||
|
||||
**Task**: AZ-452_refactor_api_base_accessor
|
||||
**Name**: Extract `getApiBase()` for `/api/...` prefix
|
||||
**Description**: Add a thin accessor that prepends an optional base URL to every API request. Default empty string preserves today's behavior. Tests and alternative deployments can supply `VITE_API_BASE_URL`.
|
||||
**Complexity**: 3 points
|
||||
**Dependencies**: None
|
||||
**Component**: `src/api/client.ts`, `src/api/sse.ts`, `src/vite-env.d.ts`
|
||||
**Tracker**: AZ-452
|
||||
**Epic**: AZ-447
|
||||
|
||||
## Problem
|
||||
|
||||
`src/api/client.ts:44` and every `api.get('/api/...')` call site assume the SPA and the suite share the same nginx (production layout). There is no override hook for test scenarios that need to redirect API traffic to a different host (e.g., MSW handlers serving from a separate origin, or e2e runs targeting a specific suite revision).
|
||||
|
||||
## Outcome
|
||||
|
||||
- A `getApiBase()` function is exported from `src/api/client.ts`.
|
||||
- `request()`, `refreshToken()`, and `createSSE()` prepend its result to the URL.
|
||||
- Default `''` preserves every existing call site unchanged.
|
||||
- Tests can override per build via `VITE_API_BASE_URL`.
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
- New `getApiBase()` function in `src/api/client.ts`.
|
||||
- Threading the value through `request()` + `refreshToken()` in `client.ts` and `createSSE()` in `sse.ts`.
|
||||
- `.env.example` and `src/vite-env.d.ts` updated.
|
||||
|
||||
### Excluded
|
||||
- Changing the `api.get` / `api.post` / `api.put` / `api.patch` / `api.delete` / `api.upload` signatures.
|
||||
- Touching the auth header or 401-retry logic.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: Default preserves today's behavior**
|
||||
Given `VITE_API_BASE_URL` is unset,
|
||||
When `api.get('/api/admin/auth/me')` is invoked,
|
||||
Then the fetch URL is `/api/admin/auth/me` (no prefix added).
|
||||
|
||||
**AC-2: Override is honored**
|
||||
Given `VITE_API_BASE_URL=http://test-host:8080` at build time,
|
||||
When `api.get('/api/admin/auth/me')` is invoked,
|
||||
Then the fetch URL is `http://test-host:8080/api/admin/auth/me`.
|
||||
|
||||
**AC-3: SSE honors the same prefix**
|
||||
Given `VITE_API_BASE_URL=http://test-host:8080`,
|
||||
When `createSSE('/api/flights/1/live-gps', ...)` is invoked,
|
||||
Then the `EventSource` URL is `http://test-host:8080/api/flights/1/live-gps?access_token=...`.
|
||||
|
||||
**AC-4: Type declaration present**
|
||||
Given a build,
|
||||
Then `ImportMetaEnv` declares `readonly VITE_API_BASE_URL: string | undefined`.
|
||||
|
||||
## Non-Functional Requirements
|
||||
|
||||
**Reliability**
|
||||
- Default empty-string fallback MUST be used when the env var is undefined or empty — no exceptions thrown at module load.
|
||||
|
||||
**Security**
|
||||
- Adhere to AC-O3 (`credentials:'include'` semantics) and AC-23 (refresh transparency). The override does not affect cookie scope — that is governed by nginx and the browser's same-origin policy.
|
||||
|
||||
## Blackbox Tests
|
||||
|
||||
| AC Ref | Initial Data / Conditions | What to Test | Expected Behavior | NFR References |
|
||||
|--------|--------------------------|--------------|-------------------|----------------|
|
||||
| AC-1 | env unset; `api.get('/api/...')` | Captured fetch URL | Path unchanged | Compat |
|
||||
| AC-2 | env set; `api.get('/api/...')` | Captured fetch URL | Prefix applied | Test override |
|
||||
| AC-3 | env set; `createSSE('/api/...')` | Captured EventSource URL | Prefix applied | Test override |
|
||||
| AC-4 | Build run | `vite-env.d.ts` declaration | Present | Compat |
|
||||
|
||||
## Constraints
|
||||
|
||||
- The API surface of `api.{get,post,put,patch,delete,upload}` and `createSSE` MUST NOT change.
|
||||
- The default branch MUST be `getApiBase() === ''` so no relative-path semantics regress.
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
**Risk 1: Trailing-slash drift in env**
|
||||
- *Risk*: Setting the env to `http://host/` produces `http://host//api/...`.
|
||||
- *Mitigation*: Trim a trailing slash in `getApiBase()` before returning.
|
||||
|
||||
**Risk 2: SSE `access_token` placement**
|
||||
- *Risk*: When the prefix is set, the existing `?access_token=` insertion in `createSSE` must still work for both `url.includes('?')` and not-includes cases.
|
||||
- *Mitigation*: The existing branch logic handles both — re-check after the change.
|
||||
@@ -0,0 +1,71 @@
|
||||
# Wrap login redirect in a thin accessor
|
||||
|
||||
**Task**: AZ-453_refactor_navigate_to_login
|
||||
**Name**: `navigateToLoginImpl()` accessor for the failed-refresh redirect
|
||||
**Description**: Replace the direct `window.location.href = '/login'` write in `request()` with a thin overridable function so tests can verify the redirect call without globally stubbing `window.location`.
|
||||
**Complexity**: 2 points
|
||||
**Dependencies**: None
|
||||
**Component**: `src/api/client.ts`
|
||||
**Tracker**: AZ-453
|
||||
**Epic**: AZ-447
|
||||
|
||||
## Problem
|
||||
|
||||
`src/api/client.ts:34` performs `window.location.href = '/login'` directly after a failed refresh. The DOM API is global; jsdom can intercept it, but a test cannot distinguish "library code redirected" from "user code redirected" without overriding `window.location` globally — a heavy and side-effecty pattern.
|
||||
|
||||
## Outcome
|
||||
|
||||
- A module-level `navigateToLoginImpl: () => void` is introduced, defaulting to the existing redirect.
|
||||
- `setNavigateToLogin(fn)` is exported for test override.
|
||||
- `request()` calls `navigateToLoginImpl()` instead of touching `window.location` directly.
|
||||
- Production behavior is identical (default impl still navigates to `/login`).
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
- `navigateToLoginImpl` and `setNavigateToLogin` additions in `src/api/client.ts`.
|
||||
- One call-site change inside `request()`.
|
||||
|
||||
### Excluded
|
||||
- Any change to the 401 / refresh flow itself.
|
||||
- Any change to the redirect target string (`/login`).
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: Production default unchanged**
|
||||
Given the default implementation,
|
||||
When a 401 returns and `refreshToken()` resolves to `false`,
|
||||
Then `window.location.href === '/login'` after the failed-refresh branch executes.
|
||||
|
||||
**AC-2: Override is honored**
|
||||
Given a test calls `setNavigateToLogin(spy)`,
|
||||
When a 401 returns and refresh fails,
|
||||
Then `spy` is called exactly once and `window.location.href` is NOT mutated by `client.ts`.
|
||||
|
||||
**AC-3: API surface preserved**
|
||||
Given the rest of the module after the change,
|
||||
When the exports are enumerated,
|
||||
Then `setToken`, `getToken`, `api`, `setNavigateToLogin` are present and no existing export is renamed or removed.
|
||||
|
||||
## Non-Functional Requirements
|
||||
|
||||
**Compatibility**
|
||||
- Tests that already override `window.location` continue to work (the default impl still goes through it).
|
||||
|
||||
## Blackbox Tests
|
||||
|
||||
| AC Ref | Initial Data / Conditions | What to Test | Expected Behavior | NFR References |
|
||||
|--------|--------------------------|--------------|-------------------|----------------|
|
||||
| AC-1 | No override; 401 + failed refresh | `window.location.href` after the call | `/login` | Compat |
|
||||
| AC-2 | `setNavigateToLogin(spy)` then 401 + failed refresh | Spy call count + `window.location.href` | Spy = 1; href untouched by `client.ts` | Compat |
|
||||
| AC-3 | Module exports diff | Existing exports | Present | API stability |
|
||||
|
||||
## Constraints
|
||||
|
||||
- Allowed-change category: "Wrap global singletons in thin accessors that tests can override" (existing-code flow Step 4 allowed list).
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
**Risk 1: Forgetting to reset the override between tests**
|
||||
- *Risk*: If a test sets the override and doesn't reset it, later tests see the wrong impl.
|
||||
- *Mitigation*: Document in `_docs/02_document/tests/test-data.md` that helpers must reset accessors in their teardown. Optional follow-up: a `resetNavigateToLogin()` helper — but adding it expands scope; defer unless decompose requires it.
|
||||
@@ -0,0 +1,64 @@
|
||||
# Document setToken/getToken for test override
|
||||
|
||||
**Task**: AZ-454_refactor_document_token_accessor
|
||||
**Name**: JSDoc the existing token accessor
|
||||
**Description**: Add a JSDoc to the existing `setToken` / `getToken` accessors so a future maintainer doesn't delete them as "dead code". They are dynamically used by test helpers — the comment captures that intent without any behavioral change.
|
||||
**Complexity**: 1 point
|
||||
**Dependencies**: None
|
||||
**Component**: `src/api/client.ts` (comment-only edit)
|
||||
**Tracker**: AZ-454
|
||||
**Epic**: AZ-447
|
||||
|
||||
## Problem
|
||||
|
||||
`src/api/client.ts:1-9` exposes `setToken(token: string | null)` and `getToken()` over the module-level `accessToken`. The pattern is intentional (AC-02 forbids bearer storage; AC-23 requires refresh transparency; tests need a hook to seed a bearer without going through the login flow). The intent is undocumented today, so the dead-code reaper could delete these one day per `coderule.mdc` ("Dead code rots — delete it").
|
||||
|
||||
The `coderule.mdc` rule already lists "test fixtures used only by currently-skipped tests" as a reason NOT to delete, but with no tests in source yet, the accessor looks dead to a casual reader.
|
||||
|
||||
## Outcome
|
||||
|
||||
- `setToken` has a JSDoc explaining: (a) it's the bearer single-source-of-truth, (b) it MUST NOT persist to storage, (c) tests override it via `setToken('test-bearer-xyz')`, (d) it's referenced by `_docs/02_document/tests/test-data.md`.
|
||||
- `getToken` has a one-line JSDoc cross-referencing `setToken`.
|
||||
- No behavioral change.
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
- JSDoc additions on `setToken` and `getToken`.
|
||||
|
||||
### Excluded
|
||||
- Any code change.
|
||||
- Adding test-only exports beyond what already exists.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: JSDoc on setToken**
|
||||
Given the repo after the change,
|
||||
When `src/api/client.ts:3` is read,
|
||||
Then a JSDoc above `setToken` mentions: test-override intent + the AC-02 storage rule + a reference to `_docs/02_document/tests/test-data.md`.
|
||||
|
||||
**AC-2: JSDoc on getToken**
|
||||
Given the repo after the change,
|
||||
When `src/api/client.ts:7` is read,
|
||||
Then a JSDoc above `getToken` cross-references `setToken`.
|
||||
|
||||
**AC-3: No code change**
|
||||
Given a `git diff src/api/client.ts`,
|
||||
When the diff is inspected,
|
||||
Then every diff line is inside a comment block (JSDoc); no executable code is altered.
|
||||
|
||||
## Blackbox Tests
|
||||
|
||||
| AC Ref | Initial Data / Conditions | What to Test | Expected Behavior | NFR References |
|
||||
|--------|--------------------------|--------------|-------------------|----------------|
|
||||
| AC-1 | Repo after change | Presence + content of JSDoc on `setToken` | Documented intent | — |
|
||||
| AC-2 | Repo after change | Presence of JSDoc on `getToken` | Documented cross-ref | — |
|
||||
| AC-3 | `git diff` | Diff content | Only comment lines | Code stability |
|
||||
|
||||
## Constraints
|
||||
|
||||
- The comment MUST NOT introduce TODOs, FIXMEs, or speculative future work — just the existing-intent rationale.
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
None — comment-only change.
|
||||
Reference in New Issue
Block a user