From c3b3e66f5175c31e2d481230becb5c32906fd8b3 Mon Sep 17 00:00:00 2001 From: Samuel Olabode Date: Wed, 12 Aug 2026 17:20:14 -0600 Subject: [PATCH 1/5] fix: improve error handling for OAuth token exchange failures --- .../guides/auth_storage_guide/scanDeletion.md | 459 ++++++++++++++++++ 1 file changed, 459 insertions(+) create mode 100644 docs/guides/auth_storage_guide/scanDeletion.md diff --git a/docs/guides/auth_storage_guide/scanDeletion.md b/docs/guides/auth_storage_guide/scanDeletion.md new file mode 100644 index 0000000..d4ebe48 --- /dev/null +++ b/docs/guides/auth_storage_guide/scanDeletion.md @@ -0,0 +1,459 @@ +# Delete individual scans — implementation guide (closes [#112](https://github.com/codrlabs/vizably/issues/112)) + +Step-by-step plan so you can implement **per-scan delete** yourself: remove one +saved report from the user’s store without wiping the account or other scans. + +Related sources of truth: + +- [`accountStorageContract.md`](./accountStorageContract.md) — on-disk layout, + truth vs cache, concurrency +- [`githubGoogleAuthStorageImplementation.md`](./githubGoogleAuthStorageImplementation.md) + — auth/storage API conventions +- [`TODO.md`](./TODO.md) — phase checklist (add a scan-delete checkbox when you ship) +- Architecture intent: [`docs/plans/architecture-map.md`](../../plans/architecture-map.md) + (per-row “Delete this scan?” → `DELETE /api/scans/:id`) +- Issue: [codrlabs/vizably#112](https://github.com/codrlabs/vizably/issues/112) + +--- + +## 0. What “delete this scan” means + +Vizably has **no scan database**. Saved reports live in storage the user owns +(GitHub repo today; Google Drive later). Deleting one scan means: + +1. Remove that scan’s immutable file: `scans/_.json`. +2. Update rebuildable caches so the dashboard no longer lists it: + - drop the row from `scans/index.json` + - refresh `vizably.json` → `summary.scanCount` / `lastScanAt` +3. Leave every other scan file, the account manifest identity, and the repo itself alone. + +**Not in scope for #112** + +- Whole-account wipe / optional repo delete (that’s [#82](https://github.com/codrlabs/vizably/issues/82) / account deletion). +- Bulk “delete all” on Account settings (still a later phase). +- Rewriting Git history — after delete, GitHub history may still contain the blob; + disclose that lightly in UI copy if you mention permanence. + +--- + +## 1. Investigate the current tree (baseline) + +Confirm these facts before coding (re-check if the branch has drifted): + +| Location | Today’s behaviour | +|----------|-------------------| +| `backend/routes/scan.js` | `GET /scans`, `GET /scans/:id` only — **no DELETE**. | +| `backend/controllers/scanController.js` | `getSavedScans` / `getSavedScan`; no delete handler. | +| `StorageService` | `saveScanResults`, `getScanById`, `loadAccount`, `_reconcileGitHubIndex`. **No `deleteScanById`.** | +| `_writeGitHubFiles` / ViaGit / ViaContents | Create/update only. **No `file.delete` / `repos.deleteFile` yet** on `main`-based branches (unless you already merged #82 wipe helpers). | +| `frontend/src/lib/apiClient.js` | `listScans()`, `getSavedScan(id)` — **no `deleteScan`.** | +| `frontend/src/views/DashboardView.jsx` | Rows open on click; footer copy says “export, or delete” but **there is no delete control.** | +| `frontend/src/App.jsx` | `openSaved` → `getSavedScan`; list via `listScans` + `mergeAccountUpdate`. | + +**GitNexus:** before editing a symbol, run impact analysis +(`impact({ target, direction: "upstream" })`) and warn on HIGH/CRITICAL. Before +commit, run `detect_changes`. + +--- + +## 2. Product sequence + +``` +Dashboard row → user clicks Delete (not the row open target) + │ + ▼ +Confirm: “Delete this scan?” (irreversible for the Vizably store) + │ + ▼ +DELETE /api/scans/:id + │ + ▼ +StorageService.deleteScanById + → delete scans/_.json + → rewrite scans/index.json (without that id) + → rewrite vizably.json summary caches + │ + ▼ +Response { scanCount, scans } + │ + ▼ +App merges into client profile → row disappears +``` + +UX notes: + +- Confirm before calling the API (inline confirm or small dialog — match existing + Account danger-zone style if you can). +- Delete control must **not** trigger `onOpen` (stop propagation on the button). +- On failure: keep the row, show a clear error; do not pretend it succeeded. +- Optional: if the user is viewing `/results?scanId=`, navigate away + after a successful delete (nice-to-have, not required for MVP). +- Keyboard: delete control should be focusable; row remains activatable with Enter/Space. + +--- + +## 3. On-disk contract (what you mutate) + +From [`accountStorageContract.md`](./accountStorageContract.md): + +``` +/ +├── vizably.json # identity + settings + summary CACHE +└── scans/ + ├── index.json # dashboard list CACHE + └── _.json # immutable truth — DELETE this one file +``` + +**Truth:** the scan file. +**Caches:** `index.json` and `summary.scanCount` / `lastScanAt`. +If a delete updates the file but fails to update the index, the next +`loadAccount` / `_reconcileGitHubIndex` drops orphan index rows (and can drop a +missing file’s row). Prefer **one atomic commit** with all three changes when +possible. + +Finding the file (same rule as `getScanById`): + +1. List `scans/` via `_listGitHubDirectory`. +2. Match `entry.name.startsWith(\`${scanId}_\`) && entry.name.endsWith('.json')` + and `entry.name !== 'index.json'`. +3. Path = `scans/${entry.name}`; keep `entry.sha` for Contents API deletes. + +--- + +## 4. Prerequisite — teach `_writeGitHubFiles` how to delete + +On a stock `main` tree, writes only **create/update**. Single-scan delete needs +a way to remove a blob in the same batch as index/manifest updates. + +### 4.1 Recommended shape + +Support file descriptors like: + +```js +{ path: 'scans/…json', delete: true, sha: '' } +``` + +alongside the existing `{ path, content, sha? }` updates. + +### 4.2 ViaContents (`_writeGitHubFilesViaContents`) + +When `file.delete`: + +```js +await octokit.rest.repos.deleteFile({ + owner, repo, path: file.path, message, sha, branch, +}); +``` + +Skip if the file is already gone (idempotent delete). You already resolve `sha` +from `_readGitHubFile` / the directory listing when missing. + +### 4.3 ViaGit (`_writeGitHubFilesViaGit`) + +Do **not** rely on `createTree` + `sha: null` alone — real GitHub often returns +**404** for that. Prefer one of: + +- **A (simple for #112):** for delete-only or mixed batches that include deletes, + fall through to Contents API (`_shouldFallbackToContentsApi` or skip Git when + any `file.delete`). +- **B (atomic, harder):** recursive `git.getTree`, filter out deleted paths, + `createTree` **without** `base_tree` for the remaining blobs, then commit. + Never send `{"tree":[]}` — GitHub rejects empty trees with **422 Invalid tree + info**. If the rebuild would be empty for some other feature, Contents fallback. + +For #112 you almost always leave other files (`index.json` rewrite + other +scans), so empty-tree is unlikely; Contents delete + two updates is enough. + +### 4.4 If #82 (account wipe) already landed + +Reuse its `delete: true` / `deleteFile` / fallback behaviour. Do **not** +re-invent a second delete path. Cherry-pick or merge that plumbing first, then +add `deleteScanById` on top. + +### 4.5 Tests for the prerequisite + +In `backend/tests/storageService.test.js` mock (`createMockGitHubClient`): + +- Implement `repos.deleteFile` if missing. +- Cover: one file deleted, sibling files untouched; missing file is ok. + +--- + +## 5. StorageService — `deleteScanById` + +### 5.1 Signature + +```js +/** + * @param {object} account session-shaped: { storage } (same as getScanById) + * @param {string} scanId + * @param {{ githubClient?: object, githubUserClient?: object }} clients + * @returns {Promise<{ scanCount: number, scans: object[], deletedId: string, path: string }>} + */ +async deleteScanById(account, scanId, clients) +``` + +### 5.2 Algorithm (mirror `saveScanResults` / `getScanById`) + +1. **Google stub** — same as other storage methods: + ```js + if (provider === 'google') { + const err = new Error('Google storage is not available until Phase 3'); + err.status = 501; + err.code = 'PROVIDER_NOT_AVAILABLE'; + throw err; + } + ``` +2. Validate `scanId` (non-empty string) → 400 `SCAN_ID_REQUIRED` if bad. +3. Resolve `owner` / `repo` / `branch` / `octokit` like `getScanById`. +4. List `scans/`; find the matching `_*.json` file. If none → 404 + `SCAN_NOT_FOUND` (same codes as get). +5. Read manifest (`_readAccountManifest`); parse + `_normalizeManifestBrand`. +6. Reconcile index (`_reconcileGitHubIndex`) so you start from scan-file truth. +7. Filter: `index.scans = index.scans.filter((e) => e.id !== scanId)`. +8. `_updateManifestSummary(manifest, index, /* lastScanAt from new head or null */)`. +9. Build write batch: + ```js + [ + { path: scanPath, delete: true, sha: match.sha /* or fileData.sha */ }, + { + path: INDEX_PATH, // 'scans/index.json' + content: JSON.stringify(index, null, 2) + '\n', + sha: existingIndexSha, + }, + { + path: MANIFEST_PATH, // or legacy path if that’s what you read + content: JSON.stringify(updatedManifest, null, 2) + '\n', + sha: manifestFile.sha, + }, + ] + ``` +10. `_writeGitHubFiles(..., 'Delete accessibility scan …')`. +11. On `_isRefConflict` (409/422 stale sha): **retry** like `saveScanResults` + (re-list, re-reconcile, rewrite). Cap retries (e.g. 3). +12. Return `{ deletedId: scanId, path: scanPath, scanCount: index.scans.length, scans: index.scans }`. + +**Idempotency:** if the file is already gone but an index row remains, treat as +success after repairing caches (filter index + write). If both are already gone, +return 404 **or** success with current list — pick one and test it; prefer +**404** for a missing id so the UI can say “already gone”, or **200** with +current list for gentler UX. Document your choice in the route tests. + +### 5.3 Unit tests (`backend/tests/storageService.test.js`) + +Add cases next to `getScanById` / `saveScanResults`: + +- Deletes the target file; leaves other scan files and unrelated root files. +- Index no longer contains that `id`; `scanCount` matches. +- Unknown id → `SCAN_NOT_FOUND` / 404. +- Google → 501 stub. +- Conflict retry: first write 422, second succeeds (optional but valuable). + +Run: `node --test tests/storageService.test.js` + +--- + +## 6. HTTP layer + +### 6.1 Route (`backend/routes/scan.js`) + +```js +router.delete('/scans/:id', controller.deleteSavedScan); +``` + +Keep it next to the existing `GET /scans/:id` registration. + +### 6.2 Controller (`backend/controllers/scanController.js`) + +Add `deleteSavedScan`, bind in the constructor (same pattern as other methods). + +Auth / storage gate — **copy from `getSavedScan`**: + +- Not authenticated or no `req.user.storage` → **401** +- Missing services → **503** +- Missing `id` param → **400** + +Happy path: + +```js +const clients = await this.authService.clientsFor(req.user, { + storageRef: req.user.storage, +}); +const result = await this.storageService.deleteScanById(req.user, id, clients); + +if (!req.user.account) { + req.user.account = { settings: { autoDelete90d: true }, scanCount: 0 }; +} +req.user.account.scanCount = result.scanCount; +// Do NOT put result.scans on the session cookie — same rule as postScan. +await this.authService.persistUser(req); + +return res.json({ + scanCount: result.scanCount, + scans: result.scans, +}); +``` + +Error mapping (mirror `getSavedScan`): + +| Condition | Status | +|-----------|--------| +| `SCAN_NOT_FOUND` / `err.status === 404` | 404 `{ error: 'Scan not found' }` | +| `STORAGE_ACCESS_DENIED` / 403 | 403 | +| `PROVIDER_NOT_AVAILABLE` / 501 | 501 | +| Other | 500 (log) | + +**Delete is not best-effort** (unlike save-on-scan). If storage fails, return +the error — the user asked to delete. + +### 6.3 Route tests (`backend/tests/scan.test.js`) + +- Unauthenticated → 401 +- Happy path: mock `deleteScanById` → 200 `{ scanCount, scans }`; session + `scanCount` updated, **no** `scans` array on session user +- Unknown id → 404 +- Optional: 403 / 501 passthrough + +### 6.4 Docs table + +Add a row to the API table in +`githubGoogleAuthStorageImplementation.md` (and `backend/README.md` if it lists +scan routes): + +| Method | Path | Notes | +|--------|------|--------| +| `DELETE` | `/api/scans/:id` | Auth + attached storage; remove one scan file + refresh index/manifest caches; returns `{ scanCount, scans }` | + +--- + +## 7. Frontend + +### 7.1 `apiClient.deleteScan(scanId)` + +```js +deleteScan(scanId) { + return this._request(`/api/scans/${encodeURIComponent(scanId)}`, { + method: 'DELETE', + }) +} +``` + +Returns `Promise<{ scanCount: number, scans: object[] }>`. + +Cover in `frontend/src/__tests__/apiClient.test.js` (same style as `listScans` / +`getSavedScan`). + +### 7.2 `App.jsx` handler + +```js +const deleteSaved = async (s) => { + if (!s?.id) return + const result = await apiClient.deleteScan(s.id) + setUser((prev) => mergeAccountUpdate(prev, { + scanCount: result.scanCount, + scans: result.scans, + })) + // optional: if viewing that scan, clear scan state / navigate to dashboard +} +``` + +Pass `onDelete={deleteSaved}` into `DashboardView` next to `onOpen={openSaved}`. + +`mergeAccountUpdate` / `toSavedScans` in `accountAdapter.js` already understand +`{ scanCount, scans }` — no adapter change required unless you want a tiny helper +`withoutScan(account, id)` for optimistic UI. + +### 7.3 `DashboardView.jsx` + +- Add a delete control per row (icon button). +- `onClick={(e) => { e.stopPropagation(); … }}` so the row doesn’t open. +- Flow: idle → confirm (“Delete this scan? This removes it from your storage.”) + → busy → call `onDelete(s)` → parent updates `saved`. +- Show per-row or banner error on failure; re-enable the button. +- Keep empty-state UX unchanged when `saved.length === 0` after the last delete. + +### 7.4 Frontend tests + +`frontend/src/__tests__/dashboardView.test.jsx`: + +- Delete confirm does **not** call `onOpen`. +- Confirm calls `onDelete` with the scan. +- Cancel leaves the list alone. + +--- + +## 8. Suggested commit sequence + +Keep the PR reviewable: + +1. `_writeGitHubFiles` delete support (`delete: true` + Contents `deleteFile`) + unit tests + *(skip if already present from #82)* +2. `StorageService.deleteScanById` + storage tests +3. `DELETE /api/scans/:id` controller/route + `scan.test.js` +4. `apiClient.deleteScan` + Dashboard/App UX + frontend tests +5. Docs: this guide checklist, contract one-liner, API table, `TODO.md` + +Run GitNexus `detect_changes` before committing. Omit Cursor co-author trailers +if the team asks to omit them. + +--- + +## 9. Docs to keep in sync + +When the code lands: + +- [ ] This guide — mark the acceptance checklist below. +- [ ] [`accountStorageContract.md`](./accountStorageContract.md) — short note under + write atomicity or a “Deleting a scan” bullet: remove file + index entry; + caches rebuildable; Git history may retain blobs. +- [ ] [`githubGoogleAuthStorageImplementation.md`](./githubGoogleAuthStorageImplementation.md) + — API table row for `DELETE /api/scans/:id`. +- [ ] [`TODO.md`](./TODO.md) — checkbox under scan/storage follow-ups. +- [ ] `backend/README.md` route table if it lists scan endpoints. + +--- + +## 10. Manual test plan + +1. Sign in, connect a repo, run 2+ scans so the dashboard has multiple rows. +2. Delete one scan → confirm → that row disappears; the other remains. +3. Refresh / re-open dashboard → still gone (`GET /api/scans`). +4. Open the GitHub repo → that `scans/_*.json` file is gone; `index.json` + no longer lists it; unrelated files untouched. +5. Delete the last scan → empty dashboard state. +6. Delete an unknown id (curl) → 404. +7. Signed out → 401. + +--- + +## 11. Acceptance checklist (closes #112) + +- [ ] Dashboard has a Delete action per scan (with confirm). +- [ ] Delete removes only that scan’s file from attached storage. +- [ ] Other scans, account identity, and the repository remain. +- [ ] `index.json` / `scanCount` update (or heal on next load). +- [ ] `DELETE /api/scans/:id` auth-gated; returns updated `{ scanCount, scans }`. +- [ ] Session stores `scanCount` only (not the full scans array). +- [ ] Google remains explicitly stubbed (501) until Phase 3. +- [ ] Backend + frontend tests cover happy path, 404, and “delete doesn’t open”. +- [ ] Docs/API table updated. + +When the checklist is green and the PR is merged, close +[#112](https://github.com/codrlabs/vizably/issues/112). + +--- + +## Quick file checklist + +| Layer | File | +|-------|------| +| Storage | `backend/services/storageService.js` | +| Tests | `backend/tests/storageService.test.js` | +| Controller | `backend/controllers/scanController.js` | +| Routes | `backend/routes/scan.js` | +| Route tests | `backend/tests/scan.test.js` | +| Client | `frontend/src/lib/apiClient.js` | +| UI | `frontend/src/views/DashboardView.jsx` | +| Wiring | `frontend/src/App.jsx` | +| FE tests | `frontend/src/__tests__/dashboardView.test.jsx`, `apiClient.test.js` | +| Docs | this file + contract + implementation guide + TODO | From d47c39dfa4486841bde5e4c8a2b61c0682869614 Mon Sep 17 00:00:00 2001 From: Samuel Olabode Date: Wed, 12 Aug 2026 17:28:01 -0600 Subject: [PATCH 2/5] feat(scan): add deleteSavedScan endpoint and associated logic Implemented a new DELETE endpoint for removing saved scans from user storage. The ScanController now includes the deleteSavedScan method, which handles authentication, validates input, and interacts with the storage service to delete the specified scan. Updated the storageService to support scan deletion and added corresponding tests to ensure functionality and error handling. Frontend integration allows users to delete scans from the dashboard. --- backend/controllers/scanController.js | 66 +++++++++ backend/routes/scan.js | 2 + backend/services/storageService.js | 202 ++++++++++++++++++++++++-- backend/tests/scan.test.js | 93 ++++++++++++ backend/tests/storageService.test.js | 135 ++++++++++++++++- frontend/src/App.jsx | 16 ++ frontend/src/lib/apiClient.js | 11 ++ 7 files changed, 509 insertions(+), 16 deletions(-) diff --git a/backend/controllers/scanController.js b/backend/controllers/scanController.js index fdaf8d4..34cd434 100644 --- a/backend/controllers/scanController.js +++ b/backend/controllers/scanController.js @@ -30,6 +30,7 @@ class ScanController { this.getScanResults = this.getScanResults.bind(this); this.getSavedScan = this.getSavedScan.bind(this); this.getSavedScans = this.getSavedScans.bind(this); + this.deleteSavedScan = this.deleteSavedScan.bind(this); this.getProblem = this.getProblem.bind(this); } @@ -173,6 +174,71 @@ class ScanController { } } + /** + * DELETE /api/scans/:id — remove one saved report from attached storage. + */ + async deleteSavedScan(req, res) { + if ( + typeof req.isAuthenticated !== 'function' || + !req.isAuthenticated() || + !req.user?.storage + ) { + return res.status(401).json({ error: 'Not authenticated' }); + } + if (!this.authService || !this.storageService) { + return res.status(503).json({ error: 'Storage is not configured' }); + } + + const { id } = req.params; + if (!id) { + return res.status(400).json({ error: 'Missing scan id' }); + } + + try { + const clients = await this.authService.clientsFor(req.user, { + storageRef: req.user.storage, + }); + const result = await this.storageService.deleteScanById( + req.user, + id, + clients, + ); + + if (!req.user.account) { + req.user.account = { + settings: { autoDelete90d: true }, + scanCount: 0, + }; + } + req.user.account.scanCount = result.scanCount; + await this.authService.persistUser(req); + + return res.json({ + scanCount: result.scanCount, + scans: result.scans, + }); + } catch (err) { + if (err.code === 'SCAN_NOT_FOUND' || err.status === 404) { + return res.status(404).json({ error: 'Scan not found' }); + } + if (err.code === 'PROVIDER_NOT_AVAILABLE' || err.status === 501) { + return res.status(501).json({ + error: err.message, + code: err.code, + }); + } + if ( + err.code === 'STORAGE_ACCESS_DENIED' || + err.code === 'STORAGE_IDENTITY_MISMATCH' || + err.status === 403 + ) { + return res.status(403).json({ error: err.message }); + } + console.error(err); + return res.status(500).json({ error: 'Internal server error' }); + } + } + /** * GET /api/scans — list the saved scans held in the user's own storage. * diff --git a/backend/routes/scan.js b/backend/routes/scan.js index 69804aa..dd15992 100644 --- a/backend/routes/scan.js +++ b/backend/routes/scan.js @@ -4,6 +4,7 @@ * - GET /api/scan-results * - GET /api/scans * - GET /api/scans/:id + * - DELETE /api/scans/:id * * Only knows about HTTP shape. All logic lives in ScanController. * @@ -21,6 +22,7 @@ function makeScanRouter(controller) { router.get('/scan-results', controller.getScanResults); router.get('/scans', controller.getSavedScans); router.get('/scans/:id', controller.getSavedScan); + router.delete('/scans/:id', controller.deleteSavedScan); return router; } diff --git a/backend/services/storageService.js b/backend/services/storageService.js index 55de330..82caee7 100644 --- a/backend/services/storageService.js +++ b/backend/services/storageService.js @@ -789,6 +789,156 @@ class StorageService { }; } + /** + * Delete one immutable saved scan by id and refresh index/manifest caches. + * @param {object} account session user (with storage binding) + * @param {string} scanId + * @param {StorageClients} clients + * @returns {Promise<{ deletedId: string, path: string, scanCount: number, scans: object[] }>} + */ + async deleteScanById(account, scanId, clients) { + if (account?.storage?.provider === 'google') { + const err = new Error(GOOGLE_NOT_AVAILABLE); + err.status = 501; + err.code = 'PROVIDER_NOT_AVAILABLE'; + throw err; + } + if (!clients.githubClient) { + throw new Error('GitHub client is required to delete a saved scan'); + } + if (!scanId || typeof scanId !== 'string') { + const err = new Error('Scan id is required'); + err.status = 400; + err.code = 'SCAN_ID_REQUIRED'; + throw err; + } + + const maxAttempts = 3; + for (let attempt = 0; attempt < maxAttempts; attempt += 1) { + try { + return await this._deleteScanByIdOnce(account, scanId, clients); + } catch (err) { + const canRetry = this._isRefConflict(err) && attempt < maxAttempts - 1; + if (!canRetry) { + throw err; + } + } + } + + throw new Error('GitHub write failed after retries'); + } + + /** + * @param {object} account + * @param {string} scanId + * @param {StorageClients} clients + * @private + */ + async _deleteScanByIdOnce(account, scanId, clients) { + const storageRef = account.storageRef ?? account.storage; + const { owner, repo } = this._parseGitHubRef(storageRef); + const octokit = clients.githubClient; + const branch = await this._resolveGitHubBranch( + octokit, + owner, + repo, + storageRef.branch, + ); + + const scanEntries = await this._listGitHubDirectory( + octokit, + owner, + repo, + SCANS_DIR, + branch, + ); + const match = scanEntries.find( + (entry) => + entry.type === 'file' && + entry.name.startsWith(`${scanId}_`) && + entry.name.endsWith('.json') && + entry.name !== 'index.json', + ); + + if (!match) { + const err = new Error('Scan not found'); + err.status = 404; + err.code = 'SCAN_NOT_FOUND'; + throw err; + } + + const scanPath = `${SCANS_DIR}/${match.name}`; + const fileData = await this._readGitHubFile( + octokit, + owner, + repo, + scanPath, + branch, + ); + if (!fileData) { + const err = new Error('Scan not found'); + err.status = 404; + err.code = 'SCAN_NOT_FOUND'; + throw err; + } + + const manifestFile = await this._readAccountManifest(octokit, owner, repo, branch); + if (!manifestFile) { + throw new Error('Account manifest not found'); + } + + const { manifest } = this._normalizeManifestBrand( + this._parseJson(manifestFile.content, 'manifest'), + ); + const { index } = await this._reconcileGitHubIndex(octokit, owner, repo, branch); + index.scans = index.scans.filter((entry) => entry.id !== scanId); + + const updatedManifest = this._updateManifestSummary( + manifest, + index, + index.scans[0]?.scannedAt, + ); + if (index.scans.length === 0) { + updatedManifest.summary.lastScanAt = null; + } + updatedManifest.account.updatedAt = new Date().toISOString(); + + const indexFile = await this._readGitHubFile(octokit, owner, repo, INDEX_PATH, branch); + const host = match.name.slice(scanId.length + 1).replace(/\.json$/, ''); + + await this._writeGitHubFiles( + octokit, + owner, + repo, + branch, + [ + { + path: scanPath, + delete: true, + sha: fileData.sha || match.sha, + }, + { + path: INDEX_PATH, + content: JSON.stringify(index, null, 2) + '\n', + sha: indexFile?.sha, + }, + { + path: MANIFEST_PATH, + content: JSON.stringify(updatedManifest, null, 2) + '\n', + ...(manifestFile.path === MANIFEST_PATH ? { sha: manifestFile.sha } : {}), + }, + ], + `Delete accessibility scan for ${host || scanId}`, + ); + + return { + deletedId: scanId, + path: scanPath, + scanCount: index.scans.length, + scans: index.scans, + }; + } + /** * One save attempt: reconcile from scan-file truth, append prepared scan, write. * @param {object} account @@ -1336,24 +1486,30 @@ class StorageService { * Contents API when the Git Database API is unavailable. Conflicts bubble up * so callers (e.g. saveScanResults) can re-reconcile against scan truth — * never rewrite caches with a refreshed sha and stale content. + * + * Batches that include deletes skip the Git Database API and use Contents + * `deleteFile` — createTree + sha:null is unreliable on real GitHub. * @private */ async _writeGitHubFiles(octokit, owner, repo, branch, files, message) { - try { - return await this._writeGitHubFilesViaGit( - octokit, - owner, - repo, - branch, - files, - message, - ); - } catch (err) { - if (!this._shouldFallbackToContentsApi(err)) { - throw Object.assign(new Error(this._formatGitHubStorageError(err)), { - status: err?.status, - cause: err, - }); + const hasDeletes = files.some((file) => Boolean(file.delete)); + if (!hasDeletes) { + try { + return await this._writeGitHubFilesViaGit( + octokit, + owner, + repo, + branch, + files, + message, + ); + } catch (err) { + if (!this._shouldFallbackToContentsApi(err)) { + throw Object.assign(new Error(this._formatGitHubStorageError(err)), { + status: err?.status, + cause: err, + }); + } } } @@ -1486,6 +1642,22 @@ class StorageService { : `${message} (${i + 1}/${files.length})`; try { + if (file.delete) { + if (!sha) { + continue; + } + const { data } = await octokit.rest.repos.deleteFile({ + owner, + repo, + path: file.path, + message: fileMessage, + sha, + branch, + }); + lastCommit = data.commit; + continue; + } + const { data } = await octokit.rest.repos.createOrUpdateFileContents({ owner, repo, diff --git a/backend/tests/scan.test.js b/backend/tests/scan.test.js index 6ecd5d7..bd31db3 100644 --- a/backend/tests/scan.test.js +++ b/backend/tests/scan.test.js @@ -415,3 +415,96 @@ test('getSavedScan returns 503 when storage services are missing', async () => { ); assert.equal(out.statusCode, 503); }); + +test('deleteSavedScan removes a scan and updates session scanCount only', async () => { + const ScanController = require('../controllers/scanController'); + const remaining = [ + { + id: 'scan-2', + url: 'https://example.com/b', + scannedAt: '2026-07-11T12:00:00Z', + }, + ]; + let persisted = false; + const ctrl = new ScanController({ + mockScanResults, + scanRunner: mockScanRunner, + authService: { + clientsFor: async () => ({ githubClient: {} }), + persistUser: async () => { + persisted = true; + }, + }, + storageService: { + deleteScanById: async () => ({ + deletedId: 'scan-1', + path: 'scans/scan-1_example.com.json', + scanCount: 1, + scans: remaining, + }), + }, + }); + + const req = { + params: { id: 'scan-1' }, + isAuthenticated: () => true, + user: { + storage: { id: 'R_kg', full_name: 'sam/repo' }, + account: { scanCount: 2 }, + }, + }; + const out = mockRes(); + await ctrl.deleteSavedScan(req, out.res); + + assert.equal(out.statusCode, 200); + assert.deepEqual(out.body, { scanCount: 1, scans: remaining }); + assert.equal(req.user.account.scanCount, 1); + assert.equal(req.user.account.scans, undefined); + assert.equal(persisted, true); +}); + +test('deleteSavedScan requires auth and attached storage', async () => { + const ScanController = require('../controllers/scanController'); + const ctrl = new ScanController({ + mockScanResults, + scanRunner: mockScanRunner, + authService: {}, + storageService: {}, + }); + const out = mockRes(); + await ctrl.deleteSavedScan( + { + params: { id: 'scan-1' }, + isAuthenticated: () => false, + user: null, + }, + out.res, + ); + assert.equal(out.statusCode, 401); +}); + +test('deleteSavedScan returns 404 for SCAN_NOT_FOUND', async () => { + const ScanController = require('../controllers/scanController'); + const ctrl = new ScanController({ + mockScanResults, + scanRunner: mockScanRunner, + authService: { clientsFor: async () => ({}) }, + storageService: { + deleteScanById: async () => { + const err = new Error('missing'); + err.code = 'SCAN_NOT_FOUND'; + throw err; + }, + }, + }); + const out = mockRes(); + await ctrl.deleteSavedScan( + { + params: { id: 'missing' }, + isAuthenticated: () => true, + user: { storage: { id: 'R_kg' } }, + }, + out.res, + ); + assert.equal(out.statusCode, 404); +}); diff --git a/backend/tests/storageService.test.js b/backend/tests/storageService.test.js index 07abd03..267bb5a 100644 --- a/backend/tests/storageService.test.js +++ b/backend/tests/storageService.test.js @@ -106,6 +106,20 @@ function createMockGitHubClient(initial = {}) { }; return { data: created }; }, + deleteFile: async ({ path, sha }) => { + if (!files[path]) { + const err = new Error('Not Found'); + err.status = 404; + throw err; + } + if (sha && files[path].sha !== sha) { + const err = new Error('Reference update failed'); + err.status = 422; + throw err; + } + delete files[path]; + return { data: { commit: { sha: `delete-${path}` } } }; + }, createOrUpdateFileContents: async ({ path, content, sha }) => { if (createOrUpdateFailures > 0) { createOrUpdateFailures -= 1; @@ -154,11 +168,12 @@ function createMockGitHubClient(initial = {}) { if (path === 'scans') { const scanFiles = Object.keys(files) - .filter((p) => p.startsWith('scans/') && !p.endsWith('index.json')) + .filter((p) => p.startsWith('scans/')) .map((p) => ({ name: p.replace('scans/', ''), type: 'file', path: p, + sha: files[p].sha, })); if (scanFiles.length === 0) { const err = new Error('Not Found'); @@ -1113,3 +1128,121 @@ test('getScanById returns not found for unknown id', async () => { (err) => err.code === 'SCAN_NOT_FOUND' && err.status === 404, ); }); + +test('deleteScanById removes one scan file and leaves the others', async () => { + const storageService = new StorageService(); + const keepId = 'keep-id'; + const dropId = 'drop-id'; + const account = { + storage: { ...STORAGE_REF, provider: 'github', branch: 'main' }, + }; + const client = createMockGitHubClient({ + files: { + 'vizably.json': { + content: JSON.stringify( + manifest({ summary: { scanCount: 2, lastScanAt: '2026-07-11T12:00:00Z' } }), + ), + sha: 'sha-manifest', + }, + 'scans/index.json': { + content: JSON.stringify({ + schemaVersion: 1, + scans: [ + { + id: dropId, + url: 'https://drop.example', + host: 'drop.example', + scannedAt: '2026-07-11T12:00:00Z', + file: `scans/${dropId}_drop.example.json`, + }, + { + id: keepId, + url: 'https://keep.example', + host: 'keep.example', + scannedAt: '2026-07-10T12:00:00Z', + file: `scans/${keepId}_keep.example.json`, + }, + ], + }), + sha: 'sha-index', + }, + [`scans/${dropId}_drop.example.json`]: { + content: JSON.stringify({ + id: dropId, + url: 'https://drop.example', + scannedAt: '2026-07-11T12:00:00Z', + result: { problems: {} }, + }), + sha: 'sha-drop', + }, + [`scans/${keepId}_keep.example.json`]: { + content: JSON.stringify({ + id: keepId, + url: 'https://keep.example', + scannedAt: '2026-07-10T12:00:00Z', + result: { problems: {} }, + }), + sha: 'sha-keep', + }, + 'README.md': { content: '# keep\n', sha: 'sha-readme' }, + }, + }); + + const result = await storageService.deleteScanById(account, dropId, { + githubClient: client, + }); + + assert.equal(result.deletedId, dropId); + assert.equal(result.scanCount, 1); + assert.equal(result.scans.length, 1); + assert.equal(result.scans[0].id, keepId); + assert.equal(client.files[`scans/${dropId}_drop.example.json`], undefined); + assert.ok(client.files[`scans/${keepId}_keep.example.json`]); + assert.equal(client.files['README.md'].content, '# keep\n'); + + const index = JSON.parse(client.files['scans/index.json'].content); + assert.equal(index.scans.length, 1); + assert.equal(index.scans[0].id, keepId); + + const updatedManifest = JSON.parse(client.files['vizably.json'].content); + assert.equal(updatedManifest.summary.scanCount, 1); +}); + +test('deleteScanById returns not found for unknown id', async () => { + const storageService = new StorageService(); + const client = createMockGitHubClient({ + files: { + 'vizably.json': { + content: JSON.stringify(manifest()), + sha: 'sha-manifest', + }, + 'scans/index.json': { + content: JSON.stringify({ schemaVersion: 1, scans: [] }), + sha: 'sha-index', + }, + }, + }); + + await assert.rejects( + () => + storageService.deleteScanById( + { storage: { ...STORAGE_REF, provider: 'github', branch: 'main' } }, + 'missing-id', + { githubClient: client }, + ), + (err) => err.code === 'SCAN_NOT_FOUND' && err.status === 404, + ); +}); + +test('deleteScanById stubs google until Phase 3', async () => { + const storageService = new StorageService(); + await assert.rejects( + () => + storageService.deleteScanById( + { storage: { provider: 'google', id: 'folder' } }, + 'scan-1', + { githubClient: {} }, + ), + (err) => err.status === 501 && err.code === 'PROVIDER_NOT_AVAILABLE', + ); +}); diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index 2874d22..cf9438a 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -284,6 +284,21 @@ function AppRoutes() { navigate(`${PATHS.results}?scanId=${encodeURIComponent(s.id)}`) } + /** Remove one saved scan from attached storage and refresh the dashboard list. */ + const deleteSaved = async (s) => { + if (!s?.id) return + const result = await apiClient.deleteScan(s.id) + setUser((prev) => mergeAccountUpdate(prev, { + scanCount: result.scanCount, + scans: result.scans, + })) + if (location.search.includes(`scanId=${encodeURIComponent(s.id)}`)) { + setScan(null) + setProblem(null) + navigate(PATHS.dashboard) + } + } + const auth = (p) => { if (p === 'google') return apiClient.githubLogin() @@ -372,6 +387,7 @@ function AppRoutes() { } + */ + deleteScan(scanId) { + return this._request(`/api/scans/${encodeURIComponent(scanId)}`, { + method: 'DELETE', + }) + } + /** * Look up a single problem by id. * @param {string} id From 07bbf9016b8852f9163233cb7c4923e823f72759 Mon Sep 17 00:00:00 2001 From: Samuel Olabode Date: Wed, 12 Aug 2026 17:28:41 -0600 Subject: [PATCH 3/5] feat(scan): implement scan deletion functionality in the dashboard Added a new DELETE endpoint for removing saved scans from user storage, along with frontend integration in the DashboardView. Users can now confirm and delete scans, with appropriate error handling and UI updates. Updated documentation to reflect the new API capabilities and added tests to ensure functionality. --- backend/README.md | 1 + docs/guides/auth_storage_guide/TODO.md | 3 + .../accountStorageContract.md | 8 + .../githubGoogleAuthStorageImplementation.md | 3 + .../guides/auth_storage_guide/scanDeletion.md | 18 +- frontend/src/__tests__/apiClient.test.js | 18 ++ frontend/src/__tests__/dashboardView.test.jsx | 45 +++- frontend/src/views/DashboardView.jsx | 221 +++++++++++++++--- 8 files changed, 279 insertions(+), 38 deletions(-) diff --git a/backend/README.md b/backend/README.md index d3a1364..f3a3fdd 100644 --- a/backend/README.md +++ b/backend/README.md @@ -225,6 +225,7 @@ See also [`docs/guides/auth_storage_guide/githubGoogleAuthStorageImplementation. | GET | `/api/scan-results?url=` | re-run a scan for a URL (used by deep links) | | GET | `/api/scans` | list saved scans from the user's store | | GET | `/api/scans/:id` | load one saved report from the user's store | +| DELETE | `/api/scans/:id` | delete one saved report from the user's store | | GET | `/api/problems/:id` | look up a single problem (legacy mock lookup) | Every backend path lives under `/api` so one serverless function can claim the diff --git a/docs/guides/auth_storage_guide/TODO.md b/docs/guides/auth_storage_guide/TODO.md index 3e845c3..4e700c9 100644 --- a/docs/guides/auth_storage_guide/TODO.md +++ b/docs/guides/auth_storage_guide/TODO.md @@ -128,6 +128,8 @@ Provider-neutral routes; Google OAuth endpoints stubbed until Phase 3. - [x] Accept `storageService` + `authService` in deps - [x] In `postScan`: if authenticated and `req.user.storage`, build clients and `saveScanResults(...)` — a storage failure logs a warning, **never** fails the scan +- [x] `DELETE /api/scans/:id` — `deleteSavedScan` → `storageService.deleteScanById` + (see [`scanDeletion.md`](./scanDeletion.md)) ### Phase 1 follow-ups (post-merge) @@ -152,6 +154,7 @@ Provider-neutral API shape; **GitHub picker wired**, Google deferred to Phase 3. - [x] `validateStorage(provider, storageRef)` → `POST /api/auth/storage/validate` - [x] `setupStorage(provider, storageRef, action)` → `POST /api/auth/storage` - [x] Keep `runScan`, `getScanResults`, `getProblem` +- [x] `deleteScan(id)` → `DELETE /api/scans/:id` ### ConnectView (`frontend/src/views/ConnectView.jsx`) — the picker diff --git a/docs/guides/auth_storage_guide/accountStorageContract.md b/docs/guides/auth_storage_guide/accountStorageContract.md index 01132b0..21d5493 100644 --- a/docs/guides/auth_storage_guide/accountStorageContract.md +++ b/docs/guides/auth_storage_guide/accountStorageContract.md @@ -191,6 +191,14 @@ summary) must be atomic or partial-write tolerant: using ETag/generation preconditions; rely on load-time reconcile to heal any gap. +### Deleting a scan + +`DELETE /api/scans/:id` removes one immutable file `scans/_.json` +and refreshes rebuildable caches (`scans/index.json` + manifest `summary`). +Other scan files and account identity stay. GitHub history may still contain +the deleted blob unless history is rewritten — disclose that in the UI if you +talk about permanence. See [`scanDeletion.md`](./scanDeletion.md). + --- ## Versioning & migration diff --git a/docs/guides/auth_storage_guide/githubGoogleAuthStorageImplementation.md b/docs/guides/auth_storage_guide/githubGoogleAuthStorageImplementation.md index 5c05742..7b07da6 100644 --- a/docs/guides/auth_storage_guide/githubGoogleAuthStorageImplementation.md +++ b/docs/guides/auth_storage_guide/githubGoogleAuthStorageImplementation.md @@ -356,6 +356,9 @@ Session cookies, not Bearer tokens. All calls use `credentials: 'include'`; no - `validateStorage(provider, storageRef)` → `POST /api/auth/storage/validate` - `setupStorage(provider, storageRef, action)` → `POST /api/auth/storage` - `logout()` → `POST /api/auth/logout` +- `listScans()` → `GET /api/scans` +- `getSavedScan(id)` → `GET /api/scans/:id` +- `deleteScan(id)` → `DELETE /api/scans/:id` - `runScan`, `getScanResults`, `getProblem` unchanged. For Google, selection is done with the **Google Picker** client library; the diff --git a/docs/guides/auth_storage_guide/scanDeletion.md b/docs/guides/auth_storage_guide/scanDeletion.md index d4ebe48..df7a4ca 100644 --- a/docs/guides/auth_storage_guide/scanDeletion.md +++ b/docs/guides/auth_storage_guide/scanDeletion.md @@ -428,15 +428,15 @@ When the code lands: ## 11. Acceptance checklist (closes #112) -- [ ] Dashboard has a Delete action per scan (with confirm). -- [ ] Delete removes only that scan’s file from attached storage. -- [ ] Other scans, account identity, and the repository remain. -- [ ] `index.json` / `scanCount` update (or heal on next load). -- [ ] `DELETE /api/scans/:id` auth-gated; returns updated `{ scanCount, scans }`. -- [ ] Session stores `scanCount` only (not the full scans array). -- [ ] Google remains explicitly stubbed (501) until Phase 3. -- [ ] Backend + frontend tests cover happy path, 404, and “delete doesn’t open”. -- [ ] Docs/API table updated. +- [x] Dashboard has a Delete action per scan (with confirm). +- [x] Delete removes only that scan’s file from attached storage. +- [x] Other scans, account identity, and the repository remain. +- [x] `index.json` / `scanCount` update (or heal on next load). +- [x] `DELETE /api/scans/:id` auth-gated; returns updated `{ scanCount, scans }`. +- [x] Session stores `scanCount` only (not the full scans array). +- [x] Google remains explicitly stubbed (501) until Phase 3. +- [x] Backend + frontend tests cover happy path, 404, and “delete doesn’t open”. +- [x] Docs/API table updated. When the checklist is green and the PR is merged, close [#112](https://github.com/codrlabs/vizably/issues/112). diff --git a/frontend/src/__tests__/apiClient.test.js b/frontend/src/__tests__/apiClient.test.js index d6bfebd..7a05ebf 100644 --- a/frontend/src/__tests__/apiClient.test.js +++ b/frontend/src/__tests__/apiClient.test.js @@ -223,4 +223,22 @@ describe('ApiClient', () => { expect(result.scanCount).toBe(2) expect(result.scans).toHaveLength(2) }) + + it('deletes a saved scan by id', async () => { + const fetchImpl = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + json: async () => ({ scanCount: 1, scans: [{ id: 'scan-2' }] }), + }) + + const client = new ApiClient({ fetchImpl }) + const result = await client.deleteScan('scan-1') + + expect(fetchImpl).toHaveBeenCalledWith( + '/api/scans/scan-1', + expect.objectContaining({ method: 'DELETE' }), + ) + expect(result.scanCount).toBe(1) + expect(result.scans).toEqual([{ id: 'scan-2' }]) + }) }) diff --git a/frontend/src/__tests__/dashboardView.test.jsx b/frontend/src/__tests__/dashboardView.test.jsx index 35d39da..31a8e0c 100644 --- a/frontend/src/__tests__/dashboardView.test.jsx +++ b/frontend/src/__tests__/dashboardView.test.jsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from 'vitest' -import { render, screen, fireEvent } from '@testing-library/react' +import { render, screen, fireEvent, waitFor } from '@testing-library/react' import DashboardView from '../views/DashboardView' const SAVED = [ @@ -77,4 +77,47 @@ describe('DashboardView', () => { fireEvent.keyDown(row, { key: 'Enter' }) expect(onOpen).toHaveBeenCalledWith(SAVED[1]) }) + + it('asks for confirm before delete and does not open the scan', async () => { + const onOpen = vi.fn() + const onDelete = vi.fn().mockResolvedValue(undefined) + render( + , + ) + + fireEvent.click(screen.getByRole('button', { name: /delete scan example.com/i })) + expect(onOpen).not.toHaveBeenCalled() + expect(screen.getByText(/delete this scan/i)).toBeInTheDocument() + + fireEvent.click(screen.getByRole('button', { name: /yes, delete/i })) + await waitFor(() => expect(onDelete).toHaveBeenCalledWith(SAVED[0])) + expect(onOpen).not.toHaveBeenCalled() + }) + + it('cancel leaves the list alone without calling onDelete', () => { + const onDelete = vi.fn() + render( + , + ) + + fireEvent.click(screen.getByRole('button', { name: /delete scan example.com/i })) + fireEvent.click(screen.getByRole('button', { name: /cancel/i })) + expect(onDelete).not.toHaveBeenCalled() + expect(screen.queryByText(/delete this scan/i)).not.toBeInTheDocument() + }) }) diff --git a/frontend/src/views/DashboardView.jsx b/frontend/src/views/DashboardView.jsx index f2ca723..65624ec 100644 --- a/frontend/src/views/DashboardView.jsx +++ b/frontend/src/views/DashboardView.jsx @@ -4,42 +4,159 @@ import { Ico } from '../lib/icons' import { PROVIDERS } from '../data/placeholders' /** Dashboard — signed-in saved scans from the loaded account index. */ -export default function DashboardView({ onNav, onOpen, saved, provider, user, storage }) { +export default function DashboardView({ onNav, onOpen, onDelete, saved, provider, user, storage }) { const pv = PROVIDERS[provider] || PROVIDERS.github const storageLabel = storage?.full_name || pv.dest + const [confirmId, setConfirmId] = useState(null) + const [deletingId, setDeletingId] = useState(null) + const [deleteError, setDeleteError] = useState(null) const band = (v) => v >= 90 ? { c: 'var(--green-600)', g: 'Good' } : v >= 70 ? { c: 'var(--sev-moderate)', g: 'Fair' } : v >= 50 ? { c: 'var(--sev-serious)', g: 'Poor' } : { c: 'var(--sev-critical)', g: 'Critical' } + const handleDelete = async (s) => { + if (!onDelete || !s?.id) return + setDeleteError(null) + setDeletingId(s.id) + try { + await onDelete(s) + setConfirmId(null) + } catch (err) { + setDeleteError(err?.message || 'Failed to delete that scan') + } finally { + setDeletingId(null) + } + } + const Row = ({ s, last }) => { const [hover, setHover] = useState(false) const b = band(s.score) + const confirming = confirmId === s.id + const busy = deletingId === s.id + return ( -
onOpen(s)} - onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); onOpen(s) } }} - onMouseEnter={() => setHover(true)} onMouseLeave={() => setHover(false)} - style={{ display: 'flex', alignItems: 'center', gap: 16, padding: '16px 18px', +
{ if (!confirming && !busy) onOpen(s) }} + onKeyDown={(e) => { + if (confirming || busy) return + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault() + onOpen(s) + } + }} + onMouseEnter={() => setHover(true)} + onMouseLeave={() => setHover(false)} + style={{ + display: 'flex', + alignItems: 'center', + gap: 16, + padding: '16px 18px', background: hover ? 'var(--bg-subtle)' : 'var(--surface-card)', - borderBottom: last ? 'none' : '1px solid var(--border-subtle)', cursor: 'pointer', - transition: 'background var(--duration-fast) var(--ease-standard)' }}> - + borderBottom: last ? 'none' : '1px solid var(--border-subtle)', + cursor: confirming || busy ? 'default' : 'pointer', + transition: 'background var(--duration-fast) var(--ease-standard)', + }} + > + {s.url[0].toUpperCase()}
-
{s.url}
-
+
+ {s.url} +
+
{Ico('Clock', 12)} Scanned {s.when}
+ {confirming && ( +
e.stopPropagation()}> +

+ Delete this scan? It will be removed from your storage. GitHub history may still retain it. +

+
+ + +
+
+ )}
- - - {s.score} - {b.g} - - + {!confirming && ( + <> + + + + {s.score} + + {b.g} + + {onDelete && ( + + )} + + + )}
) } @@ -48,20 +165,37 @@ export default function DashboardView({ onNav, onOpen, saved, provider, user, st return (
-
Signed in as {user?.email || 'your account'}
+
+ Signed in as {user?.email || 'your account'} +

Your scans

-
+
{Ico('ScanLine', 30, 'currentColor')}

No scans yet

-

+

Run your first accessibility scan and it’ll show up here — ready to revisit any time you sign in.

- + -
+
{Ico(pv.destIcon, 16, 'currentColor')}

Scans are saved to {pv.store} ({storageLabel}) — your space, not ours. Nothing for us to meter or lock behind a paywall. @@ -75,19 +209,46 @@ export default function DashboardView({ onNav, onOpen, saved, provider, user, st return (

-
+
-
Signed in as {user?.email || 'your account'}
+
+ Signed in as {user?.email || 'your account'} +

Your scans

- +
- {/* Stats */} + {deleteError && ( +
+ {deleteError} +
+ )} +
{[['Sites saved', saved.length, 'var(--text-strong)'], ['Avg. score', avg, band(avg).c]].map(([label, val, col]) => ( -
{val}
+
+ {val} +
{label}
 ))} @@ -97,7 +258,11 @@ export default function DashboardView({ onNav, onOpen, saved, provider, user, st {saved.map((s, i) => )} -
+
{Ico(pv.destIcon, 16, 'currentColor')}

These reports live in {pv.store} ({storageLabel}), synced from your {pv.name} account — so they’re yours to keep, export, or delete, and they never touch our servers. From b0fa11abcd462d671c7a2961e2d9c83c285cdf4b Mon Sep 17 00:00:00 2001 From: Samuel Olabode Date: Wed, 12 Aug 2026 17:30:45 -0600 Subject: [PATCH 4/5] fix(DashboardView): update button role and tabIndex based on confirming state --- frontend/src/views/DashboardView.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/views/DashboardView.jsx b/frontend/src/views/DashboardView.jsx index 65624ec..74dd673 100644 --- a/frontend/src/views/DashboardView.jsx +++ b/frontend/src/views/DashboardView.jsx @@ -38,8 +38,8 @@ export default function DashboardView({ onNav, onOpen, onDelete, saved, provider return (

{ if (!confirming && !busy) onOpen(s) }} onKeyDown={(e) => { if (confirming || busy) return From f2e8151ac453319df1104f2d3cd3b9b3748a05c3 Mon Sep 17 00:00:00 2001 From: Samuel Olabode Date: Wed, 12 Aug 2026 17:34:24 -0600 Subject: [PATCH 5/5] feat(ResultsView): implement scan deletion functionality with confirmation dialog --- frontend/src/App.jsx | 35 ++++-- frontend/src/__tests__/resultsView.test.jsx | 26 ++++- frontend/src/views/ResultsView.jsx | 112 ++++++++++++++++++-- 3 files changed, 158 insertions(+), 15 deletions(-) diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index cf9438a..b8e9a1a 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -64,20 +64,36 @@ function AuthLoadingIndicator() { * /results — renders the in-memory scan; on a deep link / refresh it * reloads a saved report (?scanId=) or re-fetches by ?url=. */ -function ResultsRoute({ scan, onOpenProblem }) { +function ResultsRoute({ scan, onOpenProblem, onDelete }) { const [params] = useSearchParams() const scanId = params.get('scanId') const url = params.get('url') - if (scan) return + if (scan) { + return ( + + ) + } if (scanId) { - return + return ( + + ) } if (!url) return return } -function SavedScanFetcher({ scanId, onOpenProblem }) { +function SavedScanFetcher({ scanId, onOpenProblem, onDelete }) { const navigate = useNavigate() const [viewModel, setViewModel] = useState(null) const [loading, setLoading] = useState(true) @@ -124,7 +140,14 @@ function SavedScanFetcher({ scanId, onOpenProblem }) { ) } - return + return ( + + ) } function ResultsFetcher({ url, onOpenProblem }) { @@ -365,7 +388,7 @@ function AppRoutes() { } /> - } /> + } /> } /> } /> } /> diff --git a/frontend/src/__tests__/resultsView.test.jsx b/frontend/src/__tests__/resultsView.test.jsx index 3423601..37fe3a1 100644 --- a/frontend/src/__tests__/resultsView.test.jsx +++ b/frontend/src/__tests__/resultsView.test.jsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from 'vitest' -import { render, screen, fireEvent } from '@testing-library/react' +import { render, screen, fireEvent, waitFor } from '@testing-library/react' import ResultsView from '../views/ResultsView' import { toScanViewModel } from '../lib/scanAdapter' import { scanResultFixture } from './fixtures/scanResult' @@ -38,4 +38,28 @@ describe('ResultsView', () => { fireEvent.click(screen.getByText('Images must have alternate text')) expect(onOpenProblem).toHaveBeenCalledWith(expect.objectContaining({ id: 'image-alt' })) }) + + it('hides delete when there is no saved scan id', () => { + render( {}} onDelete={vi.fn()} />) + expect(screen.queryByRole('button', { name: /delete scan/i })).not.toBeInTheDocument() + }) + + it('confirms before deleting a saved scan', async () => { + const onDelete = vi.fn().mockResolvedValue(undefined) + render( + {}} + scanId="scan-1" + onDelete={onDelete} + />, + ) + + fireEvent.click(screen.getByRole('button', { name: /delete scan/i })) + expect(screen.getByText(/delete this saved scan/i)).toBeInTheDocument() + expect(onDelete).not.toHaveBeenCalled() + + fireEvent.click(screen.getByRole('button', { name: /yes, delete/i })) + await waitFor(() => expect(onDelete).toHaveBeenCalledWith({ id: 'scan-1' })) + }) }) diff --git a/frontend/src/views/ResultsView.jsx b/frontend/src/views/ResultsView.jsx index cb0d73a..59421df 100644 --- a/frontend/src/views/ResultsView.jsx +++ b/frontend/src/views/ResultsView.jsx @@ -1,21 +1,117 @@ -import { Badge, Card, ProblemRow, ScoreDial, SeverityBadge } from '../design-system' +import { useState } from 'react' +import { Badge, Button, Card, ProblemRow, ScoreDial, SeverityBadge } from '../design-system' import { Ico } from '../lib/icons' /** Results view — score summary + category sections + what's good. */ -export default function ResultsView({ data, onOpenProblem }) { +export default function ResultsView({ data, onOpenProblem, scanId, onDelete }) { const total = data.categories.reduce((n, c) => n + c.problems.length, 0) + const canDelete = Boolean(scanId && onDelete) + const [confirming, setConfirming] = useState(false) + const [deleting, setDeleting] = useState(false) + const [deleteError, setDeleteError] = useState(null) + + const handleDelete = async () => { + if (!canDelete || deleting) return + setDeleteError(null) + setDeleting(true) + try { + await onDelete({ id: scanId }) + } catch (err) { + setDeleteError(err?.message || 'Failed to delete this scan') + setDeleting(false) + } + } return (
{/* Summary */} -
+
Scan complete
- +
+ {canDelete && !confirming && ( + + )} + +
+ + {canDelete && confirming && ( +
+
+ + {Ico('Trash2', 16, 'currentColor')} + +
+
+ Delete this saved scan? +
+

+ Removes it from your storage. Other scans stay. GitHub history may still retain it. +

+ {deleteError && ( +

+ {deleteError} +

+ )} +
+
+
+ + +
+
+ )} +