AI Implementation feature(821): First-Start Create Admin Modal (#2)
This commit was merged in pull request #2.
This commit is contained in:
@@ -1,101 +0,0 @@
|
||||
# Implementation Plan: WAL journal mode + OpenAPI 3.1 — Status check
|
||||
|
||||
## 0. Scope vs. Current Branch State
|
||||
|
||||
This reviewer request asks for three changes that were **already implemented
|
||||
and committed** in `de27a8b` on the `feature-820-1784637300954` branch. The
|
||||
code currently in the working tree (and in HEAD) matches the request verbatim:
|
||||
|
||||
1. ✅ `InitSchema` migration (`backend/src/database/migrations/1700000000000-InitSchema.ts`)
|
||||
already contains the WAL PRAGMA at line 8 — immediately after the existing
|
||||
`PRAGMA foreign_keys = ON` (line 7).
|
||||
2. ✅ `DatabaseInitService` (`backend/src/database/database-init.service.ts`)
|
||||
already enforces WAL on every startup via a private `ensureWalMode()`
|
||||
method that runs after `dataSource.initialize()`. It checks the current
|
||||
`journal_mode` and applies `PRAGMA journal_mode = WAL` if not already
|
||||
`wal`; idempotent and safe to call repeatedly.
|
||||
3. ✅ `main.ts` (`backend/src/main.ts` lines 83–94) now pipes the
|
||||
NestJS-generated OpenAPI document through a custom `toOpenApi31()`
|
||||
converter (`backend/src/common/utils/openapi31.ts`) that emits
|
||||
`openapi: '3.1.0'` and rewrites every legacy `nullable: true` marker
|
||||
into the JSON Schema 2020-12 type union.
|
||||
|
||||
Per the system instruction "If the feature is ALREADY fully implemented, you
|
||||
MUST include the exact marker text **[ALREADY_IMPLEMENTED]** at the top or in
|
||||
the status section of your plan file. Explain in the plan exactly which
|
||||
files/methods already contain the implementation", this plan documents the
|
||||
existing implementation rather than adding duplicate code.
|
||||
|
||||
## 1. Architectural Reconnaissance
|
||||
|
||||
- **Migration** (`backend/src/database/migrations/1700000000000-InitSchema.ts:7-8`):
|
||||
```ts
|
||||
await queryRunner.query(`PRAGMA foreign_keys = ON;`);
|
||||
await queryRunner.query(`PRAGMA journal_mode = WAL;`);
|
||||
```
|
||||
- **Warm-DB enforcement** (`backend/src/database/database-init.service.ts:34-78`):
|
||||
- `init()` calls `ensureWalMode()` after `dataSource.initialize()`.
|
||||
- `ensureWalMode()` queries `PRAGMA journal_mode`, switches to WAL if not
|
||||
already set, logs the transition.
|
||||
- **OpenAPI 3.1 pipeline** (`backend/src/main.ts:83-94`):
|
||||
```ts
|
||||
const swaggerConfig = new DocumentBuilder()...
|
||||
const rawDocument = SwaggerModule.createDocument(app, swaggerConfig) as Record<string, any>;
|
||||
const document = toOpenApi31(rawDocument);
|
||||
SwaggerModule.setup('api/docs', app, document, { jsonDocumentUrl: 'api/docs-json' });
|
||||
```
|
||||
- `backend/src/common/utils/openapi31.ts` exports `toOpenApi31(doc)`.
|
||||
- Rewrites `nullable: true` → `type: ['<orig>', 'null']` recursively in
|
||||
`components.schemas`, `paths`, parameters, request/response bodies,
|
||||
`properties`, `items`, `oneOf/anyOf/allOf`, and nested `schema` keys.
|
||||
- Preserves `$ref` nodes.
|
||||
- **Tests** (`tests/backend/openapi31.spec.ts`, 7 tests): all pass.
|
||||
- **Full suite**: 83/83 tests across 18 suites.
|
||||
|
||||
## 2. Impacted Files
|
||||
|
||||
None — all changes are already in place at HEAD.
|
||||
|
||||
| File | Status |
|
||||
| --- | --- |
|
||||
| `backend/src/database/migrations/1700000000000-InitSchema.ts` | ✅ line 8 |
|
||||
| `backend/src/database/database-init.service.ts` | ✅ lines 34-78 |
|
||||
| `backend/src/common/utils/openapi31.ts` | ✅ exists (created in `de27a8b`) |
|
||||
| `backend/src/main.ts` | ✅ lines 83-94 |
|
||||
| `tests/backend/openapi31.spec.ts` | ✅ exists, 7 tests passing |
|
||||
|
||||
## 3. Proposed Changes
|
||||
|
||||
No code changes required. If a follow-up task wants to extend the existing
|
||||
implementation (e.g., add an `info.license` field, support `webhooks`, or
|
||||
add a Swagger UI bundle switch), that would be a separate plan.
|
||||
|
||||
## 4. Test Strategy
|
||||
|
||||
- The existing `tests/backend/openapi31.spec.ts` (7 tests) locks the
|
||||
conversion behaviour.
|
||||
- The full suite `npm test` passes 83/83 across 18 suites.
|
||||
- A live-server smoke (documented in the previous turn's commit message)
|
||||
confirms `openapi: '3.1.0'`, 13 paths, zero `nullable: true` occurrences,
|
||||
WAL applied on cold start AND confirmed on warm restart.
|
||||
|
||||
## 5. Persistent `/data` Usage
|
||||
|
||||
No change. WAL persists to the file-backed SQLite DB; the SQLite WAL mode is
|
||||
persisted in the database header, so subsequent connections inherit it.
|
||||
|
||||
## 6. Definition of Done (already met)
|
||||
|
||||
- `git log --oneline -3` shows commit `de27a8b` "WAL journal mode + OpenAPI 3.1"
|
||||
- `npm test` → 83/83 passing
|
||||
- `npm run build` → backend + frontend succeed
|
||||
- `GET /api/docs-json` returns `openapi: "3.1.0"`
|
||||
- Cold start log: `PRAGMA journal_mode set (was='delete', now='wal')`
|
||||
- Warm start log: `PRAGMA journal_mode=wal (already set)`
|
||||
- Zero `nullable: true` occurrences in the served JSON document
|
||||
|
||||
**[ALREADY_IMPLEMENTED]** — All three requested changes exist on the branch
|
||||
in commits `de27a8b` (WAL + OpenAPI 3.1). No further code changes are needed
|
||||
unless the reviewer wants to extend the implementation with additional
|
||||
OpenAPI 3.1 features (webhooks, license, contact, etc.) or additional SQLite
|
||||
PRAGMAs.
|
||||
@@ -0,0 +1,86 @@
|
||||
# Implementation Plan: Fix Angular build error — Property 'code' does not exist on type 'CreateAdminResult'
|
||||
|
||||
## 1. Architectural Reconnaissance
|
||||
|
||||
- **Codebase style & conventions:** Angular 17 standalone components, signals, typed reactive forms. `frontend/tsconfig.json` uses `strict: false` and `strictTemplates: false`. The project still uses `@angular/compiler` (ngc) which performs its own type checking on top of TS narrowing rules.
|
||||
- **Data Layer:** N/A — this is a frontend-only type-narrowing fix.
|
||||
- **Test Framework & Structure:** `tests/jest.config.js`. Tests live under `tests/frontend/`. All compile via `npm test`.
|
||||
- **Required Tools & Dependencies:** No new dependencies. The fix is purely TypeScript / component-internal.
|
||||
|
||||
## 2. Impacted Files
|
||||
|
||||
- **To Modify:** `/repo/frontend/src/app/features/setup/setup-create-admin.component.ts` — fix the type-narrowing that Angular's compiler refuses to accept.
|
||||
- **No new files, no test changes, no docs changes.**
|
||||
|
||||
## 3. Proposed Changes
|
||||
|
||||
### Root cause
|
||||
|
||||
In `setup-create-admin.component.ts:88-117` the `submit()` method awaits the service, then reads `result.ok`. After the `if (result.ok) { ... return; }` branch, TS should narrow `result` to `CreateAdminFailure` (which has `code`). However Angular's compiler (used by `ng build`) reports:
|
||||
|
||||
```
|
||||
TS2339: Property 'code' does not exist on type 'CreateAdminResult'.
|
||||
Property 'code' does not exist on type 'CreateAdminSuccess'.
|
||||
```
|
||||
|
||||
at the two reads `result.code` on lines 109 and 116. With `strict: false` + `strictTemplates: false`, narrowing across an awaited union *should* succeed, but Angular's compiler's narrowing model is stricter in some code paths (notably after `await` of an external service) and conservatively widens back to the full union. The error message proves it: type is `CreateAdminResult`, not `CreateAdminFailure`.
|
||||
|
||||
### Minimal fix
|
||||
|
||||
Replace the discriminated-union narrowing with an explicit type guard in the service and an `else` branch in the component. This avoids relying on the compiler's narrowing at all.
|
||||
|
||||
### Step-by-step
|
||||
|
||||
1. **Add a type predicate to the service** (`setup-create-admin.service.ts`):
|
||||
- Add `export function isCreateAdminFailure(r: CreateAdminResult): r is CreateAdminFailure { return !r.ok; }`.
|
||||
- No behaviour change.
|
||||
|
||||
2. **Refactor `submit()`** in `setup-create-admin.component.ts`:
|
||||
- Replace the `if (result.ok) { ... return; } ... if (result.code === ...) ...` sequence with an `if (result.ok) { ... return; } const failure = result;` pattern. Because `result.ok` is `true | false`, the negation narrows the local copy identically. The safest cross-compiler approach is:
|
||||
```ts
|
||||
if (result.ok) {
|
||||
this.auth.setSession(result.accessToken, result.user);
|
||||
this.bootstrap.markInitialized();
|
||||
await this.router.navigateByUrl('/challenges');
|
||||
return;
|
||||
}
|
||||
const failure: CreateAdminFailure = result; // explicit local of narrowed type
|
||||
if (failure.code === 'USERNAME_TAKEN') {
|
||||
this.serverError.set('Username already exists');
|
||||
this.serverErrorCode.set('USERNAME_TAKEN');
|
||||
return;
|
||||
}
|
||||
this.serverError.set('Setup failed, please retry');
|
||||
this.serverErrorCode.set(failure.code);
|
||||
```
|
||||
- Import `CreateAdminFailure` from `./setup-create-admin.service`.
|
||||
- The cast `const failure: CreateAdminFailure = result` after the `return` is safe (TS will assert assignability; if the union ever changes it becomes a compile error here rather than two ambiguous ones).
|
||||
|
||||
Alternatively, an explicit cast: `const failure = result as CreateAdminFailure;`. Either is acceptable; the `as`-cast is one token shorter and more idiomatic. Choose:
|
||||
|
||||
```ts
|
||||
const failure = result as CreateAdminFailure;
|
||||
```
|
||||
|
||||
3. **No HTML/CSS/service file changes.**
|
||||
|
||||
## 4. Test Strategy
|
||||
|
||||
- **Target Unit Test File:** no changes. The existing `tests/frontend/setup-create-admin.spec.ts` already covers the validator and rules; the build error is a compile-time-only issue that the existing tests do not exercise.
|
||||
- **Mocking Strategy:** N/A.
|
||||
- **Verification:**
|
||||
1. `npm --workspace frontend run build` must complete without `TS2339` errors.
|
||||
2. `npm test` must still pass (20 suites, 97 tests).
|
||||
- **Do not** touch any test file. The fix is internal to the component.
|
||||
|
||||
## 5. Risk / Non-Goals
|
||||
|
||||
- **No behaviour change.** The runtime control flow is identical; the change only adds an explicit local that the compiler can definitively narrow.
|
||||
- **No public API change.** `CreateAdminResult`, `CreateAdminSuccess`, `CreateAdminFailure` shapes remain as defined.
|
||||
- **No docs change required** — implementation detail only.
|
||||
|
||||
## 6. Execution Checklist
|
||||
|
||||
1. Edit `setup-create-admin.component.ts`: import `CreateAdminFailure`, refactor lines 102-116 to use `const failure = result as CreateAdminFailure;` after the success branch returns.
|
||||
2. Run `npm --workspace frontend run build` and confirm 0 errors.
|
||||
3. Run `npm test` and confirm `Test Suites: 20 passed, 20 total`.
|
||||
Reference in New Issue
Block a user