feat(inventory): read-only detail view for firearm and magazine records (#19)

[unclesp1d3r]

Summary

Closes #19. Adds dedicated, permission-aware read-only detail routes for a single firearm (/firearms/[id]) and magazine (/magazines/[id]), giving view-only grantees a correct destination and owners a clean "just look at it" surface. The detail page is the single home for one record: it hosts permission-gated Edit / Delete / Share, while view-only grantees reach a read-only page by clicking the record name.

Plan: docs/plans/2026-07-04-001-feat-firearm-magazine-detail-view-plan.md (R1–R19, AE1–AE7).

What's implemented

• U1 — Firearm detail route (/firearms/[id]): all fields read-only including the serial for any viewer (R4); owner/edit-gated Edit, owner-only Delete/Share (R8); in-place edit form (R11); embedded read-only range-session history (R14); delete redirects to the list (R15); heading focus + back link (R16/R18). Shared app/(app)/not-found.tsx renders the accessible 404 (R9).
• U2 — Magazine owner-only enforcement (server): updateMagazine now routes through a new authorizeOwnerOnlyUpdate, so an edit-grantee's save is rejected server-side (R13, AE6) — not just hidden in the UI. The Share control stops offering edit for magazines. Backed by a live-gated integration test.
• U3 — Magazine detail route (/magazines/[id]): owner-only actions; view- and edit-grantees get a purely read-only page (R7/R8).
• U4 — List entry points & action relocation: row names link to the detail routes (R6) with an id-fragment disambiguator when names collide (R17/R52); inline Edit (both) and Sessions (firearm) move to the detail page; owner-only quick Delete/Share stay on the list (R10).
• U5 — E2E coverage: new detail-view-sharing spec proves owner full actions, view-only read-only pages (serial + read-only sessions visible, no controls), magazine view-only sharing, not-found for no-access/malformed URLs, and delete-from-detail returning to the list. Six existing specs migrated from the removed list-row buttons to the detail-page flow.

Key decisions

• Dedicated routes over in-page panels — a bookmarkable URL fits the sharing model (a grantee can link straight to a record).
• Magazine editing is owner-only — a deliberate, accepted regression of current edit-grantee capability, enforced server-side. Bulk create-on-behalf (allow_create_on_behalf) is a separate, pre-existing surface left intact and explicitly out of scope.
• Serial is shown to view-only firearm grantees — an accepted decision for this collection-management tool, independent of the list's showSerial toggle.
• Per-entity detail views (no shared read-only renderer) — YAGNI.

Testing

• just ci-check green: Biome lint + format, tsc --noEmit, pre-commit, 233 unit/integration tests, 17 Playwright e2e tests.
• New: magazine owner-only authorization integration test; detail-view-sharing e2e.

Review notes

A code-review pass (correctness / security / maintainability) ran on the branch. Security found nothing. One real bug was found and fixed: a malformed (non-uuid) path id raised a Postgres cast error instead of a clean 404 — now validated at the request boundary with e2e coverage. The dead FirearmListItem.permission field (unused after Sessions moved off the list) was removed. Remaining P3 findings (small DetailRow duplication between the two detail views) are the deliberate per-entity-layout decision and were left as-is.

Note: the branch also carries earlier repo-infra commits (justfile with the ci-check gate, TOML formatting config, SBOM gitignore) that predate this feature work.

[coderabbitai]

Walkthrough

Adds firearm and magazine detail routes with permission-aware rendering, list-to-detail navigation, owner-only magazine updates, shared not-found handling, Playwright coverage, and local tooling updates.

Changes

Firearm and Magazine Detail Views

Layer / File(s)	Summary
Contracts and authorization docs/plans/2026-07-04-001-feat-firearm-magazine-detail-view-plan.md, src/lib/uuid.ts, app/(app)/firearms/[id]/page.tsx, app/(app)/magazines/[id]/page.tsx, src/domain/firearms/service.ts, src/domain/magazines/service.ts, src/auth/authorize.ts, src/domain/magazines/__tests__/authorize-owner-only.test.ts, src/domain/magazines/__tests__/service.test.ts	Adds route parameter validation, permission-returning domain loaders, owner-only magazine update authorization, and test coverage for the new authorization and magazine count behavior.
Firearm detail page and view app/(app)/firearms/firearm-detail-view.tsx, app/(app)/firearms/range-session-history.tsx, hooks/use-delete-confirmation.ts	Adds the firearm detail client view, owner-only actions, read-only field rendering, optional range-session history controls, and redirect-aware deletion.
Magazine detail page and view app/(app)/magazines/magazine-detail-view.tsx, app/(app)/grants/share-control.tsx, src/domain/magazines/service.ts	Adds the magazine detail client view, owner-only edit/delete controls, compatible-firearm rendering, and delete confirmation wiring.
List navigation and shared not-found app/(app)/firearms/firearms-view.tsx, app/(app)/firearms/page.tsx, app/(app)/magazines/magazines-view.tsx, app/(app)/not-found.tsx	Links list rows to detail pages, removes inline edit/session entry points, simplifies list item data, and adds the shared app 404 page.
End-to-end coverage e2e/detail-view-sharing.spec.ts, e2e/fixtures/user-pool.ts, e2e/firearm-nickname.spec.ts, e2e/firearm-taxonomy.spec.ts, e2e/inventory-crud.spec.ts, e2e/magpul-settings.spec.ts, e2e/range-sessions-sharing.spec.ts, e2e/range-sessions.spec.ts	Adds detail-view sharing coverage and updates existing Playwright flows to enter firearm and magazine detail pages through the new links.

Tooling and workflow config

Layer / File(s)	Summary
Workflow and config updates justfile, .taplo.toml, .gitignore, .mdformat.toml, mise.toml, AGENTS.md	Adds workflow recipes, Taplo formatting config, SBOM ignore rules, mdformat ordering, mise spacing, and the pre-commit just ci-check requirement.

Sequence Diagram(s)

sequenceDiagram
  participant Browser
  participant FirearmDetailPage
  participant Auth
  participant FirearmsService
  Browser->>FirearmDetailPage: request /firearms/[id]
  FirearmDetailPage->>Auth: getCurrentUser()
  Auth-->>FirearmDetailPage: user or none
  FirearmDetailPage->>FirearmDetailPage: isUuid(id)
  FirearmDetailPage->>FirearmsService: getFirearm(user.id, id)
  FirearmsService-->>FirearmDetailPage: firearm + permission or NotFoundError
  FirearmDetailPage->>FirearmsService: calibersForInput, magazineCountForFirearm, listFirearms
  FirearmDetailPage-->>Browser: render FirearmDetailView

Loading

sequenceDiagram
  participant Client
  participant MagazinesService
  participant AuthorizeOwnerOnlyUpdate
  participant Database
  Client->>MagazinesService: updateMagazine(...)
  MagazinesService->>AuthorizeOwnerOnlyUpdate: check actor permission
  AuthorizeOwnerOnlyUpdate-->>MagazinesService: owner / NotAuthorizedError / NotFoundError
  MagazinesService->>Database: persist magazine changes
  Database-->>MagazinesService: updated row
  MagazinesService-->>Client: result

Loading

Possibly related PRs

• unclesp1d3r/mag_stacker#13: Shares the same authorization module changes in src/auth/authorize.ts.
• unclesp1d3r/mag_stacker#25: Uses the same Playwright fixture and auth harness changes in e2e/fixtures/user-pool.ts.
• unclesp1d3r/mag_stacker#34: Touches the same firearm session-history flow updated by the detail view.

Suggested labels: enhancement, backend, frontend, testing, priority:medium

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	Repo tooling/docs changes like justfile, AGENTS.md, TOML configs, and the plan file are outside the detail-view issue scope.	Move infrastructure and documentation updates to a separate PR, keeping this change focused on the firearm and magazine detail-view feature.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR covers the issue's read-only detail routes, permission gating, accessibility, and e2e coverage.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title follows Conventional Commits and accurately summarizes the detail-view feature.
Description check	✅ Passed	The description is directly related to the PR and describes the read-only detail routes and permission changes.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch 19-add-a-read-only-detail-view-for-firearm-and-magazine-records

---

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

Adds dedicated, permission-aware read-only detail routes for firearms and magazines so view-only grantees have a correct destination and owners get a safe “inspect” surface, while relocating per-record actions off list rows. It also tightens magazine update authorization to be owner-only server-side and updates CI/dev tooling documentation/automation around the repo’s workflow.

Changes:

• Introduce /firearms/[id] and /magazines/[id] detail pages with permission-gated actions, shared 404, and UUID boundary validation.
• Enforce magazine updates as owner-only via new authorizeOwnerOnlyUpdate and remove “edit” as a shareable permission for magazines in the Share UI.
• Update Playwright E2E specs to navigate via the new detail routes; add new E2E/integration coverage; add/align local tooling (justfile, taplo/mdformat config, SBOM ignore).

Reviewed changes

Copilot reviewed 28 out of 30 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/lib/uuid.ts	Adds isUuid() route-param guard to avoid Postgres uuid cast errors and enable clean 404s.
src/domain/magazines/service.ts	Switches magazine update authorization to owner-only (authorizeOwnerOnlyUpdate).
src/domain/magazines/tests/authorize-owner-only.test.ts	Adds live-gated integration coverage for owner-only magazine updates.
src/auth/authorize.ts	Adds authorizeOwnerOnlyUpdate() helper to mirror owner-only delete semantics.
mise.toml	Formatting/alignment tweak (shellcheck entry).
justfile	Adds repo task runner recipes including ci-check gate, test/e2e helpers, SBOM, etc.
hooks/use-delete-confirmation.ts	Adds optional post-delete redirect for detail pages while reusing the shared delete flow.
e2e/range-sessions.spec.ts	Migrates navigation to firearm detail page for sessions workflow.
e2e/range-sessions-sharing.spec.ts	Updates sharing/session flow to use detail routes and asserts view-only gating.
e2e/magpul-settings.spec.ts	Updates edit flow to go through magazine detail route.
e2e/inventory-crud.spec.ts	Updates magazine edit flow to go through detail route and returns to list.
e2e/fixtures/user-pool.ts	Adds seeded users for the new detail-view sharing spec.
e2e/firearm-taxonomy.spec.ts	Updates firearm edit flow to go through detail route.
e2e/firearm-nickname.spec.ts	Updates firearm edit flow to go through detail route and returns to list.
e2e/detail-view-sharing.spec.ts	New end-to-end spec covering detail view permissions, not-found behavior, and delete redirect.
docs/plans/2026-07-04-001-feat-firearm-magazine-detail-view-plan.md	Adds implementation-ready plan and acceptance examples for the feature.
app/(app)/not-found.tsx	Adds shared accessible 404 page for the app segment.
app/(app)/magazines/magazines-view.tsx	Converts row names to links to detail route; removes inline edit form usage; keeps owner quick actions.
app/(app)/magazines/magazine-detail-view.tsx	New magazine detail client view with read-only layout and owner-only actions/edit-in-place.
app/(app)/magazines/[id]/page.tsx	New magazine detail server route (loads record + options, UUID guard, notFound on NotFoundError).
app/(app)/grants/share-control.tsx	Removes “edit” option for magazines (view-only sharing), preserving edit sharing for firearms.
app/(app)/firearms/range-session-history.tsx	Makes onClose optional so sessions history can be embedded on the detail page.
app/(app)/firearms/page.tsx	Removes now-dead per-row permission field from firearms list items.
app/(app)/firearms/firearms-view.tsx	Converts row names to links; removes inline edit + sessions controls from list rows.
app/(app)/firearms/firearm-detail-view.tsx	New firearm detail client view including serial display and embedded read-only session history with gated controls.
app/(app)/firearms/[id]/page.tsx	New firearm detail server route (UUID guard, notFound behavior, loads suggestions/summary).
AGENTS.md	Documents a strict pre-commit gate requiring just ci-check to pass before commits.
.taplo.toml	Adds taplo formatter config for TOML alignment/format consistency.
.mdformat.toml	Reorders/sets mdformat config fields.
.gitignore	Ignores generated SBOM output (sbom.cdx.json).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (2)

app/(app)/magazines/[id]/page.tsx (1)

28-39: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Redundant resolvePermission call duplicates getMagazine's internal check.

getMagazine already calls resolvePermission and throws NotFoundError when the magazine isn't owned/shared, then this page calls resolvePermission again just to get the display value passed to permission={permission ?? "view"}. This adds an extra DB round-trip per page load and opens a narrow window where a grant revoked between the two calls silently falls back to "view" instead of surfacing not-found.

Consider having getMagazine (or a variant) return the resolved permission alongside the row so the page doesn't re-query.

Also applies to: 70-70

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/`(app)/magazines/[id]/page.tsx around lines 28 - 39, The page is
re-querying permission with resolvePermission even though getMagazine already
performs the ownership/shared access check and throws NotFoundError. Update the
magazine load flow so getMagazine (or a nearby helper) returns the resolved
permission together with the magazine row, and have page.tsx use that value for
permission={...} instead of calling resolvePermission again. Keep the existing
NotFoundError handling in the getMagazine path and remove the redundant
Promise.all permission fetch.

app/(app)/firearms/[id]/page.tsx (1)

35-42: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Redundant permission lookup — getFirearm already resolves it.

Per src/domain/firearms/service.ts:102-115, getFirearm internally calls resolvePermission and throws NotFoundError when it's null. Line 37 then calls resolvePermission again for the same (user.id, "firearm", id) tuple — a second DB round trip for data already computed. It also reopens a (very unlikely) TOCTOU window: if access were revoked between the two calls, this second lookup could return null, and the permission ?? "view" fallback on line 65 would silently grant "view" instead of surfacing a 404.

Consider having getFirearm return the resolved permission alongside the record so callers don't refetch it.

♻️ Sketch

-export async function getFirearm(
-  actorId: string,
-  id: string,
-): Promise<Firearm> {
+export async function getFirearm(
+  actorId: string,
+  id: string,
+): Promise<{ firearm: Firearm; permission: Permission }> {
   const perm = await resolvePermission(db, actorId, "firearm", id);
   if (perm === null) throw new NotFoundError();
   const [row] = await db.select().from(firearm).where(eq(firearm.id, id)).limit(1);
   if (!row) throw new NotFoundError();
-  return row;
+  return { firearm: row, permission: perm };
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/`(app)/firearms/[id]/page.tsx around lines 35 - 42, `page.tsx` is doing a
redundant permission lookup for the same firearm access check already handled by
`getFirearm`. Update the firearm page flow so the permission comes from
`getFirearm` in `src/domain/firearms/service.ts` (or have that method return the
resolved permission alongside the record), and stop calling
`resolvePermission(db, user.id, "firearm", id)` separately. Make sure the later
`permission ?? "view"` fallback uses the permission returned from `getFirearm`,
so unauthorized access still surfaces `NotFoundError` instead of silently
defaulting to view.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/`(app)/magazines/magazine-detail-view.tsx:
- Around line 105-109: The magazine detail badge is showing the raw permission
value even for non-owners, which can misleadingly display an inert “edit” grant.
Update the shared-with-you display in magazine-detail-view.tsx so the Badge text
is normalized for magazines (for example, always show “view” for any non-owner)
while keeping the existing isOwner gating for Edit/Delete/Share actions. Use the
existing isOwner and permission logic in the magazine-detail-view component to
locate and adjust the badge rendering.

In `@src/auth/authorize.ts`:
- Around line 69-74: Update the docstring in authorize.ts so it no longer
references the nonexistent R70 requirement; replace it with the correct plan
reference, likely R9, in the comment for the owner-only update behavior. Keep
the wording aligned with the existing authorizeDelete-style explanation and
verify any other nearby comments or symbols in authorize/authorizeDelete use
only valid requirement IDs from the same plan.
- Around line 75-87: Extract the shared owner-only permission check used by
authorizeOwnerOnlyUpdate and authorizeDelete into a single helper so the gating
logic lives in one place. Update authorizeOwnerOnlyUpdate to delegate to that
shared function in src/auth/authorize.ts, preserving its NotAuthorizedError
message for edit/view and NotFoundError for everything else, while keeping the
owner-only success path unchanged. Use the existing symbols
authorizeOwnerOnlyUpdate, authorizeDelete, and resolvePermission to centralize
the permission resolution and avoid duplicated branching.

---

Nitpick comments:
In `@app/`(app)/firearms/[id]/page.tsx:
- Around line 35-42: `page.tsx` is doing a redundant permission lookup for the
same firearm access check already handled by `getFirearm`. Update the firearm
page flow so the permission comes from `getFirearm` in
`src/domain/firearms/service.ts` (or have that method return the resolved
permission alongside the record), and stop calling `resolvePermission(db,
user.id, "firearm", id)` separately. Make sure the later `permission ?? "view"`
fallback uses the permission returned from `getFirearm`, so unauthorized access
still surfaces `NotFoundError` instead of silently defaulting to view.

In `@app/`(app)/magazines/[id]/page.tsx:
- Around line 28-39: The page is re-querying permission with resolvePermission
even though getMagazine already performs the ownership/shared access check and
throws NotFoundError. Update the magazine load flow so getMagazine (or a nearby
helper) returns the resolved permission together with the magazine row, and have
page.tsx use that value for permission={...} instead of calling
resolvePermission again. Keep the existing NotFoundError handling in the
getMagazine path and remove the redundant Promise.all permission fetch.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 5faf2016-7d33-4143-a4e5-68dd7b43792a

📥 Commits

Reviewing files that changed from the base of the PR and between 00c5e0c and 9af28a5.

📒 Files selected for processing (30)

• .gitignore
• .mdformat.toml
• .taplo.toml
• AGENTS.md
• app/(app)/firearms/[id]/page.tsx
• app/(app)/firearms/firearm-detail-view.tsx
• app/(app)/firearms/firearms-view.tsx
• app/(app)/firearms/page.tsx
• app/(app)/firearms/range-session-history.tsx
• app/(app)/grants/share-control.tsx
• app/(app)/magazines/[id]/page.tsx
• app/(app)/magazines/magazine-detail-view.tsx
• app/(app)/magazines/magazines-view.tsx
• app/(app)/not-found.tsx
• docs/plans/2026-07-04-001-feat-firearm-magazine-detail-view-plan.md
• e2e/detail-view-sharing.spec.ts
• e2e/firearm-nickname.spec.ts
• e2e/firearm-taxonomy.spec.ts
• e2e/fixtures/user-pool.ts
• e2e/inventory-crud.spec.ts
• e2e/magpul-settings.spec.ts
• e2e/range-sessions-sharing.spec.ts
• e2e/range-sessions.spec.ts
• hooks/use-delete-confirmation.ts
• justfile
• mise.toml
• src/auth/authorize.ts
• src/domain/magazines/__tests__/authorize-owner-only.test.ts
• src/domain/magazines/service.ts
• src/lib/uuid.ts

💤 Files with no reviewable changes (1)

• app/(app)/firearms/page.tsx