AI Implementation feature(871): Admin Area System: Database Backup/Restore and Danger Zone (#61)
This commit was merged in pull request #61.
This commit is contained in:
@@ -0,0 +1,30 @@
|
||||
# Implementation Plan: Job 871 — follow-up tweaks
|
||||
|
||||
## 1. Architectural Reconnaissance
|
||||
- The previous implementation introduced a `RestoreService` that takes a `ConfigService` constructor parameter but does not retain it on `this`. Internally `stageArchive()` calls `resolveSystemStagingDir(this.requireConfig())`, where `requireConfig()` returns the parameter via a cast `(this as any).configService`. This works but is a hack — the parameter should simply be retained as a class field.
|
||||
- The previous `DangerZoneService.wipeChallenges()` snapshots `<UPLOAD_DIR>/challenges` to a side directory, transactionally deletes the DB rows, and then removes the snapshot. It currently never actually deletes the live `<UPLOAD_DIR>/challenges` directory on success, so the filesystem files persist even though the DB rows are gone. The user wants the live directory physically removed after the DB commit succeeds, with the snapshot retained as the rollback source until the disk delete is complete so that a disk-delete failure can be restored from it.
|
||||
|
||||
## 2. Impacted Files
|
||||
- **To Modify:**
|
||||
- `backend/src/modules/admin/system/restore.service.ts` — retain `ConfigService` on the instance and stop using the `requireConfig()` shim; use it directly in `stageArchive()`.
|
||||
- `backend/src/modules/admin/system/danger-zone.service.ts` — in `wipeChallenges()`, after the DB transaction commits, physically remove the live `<UPLOAD_DIR>/challenges` directory, and only after that succeeds remove the snapshot; on any failure after the snapshot was created, copy the snapshot back into place so a disk-delete failure can be reversed.
|
||||
|
||||
## 3. Proposed Changes
|
||||
1. **`restore.service.ts` cleanup:**
|
||||
- Convert the constructor's `config: ConfigService` parameter into `private readonly configService: ConfigService` so it is stored on the instance.
|
||||
- At the top of the constructor body, capture each derived value (`uploadDir`, `databasePath`, `stageTtlMs`, `restoreUploadLimit`) from `configService` once.
|
||||
- Remove the private `requireConfig()` shim method entirely.
|
||||
- In `stageArchive()`, change the call to `resolveSystemStagingDir(this.configService)`.
|
||||
2. **`danger-zone.service.ts` physical delete:**
|
||||
- In `wipeChallenges()`, after the `dataSource.transaction(...)` block returns successfully (we already have the row counts in `counts`), add: `FilesystemTransactionService.rmSafe(challengesDir)` to physically remove the live `<UPLOAD_DIR>/challenges` directory and its contents.
|
||||
- If that rm succeeds, then `rmSafe` the `stagingBackup` snapshot.
|
||||
- If the rm of `challengesDir` itself throws, retain the snapshot so the catch branch can restore from it (keep `staged = true`), rethrow the error to enter the catch branch, and from the existing catch branch copy `stagingBackup` back into `challengesDir` exactly as before. Update the in-method narrative so that order of operations matches: stage → db commit → live rm → snapshot rm.
|
||||
- Do not change the DB ordering: the transaction must commit before any filesystem delete so a DB failure does not leave a partial filesystem state.
|
||||
|
||||
## 4. Test Strategy
|
||||
- **Target Unit Test File:**
|
||||
- Existing `tests/backend/admin-system-danger.spec.ts` already verifies that after `wipe-challenges` the `challenge` table is empty. Extend it with one assertion: after a successful wipe, the live `<UPLOAD_DIR>/challenges` directory is also physically empty/absent.
|
||||
- Existing `tests/backend/admin-system-backup.spec.ts` and `tests/backend/admin-system-restore-validation.spec.ts` already validate `BackupService` and `RestoreService.stageArchive()` behavior; no behavioral changes are expected from the constructor cleanup since `stageArchive()` still resolves the same staging directory. Re-run those tests after the edit to confirm no regressions.
|
||||
- No new frontend tests are required — these are strictly backend concerns.
|
||||
- **Mocking Strategy:** Reuse the existing per-suite isolated temp upload directories (`mkdtempSync(path.join(os.tmpdir(), 'hipctf-…'))`) and assert against the live path with `fs.existsSync`/`fs.readdirSync`. No new mocking infrastructure is needed; the danger-zone logic now relies on the same primitives (fs + SQLite transaction) the existing tests already exercise.
|
||||
- **Verification:** Run `npm test` to confirm both `admin-system-danger.spec.ts` and the related system suites stay green; also run `npx --workspace backend tsc -p tsconfig.build.json --noEmit` to confirm the `requireConfig()` removal does not break the build.
|
||||
@@ -1,48 +0,0 @@
|
||||
# Implementation Plan: Job 915 — Enable Admin Blog Post Deletion Confirmation
|
||||
|
||||
## Status
|
||||
|
||||
The requested job is **not fully implemented**. The backend DELETE endpoint, persistence service, Angular API client, parent delete workflow, and confirmation modal already exist, but the parent binds the modal's disabled state to the selected post rather than the request-in-flight state. The UI therefore cannot complete an otherwise wired deletion flow.
|
||||
|
||||
## 1. Architectural Reconnaissance
|
||||
|
||||
- **Codebase style & conventions:** Node.js npm-workspace monorepo using TypeScript, NestJS 10 on the backend, and Angular 17 standalone components on the frontend. Angular admin pages use smart/container components with `signal()` state, signal inputs/outputs, `ChangeDetectionStrategy.OnPush`, and Angular control-flow syntax. The admin blog parent owns service calls and modal lifecycle; `BlogDeleteModalComponent` is presentational and only receives inputs/emits cancel/confirm events.
|
||||
- **Data Layer:** TypeORM `BlogPostEntity` backed by SQLite via `better-sqlite3`. Blog deletion is already implemented in `backend/src/modules/blog/admin-blog.service.ts:65-69` with a repository lookup, not-found error, and `delete({ id })`; no schema or migration change is required.
|
||||
- **Test Framework & Structure:** Root Jest multi-project configuration in `tests/jest.config.js`, with frontend tests under the dedicated `tests/frontend/` folder and backend tests under `tests/backend/`. The root `npm test` command runs all projects; focused frontend execution is available through `npm run test:frontend`. Existing blog tests are mostly pure-logic/source-contract tests, while the API suite already covers DELETE persistence and 404 behavior.
|
||||
- **Required Tools & Dependencies:** No new package or system dependency is required. Existing Node/npm, Angular, Jest, jsdom, ts-jest, and repository dependencies are sufficient. `setup.sh` needs no change; it already installs dependencies, rebuilds `better-sqlite3`, and builds both workspaces. No `/data` fixture or persistent test asset is needed because this is a frontend state-binding fix and existing backend tests use in-memory SQLite.
|
||||
|
||||
## 2. Impacted Files
|
||||
|
||||
- **To Modify:**
|
||||
- `frontend/src/app/features/admin/blog/blog.component.ts` — bind the delete modal's `deleting` input to `actionInFlight()` rather than `deleting() !== null`, preserving the selected post while enabling confirmation before the request starts and disabling it during the request.
|
||||
- `tests/frontend/blog-admin-delete.spec.ts` — add a minimal focused regression test in the dedicated frontend test folder covering the modal binding and deletion state contract.
|
||||
- **To Create:**
|
||||
- `tests/frontend/blog-admin-delete.spec.ts` — new focused frontend regression test file, if no existing delete-modal/admin-page test is suitable for extension.
|
||||
|
||||
No backend controller, service, entity, DTO, route, database migration, API client, delete modal component, documentation, dependency manifest, or setup script needs modification.
|
||||
|
||||
## 3. Proposed Changes
|
||||
|
||||
1. **Database / Schema Migration:**
|
||||
- No change. `AdminBlogService.remove()` already deletes the requested `blog_post` row and returns not-found for a missing ID.
|
||||
- Do not add migration or seed data. The expected persistence behavior is already provided by the existing DELETE API and is covered by `tests/backend/blog-admin.spec.ts:220-240`.
|
||||
|
||||
2. **Backend Logic & APIs:**
|
||||
- No change. `AdminBlogController.remove()` already exposes `DELETE /api/v1/admin/blog/posts/:id`, applies the admin guard/role protection, and returns HTTP 204.
|
||||
- `BlogApiService.deletePost()` already calls the endpoint with the encoded ID and credentials.
|
||||
- The parent `AdminBlogComponent.onDeleteConfirm()` already sets `actionInFlight` to true before awaiting the DELETE request, closes the modal after success, reloads the complete admin list, reports `Post deleted.`, and resets the in-flight signal in `finally`. Its error path keeps the modal open and exposes the server message.
|
||||
|
||||
3. **Frontend UI Integration:**
|
||||
- In `frontend/src/app/features/admin/blog/blog.component.ts:111-118`, change only the `[deleting]` binding from `[deleting]="deleting() !== null"` to `[deleting]="actionInFlight()"`.
|
||||
- Keep `[post]="deleting()"` unchanged so the modal continues displaying the selected title and the parent retains the target ID until success or cancel.
|
||||
- Keep the existing `actionInFlight` signal shared by create, edit, and delete operations. When `openDelete()` sets the selected post and opens the modal, `actionInFlight()` remains false, so `post-delete-ok` is enabled. When `onDeleteConfirm()` begins, it becomes true and the modal disables Delete while awaiting the request. On success, `closeModal()` clears the selection and `load()` refreshes the table; on failure, the selection/modal remain visible and the button is re-enabled after `finally` so retry is possible.
|
||||
- Do not change `BlogDeleteModalComponent`: its `[disabled]="deleting()"` behavior is already correct when supplied the proper request-state signal.
|
||||
|
||||
## 4. Test Strategy
|
||||
|
||||
- **Target Unit Test File:** Add `tests/frontend/blog-admin-delete.spec.ts` under the existing dedicated Jest frontend test root. The test should be minimal and focused on the regression contract, not a browser/UI visual test.
|
||||
- **Test cases:**
|
||||
1. Read/verify the admin component template contract that the delete modal receives `actionInFlight()` for its `deleting` input, and that it no longer derives the disabled state from the selected `deleting()` post. This matches the repository's existing lightweight source-contract testing style and directly catches reintroduction of the bug.
|
||||
2. Optionally instantiate the standalone `BlogDeleteModalComponent` with signal inputs using the existing Jest/jsdom Angular setup, set `deleting` false and true via `fixture.componentRef.setInput`, run `detectChanges()`, and assert the confirm button is enabled before the request and disabled during it. Keep this only if the current test environment supports standalone component compilation without introducing infrastructure; otherwise the source-contract test is sufficient for this one-line wiring regression.
|
||||
- **Mocking Strategy:** Do not involve HTTP, the backend, SQLite, authentication, or real navigation in the focused frontend test. If component instantiation is used, provide only the modal's standalone imports and use signal input APIs; no service mock is needed because the modal emits outputs and contains no service dependency. The existing backend DELETE integration test remains the persistence/error boundary test.
|
||||
- **Verification:** Run the single root command `npm test` after implementation so both existing backend API coverage and the new frontend regression test execute. The implementer should also run the repository's available frontend-focused Jest project if needed during iteration; no new command or setup change is required.
|
||||
Reference in New Issue
Block a user