Add PATCH and DELETE endpoints with ownership checks (403 vs 404), inline rename on DiagramCard and editor header, delete confirmation dialog, and differentiated error states for forbidden/not-found. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
264 lines
14 KiB
Markdown
264 lines
14 KiB
Markdown
# Story 1.3: Diagram Access Control & Management
|
|
|
|
Status: done
|
|
|
|
## Story
|
|
|
|
As a diagram owner,
|
|
I want to rename, delete, and control access to my diagrams,
|
|
so that I can manage my workspace and protect my work.
|
|
|
|
## Acceptance Criteria
|
|
|
|
1. **Given** I am viewing a diagram I own, **When** I click the diagram title in the dashboard card or editor header, **Then** it becomes an inline editable field, **And** changes are saved on blur or Enter keypress via PATCH /diagrams/:id.
|
|
|
|
2. **Given** I own a diagram, **When** I click the delete action on a diagram card, **Then** a confirmation dialog appears warning that deletion is permanent, **And** on confirmation the diagram is soft-deleted (sets `deletedAt` timestamp) and removed from my dashboard.
|
|
|
|
3. **Given** I am not the owner of a diagram, **When** I attempt to access the diagram via direct URL, **Then** I receive a 403 Forbidden response, **And** I see a "You don't have access to this diagram" message.
|
|
|
|
4. **Given** any API request to diagram endpoints (GET /:id, PATCH /:id, DELETE /:id), **When** the request is processed, **Then** server-side middleware verifies the authenticated user is the diagram owner (or has a valid share token — covered in Epic 6), **And** unauthorized requests return 403 before any data mutation.
|
|
|
|
## Tasks / Subtasks
|
|
|
|
- [x] Task 1: Add PATCH and DELETE endpoints to diagram router (AC: #1, #2, #4)
|
|
- [x] 1.1: Add `updateDiagramSchema` Zod schema for PATCH (title only for now)
|
|
- [x] 1.2: Add PATCH /:id endpoint — ownership check → update title → return updated diagram
|
|
- [x] 1.3: Add DELETE /:id endpoint — ownership check → soft-delete via `deletedAt = new Date()` → return success
|
|
- [x] 1.4: Refactor GET /:id to distinguish 404 (not found) from 403 (not owner): query without userId filter first, then check ownership
|
|
- [x] Task 2: Add inline rename to DiagramCard (AC: #1)
|
|
- [x] 2.1: Add editable title state to DiagramCard — click title → input field, save on blur/Enter, cancel on Escape
|
|
- [x] 2.2: PATCH mutation via Hono RPC client with React Query invalidation
|
|
- [x] 2.3: Add kebab menu (DropdownMenu) to DiagramCard with Rename and Delete actions
|
|
- [x] Task 3: Add delete confirmation dialog (AC: #2)
|
|
- [x] 3.1: AlertDialog with warning text: "This action cannot be undone. The diagram will be permanently deleted."
|
|
- [x] 3.2: DELETE mutation via Hono RPC client with React Query invalidation + toast confirmation
|
|
- [x] Task 4: Add inline rename to diagram editor header (AC: #1)
|
|
- [x] 4.1: Replace static title in editor page with editable field — same blur/Enter/Escape pattern
|
|
- [x] 4.2: PATCH mutation with React Query invalidation
|
|
- [x] Task 5: Handle 403 error state in diagram editor (AC: #3)
|
|
- [x] 5.1: Detect 403 response from GET /:id and show "You don't have access to this diagram" with appropriate icon
|
|
- [x] 5.2: Differentiate 403 (access denied) from 404 (not found) in error UI
|
|
- [x] Task 6: Tests (AC: all)
|
|
- [x] 6.1: API schema validation tests for updateDiagramSchema
|
|
- [x] 6.2: Ownership check logic tests (403 vs 404 distinction)
|
|
|
|
## Dev Notes
|
|
|
|
### API — PATCH and DELETE Endpoints
|
|
|
|
Modify `packages/api/src/modules/diagram/router.ts` to add two new endpoints:
|
|
|
|
**PATCH /:id — Rename diagram:**
|
|
```typescript
|
|
const updateDiagramBodySchema = z.object({
|
|
title: z.string().min(1).max(255).optional(),
|
|
}).refine((data) => Object.keys(data).length > 0, {
|
|
message: "At least one field must be provided",
|
|
});
|
|
|
|
.patch("/:id", enforceAuth, validate("json", updateDiagramBodySchema), async (c) => {
|
|
const [d] = await db.select().from(diagram)
|
|
.where(and(eq(diagram.id, c.req.param("id")), isNull(diagram.deletedAt)));
|
|
|
|
if (!d) {
|
|
throw new HttpException(HttpStatusCode.NOT_FOUND, { code: "error.notFound" });
|
|
}
|
|
if (d.userId !== c.var.user.id) {
|
|
throw new HttpException(HttpStatusCode.FORBIDDEN, { code: "error.forbidden", message: "You don't have access to this diagram" });
|
|
}
|
|
|
|
const [updated] = await db.update(diagram)
|
|
.set(c.req.valid("json"))
|
|
.where(eq(diagram.id, c.req.param("id")))
|
|
.returning();
|
|
|
|
return c.json({ data: updated });
|
|
})
|
|
```
|
|
|
|
**DELETE /:id — Soft-delete diagram:**
|
|
```typescript
|
|
.delete("/:id", enforceAuth, async (c) => {
|
|
const [d] = await db.select().from(diagram)
|
|
.where(and(eq(diagram.id, c.req.param("id")), isNull(diagram.deletedAt)));
|
|
|
|
if (!d) {
|
|
throw new HttpException(HttpStatusCode.NOT_FOUND, { code: "error.notFound" });
|
|
}
|
|
if (d.userId !== c.var.user.id) {
|
|
throw new HttpException(HttpStatusCode.FORBIDDEN, { code: "error.forbidden", message: "You don't have access to this diagram" });
|
|
}
|
|
|
|
await db.update(diagram)
|
|
.set({ deletedAt: new Date() })
|
|
.where(eq(diagram.id, c.req.param("id")));
|
|
|
|
return c.json({ data: { success: true } });
|
|
})
|
|
```
|
|
|
|
**Refactor GET /:id — 403 vs 404 distinction:**
|
|
|
|
Current GET /:id combines ownership + existence in one WHERE clause, returning 404 for both cases. Refactor to:
|
|
|
|
```typescript
|
|
.get("/:id", enforceAuth, async (c) => {
|
|
const [d] = await db.select().from(diagram)
|
|
.where(and(eq(diagram.id, c.req.param("id")), isNull(diagram.deletedAt)));
|
|
|
|
if (!d) {
|
|
throw new HttpException(HttpStatusCode.NOT_FOUND, { code: "error.notFound" });
|
|
}
|
|
if (d.userId !== c.var.user.id) {
|
|
throw new HttpException(HttpStatusCode.FORBIDDEN, { code: "error.forbidden", message: "You don't have access to this diagram" });
|
|
}
|
|
|
|
return c.json({ data: d });
|
|
})
|
|
```
|
|
|
|
Note: When share tokens are added (Epic 6), the ownership check will expand to also accept valid share tokens: `if (d.userId !== c.var.user.id && !validShareToken) { 403 }`.
|
|
|
|
### Frontend — DiagramCard Enhancements
|
|
|
|
Modify `apps/web/src/modules/diagram/components/DiagramCard.tsx`:
|
|
|
|
**Add kebab menu with Rename/Delete actions:**
|
|
- Use `DropdownMenu` (not ContextMenu) — consistent with Story 1.2's ProjectContextMenu pattern
|
|
- Menu items: Rename (opens inline edit), Delete (opens AlertDialog)
|
|
- Import: `DropdownMenu`, `DropdownMenuTrigger`, `DropdownMenuContent`, `DropdownMenuItem` from `@turbostarter/ui-web/dropdown-menu`
|
|
- Import: `AlertDialog`, `AlertDialogAction`, `AlertDialogCancel`, `AlertDialogContent`, `AlertDialogDescription`, `AlertDialogFooter`, `AlertDialogHeader`, `AlertDialogTitle` from `@turbostarter/ui-web/alert-dialog`
|
|
|
|
**Inline editable title:**
|
|
- State: `isEditing` boolean, `editValue` string
|
|
- Click title (or Rename from menu) → show `Input` component
|
|
- Save on blur/Enter → `api.diagrams[":id"].$patch({ param: { id }, json: { title: editValue } })`
|
|
- Cancel on Escape → revert to original title
|
|
- React Query invalidation: `["diagrams"]` and `["diagram", id]`
|
|
- Use `toast()` from `sonner` for success/error feedback
|
|
|
|
**Delete confirmation:**
|
|
- AlertDialog matches Story 1.2's project delete pattern
|
|
- On confirm → `api.diagrams[":id"].$delete({ param: { id } })`
|
|
- React Query invalidation: `["diagrams"]` + navigate away if in editor
|
|
- Toast: "Diagram deleted"
|
|
|
|
### Frontend — Editor Header Rename
|
|
|
|
Modify `apps/web/src/app/[locale]/dashboard/(user)/diagram/[id]/page.tsx`:
|
|
|
|
Replace the static `<h1>{data?.data?.title}</h1>` with an inline editable title using the same pattern as DiagramCard. The editor page already fetches the diagram — just add edit state and PATCH mutation.
|
|
|
|
### Frontend — 403 Error Handling
|
|
|
|
The editor page currently shows a generic error for any API failure. Enhance to distinguish:
|
|
|
|
```typescript
|
|
const { data, isLoading, error } = useQuery({
|
|
queryKey: ["diagram", params.id],
|
|
queryFn: async () => {
|
|
const res = await api.diagrams[":id"].$get({ param: { id: params.id } });
|
|
if (res.status === 403) {
|
|
throw new Error("forbidden");
|
|
}
|
|
if (!res.ok) {
|
|
throw new Error("not-found");
|
|
}
|
|
return await res.json();
|
|
},
|
|
});
|
|
```
|
|
|
|
Render different error UIs:
|
|
- 403: `Icons.ShieldAlert` + "You don't have access to this diagram"
|
|
- 404/other: `Icons.AlertTriangle` + "Failed to load diagram. It may have been deleted."
|
|
|
|
### Project Structure Notes
|
|
|
|
- `packages/api/src/modules/diagram/router.ts` — MODIFIED: add PATCH /:id, DELETE /:id, refactor GET /:id
|
|
- `apps/web/src/modules/diagram/components/DiagramCard.tsx` — MODIFIED: add kebab menu, inline rename, delete dialog
|
|
- `apps/web/src/app/[locale]/dashboard/(user)/diagram/[id]/page.tsx` — MODIFIED: add editable title, 403 handling
|
|
- `packages/api/tests/diagram/diagram-access-control.test.ts` — NEW: ownership and access control schema tests
|
|
- Alignment: All paths match architecture doc patterns. No new modules needed.
|
|
|
|
### Anti-Patterns to Avoid
|
|
|
|
- **NEVER hard-delete diagrams** — always soft-delete via `deletedAt = new Date()`. The `deletedAt` column already exists.
|
|
- **NEVER return 404 for ownership failures** — return 403 to distinguish "doesn't exist" from "not authorized". This is a security-aware choice for this story (the AC explicitly requires 403).
|
|
- **NEVER use raw fetch** — always use Hono RPC client (`api.diagrams[":id"].$patch(...)`)
|
|
- **NEVER inline `.parse()` in Hono handlers** — use `validate("json", schema)` middleware
|
|
- **NEVER put business logic in API routers** — ownership check is a simple comparison, acceptable inline. If it grows, extract to a helper.
|
|
- **DO NOT** change the diagram DB schema — `deletedAt` column already exists from Story 1.1
|
|
- **DO NOT** implement share token access control — that's Epic 6. Leave a comment placeholder in the ownership check.
|
|
- **DO NOT** add drag-and-drop — that's Story 1.4
|
|
- **DO NOT** add `Icons.ShieldAlert` to icons.tsx without checking if it already exists — check first
|
|
|
|
### Previous Story Intelligence (Story 1.2)
|
|
|
|
**Key learnings to carry forward:**
|
|
- `DropdownMenu` preferred over `ContextMenu` for action menus (more discoverable, accessible)
|
|
- `db.transaction()` for multi-step operations — NOT needed here since each endpoint does a single mutation
|
|
- `timeAgo()` exported from DiagramCard for reuse
|
|
- Hono RPC client pattern: `api.diagrams[":id"].$get({ param: { id } })` — NOT raw fetch
|
|
- `toast()` from `sonner` for user feedback
|
|
- React Query invalidation: `queryClient.invalidateQueries({ queryKey: ["diagrams"] })`
|
|
- PATCH validation: use `.refine()` to require at least one field (lesson from code review issue #4)
|
|
- 107 existing tests pass — don't break them
|
|
|
|
### Git Intelligence
|
|
|
|
Recent commits:
|
|
- `85e06c2 feat: implement Story 1.2 — organize diagrams into projects` — 12 files, project CRUD + sidebar
|
|
- `392da38 feat: implement Story 1.1 — create and view diagrams` — 20 files, diagram schema + CRUD + dashboard
|
|
|
|
Established patterns:
|
|
- Commit message: `feat: implement Story X.Y — description`
|
|
- Co-located tests: `packages/api/tests/diagram/`
|
|
- Zod schemas exported from router files for type-safe client usage
|
|
|
|
### References
|
|
|
|
- [Source: _bmad-output/planning-artifacts/epics.md#Story 1.3] — Full AC and technical notes
|
|
- [Source: _bmad-output/planning-artifacts/architecture.md#Implementation Patterns] — Naming, structure, enforcement rules
|
|
- [Source: _bmad-output/planning-artifacts/architecture.md#Enforcement Guidelines] — 7 mandatory rules for AI agents
|
|
- [Source: _bmad-output/implementation-artifacts/1-2-organize-diagrams-into-projects.md] — Previous story learnings
|
|
- [Source: _bmad-output/project-context.md] — 62 critical implementation rules
|
|
- [Source: packages/api/src/modules/diagram/router.ts] — Current diagram router (GET /, GET /:id, POST /)
|
|
- [Source: packages/db/src/schema/diagram.ts] — Diagram schema with deletedAt column
|
|
- [Source: apps/web/src/modules/diagram/components/DiagramCard.tsx] — Current card component (no menu/edit)
|
|
- [Source: apps/web/src/app/[locale]/dashboard/(user)/diagram/[id]/page.tsx] — Current editor page (static title)
|
|
|
|
## Dev Agent Record
|
|
|
|
### Agent Model Used
|
|
|
|
Claude Opus 4.6
|
|
|
|
### Debug Log References
|
|
|
|
- TypeScript: 0 errors (both `packages/api` and `apps/web`)
|
|
- Tests: 120 passed (107 existing + 13 new) — 0 failures
|
|
|
|
### Completion Notes List
|
|
|
|
- Task 1: Added `updateDiagramBodySchema` with `.refine()` for at least one field. PATCH /:id and DELETE /:id endpoints with ownership check (403 vs 404 distinction). Refactored GET /:id to separate existence from ownership.
|
|
- Task 2: DiagramCard converted to client component with inline editable title, PATCH mutation via Hono RPC, DropdownMenu (kebab) with Rename/Delete actions.
|
|
- Task 3: AlertDialog for delete confirmation with destructive styling. DELETE mutation via Hono RPC with React Query invalidation + toast.
|
|
- Task 4: Editor page title replaced with inline editable field using same blur/Enter/Escape pattern as DiagramCard.
|
|
- Task 5: 403 detection via `res.status === 403` in useQuery. Differentiated error UIs: `Icons.Lock` for 403, `Icons.AlertTriangle` for 404. Note: Used `Icons.Lock` instead of `Icons.ShieldAlert` (not available in icons.tsx).
|
|
- Task 6: 13 tests total — 7 schema validation tests for `updateDiagramBodySchema` + 6 ownership check logic tests (403 vs 404 contract, shared helper, Epic 6 placeholder).
|
|
- **Code Review Fixes (Claude Opus 4.6):**
|
|
- H1: Added onClick to DiagramCard title for inline edit (AC #1 compliance)
|
|
- H2: Added 6 ownership check logic tests (Task 6.2)
|
|
- M1+L2: Replaced useState error anti-pattern with typed DiagramError class + useQuery error property
|
|
- M2: Added custom retry function to skip retries for 403/404 DiagramErrors
|
|
- M3: Extracted `getOwnedDiagram()` helper to DRY ownership check across GET/PATCH/DELETE
|
|
- L1: Added useEffect to sync editValue when diagram.title changes externally
|
|
|
|
### File List
|
|
|
|
- `packages/api/src/modules/diagram/router.ts` — MODIFIED: Added `updateDiagramBodySchema`, PATCH /:id, DELETE /:id, refactored GET /:id for 403 vs 404
|
|
- `apps/web/src/modules/diagram/components/DiagramCard.tsx` — MODIFIED: Added inline rename, kebab menu (DropdownMenu), delete confirmation (AlertDialog), PATCH/DELETE mutations
|
|
- `apps/web/src/app/[locale]/dashboard/(user)/diagram/[id]/page.tsx` — MODIFIED: Added inline editable title, 403/404 error differentiation
|
|
- `packages/api/tests/diagram/diagram-access-control.test.ts` — NEW: 7 schema validation tests for updateDiagramBodySchema
|