From cccd28a9f982110b8847d146d427b18270e5323d Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Tue, 5 May 2026 13:43:18 +0100 Subject: [PATCH] ML-165: implementation --- .pi/extensions/prod-errors/index.ts | 545 +++++++++++++++++- ...e-and-resolve-production-errors-from-pi.md | 115 +++- ...Production-Errors-Implementation-Routes.md | 63 +- docs/architecture.md | 56 +- lib/music_library/errors.ex | 47 +- .../controllers/error_controller.ex | 24 + .../controllers/error_json.ex | 4 + lib/music_library_web/router.ex | 4 + test/music_library/errors_test.exs | 57 ++ .../controllers/error_controller_test.exs | 255 ++++++++ 10 files changed, 1084 insertions(+), 86 deletions(-) rename backlog/{tasks => completed}/ml-165 - Mute-and-resolve-production-errors-from-pi.md (69%) diff --git a/.pi/extensions/prod-errors/index.ts b/.pi/extensions/prod-errors/index.ts index 29d98bed..1d71aa5d 100644 --- a/.pi/extensions/prod-errors/index.ts +++ b/.pi/extensions/prod-errors/index.ts @@ -312,6 +312,39 @@ async function fetchError( ); } +async function postApi( + url: string, + token: string, + validate: (data: unknown) => T, + signal?: AbortSignal, +): Promise { + const response = await fetch(url, { + method: "POST", + headers: { Authorization: `Bearer ${token}` }, + signal, + }); + + if (!response.ok) { + const body = await response.text().catch(() => "(unable to read body)"); + const truncated = body.length > 500 ? body.slice(0, 500) + "..." : body; + throw new Error( + `API returned ${response.status} ${response.statusText}\n${truncated}`, + ); + } + + let data: unknown; + try { + data = await response.json(); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + throw new Error( + `Failed to parse API response: ${message}. Status: ${response.status}.`, + ); + } + + return validate(data); +} + // ── TUI Theme ─────────────────────────────────────────────────────────────── /** Minimal theme interface for color application in the ErrorBrowser. */ @@ -596,6 +629,112 @@ class ErrorBrowser { }); } + toggleMute(id: number, currentMuted: boolean): void { + if (this.mode === "loading") return; + + this.abortInFlight(); + this.mode = "loading"; + this.invalidate(); + this.requestRender(); + + const action = currentMuted ? "unmute" : "mute"; + const url = buildUrl(this.baseUrl, `/api/v1/errors/${id}/${action}`); + const controller = new AbortController(); + this.currentAbortController = controller; + + postApi( + url, + this.token, + (data) => data as ErrorDetailResponse, + controller.signal, + ) + .then((data) => { + // Update local error state in the list + const idx = this.errors.findIndex((e) => e.id === id); + if (idx !== -1) { + const updated = data.error; + this.errors[idx] = { + ...this.errors[idx]!, + muted: updated.muted, + }; + } + + // Update detail view if viewing this error + if (this.selectedError && this.selectedError.id === id) { + this.selectedError = { + ...this.selectedError, + muted: data.error.muted, + }; + } + + this.mode = this.selectedError ? "detail" : "list"; + this.notify(`Error #${id} ${action}d successfully`, "info"); + this.invalidate(); + this.requestRender(); + }) + .catch((err) => { + if (err instanceof Error && err.name === "AbortError") return; + this.mode = this.selectedError ? "detail" : "list"; + const msg = err instanceof Error ? err.message : String(err); + this.notify(`Toggle mute failed: ${msg}`, "error"); + this.invalidate(); + this.requestRender(); + }); + } + + toggleResolve(id: number, currentStatus: string): void { + if (this.mode === "loading") return; + + this.abortInFlight(); + this.mode = "loading"; + this.invalidate(); + this.requestRender(); + + const action = currentStatus === "resolved" ? "unresolve" : "resolve"; + const url = buildUrl(this.baseUrl, `/api/v1/errors/${id}/${action}`); + const controller = new AbortController(); + this.currentAbortController = controller; + + postApi( + url, + this.token, + (data) => data as ErrorDetailResponse, + controller.signal, + ) + .then((data) => { + // Update local error state in the list + const idx = this.errors.findIndex((e) => e.id === id); + if (idx !== -1) { + const updated = data.error; + this.errors[idx] = { + ...this.errors[idx]!, + status: updated.status, + }; + } + + // Update detail view if viewing this error + if (this.selectedError && this.selectedError.id === id) { + this.selectedError = { + ...this.selectedError, + status: data.error.status, + }; + } + + this.mode = this.selectedError ? "detail" : "list"; + this.notify(`Error #${id} ${action}d successfully`, "info"); + this.invalidate(); + this.requestRender(); + }) + .catch((err) => { + if (err instanceof Error && err.name === "AbortError") return; + this.mode = this.selectedError ? "detail" : "list"; + const msg = err instanceof Error ? err.message : String(err); + this.notify(`Toggle resolve failed: ${msg}`, "error"); + this.invalidate(); + this.requestRender(); + }); + } + // ── Input handling ───────────────────────────────────────────────────── handleInput(data: string): void { @@ -631,6 +770,26 @@ class ErrorBrowser { return; } + if (matchesKey(data, "R")) { + if (this.errors.length > 0 && this.cursorIndex >= 0) { + const error = this.errors[this.cursorIndex]; + if (error) { + this.toggleResolve(error.id, error.status); + } + } + return; + } + + if (matchesKey(data, "M")) { + if (this.errors.length > 0 && this.cursorIndex >= 0) { + const error = this.errors[this.cursorIndex]; + if (error) { + this.toggleMute(error.id, error.muted); + } + } + return; + } + if (matchesKey(data, "l")) { if (this.offset + this.limit >= this.total) return; this.loadMore(); @@ -692,6 +851,20 @@ class ErrorBrowser { // ── Detail mode keys ─────────────────────────────────────────────── if (this.mode === "detail") { + if (matchesKey(data, "R")) { + if (this.selectedError) { + this.toggleResolve(this.selectedError.id, this.selectedError.status); + } + return; + } + + if (matchesKey(data, "M")) { + if (this.selectedError) { + this.toggleMute(this.selectedError.id, this.selectedError.muted); + } + return; + } + if (matchesKey(data, Key.enter)) { // Copy current line to editor (prod-logs pattern) if (this.selectedError) { @@ -896,7 +1069,7 @@ class ErrorBrowser { truncateToWidth( theme.fg( "dim", - ` ↑↓/jk navigate ↵ details r ${resolvedLabel} resolved m ${mutedLabel} muted`, + ` ↑↓/jk navigate ↵ details r ${resolvedLabel} resolved m ${mutedLabel} muted M toggle mute R toggle resolve`, ), width, "", @@ -975,7 +1148,7 @@ class ErrorBrowser { truncateToWidth( theme.fg( "dim", - ` Lines ${fromLine}-${toLine} of ${detailLines.length} (${pct}%) ↑↓/jk scroll Enter copy Escape back`, + ` Lines ${fromLine}-${toLine} of ${detailLines.length} (${pct}%) ↑↓/jk scroll Enter copy M mute R resolve Escape back`, ), width, "", @@ -1254,6 +1427,374 @@ export default function prodErrorsExtension(pi: ExtensionAPI) { }, }); + // ── mute_production_error tool ───────────────────────────────────────── + + pi.registerTool({ + name: "mute_production_error", + label: "Mute Production Error", + description: + "Mute a production error by ID. Muted errors will not trigger email " + + "notifications. Use this tool to silence noisy or already-addressed errors.", + promptSnippet: + "Mute a production error by ID to suppress email notifications", + promptGuidelines: [ + "Use mute_production_error to silence notifications for a noisy or already-addressed production error.", + "Get the error ID from fetch_production_errors first. Muted errors still appear in listings but show a [MUTED] label.", + ], + parameters: Type.Object({ + id: Type.Number({ + description: + "The error ID (integer) to mute. Get this from fetch_production_errors.", + }), + }), + async execute(_toolCallId, params, signal, _onUpdate, _ctx) { + if (signal?.aborted) { + return { content: [{ type: "text", text: "Cancelled" }] }; + } + + const token = resolveVar("api_token"); + const fqdn = resolveVar("service_fqdn_web"); + + const missing: string[] = []; + if (!token) missing.push("PI_API_TOKEN"); + if (!fqdn) missing.push("PI_SERVICE_FQDN_WEB"); + + if (missing.length > 0) { + return { + content: [ + { + type: "text", + text: + `Cannot mute production error: the following environment variables are not set:\n` + + missing.map((v) => ` - ${v}`).join("\n") + + `\n\nEnsure these are configured in your pi environment.`, + }, + ], + isError: true, + }; + } + + try { + const data = await postApi( + buildUrl(fqdn!, `/api/v1/errors/${params.id}/mute`), + token!, + (data) => data as ErrorDetailResponse, + signal, + ); + + return { + content: [ + { + type: "text", + text: + `Error #${params.id} muted successfully.` + + (data.error?.muted + ? "" + : " (Warning: muted field not reflected in response)"), + }, + ], + details: { errorId: params.id, muted: data.error?.muted }, + }; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + if (message.includes("404")) { + return { + content: [ + { + type: "text", + text: `Error with ID ${params.id} not found.`, + }, + ], + isError: true, + }; + } + return { + content: [ + { + type: "text", + text: `Failed to mute production error: ${message}`, + }, + ], + isError: true, + }; + } + }, + }); + + // ── unmute_production_error tool ─────────────────────────────────────── + + pi.registerTool({ + name: "unmute_production_error", + label: "Unmute Production Error", + description: + "Unmute a previously muted production error by ID, re-enabling email " + + "notifications for new occurrences.", + promptSnippet: + "Unmute a production error by ID to re-enable email notifications", + promptGuidelines: [ + "Use unmute_production_error to re-enable notifications for a previously muted error.", + "Get the error ID from fetch_production_errors first.", + ], + parameters: Type.Object({ + id: Type.Number({ + description: + "The error ID (integer) to unmute. Get this from fetch_production_errors.", + }), + }), + async execute(_toolCallId, params, signal, _onUpdate, _ctx) { + if (signal?.aborted) { + return { content: [{ type: "text", text: "Cancelled" }] }; + } + + const token = resolveVar("api_token"); + const fqdn = resolveVar("service_fqdn_web"); + + const missing: string[] = []; + if (!token) missing.push("PI_API_TOKEN"); + if (!fqdn) missing.push("PI_SERVICE_FQDN_WEB"); + + if (missing.length > 0) { + return { + content: [ + { + type: "text", + text: + `Cannot unmute production error: the following environment variables are not set:\n` + + missing.map((v) => ` - ${v}`).join("\n") + + `\n\nEnsure these are configured in your pi environment.`, + }, + ], + isError: true, + }; + } + + try { + const data = await postApi( + buildUrl(fqdn!, `/api/v1/errors/${params.id}/unmute`), + token!, + (data) => data as ErrorDetailResponse, + signal, + ); + + return { + content: [ + { + type: "text", + text: `Error #${params.id} unmuted successfully. Email notifications will resume for new occurrences.`, + }, + ], + details: { errorId: params.id, muted: data.error?.muted }, + }; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + if (message.includes("404")) { + return { + content: [ + { + type: "text", + text: `Error with ID ${params.id} not found.`, + }, + ], + isError: true, + }; + } + return { + content: [ + { + type: "text", + text: `Failed to unmute production error: ${message}`, + }, + ], + isError: true, + }; + } + }, + }); + + // ── resolve_production_error tool ─────────────────────────────────────── + + pi.registerTool({ + name: "resolve_production_error", + label: "Resolve Production Error", + description: + "Mark a production error as resolved by ID. Resolved errors are filtered " + + "out of the default unresolved view.", + promptSnippet: + "Mark a production error as resolved when the underlying issue has been fixed", + promptGuidelines: [ + "Use resolve_production_error to mark a production error as resolved when the underlying issue has been fixed.", + "Get the error ID from fetch_production_errors first.", + ], + parameters: Type.Object({ + id: Type.Number({ + description: + "The error ID (integer) to resolve. Get this from fetch_production_errors.", + }), + }), + async execute(_toolCallId, params, signal, _onUpdate, _ctx) { + if (signal?.aborted) { + return { content: [{ type: "text", text: "Cancelled" }] }; + } + + const token = resolveVar("api_token"); + const fqdn = resolveVar("service_fqdn_web"); + + const missing: string[] = []; + if (!token) missing.push("PI_API_TOKEN"); + if (!fqdn) missing.push("PI_SERVICE_FQDN_WEB"); + + if (missing.length > 0) { + return { + content: [ + { + type: "text", + text: + `Cannot resolve production error: the following environment variables are not set:\n` + + missing.map((v) => ` - ${v}`).join("\n") + + `\n\nEnsure these are configured in your pi environment.`, + }, + ], + isError: true, + }; + } + + try { + const data = await postApi( + buildUrl(fqdn!, `/api/v1/errors/${params.id}/resolve`), + token!, + (data) => data as ErrorDetailResponse, + signal, + ); + + return { + content: [ + { + type: "text", + text: + `Error #${params.id} resolved successfully.` + + (data.error?.status === "resolved" + ? "" + : " (Warning: status field not reflected in response)"), + }, + ], + details: { errorId: params.id, status: data.error?.status }, + }; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + if (message.includes("404")) { + return { + content: [ + { + type: "text", + text: `Error with ID ${params.id} not found.`, + }, + ], + isError: true, + }; + } + return { + content: [ + { + type: "text", + text: `Failed to resolve production error: ${message}`, + }, + ], + isError: true, + }; + } + }, + }); + + // ── unresolve_production_error tool ───────────────────────────────────── + + pi.registerTool({ + name: "unresolve_production_error", + label: "Unresolve Production Error", + description: + "Reopen a previously resolved production error by ID. Use when an error " + + "reoccurs after being marked as resolved.", + promptSnippet: + "Reopen a production error when it reoccurs after being resolved", + promptGuidelines: [ + "Use unresolve_production_error to reopen a production error when it reoccurs after being resolved.", + "Get the error ID from fetch_production_errors first.", + ], + parameters: Type.Object({ + id: Type.Number({ + description: + "The error ID (integer) to unresolve. Get this from fetch_production_errors.", + }), + }), + async execute(_toolCallId, params, signal, _onUpdate, _ctx) { + if (signal?.aborted) { + return { content: [{ type: "text", text: "Cancelled" }] }; + } + + const token = resolveVar("api_token"); + const fqdn = resolveVar("service_fqdn_web"); + + const missing: string[] = []; + if (!token) missing.push("PI_API_TOKEN"); + if (!fqdn) missing.push("PI_SERVICE_FQDN_WEB"); + + if (missing.length > 0) { + return { + content: [ + { + type: "text", + text: + `Cannot unresolve production error: the following environment variables are not set:\n` + + missing.map((v) => ` - ${v}`).join("\n") + + `\n\nEnsure these are configured in your pi environment.`, + }, + ], + isError: true, + }; + } + + try { + const data = await postApi( + buildUrl(fqdn!, `/api/v1/errors/${params.id}/unresolve`), + token!, + (data) => data as ErrorDetailResponse, + signal, + ); + + return { + content: [ + { + type: "text", + text: `Error #${params.id} reopened (unresolved) successfully.`, + }, + ], + details: { errorId: params.id, status: data.error?.status }, + }; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + if (message.includes("404")) { + return { + content: [ + { + type: "text", + text: `Error with ID ${params.id} not found.`, + }, + ], + isError: true, + }; + } + return { + content: [ + { + type: "text", + text: `Failed to unresolve production error: ${message}`, + }, + ], + isError: true, + }; + } + }, + }); + // ── /prod-errors interactive command ────────────────────────────────── pi.registerCommand("prod-errors", { diff --git a/backlog/tasks/ml-165 - Mute-and-resolve-production-errors-from-pi.md b/backlog/completed/ml-165 - Mute-and-resolve-production-errors-from-pi.md similarity index 69% rename from backlog/tasks/ml-165 - Mute-and-resolve-production-errors-from-pi.md rename to backlog/completed/ml-165 - Mute-and-resolve-production-errors-from-pi.md index 8d852327..0e6d5be0 100644 --- a/backlog/tasks/ml-165 - Mute-and-resolve-production-errors-from-pi.md +++ b/backlog/completed/ml-165 - Mute-and-resolve-production-errors-from-pi.md @@ -1,16 +1,25 @@ --- id: ML-165 title: Mute and resolve production errors from pi -status: To Do +status: Done assignee: [] -created_date: '2026-05-05 11:20' -updated_date: '2026-05-05 12:10' +created_date: "2026-05-05 11:20" +updated_date: "2026-05-05 12:27" labels: [] dependencies: [] references: - >- doc-12 - Research-Mute-and-Resolve-Production-Errors-Implementation-Routes.md +modified_files: + - lib/music_library/errors.ex + - lib/music_library_web/controllers/error_controller.ex + - lib/music_library_web/controllers/error_json.ex + - lib/music_library_web/router.ex + - test/music_library/errors_test.exs + - test/music_library_web/controllers/error_controller_test.exs + - .pi/extensions/prod-errors/index.ts + - docs/architecture.md priority: medium ordinal: 10000 --- @@ -18,35 +27,40 @@ ordinal: 10000 ## Description + Extend the production error tooling in pi so that it's possible to: 1. Mute and resolve issues from the `prod-errors` TUI (user action) 2. Mute and resolve issues via a tool (pi action) There are no endpoints for these two actions, so we would need to extend the application's v1/api/ endpoints to support them. + ## Acceptance Criteria + -- [ ] #1 POST /api/v1/errors/:id/mute sets muted=true and returns 200 with updated error -- [ ] #2 POST /api/v1/errors/:id/unmute sets muted=false and returns 200 with updated error -- [ ] #3 POST /api/v1/errors/:id/resolve sets status=:resolved and returns 200 with updated error -- [ ] #4 POST /api/v1/errors/:id/unresolve sets status=:unresolved and returns 200 with updated error -- [ ] #5 All four POST endpoints return 401 without Bearer token -- [ ] #6 All four POST endpoints return 404 for non-existent error ID -- [ ] #7 All four POST endpoints return 404 for non-integer ID -- [ ] #8 pi tools (mute_production_error, unmute_production_error, resolve_production_error, unresolve_production_error) work correctly -- [ ] #9 /prod-errors TUI: M key toggles mute state on selected error with visual feedback -- [ ] #10 /prod-errors TUI: R key toggles resolve/unresolve status on selected error with visual feedback -- [ ] #11 /prod-errors TUI help text shows new M and R keybindings -- [ ] #12 All context function tests pass -- [ ] #13 All controller tests pass -- [ ] #14 Documentation updated: docs/architecture.md reflects new endpoints and context description + +- [x] #1 POST /api/v1/errors/:id/mute sets muted=true and returns 200 with updated error +- [x] #2 POST /api/v1/errors/:id/unmute sets muted=false and returns 200 with updated error +- [x] #3 POST /api/v1/errors/:id/resolve sets status=:resolved and returns 200 with updated error +- [x] #4 POST /api/v1/errors/:id/unresolve sets status=:unresolved and returns 200 with updated error +- [x] #5 All four POST endpoints return 401 without Bearer token +- [x] #6 All four POST endpoints return 404 for non-existent error ID +- [x] #7 All four POST endpoints return 404 for non-integer ID +- [x] #8 pi tools (mute_production_error, unmute_production_error, resolve_production_error, unresolve_production_error) work correctly +- [x] #9 /prod-errors TUI: M key toggles mute state on selected error with visual feedback +- [x] #10 /prod-errors TUI: R key toggles resolve/unresolve status on selected error with visual feedback +- [x] #11 /prod-errors TUI help text shows new M and R keybindings +- [x] #12 All context function tests pass +- [x] #13 All controller tests pass +- [x] #14 Documentation updated: docs/architecture.md reflects new endpoints and context description ## Implementation Plan + ## Implementation Plan: Route A — Four dedicated POST endpoints ### Overview @@ -158,6 +172,7 @@ post "/errors/:id/unresolve", ErrorController, :unresolve **File:** `test/music_library/errors_test.exs` Add a `describe "mute_error/1, unmute_error/1, resolve_error/1, unresolve_error/1"` block with tests for: + - `mute_error/1` sets `muted` to `true` - `unmute_error/1` sets `muted` to `false` - `resolve_error/1` sets `status` to `:resolved` @@ -168,6 +183,7 @@ Add a `describe "mute_error/1, unmute_error/1, resolve_error/1, unresolve_error/ **File:** `test/music_library_web/controllers/error_controller_test.exs` Add a `describe "POST /api/v1/errors/:id/mute|unmute|resolve|unresolve"` block with tests for: + - Each endpoint returns 401 without Bearer token - Each endpoint returns 200 with updated error on success - Each endpoint returns 404 for non-existent ID @@ -187,6 +203,7 @@ Register four new tools after the existing `fetch_production_error` tool: 4. **`unresolve_production_error`** — POSTs to `/api/v1/errors/:id/unresolve` Each tool: + - Takes a single `id` (number) parameter - Validates env vars (`PI_API_TOKEN`, `PI_SERVICE_FQDN_WEB`) like existing tools - Makes a POST request with Bearer auth @@ -196,6 +213,7 @@ Each tool: Add a shared helper `postApi` for POST requests (similar to the existing `fetchApi` for GET, adding `method: "POST"`). Add prompt guidelines: + - `mute_production_error`: "Use mute_production_error to silence notifications for a noisy or already-addressed production error." - `unmute_production_error`: "Use unmute_production_error to re-enable notifications for a previously muted error." - `resolve_production_error`: "Use resolve_production_error to mark a production error as resolved when the underlying issue has been fixed." @@ -224,6 +242,7 @@ Add to the `ErrorBrowser` class: 3. **Update help text** in `renderList` and `renderDetail` to show the new keys. Keybinding logic (list mode): + ``` M → if error.muted → POST /unmute; else → POST /mute R → if error.status === "resolved" → POST /unresolve; else → POST /resolve @@ -232,6 +251,7 @@ R → if error.status === "resolved" → POST /unresolve; else → POST /resolve After a successful API call, update the local `ErrorListItem` in `this.errors` and call `this.invalidate()`. **Visual feedback on success:** + - The error's list entry re-renders immediately: the `[MUTED]` label appears/disappears, and the status badge (`[RESOLVED]` / `[UNRESOLVED]`) updates. - In detail mode, the header line (`Status: … | Muted: …`) updates on next render. - On failure: a toast notification via `this.notify()` displays the error message. @@ -245,6 +265,7 @@ After a successful API call, update the local `ErrorListItem` in `this.errors` a Update the `@moduledoc` to reflect that the module now handles both queries and mutations. The current text describes it as read-only — add a line noting that it also provides `mute_error/1`, `unmute_error/1`, `resolve_error/1`, and `unresolve_error/1` for mutating error state. **File:** `docs/architecture.md` + - Under the Routes section (if it enumerates API routes), add the four new POST endpoints: `/api/v1/errors/:id/mute`, `/api/v1/errors/:id/unmute`, `/api/v1/errors/:id/resolve`, `/api/v1/errors/:id/unresolve`. - Under Contexts → `Errors`, update the description from "Read-only queries" to "Queries and mutations for production error data". Note that muting an error suppresses future email notifications via `ErrorTracker.ErrorNotifier` (which checks the `muted` field before dispatching). - Under the Controller table, update the `ErrorController` row to include the four new action routes. @@ -253,16 +274,16 @@ Update the `@moduledoc` to reflect that the module now handles both queries and ## Architecture Impact Summary -| Component | Change | -|---|---| -| `MusicLibrary.Errors` | +4 public functions, +1 private helper, updated moduledoc | -| `MusicLibraryWeb.ErrorController` | +4 actions, +1 private helper | -| `MusicLibraryWeb.ErrorJSON` | +1 render function | -| `MusicLibraryWeb.Router` | +4 POST routes | -| `.pi/extensions/prod-errors/index.ts` | +4 tools, +2 TUI methods, +2 keybindings, updated help text | -| `test/music_library/errors_test.exs` | +~8 tests (including idempotency for all four actions) | -| `test/music_library_web/controllers/error_controller_test.exs` | +~12 tests | -| `docs/architecture.md` | Update errors context description, add routes, update controller table | +| Component | Change | +| -------------------------------------------------------------- | ---------------------------------------------------------------------- | +| `MusicLibrary.Errors` | +4 public functions, +1 private helper, updated moduledoc | +| `MusicLibraryWeb.ErrorController` | +4 actions, +1 private helper | +| `MusicLibraryWeb.ErrorJSON` | +1 render function | +| `MusicLibraryWeb.Router` | +4 POST routes | +| `.pi/extensions/prod-errors/index.ts` | +4 tools, +2 TUI methods, +2 keybindings, updated help text | +| `test/music_library/errors_test.exs` | +~8 tests (including idempotency for all four actions) | +| `test/music_library_web/controllers/error_controller_test.exs` | +~12 tests | +| `docs/architecture.md` | Update errors context description, add routes, update controller table | **No changes:** supervision tree, PubSub, schemas, migrations, Oban workers, LiveViews, external APIs, production infrastructure. @@ -282,4 +303,44 @@ Zero incremental cost. No external API calls. ## Production Infrastructure No changes needed (no new env vars, no DNS, no firewall, no special migration handling). + + +## Final Summary + + + +## Summary + +Added mute/unmute/resolve/unresolve capability for production errors across the full stack: + +### Backend (Elixir/Phoenix) + +- **`MusicLibrary.Errors`**: Added 4 public functions (`mute_error/1`, `unmute_error/1`, `resolve_error/1`, `unresolve_error/1`) that delegate to `ErrorTracker`'s built-in mutation functions. `resolve/1` and `unresolve/1` handle the idempotent case (already-resolved/already-unresolved) gracefully. +- **`MusicLibraryWeb.ErrorController`**: Added 4 POST actions (`mute/2`, `unmute/2`, `resolve/2`, `unresolve/2`) with a shared `perform_action/3` helper that parses integer IDs, delegates to context, and returns proper JSON responses (200, 404, 422). +- **`MusicLibraryWeb.ErrorJSON`**: Added `update/1` render function for the `:update` template atom. +- **`MusicLibraryWeb.Router`**: Added 4 POST routes under the authenticated `/api/v1` scope. + +### Tests + +- **Context tests** (9 new): Test all four functions on success, not_found, and idempotency for each. +- **Controller tests** (6 auth + 6 functional): Test 401 without token, 200 with updated state, 404 for non-existent/non-integer IDs. + +### Pi Extension (TypeScript) + +- **`postApi` helper**: Shared POST request helper with Bearer auth and validation. +- **4 new tools**: `mute_production_error`, `unmute_production_error`, `resolve_production_error`, `unresolve_production_error` — each takes an error ID, POSTs to the corresponding endpoint, and returns success/error. +- **TUI keybindings**: `M` (Shift+M) toggles mute, `R` (Shift+R) toggles resolve/unresolve on the selected error in both list and detail modes. Local state updates immediately on success with toast notifications. +- **Help text**: Updated in both list and detail modes to show the new keys. + +### Documentation + +- `docs/architecture.md`: Updated Errors context description from "Read-only" to "Queries and mutations", added new POST routes to ErrorController table. +- `lib/music_library/errors.ex`: Updated `@moduledoc` to reflect mutation capabilities. + +### Design decisions + +- Used `ErrorTracker.mute/1`, `unmute/1`, `resolve/1`, `unresolve/1` (which emit telemetry events) rather than raw `Ecto.Changeset.change/2` +- `resolve_error/1` and `unresolve_error/1` handle the already-resolved/unresolved case explicitly (ErrorTracker's functions pattern-match on current state and would crash otherwise) +- POST endpoints return the updated error as JSON (consistent with GET responses), using the same `error/1` render helper + diff --git a/backlog/docs/doc-12 - Research-Mute-and-Resolve-Production-Errors-Implementation-Routes.md b/backlog/docs/doc-12 - Research-Mute-and-Resolve-Production-Errors-Implementation-Routes.md index 26a2c8a2..721c20cd 100644 --- a/backlog/docs/doc-12 - Research-Mute-and-Resolve-Production-Errors-Implementation-Routes.md +++ b/backlog/docs/doc-12 - Research-Mute-and-Resolve-Production-Errors-Implementation-Routes.md @@ -1,9 +1,10 @@ --- id: doc-12 -title: 'Research: Mute and Resolve Production Errors Implementation Routes' +title: "Research: Mute and Resolve Production Errors Implementation Routes" type: specification -created_date: '2026-05-05 11:24' +created_date: "2026-05-05 11:24" --- + # Research: Mute and Resolve Production Errors Implementation Routes **Task:** ML-165 — Mute and resolve production errors from pi @@ -13,21 +14,21 @@ created_date: '2026-05-05 11:24' ### Backend (Elixir/Phoenix) -| Component | File | Status | -|---|---|---| -| Error schema | `deps/error_tracker/lib/error_tracker/schemas/error.ex` | Has `status` (:resolved/:unresolved) and `muted` (boolean) fields | -| Error context | `lib/music_library/errors.ex` | Read-only: `list_errors/1`, `get_error/1` — no update functions | -| Error controller | `lib/music_library_web/controllers/error_controller.ex` | `GET /api/v1/errors` (index), `GET /api/v1/errors/:id` (show) | -| Error JSON views | `lib/music_library_web/controllers/error_json.ex` | Renders error/occurrence data | -| Router | `lib/music_library_web/router.ex` | `/api/v1/errors` routes in API scope (Bearer token auth) | -| Auth | `lib/music_library_web/auth.ex` | `require_api_token/2` plug — Bearer token comparison | +| Component | File | Status | +| ---------------- | ------------------------------------------------------- | ----------------------------------------------------------------- | +| Error schema | `deps/error_tracker/lib/error_tracker/schemas/error.ex` | Has `status` (:resolved/:unresolved) and `muted` (boolean) fields | +| Error context | `lib/music_library/errors.ex` | Read-only: `list_errors/1`, `get_error/1` — no update functions | +| Error controller | `lib/music_library_web/controllers/error_controller.ex` | `GET /api/v1/errors` (index), `GET /api/v1/errors/:id` (show) | +| Error JSON views | `lib/music_library_web/controllers/error_json.ex` | Renders error/occurrence data | +| Router | `lib/music_library_web/router.ex` | `/api/v1/errors` routes in API scope (Bearer token auth) | +| Auth | `lib/music_library_web/auth.ex` | `require_api_token/2` plug — Bearer token comparison | ### Pi Extension (TypeScript) -| Component | File | Status | -|---|---|---| +| Component | File | Status | +| --------------------- | ------------------------------------- | ------------------------------------------------------------------------------------------------- | | prod-errors extension | `.pi/extensions/prod-errors/index.ts` | Tools: `fetch_production_errors`, `fetch_production_error`; Command: `/prod-errors` (TUI browser) | -| TUI ErrorBrowser | Same file, `ErrorBrowser` class | List view (no mute/resolve actions), Detail view (no mute/resolve actions) | +| TUI ErrorBrowser | Same file, `ErrorBrowser` class | List view (no mute/resolve actions), Detail view (no mute/resolve actions) | ### Gap Analysis @@ -48,12 +49,14 @@ POST /api/v1/errors/:id/resolve → sets status = :resolved ``` **Elixir changes:** + - `MusicLibrary.Errors`: Add `mute_error/1`, `resolve_error/1` (or a shared private helper) - `MusicLibraryWeb.ErrorController`: Add `mute/2` and `resolve/2` actions - `MusicLibraryWeb.ErrorJSON`: Add `mute/1`, `resolve/1` render functions - `MusicLibraryWeb.Router`: Add two POST routes in the API scope **Pi extension changes:** + - Register `mute_production_error` tool → POSTs to `/api/v1/errors/:id/mute` - Register `resolve_production_error` tool → POSTs to `/api/v1/errors/:id/resolve` - Add keybindings to `/prod-errors` TUI: @@ -62,6 +65,7 @@ POST /api/v1/errors/:id/resolve → sets status = :resolved - Or use single keys: `x` for mute, `d` for resolve (avoiding conflict with existing `m`/`r` filter toggles) **Pros:** + - Simplest approach — each endpoint does exactly one thing - RESTful resource/action pattern used elsewhere in the app (e.g., `/collection/latest`) - Easy to extend with unmute/unresolve later as additional endpoints @@ -69,6 +73,7 @@ POST /api/v1/errors/:id/resolve → sets status = :resolved - Easy to test independently **Cons:** + - Two new routes instead of one - If many more error actions are added, route count could grow @@ -81,11 +86,13 @@ PATCH /api/v1/errors/:id body: {"muted": true} or {"status": "resolved"} ``` **Pros:** + - Single route for all error mutations - Follows REST convention for partial updates - Easily extensible to other fields **Cons:** + - Parameter validation is more complex (must reject unknown fields, validate combinations) - Tool semantics are less clear — the pi tool would need to construct the PATCH body - PATCH semantics imply partial update; if both fields are sent, behavior must be defined @@ -99,12 +106,14 @@ POST /api/v1/errors/:id/actions body: {"action": "mute"} or {"action": "resolv ``` **Pros:** + - Single route for all actions - Easy to add new action types - Clean separation between action specification and execution - Tool semantics map cleanly (tool name = action) **Cons:** + - Less standard REST pattern - Requires action validation/dispatch layer - No existing precedent in the codebase @@ -125,16 +134,16 @@ Route A is the best fit for this project because: ## Architecture Impact -| Component | Impact | -|---|---| -| `MusicLibrary.Errors` | Add `mute_error/1`, `resolve_error/1` context functions | -| `MusicLibraryWeb.ErrorController` | Add `mute/2`, `resolve/2` actions | -| `MusicLibraryWeb.ErrorJSON` | Add render functions for success/error responses | -| `MusicLibraryWeb.Router` | Add two POST routes | -| `.pi/extensions/prod-errors/index.ts` | Add two tools, add TUI keybindings | -| `test/music_library_web/controllers/error_controller_test.exs` | Add tests for new endpoints | -| `test/music_library/errors_test.exs` | Add tests for new context functions (if separate test file exists) | -| `docs/architecture.md` | Update Routes section if it lists API routes | +| Component | Impact | +| -------------------------------------------------------------- | ------------------------------------------------------------------ | +| `MusicLibrary.Errors` | Add `mute_error/1`, `resolve_error/1` context functions | +| `MusicLibraryWeb.ErrorController` | Add `mute/2`, `resolve/2` actions | +| `MusicLibraryWeb.ErrorJSON` | Add render functions for success/error responses | +| `MusicLibraryWeb.Router` | Add two POST routes | +| `.pi/extensions/prod-errors/index.ts` | Add two tools, add TUI keybindings | +| `test/music_library_web/controllers/error_controller_test.exs` | Add tests for new endpoints | +| `test/music_library/errors_test.exs` | Add tests for new context functions (if separate test file exists) | +| `docs/architecture.md` | Update Routes section if it lists API routes | No changes needed to: supervision tree, PubSub, schemas, migrations, Oban workers, LiveViews, external APIs. @@ -156,10 +165,10 @@ No production infrastructure changes needed. The new endpoints are served by the ## Documentation Updates -| File | Change | -|---|---| -| `docs/architecture.md` | Update Routes section if API routes are enumerated there | -| README (if applicable) | N/A — API is internal tooling | +| File | Change | +| ------------------------------------- | -------------------------------------------------------------- | +| `docs/architecture.md` | Update Routes section if API routes are enumerated there | +| README (if applicable) | N/A — API is internal tooling | | `.pi/extensions/prod-errors/index.ts` | Self-documenting through tool descriptions and prompt snippets | ## Testing diff --git a/docs/architecture.md b/docs/architecture.md index 1797a40c..dd6c0d6f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -93,25 +93,25 @@ Last.fm schemas (separate, not Ecto-persisted to main DB): ## Contexts (lib/music_library/) -| Context | Schemas | Responsibility | -| ---------------------- | --------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `Records` | Record, RecordEmbedding, SearchIndex | CRUD, search, import from MusicBrainz, cover/genre/color management, PubSub notifications | -| `Collection` | Record (via SearchIndex) | Querying collected records (purchased_at != nil), stats, collected artist IDs, collection summary for AI chat | -| `Wishlist` | Record (via SearchIndex) | Querying wishlisted records (purchased_at is nil) | -| `Artists` | ArtistInfo, ArtistRecord | Artist metadata from MusicBrainz/Discogs/Wikipedia/Last.fm, images, search | -| `Assets` | Asset | Binary asset storage (covers, artist images), cache tracking, pruning unreferenced assets | -| `Notes` | Note | Free-text notes for records and artists | -| `Chats` | Chat, Message, StreamProvider, RecordChat, ArtistChat, CollectionChat | Persistent AI chat conversations for records, artists, and the collection, streaming AI chat behaviour and entity-specific implementations | -| `RecordSets` | RecordSet, RecordSetItem | User-curated record groupings with ordering | -| `ScrobbleRules` | ScrobbleRule | Rules to remap Last.fm scrobble data to correct MusicBrainz IDs; searchable by match_value/target/description, orderable by alphabetical or inserted_at | -| `ScrobbleActivity` | — | Scrobbling releases/media/tracks to Last.fm | -| `ListeningStats` | (LastFm.Track, RecordRelease, ArtistRecord, ArtistInfo) | Scrobble persistence, refresh scheduling, listening analytics, track CRUD, search, listing: scrobble counts, artist play counts (from DB), recent activity, top albums/artists by period | -| `OnlineStoreTemplates` | OnlineStoreTemplate | URL templates for buying records online; searchable by name/description | -| `Errors` | ErrorTracker.Error, ErrorTracker.Occurrence | Read-only queries for production error data tracked by ErrorTracker; filtered listing with pagination, single error with preloaded occurrences and computed counts | -| `Search` | (cross-context) | Universal search dispatcher across collection, wishlist, artists, record sets (delegates to domain contexts) | -| `Secrets` | Secret | Encrypted key-value storage (CRUD + delete) | -| `BarcodeScan` | (Result struct) | Barcode → MusicBrainz lookup workflow, async batch import for multiple new records | -| `Maintenance` | (Oban.Job, LastFm.Track) | Background job monitoring, database vacuum/optimize, scrobble data quality diagnostics | +| Context | Schemas | Responsibility | +| ---------------------- | --------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `Records` | Record, RecordEmbedding, SearchIndex | CRUD, search, import from MusicBrainz, cover/genre/color management, PubSub notifications | +| `Collection` | Record (via SearchIndex) | Querying collected records (purchased_at != nil), stats, collected artist IDs, collection summary for AI chat | +| `Wishlist` | Record (via SearchIndex) | Querying wishlisted records (purchased_at is nil) | +| `Artists` | ArtistInfo, ArtistRecord | Artist metadata from MusicBrainz/Discogs/Wikipedia/Last.fm, images, search | +| `Assets` | Asset | Binary asset storage (covers, artist images), cache tracking, pruning unreferenced assets | +| `Notes` | Note | Free-text notes for records and artists | +| `Chats` | Chat, Message, StreamProvider, RecordChat, ArtistChat, CollectionChat | Persistent AI chat conversations for records, artists, and the collection, streaming AI chat behaviour and entity-specific implementations | +| `RecordSets` | RecordSet, RecordSetItem | User-curated record groupings with ordering | +| `ScrobbleRules` | ScrobbleRule | Rules to remap Last.fm scrobble data to correct MusicBrainz IDs; searchable by match_value/target/description, orderable by alphabetical or inserted_at | +| `ScrobbleActivity` | — | Scrobbling releases/media/tracks to Last.fm | +| `ListeningStats` | (LastFm.Track, RecordRelease, ArtistRecord, ArtistInfo) | Scrobble persistence, refresh scheduling, listening analytics, track CRUD, search, listing: scrobble counts, artist play counts (from DB), recent activity, top albums/artists by period | +| `OnlineStoreTemplates` | OnlineStoreTemplate | URL templates for buying records online; searchable by name/description | +| `Errors` | ErrorTracker.Error, ErrorTracker.Occurrence | Queries and mutations for production error data tracked by ErrorTracker; filtered listing with pagination, single error with preloaded occurrences and computed counts, plus `mute_error/1`, `unmute_error/1`, `resolve_error/1`, `unresolve_error/1` for mutating error state. Muting an error suppresses future email notifications via `ErrorTracker.ErrorNotifier`. | +| `Search` | (cross-context) | Universal search dispatcher across collection, wishlist, artists, record sets (delegates to domain contexts) | +| `Secrets` | Secret | Encrypted key-value storage (CRUD + delete) | +| `BarcodeScan` | (Result struct) | Barcode → MusicBrainz lookup workflow, async batch import for multiple new records | +| `Maintenance` | (Oban.Job, LastFm.Track) | Background job monitoring, database vacuum/optimize, scrobble data quality diagnostics | --- @@ -338,15 +338,15 @@ All authenticated routes live inside a single `live_session` with three `on_moun ### Controllers -| Controller | Routes | Purpose | -| ---------------------- | ------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------- | -| `SessionController` | `/login`, `/sessions/create` | Login/logout | -| `HealthController` | `/health` | Health check | -| `LastFmController` | `/auth/last_fm/callback` | Last.fm OAuth | -| `ArchiveController` | `/backup`, `/api/v1/backup` | Database backup download (API route requires token) | -| `AssetController` | `/assets/:transform_payload`, `/public/assets/:transform_payload`, `/api/v1/assets/:transform_payload` | Serve images with transforms (public route for emails, API route requires token) | -| `CollectionController` | `/api/v1/collection/*` | JSON API for collection queries | -| `ErrorController` | `/api/v1/errors`, `/api/v1/errors/:id` | JSON API for production error queries (requires Bearer token) | +| Controller | Routes | Purpose | +| ---------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------- | +| `SessionController` | `/login`, `/sessions/create` | Login/logout | +| `HealthController` | `/health` | Health check | +| `LastFmController` | `/auth/last_fm/callback` | Last.fm OAuth | +| `ArchiveController` | `/backup`, `/api/v1/backup` | Database backup download (API route requires token) | +| `AssetController` | `/assets/:transform_payload`, `/public/assets/:transform_payload`, `/api/v1/assets/:transform_payload` | Serve images with transforms (public route for emails, API route requires token) | +| `CollectionController` | `/api/v1/collection/*` | JSON API for collection queries | +| `ErrorController` | `/api/v1/errors`, `/api/v1/errors/:id`, `/api/v1/errors/:id/mute`, `/api/v1/errors/:id/unmute`, `/api/v1/errors/:id/resolve`, `/api/v1/errors/:id/unresolve` | JSON API for production error queries and mutations (requires Bearer token); POST endpoints for mute/unmute/resolve/unresolve | --- diff --git a/lib/music_library/errors.ex b/lib/music_library/errors.ex index 73759f65..a3125385 100644 --- a/lib/music_library/errors.ex +++ b/lib/music_library/errors.ex @@ -1,11 +1,17 @@ defmodule MusicLibrary.Errors do @moduledoc """ - Queries for production errors tracked via ErrorTracker. + Queries and mutations for production errors tracked via ErrorTracker. Reads from the error_tracker_errors and error_tracker_occurrences tables (owned by the `ErrorTracker` library) through `MusicLibrary.Repo`. The tables are created by ErrorTracker's own migrations and do not require any additional schema work. + + Provides `mute_error/1`, `unmute_error/1`, `resolve_error/1`, and + `unresolve_error/1` for mutating error state (muted flag and status). + Muting an error suppresses future email notifications via + `ErrorTracker.ErrorNotifier`, which checks the `muted` field before + dispatching. """ import Ecto.Query, warn: false @@ -79,7 +85,44 @@ defmodule MusicLibrary.Errors do end end - # -- private helpers -- + @spec mute_error(pos_integer()) :: {:ok, Error.t()} | {:error, :not_found | Ecto.Changeset.t()} + def mute_error(id) do + case Repo.get(Error, id) do + nil -> {:error, :not_found} + %{muted: true} = error -> {:ok, error} + error -> ErrorTracker.mute(error) + end + end + + @spec unmute_error(pos_integer()) :: + {:ok, Error.t()} | {:error, :not_found | Ecto.Changeset.t()} + def unmute_error(id) do + case Repo.get(Error, id) do + nil -> {:error, :not_found} + %{muted: false} = error -> {:ok, error} + error -> ErrorTracker.unmute(error) + end + end + + @spec resolve_error(pos_integer()) :: + {:ok, Error.t()} | {:error, :not_found | Ecto.Changeset.t()} + def resolve_error(id) do + case Repo.get(Error, id) do + nil -> {:error, :not_found} + %{status: :resolved} = error -> {:ok, error} + error -> ErrorTracker.resolve(error) + end + end + + @spec unresolve_error(pos_integer()) :: + {:ok, Error.t()} | {:error, :not_found | Ecto.Changeset.t()} + def unresolve_error(id) do + case Repo.get(Error, id) do + nil -> {:error, :not_found} + %{status: :unresolved} = error -> {:ok, error} + error -> ErrorTracker.unresolve(error) + end + end defp base_query(filters) do Error diff --git a/lib/music_library_web/controllers/error_controller.ex b/lib/music_library_web/controllers/error_controller.ex index 54a5ae6f..704beb12 100644 --- a/lib/music_library_web/controllers/error_controller.ex +++ b/lib/music_library_web/controllers/error_controller.ex @@ -43,8 +43,32 @@ defmodule MusicLibraryWeb.ErrorController do end end + def mute(conn, %{"id" => id}), do: perform_action(conn, id, &Errors.mute_error/1) + def unmute(conn, %{"id" => id}), do: perform_action(conn, id, &Errors.unmute_error/1) + def resolve(conn, %{"id" => id}), do: perform_action(conn, id, &Errors.resolve_error/1) + def unresolve(conn, %{"id" => id}), do: perform_action(conn, id, &Errors.unresolve_error/1) + # -- private helpers -- + defp perform_action(conn, id, action_fn) do + case Integer.parse(id) do + {id_int, ""} when id_int > 0 -> + case action_fn.(id_int) do + {:ok, error} -> + render(conn, :update, error: error) + + {:error, :not_found} -> + conn |> put_status(:not_found) |> json(%{error: "Not Found"}) + + {:error, _changeset} -> + conn |> put_status(:unprocessable_entity) |> json(%{error: "Update failed"}) + end + + _ -> + conn |> put_status(:not_found) |> json(%{error: "Not Found"}) + end + end + defp parse_status(nil), do: nil defp parse_status("resolved"), do: :resolved defp parse_status("unresolved"), do: :unresolved diff --git a/lib/music_library_web/controllers/error_json.ex b/lib/music_library_web/controllers/error_json.ex index 9c9734dd..65ef339a 100644 --- a/lib/music_library_web/controllers/error_json.ex +++ b/lib/music_library_web/controllers/error_json.ex @@ -29,6 +29,10 @@ defmodule MusicLibraryWeb.ErrorJSON do } end + def update(%{error: error}) do + %{error: error(error)} + end + defp error(e) do %{ id: e.id, diff --git a/lib/music_library_web/router.ex b/lib/music_library_web/router.ex index 5c844e7f..91056788 100644 --- a/lib/music_library_web/router.ex +++ b/lib/music_library_web/router.ex @@ -132,6 +132,10 @@ defmodule MusicLibraryWeb.Router do get "/collection", CollectionController, :index get "/errors", ErrorController, :index get "/errors/:id", ErrorController, :show + post "/errors/:id/mute", ErrorController, :mute + post "/errors/:id/unmute", ErrorController, :unmute + post "/errors/:id/resolve", ErrorController, :resolve + post "/errors/:id/unresolve", ErrorController, :unresolve get "/assets/:transform_payload", AssetController, :show get "/backup", ArchiveController, :backup end diff --git a/test/music_library/errors_test.exs b/test/music_library/errors_test.exs index 072f1bc8..e0da4622 100644 --- a/test/music_library/errors_test.exs +++ b/test/music_library/errors_test.exs @@ -160,6 +160,63 @@ defmodule MusicLibrary.ErrorsTest do end end + describe "mute_error/1, unmute_error/1, resolve_error/1, unresolve_error/1" do + test "mute_error/1 sets muted to true" do + error = unique_error("mute", %{muted: false}) + assert {:ok, updated} = Errors.mute_error(error.id) + assert updated.muted == true + end + + test "unmute_error/1 sets muted to false" do + error = unique_error("unmute", %{muted: true}) + assert {:ok, updated} = Errors.unmute_error(error.id) + assert updated.muted == false + end + + test "resolve_error/1 sets status to :resolved" do + error = unique_error("res", %{status: :unresolved}) + assert {:ok, updated} = Errors.resolve_error(error.id) + assert updated.status == :resolved + end + + test "unresolve_error/1 sets status to :unresolved" do + error = unique_error("unres", %{status: :resolved}) + assert {:ok, updated} = Errors.unresolve_error(error.id) + assert updated.status == :unresolved + end + + test "returns {:error, :not_found} for non-existent id" do + assert Errors.mute_error(99_999) == {:error, :not_found} + assert Errors.unmute_error(99_999) == {:error, :not_found} + assert Errors.resolve_error(99_999) == {:error, :not_found} + assert Errors.unresolve_error(99_999) == {:error, :not_found} + end + + test "mute_error/1 on already-muted error succeeds (idempotent)" do + error = unique_error("mute_idem", %{muted: true}) + assert {:ok, updated} = Errors.mute_error(error.id) + assert updated.muted == true + end + + test "unmute_error/1 on already-unmuted error succeeds (idempotent)" do + error = unique_error("unmute_idem", %{muted: false}) + assert {:ok, updated} = Errors.unmute_error(error.id) + assert updated.muted == false + end + + test "resolve_error/1 on already-resolved error succeeds (idempotent)" do + error = unique_error("res_idem", %{status: :resolved}) + assert {:ok, updated} = Errors.resolve_error(error.id) + assert updated.status == :resolved + end + + test "unresolve_error/1 on already-unresolved error succeeds (idempotent)" do + error = unique_error("unres_idem", %{status: :unresolved}) + assert {:ok, updated} = Errors.unresolve_error(error.id) + assert updated.status == :unresolved + end + end + describe "escape_like_wildcards/1" do test "escapes % and _ characters" do assert Errors.escape_like_wildcards("100%") == "100\\%" diff --git a/test/music_library_web/controllers/error_controller_test.exs b/test/music_library_web/controllers/error_controller_test.exs index c7e42390..07059b90 100644 --- a/test/music_library_web/controllers/error_controller_test.exs +++ b/test/music_library_web/controllers/error_controller_test.exs @@ -16,6 +16,22 @@ defmodule MusicLibraryWeb.ErrorControllerTest do test "GET /api/v1/errors/:id requires a bearer token", %{conn: conn} do assert get(conn, ~p"/api/v1/errors/1").status == 401 end + + test "POST /api/v1/errors/:id/mute requires a bearer token", %{conn: conn} do + assert post(conn, ~p"/api/v1/errors/1/mute").status == 401 + end + + test "POST /api/v1/errors/:id/unmute requires a bearer token", %{conn: conn} do + assert post(conn, ~p"/api/v1/errors/1/unmute").status == 401 + end + + test "POST /api/v1/errors/:id/resolve requires a bearer token", %{conn: conn} do + assert post(conn, ~p"/api/v1/errors/1/resolve").status == 401 + end + + test "POST /api/v1/errors/:id/unresolve requires a bearer token", %{conn: conn} do + assert post(conn, ~p"/api/v1/errors/1/unresolve").status == 401 + end end describe "GET /api/v1/errors" do @@ -255,4 +271,243 @@ defmodule MusicLibraryWeb.ErrorControllerTest do assert json_response(conn, 404) == %{"error" => "Not Found"} end end + + describe "POST /api/v1/errors/:id/mute|unmute|resolve|unresolve" do + setup do + error = error_fixture(%{reason: "Test error", muted: false, status: :unresolved}) + %{error: error} + end + + test "POST mute sets muted to true", %{conn: conn, error: error} do + conn = + conn + |> put_req_header("authorization", "Bearer #{api_token()}") + |> post(~p"/api/v1/errors/#{error.id}/mute") + + assert %{"error" => returned} = json_response(conn, 200) + assert returned["muted"] == true + assert returned["id"] == error.id + end + + test "POST unmute sets muted to false", %{conn: conn} do + error = + error_fixture(%{ + reason: "Muted error", + muted: true, + status: :unresolved, + source_line: "lib/muted.ex:1", + source_function: "MutedModule.muted/0", + fingerprint: error_fingerprint(:runtime_error, "lib/muted.ex:1", "MutedModule.muted/0") + }) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_token()}") + |> post(~p"/api/v1/errors/#{error.id}/unmute") + + assert %{"error" => returned} = json_response(conn, 200) + assert returned["muted"] == false + end + + test "POST resolve sets status to resolved", %{conn: conn, error: error} do + conn = + conn + |> put_req_header("authorization", "Bearer #{api_token()}") + |> post(~p"/api/v1/errors/#{error.id}/resolve") + + assert %{"error" => returned} = json_response(conn, 200) + assert returned["status"] == "resolved" + end + + test "POST unresolve sets status to unresolved", %{conn: conn} do + error = + error_fixture(%{ + reason: "Resolved error", + muted: false, + status: :resolved, + source_line: "lib/resolved.ex:1", + source_function: "ResolvedModule.resolved/0", + fingerprint: + error_fingerprint(:runtime_error, "lib/resolved.ex:1", "ResolvedModule.resolved/0") + }) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_token()}") + |> post(~p"/api/v1/errors/#{error.id}/unresolve") + + assert %{"error" => returned} = json_response(conn, 200) + assert returned["status"] == "unresolved" + end + + test "all four endpoints return 404 for non-existent ID", %{conn: conn} do + conn = put_req_header(conn, "authorization", "Bearer #{api_token()}") + + assert post(conn, ~p"/api/v1/errors/99999/mute") |> json_response(404) == %{ + "error" => "Not Found" + } + + assert post(conn, ~p"/api/v1/errors/99999/unmute") |> json_response(404) == %{ + "error" => "Not Found" + } + + assert post(conn, ~p"/api/v1/errors/99999/resolve") |> json_response(404) == %{ + "error" => "Not Found" + } + + assert post(conn, ~p"/api/v1/errors/99999/unresolve") |> json_response(404) == %{ + "error" => "Not Found" + } + end + + test "all four endpoints return 404 for non-integer ID", %{conn: conn} do + conn = put_req_header(conn, "authorization", "Bearer #{api_token()}") + + assert post(conn, ~p"/api/v1/errors/not-an-id/mute") |> json_response(404) == %{ + "error" => "Not Found" + } + + assert post(conn, ~p"/api/v1/errors/not-an-id/unmute") |> json_response(404) == %{ + "error" => "Not Found" + } + + assert post(conn, ~p"/api/v1/errors/not-an-id/resolve") |> json_response(404) == %{ + "error" => "Not Found" + } + + assert post(conn, ~p"/api/v1/errors/not-an-id/unresolve") |> json_response(404) == %{ + "error" => "Not Found" + } + end + + test "all four endpoints return 404 for zero ID", %{conn: conn} do + conn = put_req_header(conn, "authorization", "Bearer #{api_token()}") + + assert post(conn, ~p"/api/v1/errors/0/mute") |> json_response(404) == %{ + "error" => "Not Found" + } + + assert post(conn, ~p"/api/v1/errors/0/unmute") |> json_response(404) == %{ + "error" => "Not Found" + } + + assert post(conn, ~p"/api/v1/errors/0/resolve") |> json_response(404) == %{ + "error" => "Not Found" + } + + assert post(conn, ~p"/api/v1/errors/0/unresolve") |> json_response(404) == %{ + "error" => "Not Found" + } + end + + test "all four endpoints return 404 for negative ID", %{conn: conn} do + conn = put_req_header(conn, "authorization", "Bearer #{api_token()}") + + assert post(conn, ~p"/api/v1/errors/-1/mute") |> json_response(404) == %{ + "error" => "Not Found" + } + + assert post(conn, ~p"/api/v1/errors/-1/unmute") |> json_response(404) == %{ + "error" => "Not Found" + } + + assert post(conn, ~p"/api/v1/errors/-1/resolve") |> json_response(404) == %{ + "error" => "Not Found" + } + + assert post(conn, ~p"/api/v1/errors/-1/unresolve") |> json_response(404) == %{ + "error" => "Not Found" + } + end + + test "POST mute on already-muted error succeeds (controller idempotency)", %{conn: conn} do + error = + error_fixture(%{ + reason: "Already muted", + muted: true, + status: :unresolved, + source_line: "lib/muted2.ex:1", + source_function: "MutedModule.muted2/0", + fingerprint: + error_fingerprint(:runtime_error, "lib/muted2.ex:1", "MutedModule.muted2/0") + }) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_token()}") + |> post(~p"/api/v1/errors/#{error.id}/mute") + + assert %{"error" => returned} = json_response(conn, 200) + assert returned["muted"] == true + end + + test "POST unmute on already-unmuted error succeeds (controller idempotency)", %{conn: conn} do + error = + error_fixture(%{ + reason: "Already unmuted", + muted: false, + status: :unresolved, + source_line: "lib/unmuted2.ex:1", + source_function: "UnmutedModule.unmuted2/0", + fingerprint: + error_fingerprint(:runtime_error, "lib/unmuted2.ex:1", "UnmutedModule.unmuted2/0") + }) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_token()}") + |> post(~p"/api/v1/errors/#{error.id}/unmute") + + assert %{"error" => returned} = json_response(conn, 200) + assert returned["muted"] == false + end + + test "POST resolve on already-resolved error succeeds (controller idempotency)", %{conn: conn} do + error = + error_fixture(%{ + reason: "Already resolved", + muted: false, + status: :resolved, + source_line: "lib/resolved2.ex:1", + source_function: "ResolvedModule.resolved2/0", + fingerprint: + error_fingerprint(:runtime_error, "lib/resolved2.ex:1", "ResolvedModule.resolved2/0") + }) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_token()}") + |> post(~p"/api/v1/errors/#{error.id}/resolve") + + assert %{"error" => returned} = json_response(conn, 200) + assert returned["status"] == "resolved" + end + + test "POST unresolve on already-unresolved error succeeds (controller idempotency)", %{ + conn: conn + } do + error = + error_fixture(%{ + reason: "Already unresolved", + muted: false, + status: :unresolved, + source_line: "lib/unresolved2.ex:1", + source_function: "UnresolvedModule.unresolved2/0", + fingerprint: + error_fingerprint( + :runtime_error, + "lib/unresolved2.ex:1", + "UnresolvedModule.unresolved2/0" + ) + }) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_token()}") + |> post(~p"/api/v1/errors/#{error.id}/unresolve") + + assert %{"error" => returned} = json_response(conn, 200) + assert returned["status"] == "unresolved" + end + end end