[AZ-448] [AZ-449] [AZ-453] Externalize OWM config; wrap login redirect

Batch 2 of testability refactor under epic AZ-447. All three changes are
minimal-surgical and preserve production behavior.

AZ-448 (C01) — Externalize OWM API key
- src/features/flights/flightPlanUtils.ts: read VITE_OWM_API_KEY at call
  time; if unset, getWeatherData returns null (matches the existing
  try/catch fallback contract, AC-3).
- Hardcoded literal removed; grep src/ for the old key returns no hits
  (AC-2 / NFT-SEC-09 static-string check now green).
- AC-1 honored: when the key is set, the outbound URL contains
  appid=<key>.

AZ-449 (C02) — Externalize OWM base URL
- Same call site reads VITE_OWM_BASE_URL with trim-trailing-slash
  normalization; falls back to the public api.openweathermap.org/data/2.5
  endpoint when unset (AC-1).
- Stub-friendly: VITE_OWM_BASE_URL=http://owm-stub:8081/data/2.5
  redirects every call to the e2e stub (AC-2).

AZ-453 (C06) — Wrap login redirect in setNavigateToLogin accessor
- src/api/client.ts: navigateToLoginImpl module-level fn defaults to the
  existing window.location.href = '/login' write; setNavigateToLogin(fn)
  lets tests assert "redirect invoked" without globally stubbing
  window.location.
- request() now calls navigateToLoginImpl() instead of writing
  window.location directly.

Batch 1 task specs (AZ-450/451/452/454) moved from
_docs/02_tasks/todo/ to _docs/02_tasks/done/.

State pointer advanced to refactor Phase 4 (implement, batch 2 of 2).

Static checks:
- bun run tsc --noEmit: 0 errors
- grep '335799082893fad97fa36118b131f919' src/: 0 hits
- grep 'window.location.href' src/: 2 hits, both inside the
  navigateToLoginImpl default (jsdoc + the default impl body) — no
  caller writes window.location directly.

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Oleksandr Bezdieniezhnykh
2026-05-11 00:44:00 +03:00
parent db181043ca
commit ed81034511
7 changed files with 30 additions and 6 deletions
@@ -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,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.