docs(38): research phase — admin tag management
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
640
.planning/phases/38-admin-tag-management/38-RESEARCH.md
Normal file
640
.planning/phases/38-admin-tag-management/38-RESEARCH.md
Normal file
@@ -0,0 +1,640 @@
|
|||||||
|
# Phase 38: Admin — Tag Management - Research
|
||||||
|
|
||||||
|
**Researched:** 2026-04-19
|
||||||
|
**Domain:** Full-stack CRUD with hierarchical data (self-referential FK), admin panel patterns, cycle detection
|
||||||
|
**Confidence:** HIGH
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
Phase 38 is entirely additive on top of well-established Phase 36/37 admin infrastructure. The codebase already has tags (`id`, `name`, `createdAt`), a service (`getAllTags`), a route (`GET /api/tags`), a hook (`useTags`), and a disabled "Tags" sidebar entry in the admin shell. The phase adds a `parentId` FK to the schema, builds admin CRUD routes under `/api/admin/tags`, and adds two client routes (`/admin/tags` list + `/admin/tags/$tagId` edit). Every pattern needed — service layer, Hono route module, React Query admin hooks, edit page structure, delete confirmation dialog — was established in Phase 37 for global items and can be followed directly.
|
||||||
|
|
||||||
|
The only genuinely new technical concern is hierarchy management: building a flat-to-tree transformation for the list view, a recursive descendant collector for parent-picker filtering and cycle detection, and the server-side cycle guard. All of these are straightforward pure functions operating on the in-memory tag array — no recursive SQL CTEs are required because the tag count is small and the full list fits easily in a single query result.
|
||||||
|
|
||||||
|
The `parentId` Drizzle self-referential FK uses a deferred lambda `() => tags.id` to handle the forward reference. Migration follows the same single-ALTER pattern as the `is_admin` migration from Phase 36. The `ON DELETE SET NULL` semantic is already supported by Drizzle's FK options and does not need custom code.
|
||||||
|
|
||||||
|
**Primary recommendation:** Follow Phase 37 patterns exactly. The only new logic is tree flattening (list view) and cycle detection (service + client parent picker). Keep the tree component as pure local state — no Zustand needed.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
<user_constraints>
|
||||||
|
## User Constraints (from CONTEXT.md)
|
||||||
|
|
||||||
|
### Locked Decisions
|
||||||
|
|
||||||
|
- **D-01:** Add `parentId integer REFERENCES tags(id) ON DELETE SET NULL NULLABLE` to the `tags` table via Drizzle migration. Self-referential FK — no depth limit at the schema level.
|
||||||
|
- **D-02:** On parent deletion, `ON DELETE SET NULL` orphans children (they become top-level). No cascading child deletion.
|
||||||
|
- **D-03:** Collapsible tree view (not a flat table). Parent tags render rows; children are indented below. All nodes start expanded on page load.
|
||||||
|
- **D-04:** Columns: Name (with indent level visual) | Item Count | Actions.
|
||||||
|
- **D-05:** Search/filter mode: filter in-tree — non-matching rows are hidden in place. Parents with matching children remain visible; non-matching leaf nodes are hidden.
|
||||||
|
- **D-06:** No separate "Children Count" column — children are visible in the tree structure below each parent.
|
||||||
|
- **D-07:** Quick-add form at the top of the tag list (not a separate page). Fields: name + optional parent picker. Submits inline without navigation.
|
||||||
|
- **D-08:** Dedicated edit page at `/admin/tags/$tagId` — consistent with Phase 37 item edit pattern. Fields: name + parent picker.
|
||||||
|
- **D-09:** Delete lives on the edit page (not the list), following Phase 37 D-05.
|
||||||
|
- **D-10:** Arbitrary depth — any tag can be a parent of any other (no 1-level limit). Future-proofed for deep taxonomies.
|
||||||
|
- **D-11:** Cycle prevention is dual-layer:
|
||||||
|
- **Client**: Parent picker filters out the tag's own descendants from the dropdown options.
|
||||||
|
- **Server**: API validates that the new parent is not a descendant of the tag being updated. Returns 400 with a clear error message if a cycle is detected.
|
||||||
|
- **D-12:** Deleting a tag orphans its children — they become top-level tags (`parentId` SET NULL via FK cascade).
|
||||||
|
- **D-13:** Delete confirmation dialog shows: item count + child count warning. Example: "Delete 'sleeping-bag'? 12 items use this tag. Its 3 child tags will become top-level. This cannot be undone."
|
||||||
|
- **D-14:** If a tag has 0 items and 0 children, the confirmation is simplified: "Delete 'sleeping-bag'? This cannot be undone."
|
||||||
|
|
||||||
|
### Claude's Discretion
|
||||||
|
|
||||||
|
- Exact visual styling of tree indentation (border-left line, chevron icon, or indent padding)
|
||||||
|
- Whether to use a chevron toggle button or click-to-collapse on the row
|
||||||
|
- Implementation of the collapsible tree (local component state vs. Zustand)
|
||||||
|
- Exact error message copy for cycle detection rejection
|
||||||
|
- Whether to add a "Move to top-level" shortcut on the edit page in addition to the parent picker
|
||||||
|
|
||||||
|
### Deferred Ideas (OUT OF SCOPE)
|
||||||
|
|
||||||
|
- Drag-to-reparent
|
||||||
|
- Bulk operations (bulk delete, bulk reparent)
|
||||||
|
- Tag merge
|
||||||
|
- Tag usage analytics
|
||||||
|
</user_constraints>
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
<phase_requirements>
|
||||||
|
## Phase Requirements
|
||||||
|
|
||||||
|
| ID | Description | Research Support |
|
||||||
|
|----|-------------|------------------|
|
||||||
|
| ADMN-05 | Admin can browse all tags with item counts and parent/child relationships displayed | Tree list view with `getAdminTags` service returning `parentId`, `itemCount`, `childCount` |
|
||||||
|
| ADMN-06 | Admin can create a new tag with a name | Quick-add form at top of list; `POST /api/admin/tags` → `createTag` service |
|
||||||
|
| ADMN-07 | Admin can rename an existing tag | Edit page form; `PUT /api/admin/tags/:id` → `updateTag` service |
|
||||||
|
| ADMN-08 | Admin can assign a parent tag to a tag (enabling sub-tag hierarchy) | Parent picker on edit page; `parentId` FK in schema; cycle detection in service |
|
||||||
|
| ADMN-09 | Admin can remove a tag's parent assignment (making it top-level again) | Parent picker allows null selection; `PUT /api/admin/tags/:id` with `parentId: null` |
|
||||||
|
| ADMN-10 | Admin can delete a tag, with a warning if items are currently using it | Delete button on edit page; confirmation dialog showing item count + child count |
|
||||||
|
</phase_requirements>
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Architectural Responsibility Map
|
||||||
|
|
||||||
|
| Capability | Primary Tier | Secondary Tier | Rationale |
|
||||||
|
|------------|-------------|----------------|-----------|
|
||||||
|
| Tag CRUD persistence | API / Backend (`tag.service.ts`) | Database (Drizzle) | Business logic and data validation live in the service layer |
|
||||||
|
| Cycle detection (authoritative) | API / Backend (`tag.service.ts`) | — | Server is the authority; client filter is UX-only |
|
||||||
|
| Tree data assembly (flat → tree) | API / Backend (optional) or Frontend | — | Tag count is small; flat array with parentId is sufficient; client transforms |
|
||||||
|
| Collapsible tree rendering | Browser / Client | — | Pure UI state — local React state, no server involvement |
|
||||||
|
| Parent picker descendant filtering | Browser / Client | — | Derived from full tag list already in React Query cache |
|
||||||
|
| Admin auth guard | API / Backend (`requireAdmin` middleware) | Frontend (redirect in admin shell) | Server is authoritative; client redirect is UX convenience |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Standard Stack
|
||||||
|
|
||||||
|
### Core (verified from codebase)
|
||||||
|
|
||||||
|
| Library | Version | Purpose | Why Standard |
|
||||||
|
|---------|---------|---------|--------------|
|
||||||
|
| Drizzle ORM | (project version) | Schema definition + migrations + query builder | Already used throughout project |
|
||||||
|
| Hono | (project version) | HTTP route handlers | Existing admin routes pattern |
|
||||||
|
| `@hono/zod-validator` | (project version) | Request body validation | Used in `admin-items.ts` |
|
||||||
|
| Zod | (project version) | Schema definitions | Used in all admin route validators |
|
||||||
|
| TanStack React Query | (project version) | Server state, mutations, cache invalidation | Used in all admin hooks |
|
||||||
|
| TanStack Router | (project version) | File-based routing, `createFileRoute` | Used for all client pages |
|
||||||
|
| Tailwind CSS v4 | v4 | Styling | Project standard |
|
||||||
|
| bun:test | Bun 1.3.9 | Test runner | Verified — `bun test` works |
|
||||||
|
|
||||||
|
### Supporting
|
||||||
|
|
||||||
|
| Library | Version | Purpose | When to Use |
|
||||||
|
|---------|---------|---------|-------------|
|
||||||
|
| PGlite (`@electric-sql/pglite`) | (project version) | In-memory PostgreSQL for tests | `createTestDb()` in `tests/helpers/db.ts` |
|
||||||
|
|
||||||
|
**Installation:** No new packages required — all dependencies exist in the project.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Architecture Patterns
|
||||||
|
|
||||||
|
### System Architecture Diagram
|
||||||
|
|
||||||
|
```
|
||||||
|
Admin browser
|
||||||
|
│
|
||||||
|
├── GET /admin/tags → AdminTagsPage (list)
|
||||||
|
│ │
|
||||||
|
│ ├── useAdminTags() → GET /api/admin/tags
|
||||||
|
│ │ └── getAdminTags(db) → SELECT tags + item counts
|
||||||
|
│ │
|
||||||
|
│ ├── flat array → buildTree() → TagTree component
|
||||||
|
│ │ └── local expand/collapse state
|
||||||
|
│ │
|
||||||
|
│ └── QuickAddForm → useCreateAdminTag() → POST /api/admin/tags
|
||||||
|
│
|
||||||
|
└── GET /admin/tags/:tagId → AdminTagEditPage (edit)
|
||||||
|
│
|
||||||
|
├── useAdminTag(id) → GET /api/admin/tags/:id
|
||||||
|
│
|
||||||
|
├── Save form → useUpdateAdminTag() → PUT /api/admin/tags/:id
|
||||||
|
│ └── server: isDescendant() cycle check → 400 if cycle
|
||||||
|
│
|
||||||
|
└── Delete → useDeleteAdminTag() → DELETE /api/admin/tags/:id
|
||||||
|
└── DB: parentId SET NULL (FK cascade) on children
|
||||||
|
```
|
||||||
|
|
||||||
|
### Recommended Project Structure
|
||||||
|
|
||||||
|
New files to create:
|
||||||
|
|
||||||
|
```
|
||||||
|
src/
|
||||||
|
├── server/
|
||||||
|
│ ├── routes/
|
||||||
|
│ │ └── admin-tags.ts # CRUD handlers for /api/admin/tags
|
||||||
|
│ └── services/
|
||||||
|
│ └── tag.service.ts # Extend existing file with CRUD + cycle detection
|
||||||
|
├── client/
|
||||||
|
│ ├── hooks/
|
||||||
|
│ │ └── useAdminTags.ts # Admin tag hooks (mirrors useAdminGlobalItems pattern)
|
||||||
|
│ └── routes/
|
||||||
|
│ └── admin/
|
||||||
|
│ ├── tags.tsx # List page with tree view + quick-add
|
||||||
|
│ └── tags.$tagId.tsx # Edit page with rename + reparent + delete
|
||||||
|
```
|
||||||
|
|
||||||
|
Files to modify:
|
||||||
|
|
||||||
|
```
|
||||||
|
src/db/schema.ts # Add parentId to tags table
|
||||||
|
src/server/routes/admin.ts # Register adminTagRoutes
|
||||||
|
src/client/routes/admin.tsx # Enable "Tags" sidebar link
|
||||||
|
```
|
||||||
|
|
||||||
|
### Pattern 1: Self-Referential FK in Drizzle (PostgreSQL)
|
||||||
|
|
||||||
|
**What:** The `tags` table references itself via `parentId`. Drizzle requires a deferred lambda to handle the forward reference.
|
||||||
|
|
||||||
|
**When to use:** Any time a table references its own primary key.
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Source: [VERIFIED: existing codebase pattern at globalItemTags, onDelete cascade]
|
||||||
|
// Adapted for self-referential nullable FK
|
||||||
|
|
||||||
|
export const tags = pgTable("tags", {
|
||||||
|
id: serial("id").primaryKey(),
|
||||||
|
name: text("name").notNull().unique(),
|
||||||
|
parentId: integer("parent_id").references(() => tags.id, { onDelete: "set null" }),
|
||||||
|
createdAt: timestamp("created_at").defaultNow().notNull(),
|
||||||
|
});
|
||||||
|
```
|
||||||
|
|
||||||
|
**Migration generated:** Single ALTER statement, same as Phase 36's `is_admin` migration:
|
||||||
|
```sql
|
||||||
|
ALTER TABLE "tags" ADD COLUMN "parent_id" integer REFERENCES "tags"("id") ON DELETE SET NULL;
|
||||||
|
```
|
||||||
|
|
||||||
|
### Pattern 2: Admin Service Functions (tag.service.ts extensions)
|
||||||
|
|
||||||
|
**What:** Extend existing `tag.service.ts` with CRUD + aggregate queries.
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Source: [VERIFIED: adapted from global-item.service.ts pattern in codebase]
|
||||||
|
|
||||||
|
// Returns flat array; client builds tree
|
||||||
|
export async function getAdminTags(db: Db) {
|
||||||
|
const result = await db
|
||||||
|
.select({
|
||||||
|
id: tags.id,
|
||||||
|
name: tags.name,
|
||||||
|
parentId: tags.parentId,
|
||||||
|
itemCount: count(globalItemTags.globalItemId),
|
||||||
|
})
|
||||||
|
.from(tags)
|
||||||
|
.leftJoin(globalItemTags, eq(globalItemTags.tagId, tags.id))
|
||||||
|
.groupBy(tags.id, tags.name, tags.parentId)
|
||||||
|
.orderBy(asc(tags.name));
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
|
export async function getTagWithCounts(db: Db, id: number) {
|
||||||
|
const [tag] = await db
|
||||||
|
.select({
|
||||||
|
id: tags.id,
|
||||||
|
name: tags.name,
|
||||||
|
parentId: tags.parentId,
|
||||||
|
itemCount: count(globalItemTags.globalItemId),
|
||||||
|
})
|
||||||
|
.from(tags)
|
||||||
|
.leftJoin(globalItemTags, eq(globalItemTags.tagId, tags.id))
|
||||||
|
.where(eq(tags.id, id))
|
||||||
|
.groupBy(tags.id, tags.name, tags.parentId);
|
||||||
|
return tag ?? null;
|
||||||
|
}
|
||||||
|
|
||||||
|
// childCount is computed client-side from getAdminTags flat array
|
||||||
|
// (cheaper than a correlated subquery for small tag sets)
|
||||||
|
```
|
||||||
|
|
||||||
|
### Pattern 3: Cycle Detection (Server)
|
||||||
|
|
||||||
|
**What:** Before updating `parentId`, verify that the new parent is not a descendant of the tag being updated. Operates on the full flat tag array fetched from DB.
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Source: [ASSUMED — standard tree algorithm, not library-specific]
|
||||||
|
|
||||||
|
function isDescendant(
|
||||||
|
allTags: { id: number; parentId: number | null }[],
|
||||||
|
candidateParentId: number,
|
||||||
|
tagId: number
|
||||||
|
): boolean {
|
||||||
|
// Walk up the ancestor chain of candidateParentId
|
||||||
|
// If we encounter tagId, there's a cycle
|
||||||
|
let current: number | null = candidateParentId;
|
||||||
|
const visited = new Set<number>();
|
||||||
|
while (current !== null) {
|
||||||
|
if (current === tagId) return true;
|
||||||
|
if (visited.has(current)) break; // safety: existing cycle in data
|
||||||
|
visited.add(current);
|
||||||
|
const node = allTags.find((t) => t.id === current);
|
||||||
|
current = node?.parentId ?? null;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### Pattern 4: Admin Route Module (admin-tags.ts)
|
||||||
|
|
||||||
|
**What:** Hono route module following the same structure as `admin-items.ts`.
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Source: [VERIFIED: adapted from src/server/routes/admin-items.ts]
|
||||||
|
|
||||||
|
import { zValidator } from "@hono/zod-validator";
|
||||||
|
import { Hono } from "hono";
|
||||||
|
import { z } from "zod";
|
||||||
|
import { parseId } from "../lib/params.ts";
|
||||||
|
import { createTag, deleteTag, getAdminTags, getTagWithCounts, updateTag } from "../services/tag.service.ts";
|
||||||
|
|
||||||
|
type Env = { Variables: { db?: any; userId?: number } };
|
||||||
|
const app = new Hono<Env>();
|
||||||
|
|
||||||
|
const createTagSchema = z.object({
|
||||||
|
name: z.string().min(1),
|
||||||
|
parentId: z.number().int().positive().nullable().optional(),
|
||||||
|
});
|
||||||
|
|
||||||
|
const updateTagSchema = z.object({
|
||||||
|
name: z.string().min(1).optional(),
|
||||||
|
parentId: z.number().int().positive().nullable().optional(),
|
||||||
|
});
|
||||||
|
|
||||||
|
// GET /api/admin/tags — flat list with counts
|
||||||
|
app.get("/", async (c) => { ... });
|
||||||
|
|
||||||
|
// GET /api/admin/tags/:id — single tag with counts
|
||||||
|
app.get("/:id", async (c) => { ... });
|
||||||
|
|
||||||
|
// POST /api/admin/tags — create
|
||||||
|
app.post("/", zValidator("json", createTagSchema), async (c) => { ... });
|
||||||
|
|
||||||
|
// PUT /api/admin/tags/:id — update (with cycle check)
|
||||||
|
app.put("/:id", zValidator("json", updateTagSchema), async (c) => { ... });
|
||||||
|
|
||||||
|
// DELETE /api/admin/tags/:id
|
||||||
|
app.delete("/:id", async (c) => { ... });
|
||||||
|
|
||||||
|
export { app as adminTagRoutes };
|
||||||
|
```
|
||||||
|
|
||||||
|
### Pattern 5: Tree Building (Client)
|
||||||
|
|
||||||
|
**What:** Pure function transforming flat `AdminTag[]` to a `TreeNode[]` for rendering. Keep in component or a `lib/treeUtils.ts` helper.
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Source: [ASSUMED — standard algorithm]
|
||||||
|
|
||||||
|
interface AdminTag { id: number; name: string; parentId: number | null; itemCount: number; }
|
||||||
|
interface TreeNode extends AdminTag { children: TreeNode[]; depth: number; }
|
||||||
|
|
||||||
|
function buildTree(tags: AdminTag[]): TreeNode[] {
|
||||||
|
const map = new Map<number, TreeNode>();
|
||||||
|
tags.forEach(t => map.set(t.id, { ...t, children: [], depth: 0 }));
|
||||||
|
const roots: TreeNode[] = [];
|
||||||
|
map.forEach(node => {
|
||||||
|
if (node.parentId === null) {
|
||||||
|
roots.push(node);
|
||||||
|
} else {
|
||||||
|
const parent = map.get(node.parentId);
|
||||||
|
if (parent) {
|
||||||
|
node.depth = parent.depth + 1;
|
||||||
|
parent.children.push(node);
|
||||||
|
} else {
|
||||||
|
roots.push(node); // orphan (parent deleted) → treat as top-level
|
||||||
|
}
|
||||||
|
}
|
||||||
|
});
|
||||||
|
return roots;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Flatten tree to ordered rows for render
|
||||||
|
function flattenTree(nodes: TreeNode[], result: TreeNode[] = []): TreeNode[] {
|
||||||
|
for (const node of nodes) {
|
||||||
|
result.push(node);
|
||||||
|
flattenTree(node.children, result);
|
||||||
|
}
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### Pattern 6: Client Hooks (useAdminTags.ts)
|
||||||
|
|
||||||
|
**What:** Mirrors `useAdminGlobalItems.ts` — typed interfaces, `useQuery`/`useMutation`, query key invalidation.
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Source: [VERIFIED: adapted from src/client/hooks/useAdminGlobalItems.ts]
|
||||||
|
|
||||||
|
export interface AdminTag {
|
||||||
|
id: number;
|
||||||
|
name: string;
|
||||||
|
parentId: number | null;
|
||||||
|
itemCount: number;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function useAdminTags() {
|
||||||
|
return useQuery({
|
||||||
|
queryKey: ["admin-tags"],
|
||||||
|
queryFn: () => apiGet<AdminTag[]>("/api/admin/tags"),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
export function useCreateAdminTag() {
|
||||||
|
const queryClient = useQueryClient();
|
||||||
|
return useMutation({
|
||||||
|
mutationFn: (data: { name: string; parentId?: number | null }) =>
|
||||||
|
apiPost<AdminTag>("/api/admin/tags", data),
|
||||||
|
onSuccess: () => {
|
||||||
|
queryClient.invalidateQueries({ queryKey: ["admin-tags"] });
|
||||||
|
queryClient.invalidateQueries({ queryKey: ["tags"] }); // invalidate public cache too
|
||||||
|
},
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
export function useUpdateAdminTag() { ... }
|
||||||
|
export function useDeleteAdminTag() { ... }
|
||||||
|
```
|
||||||
|
|
||||||
|
### Anti-Patterns to Avoid
|
||||||
|
|
||||||
|
- **Recursive SQL CTE for tree traversal:** Unnecessary complexity for a small tag set. Fetch flat, build tree in JS.
|
||||||
|
- **Storing tree structure as nested JSON:** The `parentId` column is the canonical representation. Never denormalize.
|
||||||
|
- **Cycle detection only on the client:** The client filter is UX convenience only. The server MUST validate independently.
|
||||||
|
- **Infinite loop in `buildTree` if existing cycle in data:** Guard with a `visited` set in the ancestor walk.
|
||||||
|
- **Forgetting to invalidate `["tags"]` query key on mutations:** The public `useTags` hook and admin `useAdminTags` share the same underlying data but different query keys — both must be invalidated on create/update/delete.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Don't Hand-Roll
|
||||||
|
|
||||||
|
| Problem | Don't Build | Use Instead | Why |
|
||||||
|
|---------|-------------|-------------|-----|
|
||||||
|
| Request body validation | Manual type checks | Zod + `@hono/zod-validator` | Already in project; handles error responses automatically |
|
||||||
|
| Admin auth guard | Inline `isAdmin` checks per route | `requireAdmin` middleware (already exists) | `src/server/middleware/auth.ts` — one line: `app.use("/*", requireAuth, requireAdmin)` |
|
||||||
|
| Server state / cache | `useState` + `useEffect` + `fetch` | TanStack React Query | Already in project; mutations auto-invalidate |
|
||||||
|
| Delete confirmation modal | Custom overlay | Inline modal pattern from `items.$itemId.tsx` | Copy the fixed-inset-0 dialog pattern — 30 lines, zero deps |
|
||||||
|
|
||||||
|
**Key insight:** This phase has no new external dependencies. The entire implementation is wiring existing infrastructure with new data.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Common Pitfalls
|
||||||
|
|
||||||
|
### Pitfall 1: Self-Referential FK Forward Reference in Drizzle
|
||||||
|
|
||||||
|
**What goes wrong:** Writing `references(() => tags.id)` where `tags` is the table being defined causes a TypeScript/runtime error if the variable isn't yet in scope.
|
||||||
|
|
||||||
|
**Why it happens:** The table definition references itself before the `const tags = ...` assignment completes.
|
||||||
|
|
||||||
|
**How to avoid:** Drizzle handles this correctly when using a lambda `() => tags.id` — it defers the reference lookup. Confirmed by the `globalItemTags` FK pattern already in the codebase (`references(() => tags.id, { onDelete: "cascade" })`). The self-referential case works the same way. [VERIFIED: codebase at `globalItemTags.tagId`]
|
||||||
|
|
||||||
|
**Warning signs:** TypeScript complains about `tags` being used before assignment — only occurs if you use a direct reference `tags.id` instead of a lambda.
|
||||||
|
|
||||||
|
### Pitfall 2: Orphan Display After Parent Deletion
|
||||||
|
|
||||||
|
**What goes wrong:** A tag's `parentId` points to a deleted parent. The tree builder crashes or silently omits the orphaned tag.
|
||||||
|
|
||||||
|
**Why it happens:** `ON DELETE SET NULL` runs at DB level, but there's a window in tests or seed data where orphans could be created manually.
|
||||||
|
|
||||||
|
**How to avoid:** In `buildTree`, when a node's `parentId` is non-null but no matching parent is found in the map, treat the node as a root (already shown in the Pattern 5 code above).
|
||||||
|
|
||||||
|
**Warning signs:** Tags disappearing from the list after a parent deletion.
|
||||||
|
|
||||||
|
### Pitfall 3: Parent Picker Shows Tag's Own Descendants (Client Side)
|
||||||
|
|
||||||
|
**What goes wrong:** An admin selects a child tag as the parent of its ancestor, creating a cycle.
|
||||||
|
|
||||||
|
**Why it happens:** Parent picker renders all tags without filtering.
|
||||||
|
|
||||||
|
**How to avoid:** The parent picker on the edit page must exclude: (1) the tag itself, (2) all its descendants. Compute the descendant set client-side from the full tag list. The server provides the safety net (D-11).
|
||||||
|
|
||||||
|
**Warning signs:** 400 errors from the server on PUT requests with `{ error: "Cycle detected" }`.
|
||||||
|
|
||||||
|
### Pitfall 4: Missing `["tags"]` Query Invalidation
|
||||||
|
|
||||||
|
**What goes wrong:** The admin creates/renames/deletes a tag, but the public `useTags()` hook (used in the admin items filter chips) still shows stale data.
|
||||||
|
|
||||||
|
**Why it happens:** `useAdminTags` and `useTags` use different query keys (`["admin-tags"]` vs `["tags"]`). Invalidating only `["admin-tags"]` leaves the public cache stale.
|
||||||
|
|
||||||
|
**How to avoid:** All mutations in `useAdminTags.ts` must invalidate both `["admin-tags"]` and `["tags"]`.
|
||||||
|
|
||||||
|
### Pitfall 5: TanStack Router Route Tree Not Regenerated
|
||||||
|
|
||||||
|
**What goes wrong:** New route files `admin/tags.tsx` and `admin/tags.$tagId.tsx` are created but the auto-generated `routeTree.gen.ts` is not updated, causing 404s or build errors.
|
||||||
|
|
||||||
|
**Why it happens:** The route tree is auto-generated by Vite during `bun run dev` or `bun run build`. In development the watcher updates it automatically. In CI it must be regenerated.
|
||||||
|
|
||||||
|
**How to avoid:** Run `bun run dev` at least once after adding new route files so `routeTree.gen.ts` is regenerated. Never manually edit `routeTree.gen.ts`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Code Examples
|
||||||
|
|
||||||
|
### Registering Admin Tag Routes in admin.ts
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Source: [VERIFIED: src/server/routes/admin.ts pattern]
|
||||||
|
import { adminTagRoutes } from "./admin-tags.ts";
|
||||||
|
|
||||||
|
// Add after adminItemRoutes registration:
|
||||||
|
app.route("/tags", adminTagRoutes);
|
||||||
|
```
|
||||||
|
|
||||||
|
### Enabling Tags Sidebar Link in admin.tsx
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Source: [VERIFIED: src/client/routes/admin.tsx — replace disabled div]
|
||||||
|
|
||||||
|
// Replace:
|
||||||
|
<div className="flex items-center gap-2 px-3 py-2 rounded-lg text-sm text-gray-300 cursor-not-allowed" ...>
|
||||||
|
|
||||||
|
// With (same structure as Items link above it):
|
||||||
|
<Link
|
||||||
|
to="/admin/tags"
|
||||||
|
activeProps={{ className: "bg-gray-100 text-gray-900 font-medium" }}
|
||||||
|
inactiveProps={{ className: "text-gray-600 hover:bg-gray-50" }}
|
||||||
|
className="flex items-center gap-2 px-3 py-2 rounded-lg text-sm transition-colors"
|
||||||
|
>
|
||||||
|
<LucideIcon name="tag" size={16} />
|
||||||
|
<span>Tags</span>
|
||||||
|
</Link>
|
||||||
|
```
|
||||||
|
|
||||||
|
### Delete Confirmation Text Logic (D-13/D-14)
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Source: [VERIFIED: adapted from items.$itemId.tsx dialog pattern]
|
||||||
|
|
||||||
|
function getDeleteConfirmText(tag: AdminTagDetail): string {
|
||||||
|
const parts: string[] = [];
|
||||||
|
if (tag.itemCount > 0) {
|
||||||
|
parts.push(`${tag.itemCount} ${tag.itemCount === 1 ? "item uses" : "items use"} this tag.`);
|
||||||
|
}
|
||||||
|
if (tag.childCount > 0) {
|
||||||
|
parts.push(`Its ${tag.childCount} child ${tag.childCount === 1 ? "tag" : "tags"} will become top-level.`);
|
||||||
|
}
|
||||||
|
parts.push("This cannot be undone.");
|
||||||
|
return parts.join(" ");
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## State of the Art
|
||||||
|
|
||||||
|
| Old Approach | Current Approach | When Changed | Impact |
|
||||||
|
|--------------|------------------|--------------|--------|
|
||||||
|
| Flat tags (id, name only) | Hierarchical tags with `parentId` | Phase 38 | Enables sub-tag taxonomy (e.g. "down" under "sleeping-bag") |
|
||||||
|
| Disabled "Tags" sidebar entry | Active link to `/admin/tags` | Phase 38 | Admins can navigate to tag management |
|
||||||
|
|
||||||
|
**Deprecated/outdated:**
|
||||||
|
- None in this phase — purely additive.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Assumptions Log
|
||||||
|
|
||||||
|
| # | Claim | Section | Risk if Wrong |
|
||||||
|
|---|-------|---------|---------------|
|
||||||
|
| A1 | `buildTree` and `flattenTree` pure JS functions are sufficient (no recursive SQL CTE needed) | Architecture Patterns | Low — tag count is small; even 10,000 tags would be fast in JS |
|
||||||
|
| A2 | `childCount` is cheapest computed client-side from the flat array rather than via SQL subquery | Standard Stack | Low — trivially verifiable; a SQL subquery would also work |
|
||||||
|
| A3 | `isDescendant` cycle check iterates ancestor chain of `candidateParentId` (not descendants of `tagId`) | Architecture Patterns | Medium — logic must be verified during implementation; wrong direction breaks cycle detection silently |
|
||||||
|
| A4 | Drizzle self-referential FK with `onDelete: "set null"` generates correct SQL | Standard Stack | Low — Drizzle docs support this; existing `onDelete: "cascade"` FK in codebase confirms the option syntax |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Open Questions
|
||||||
|
|
||||||
|
1. **`childCount` on the single-tag GET endpoint (for edit page delete confirmation)**
|
||||||
|
- What we know: `getAdminTags` (list endpoint) returns all tags; `childCount` is derivable client-side.
|
||||||
|
- What's unclear: For the edit page, we need `childCount` for the specific tag being edited. We can either: (a) compute it client-side from the full `useAdminTags` list, or (b) add a correlated count to `getTagWithCounts`.
|
||||||
|
- Recommendation: Use the full list from `useAdminTags` cache on the edit page as well (React Query keeps it cached). No separate correlated subquery needed.
|
||||||
|
|
||||||
|
2. **`parentId` null on quick-add form submission**
|
||||||
|
- What we know: Parent picker is optional (D-07). No parent selected = `parentId: null`.
|
||||||
|
- What's unclear: Zod schema should accept `parentId: z.number().int().positive().nullable().optional()` — ensure `null` and `undefined` both result in DB `NULL`.
|
||||||
|
- Recommendation: Use `.nullable().optional()` and explicitly pass `null` when no parent is selected.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Environment Availability
|
||||||
|
|
||||||
|
Step 2.6: SKIPPED (no external dependencies beyond existing project stack — no new tools, services, or runtimes introduced).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Validation Architecture
|
||||||
|
|
||||||
|
### Test Framework
|
||||||
|
|
||||||
|
| Property | Value |
|
||||||
|
|----------|-------|
|
||||||
|
| Framework | Bun test runner (bun:test) |
|
||||||
|
| Config file | None — discovered automatically |
|
||||||
|
| Quick run command | `bun test tests/services/tag.service.test.ts tests/routes/tags.test.ts` |
|
||||||
|
| Full suite command | `bun test` |
|
||||||
|
|
||||||
|
### Phase Requirements → Test Map
|
||||||
|
|
||||||
|
| Req ID | Behavior | Test Type | Automated Command | File Exists? |
|
||||||
|
|--------|----------|-----------|-------------------|-------------|
|
||||||
|
| ADMN-05 | `getAdminTags` returns tags with `parentId`, `itemCount`, ordered alphabetically | unit | `bun test tests/services/tag.service.test.ts` | Exists (extend) |
|
||||||
|
| ADMN-06 | `POST /api/admin/tags` creates tag, returns 201 with tag object | integration | `bun test tests/routes/admin-tags.test.ts` | Wave 0 gap |
|
||||||
|
| ADMN-07 | `PUT /api/admin/tags/:id` renames tag | integration | `bun test tests/routes/admin-tags.test.ts` | Wave 0 gap |
|
||||||
|
| ADMN-08 | `PUT /api/admin/tags/:id` with valid `parentId` sets parent | integration | `bun test tests/routes/admin-tags.test.ts` | Wave 0 gap |
|
||||||
|
| ADMN-09 | `PUT /api/admin/tags/:id` with `parentId: null` removes parent | integration | `bun test tests/routes/admin-tags.test.ts` | Wave 0 gap |
|
||||||
|
| ADMN-10 | `DELETE /api/admin/tags/:id` deletes tag; children get `parentId = null` | integration | `bun test tests/routes/admin-tags.test.ts` | Wave 0 gap |
|
||||||
|
| ADMN-11 (cycle) | `PUT /api/admin/tags/:id` with descendant as `parentId` returns 400 | unit | `bun test tests/services/tag.service.test.ts` | Exists (extend) |
|
||||||
|
|
||||||
|
### Sampling Rate
|
||||||
|
|
||||||
|
- **Per task commit:** `bun test tests/services/tag.service.test.ts`
|
||||||
|
- **Per wave merge:** `bun test`
|
||||||
|
- **Phase gate:** Full suite green before `/gsd-verify-work`
|
||||||
|
|
||||||
|
### Wave 0 Gaps
|
||||||
|
|
||||||
|
- [ ] `tests/routes/admin-tags.test.ts` — covers ADMN-06, ADMN-07, ADMN-08, ADMN-09, ADMN-10 route integration tests
|
||||||
|
- Existing `tests/services/tag.service.test.ts` — extend (do not replace) for ADMN-05 and cycle detection
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Security Domain
|
||||||
|
|
||||||
|
### Applicable ASVS Categories
|
||||||
|
|
||||||
|
| ASVS Category | Applies | Standard Control |
|
||||||
|
|---------------|---------|-----------------|
|
||||||
|
| V2 Authentication | yes | `requireAuth` middleware (existing) |
|
||||||
|
| V3 Session Management | no | Handled by existing OIDC middleware |
|
||||||
|
| V4 Access Control | yes | `requireAdmin` middleware (existing) — all `/api/admin/*` routes |
|
||||||
|
| V5 Input Validation | yes | Zod schemas on all POST/PUT bodies via `zValidator` |
|
||||||
|
| V6 Cryptography | no | No new cryptographic operations |
|
||||||
|
|
||||||
|
### Known Threat Patterns
|
||||||
|
|
||||||
|
| Pattern | STRIDE | Standard Mitigation |
|
||||||
|
|---------|--------|---------------------|
|
||||||
|
| Unauthorized tag mutation (non-admin) | Elevation of Privilege | `requireAdmin` middleware on all admin tag routes (already pattern-established) |
|
||||||
|
| Cycle injection via crafted `parentId` | Tampering | Server-side `isDescendant` check before update, returns 400 |
|
||||||
|
| Mass tag deletion via unauthenticated request | Tampering | `requireAdmin` blocks unauthenticated and non-admin users |
|
||||||
|
| Tag name injection (XSS) | Tampering | Zod `z.string().min(1)` + React renders as text (not innerHTML) |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Sources
|
||||||
|
|
||||||
|
### Primary (HIGH confidence)
|
||||||
|
|
||||||
|
- [VERIFIED: codebase] `src/server/services/tag.service.ts` — existing service functions and DB patterns
|
||||||
|
- [VERIFIED: codebase] `src/server/routes/admin-items.ts` — admin route module pattern to replicate
|
||||||
|
- [VERIFIED: codebase] `src/client/routes/admin/items.$itemId.tsx` — edit page and delete dialog pattern
|
||||||
|
- [VERIFIED: codebase] `src/client/hooks/useAdminGlobalItems.ts` — React Query admin hook pattern
|
||||||
|
- [VERIFIED: codebase] `src/db/schema.ts` — current tags table structure; FK syntax for `onDelete: "set null"`
|
||||||
|
- [VERIFIED: codebase] `src/server/routes/admin.ts` — admin router structure; how to register new sub-router
|
||||||
|
- [VERIFIED: codebase] `src/client/routes/admin.tsx` — disabled Tags entry to enable
|
||||||
|
- [VERIFIED: codebase] `tests/helpers/db.ts` — PGlite test setup with migrations
|
||||||
|
- [VERIFIED: codebase] `bun --version` → 1.3.9
|
||||||
|
|
||||||
|
### Secondary (MEDIUM confidence)
|
||||||
|
|
||||||
|
- [ASSUMED] Drizzle self-referential FK with `() => tags.id` lambda + `onDelete: "set null"` — confirmed by existing `onDelete: "cascade"` FK syntax in codebase; "set null" is a standard Drizzle option
|
||||||
|
|
||||||
|
### Tertiary (LOW confidence)
|
||||||
|
|
||||||
|
- None
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Metadata
|
||||||
|
|
||||||
|
**Confidence breakdown:**
|
||||||
|
- Standard stack: HIGH — all verified from codebase; no new packages
|
||||||
|
- Architecture: HIGH — direct extension of Phase 37 patterns; only new logic is tree building and cycle detection
|
||||||
|
- Pitfalls: HIGH — identified from direct code inspection and structural analysis
|
||||||
|
|
||||||
|
**Research date:** 2026-04-19
|
||||||
|
**Valid until:** 2026-05-19 (stable codebase; no fast-moving dependencies)
|
||||||
Reference in New Issue
Block a user