Files
simone 89adf7a1d4 docs(14): UI design contract for CRM Attio-style redesign
UI-SPEC approved (5/6 PASS, 1 non-blocking flag on focal point).
Locks reuse of Phase 11 primitives (EditableCell, OptionSelect,
OptionMultiSelect) for the leads table, plus scope for the four
CRM bug fixes (i18n, type-safety, dead code).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-14 11:23:28 +02:00

528 lines
47 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Phase 14: CRM Attio-style & Fix - Research
**Researched:** 2026-06-13
**Domain:** Next.js 16 App Router admin CRM table redesign (database-view UX) + bug fixes (i18n, react-hook-form typing, dead code branches)
**Confidence:** HIGH
## Summary
Phase 14 is almost entirely a "reuse, don't reinvent" phase. Phase 11 already built and shipped the exact primitives this phase needs — `EditableCell`, `OptionSelect`, `OptionMultiSelect`, `getOptionColor`, and a Notion/Attio-style database-view table pattern (`ServiceTable.tsx` + `CatalogSearch.tsx` + the catalog server actions in `src/app/admin/catalog/actions.ts`). The polymorphic `tags` table (migration `0006_add_tags_table.sql`) was explicitly designed in its comments to support `entity_type: "leads"` in Phase 14 — **no new migration is needed** for CRM-09.
The work for CRM-08/CRM-09 is a structural port: replace `LeadTable.tsx`'s static `<Table>` (shadcn wrapper components) with a raw `<table>` following `ServiceTable.tsx`'s exact layout, wire `EditableCell` to a new `updateLeadField` server action (mirroring `updateServiceField`), wire `OptionSelect` for `status`/`next_action` and `OptionMultiSelect` for lead tags to new `addLeadTag`/`removeLeadTag`/`renameLeadTag` actions (mirroring `addServiceOption`/`removeServiceOption`/`renameServiceOption`), and add a `getLeadsWithTags`/`getLeadFieldOptions` query pair (mirroring `getAllServices`/`getCatalogFieldOptions`) in `src/lib/admin-queries.ts` or `src/lib/lead-service.ts`.
The three bug fixes (CRM-10, CRM-11, CRM-12) are independent, small, and isolated:
- **CRM-10**: `FollowUpWidget.tsx` has ~6 hardcoded English strings that need Italian translations consistent with the rest of the app's tone (already established in `LeadDetail.tsx`/`LeadTable.tsx`).
- **CRM-11**: `LeadForm.tsx` uses `useForm<any>` and `field.value as any` workarounds — needs to use the actual Zod-inferred type (`CreateLeadInput`/`UpdateLeadInput`, already exported from `lead-validators.ts`) with `field.value || ""` patterns already correctly applied elsewhere.
- **CRM-12**: `SendQuoteModal.tsx`'s "new quote" tab has a dead `if (tab === "new")` branch inside `onSubmit` that can never fire because the "new" tab's button has its own `onClick` and never submits the form — needs removing the unreachable branch and/or restructuring so the form only handles the "existing quote" path.
**Primary recommendation:** Build a `LeadTable.tsx` rewrite that is a near 1:1 structural port of `ServiceTable.tsx`, reusing `EditableCell`/`OptionSelect`/`OptionMultiSelect`/`getOptionColor` as-is, adding lead-scoped server actions in `src/app/admin/leads/actions.ts` that mirror the catalog actions' shape, and fixing the three bugs as small isolated edits to their respective files.
## Architectural Responsibility Map
| Capability | Primary Tier | Secondary Tier | Rationale |
|------------|-------------|----------------|-----------|
| Lead table inline editing (CRM-08) | Frontend Server (RSC) + API/Backend (Server Actions) | Browser (client component for interactivity) | `LeadTable` becomes a client component (`"use client"`) like `ServiceTable`; persistence via Next.js Server Actions in `src/app/admin/leads/actions.ts`, same pattern as `src/app/admin/catalog/actions.ts` |
| Lead tag multi-select (CRM-09) | Database (`tags` table, polymorphic) | API/Backend (server actions) | Reuses existing polymorphic `tags` table with `entity_type: "leads"` — no schema change; CRUD via new `addLeadTag`/`removeLeadTag`/`renameLeadTag` server actions |
| FollowUpWidget i18n (CRM-10) | Frontend Server (RSC) | — | Pure server component (`async function FollowUpWidget()`), string-literal translation only, no data layer change |
| LeadForm type safety (CRM-11) | Browser/Client component | — | `react-hook-form` + `zodResolver`, client-side form; type fix only, no backend change |
| SendQuoteModal dead branches (CRM-12) | Browser/Client component | API/Backend (existing `assignQuoteToLead` action) | Client-side control-flow fix; existing server action `assignQuoteToLead` in `src/app/admin/leads/actions.ts` is reused unchanged |
| Lead search/filter (Attio-style UX, implied by CRM-08 design ref) | Browser/Client component | — | Client-side `useMemo` filter, same pattern as `CatalogSearch.tsx` — no reload, no backend query change |
## Standard Stack
### Core (already installed — no new dependencies)
| Library | Version (installed) | Latest (npm) | Purpose | Why Standard |
|---------|---------|---------|---------|--------------|
| next | 16.2.6 | 16.2.9 [VERIFIED: npm registry] | App Router, Server Actions, RSC | Already the project framework; patch bump only, not in scope |
| react-hook-form | ^7.75.0 | 7.79.0 [VERIFIED: npm registry] | LeadForm validation (CRM-11) | Already used across the app; fixing type-relaxation, not swapping libraries |
| @hookform/resolvers | ^5.2.2 | 5.4.0 [VERIFIED: npm registry] | zodResolver bridge | Already used; no upgrade needed for this phase's scope |
| zod | ^4.4.3 | 4.4.3 [VERIFIED: npm registry] | Schema validation (`lead-validators.ts`) | Already current; `createLeadSchema`/`updateLeadSchema` exist and are correctly typed — CRM-11 fix is about *using* the inferred types in the form, not the schema itself |
| drizzle-orm | ^0.45.2 | — | DB access for `leads`, `tags`, `activities`, `reminders` | Existing ORM; polymorphic `tags` table already supports the new `entity_type` |
| date-fns + date-fns/locale/it | ^4.4.0 | — | `formatDistanceToNow`/`format` with Italian locale | Already used in `LeadDetail.tsx`/`LeadTable.tsx`; reuse for any new date displays |
| lucide-react | ^1.14.0 | — | Icons (`Search`, `Plus`, `Check`, `X`, `Pencil`, `AlertCircle`) | Already used by `OptionSelect`/`OptionMultiSelect`/`CatalogSearch`/`FollowUpWidget` |
**No installation needed** — this phase uses zero new packages. All required primitives ship from Phase 11.
### Supporting (existing internal modules to reuse)
| Module | Path | Purpose | When to Use |
|--------|------|---------|-------------|
| `EditableCell` | `src/components/ui/editable-cell.tsx` | Click-to-edit text/number/textarea/toggle cell | Lead table: `name`, `email`, `phone`, `company`, `next_action`, `notes` |
| `OptionSelect` | `src/components/ui/option-select.tsx` | Single-select dropdown with create/rename, badge-colored | Lead table: `status` (pipeline stage badge) |
| `OptionMultiSelect` | `src/components/ui/option-multi-select.tsx` | Multi-select pill list with create/rename | Lead table: `tags` (CRM-09) |
| `getOptionColor` | `src/components/ui/option-colors.ts` | Deterministic hash-based pastel color for badges | Reused automatically by `OptionSelect`/`OptionMultiSelect` — also usable for the `STAGE_COLOR` replacement if desired |
### Alternatives Considered
| Instead of | Could Use | Tradeoff |
|------------|-----------|----------|
| Reusing `tags` polymorphic table with `entity_type: "leads"` | New `lead_tags` dedicated table | Rejected — migration 0006's own code comment explicitly reserves this for Phase 14; adding a new table would duplicate the pattern and contradict the design already shipped |
| Hand-rolled inline-edit cell | TanStack Table + a cell-editing plugin | Rejected — Phase 11 already solved this with zero new dependencies; introducing TanStack Table now would be inconsistent with the catalog table and add a new dependency for no functional gain |
| Manual `STAGE_COLOR` record (current `LeadTable.tsx`) | `getOptionColor()` hash-based palette | Recommended to replace `STAGE_COLOR` with `OptionSelect` + `getOptionColor` for `status`, for visual consistency with the tags column and Attio/Pipedrive's uniform "status pill" look — but the 6 lead stages already have meaningful semantic colors (green=won, red=lost) that a hash function won't reproduce. **Discretion**: keep a small `STAGE_COLOR` map for the `status` `OptionSelect`'s badge color override (see Open Questions), or accept hash colors for simplicity |
**Installation:** None required.
## Architecture Patterns
### System Architecture Diagram
```
┌─────────────────────────────────────────────────────────────────────┐
│ Browser (/admin/leads) │
│ │
│ LeadsSearch (client, "use client") │
│ │ useState(query) ──useMemo──> filtered leads (client-side filter) │
│ ▼ │
│ LeadTable (client, "use client") │
│ ├─ LeadRow per lead │
│ │ ├─ EditableCell (name, email, phone, company, next_action) ────┐│
│ │ ├─ OptionSelect (status: pipeline stage) ──────────────────┐ ││
│ │ └─ OptionMultiSelect (tags) ──────────────────────────────┐ │ ││
│ └─ Link → /admin/leads/[id] (detail page, unchanged) │ │ ││
└───────────────────────────────────────────────────────────────────┼──┼┼┘
│ ││
Server Actions ("use server", src/app/admin/leads/actions.ts)│ ││
┌──────────────────────────────────────────────────────────┐│ ││
│ updateLeadField(leadId, field, value) <───────────────────┘ ││
│ addLeadTag / removeLeadTag / renameLeadTag <───────────────────┘│
│ (writes to `tags` table, entity_type = "leads") <────────────┘
│ → revalidatePath("/admin/leads") │
└──────────────────────────────────────────────────────────────┘
┌──────────────────────────────────────────────────────────┐
│ Neon Postgres (Drizzle ORM) │
│ leads (status, next_action, name, email, phone, company)│
│ tags (entity_type="leads", entity_id=lead.id, name) │
│ activities / reminders (unchanged — used by detail page) │
└──────────────────────────────────────────────────────────┘
Page load: getLeadsWithTags() + getLeadFieldOptions()
(new query fns in src/lib/admin-queries.ts or lead-service.ts)
mirror getAllServices() / getCatalogFieldOptions()
```
### Recommended Project Structure
```
src/
├── app/admin/leads/
│ ├── page.tsx # server component — fetch leads+tags+options, render LeadsSearch
│ ├── actions.ts # extend: updateLeadField, addLeadTag, removeLeadTag, renameLeadTag
│ ├── LeadsSearch.tsx # NEW — client-side filter wrapper, mirrors CatalogSearch.tsx
│ └── [id]/page.tsx # unchanged
├── components/admin/leads/
│ ├── LeadTable.tsx # REWRITE — database-view table, mirrors ServiceTable.tsx
│ ├── LeadForm.tsx # FIX (CRM-11) — remove `any` / type-relaxation
│ ├── SendQuoteModal.tsx # FIX (CRM-12) — remove unreachable branch
│ ├── LeadDetail.tsx # unchanged (or minor: surface tags if desired — discretion)
│ └── LogActivityModal.tsx # unchanged
├── components/admin/dashboard/
│ └── FollowUpWidget.tsx # FIX (CRM-10) — translate to Italian
└── lib/
├── lead-validators.ts # possibly extend: editable field allowlist / tag entity constant
├── lead-service.ts # OR admin-queries.ts — add getLeadsWithTags / getLeadFieldOptions
└── admin-queries.ts # has TAG_ENTITY="services" precedent — add LEADS_TAG_ENTITY="leads"
```
### Pattern 1: Database-View Table Row with Inline Edit (CRM-08)
**What:** Each table row renders read-only by default; clicking a cell turns it into an input (text/textarea/number/toggle) that commits on Enter/blur, or opens a dropdown for select/multi-select fields.
**When to use:** `/admin/leads` table — `name`, `email`, `phone`, `company`, `status`, `next_action`, `tags`.
**Example (from `ServiceTable.tsx`, directly portable):**
```tsx
// Source: src/components/admin/catalog/ServiceTable.tsx (Phase 11, shipped)
function LeadRow({ lead, options }: { lead: LeadWithTags; options: LeadFieldOptions }) {
const router = useRouter();
const [, startTransition] = useTransition();
const [error, setError] = useState<string | null>(null);
function run(fn: () => Promise<unknown>) {
setError(null);
startTransition(async () => {
try {
await fn();
router.refresh();
} catch (e) {
setError(e instanceof Error ? e.message : "Errore nel salvataggio");
}
});
}
return (
<tr className="border-b border-[#e5e7eb] hover:bg-[#f9f9f9] transition-colors duration-150">
<td className="py-2 px-3 font-medium min-w-[160px]">
<EditableCell
value={lead.name}
type="text"
required
onSave={(v) => run(() => updateLeadField(lead.id, "name", v))}
/>
</td>
<td className="py-2 px-3 min-w-[140px]">
<OptionSelect
value={lead.status}
options={options.status}
onChange={(v) => run(() => updateLeadField(lead.id, "status", v ?? "contacted"))}
/>
</td>
<td className="py-2 px-3 min-w-[180px]">
<OptionMultiSelect
values={lead.tags}
options={options.tags}
onAdd={(v) => run(() => addLeadTag(lead.id, v))}
onRemove={(v) => run(() => removeLeadTag(lead.id, v))}
onRename={(o, n) => run(() => renameLeadTag(o, n))}
/>
</td>
{/* ... next_action, email, phone, company via EditableCell ... */}
</tr>
);
}
```
### Pattern 2: Server Action — Field Allowlist + Validation per Field (CRM-08)
**What:** A single `updateLeadField(leadId, fieldName, value)` server action with an `EDITABLE_FIELDS` const-array allowlist, switching on `fieldName` to apply field-specific normalization/validation.
**When to use:** Any inline-editable scalar column.
**Example:**
```ts
// Source: src/app/admin/catalog/actions.ts updateServiceField (Phase 11, shipped)
const EDITABLE_FIELDS = ["name", "email", "phone", "company", "status", "next_action"] as const;
type EditableField = (typeof EDITABLE_FIELDS)[number];
export async function updateLeadField(leadId: string, fieldName: EditableField, value: string) {
await requireAdmin();
if (!EDITABLE_FIELDS.includes(fieldName)) throw new Error(`Campo non editabile: ${fieldName}`);
if (fieldName === "name") {
const s = value.trim();
if (s.length === 0) throw new Error("Nome richiesto");
await db.update(leads).set({ name: s, updated_at: new Date() }).where(eq(leads.id, leadId));
} else if (fieldName === "status") {
if (!LEAD_STAGES.includes(value as typeof LEAD_STAGES[number])) {
throw new Error("Stato non valido");
}
await db.update(leads).set({ status: value, updated_at: new Date() }).where(eq(leads.id, leadId));
} else {
// email/phone/company/next_action — nullable text fields
await db.update(leads).set({ [fieldName]: value.trim() || null, updated_at: new Date() }).where(eq(leads.id, leadId));
}
revalidatePath("/admin/leads");
revalidatePath(`/admin/leads/${leadId}`);
}
```
**Note (requireAdmin check):** `src/app/admin/catalog/actions.ts` calls `await requireAdmin()` (session check via `getServerSession(authOptions)`) at the top of every mutating action. **`src/app/admin/leads/actions.ts` currently does NOT have this check** — `createLead`/`updateLead`/`deleteLead`/`logActivity`/`assignQuoteToLead` have no auth guard. This is a pre-existing gap, not introduced by this phase, but new actions added in Phase 14 (`updateLeadField`, `addLeadTag`, etc.) should follow the catalog convention and include `requireAdmin()` for consistency — flagged in Pitfalls below.
### Pattern 3: Polymorphic Tag CRUD (CRM-09)
**What:** Multi-select tag pool stored in the shared `tags` table, scoped by `entity_type`. Adding/removing a tag is an insert/delete keyed by `(entity_type, entity_id, name)`; renaming propagates to all rows sharing that tag name within the entity_type scope.
**When to use:** Lead tags (CRM-09) — `entity_type = "leads"`.
**Example:**
```ts
// Source: src/app/admin/catalog/actions.ts addServiceOption/removeServiceOption/renameServiceOption
const LEADS_TAG_ENTITY = "leads";
export async function addLeadTag(leadId: string, value: string) {
await requireAdmin();
const trimmed = value.trim();
if (trimmed.length === 0) throw new Error("Valore richiesto");
await db.insert(tags)
.values({ entity_type: LEADS_TAG_ENTITY, entity_id: leadId, name: trimmed })
.onConflictDoNothing(); // unique index: tags_entity_name_unique (entity_type, entity_id, name)
revalidatePath("/admin/leads");
}
export async function removeLeadTag(leadId: string, value: string) {
await requireAdmin();
await db.delete(tags).where(
and(eq(tags.entity_type, LEADS_TAG_ENTITY), eq(tags.entity_id, leadId), eq(tags.name, value))
);
revalidatePath("/admin/leads");
}
export async function renameLeadTag(oldValue: string, newValue: string) {
await requireAdmin();
const next = newValue.trim();
if (!next || next === oldValue) return;
await db.update(tags).set({ name: next })
.where(and(eq(tags.entity_type, LEADS_TAG_ENTITY), eq(tags.name, oldValue)));
revalidatePath("/admin/leads");
}
```
### Pattern 4: Query Layer — Entity + Tags Join (CRM-09 data fetch)
**What:** `getAllServices()`'s left-join + Map-reduce pattern, ported to leads.
**Example:**
```ts
// Source: src/lib/admin-queries.ts getAllServices (Phase 11, shipped) — port to leads
const LEADS_TAG_ENTITY = "leads";
export type LeadWithTags = Lead & { tags: string[] };
export async function getLeadsWithTags(): Promise<LeadWithTags[]> {
const rows = await db
.select({
// ...all `leads` columns...
tag_name: tags.name,
})
.from(leads)
.leftJoin(tags, and(eq(tags.entity_id, leads.id), eq(tags.entity_type, LEADS_TAG_ENTITY)))
.orderBy(desc(leads.updated_at), asc(tags.name));
const leadMap = new Map<string, LeadWithTags>();
for (const row of rows) {
const { tag_name, ...leadFields } = row;
if (!leadMap.has(row.id)) leadMap.set(row.id, { ...leadFields, tags: [] });
if (tag_name) leadMap.get(row.id)!.tags.push(tag_name);
}
return Array.from(leadMap.values());
}
export type LeadFieldOptions = { status: string[]; tags: string[] };
export async function getLeadFieldOptions(): Promise<LeadFieldOptions> {
const tagRows = await db.selectDistinct({ name: tags.name }).from(tags)
.where(eq(tags.entity_type, LEADS_TAG_ENTITY));
return {
status: [...LEAD_STAGES], // fixed enum, not a dynamic pool
tags: tagRows.map((r) => r.name).sort((a, b) => a.localeCompare(b, "it")),
};
}
```
### Pattern 5: Client-Side Instant Search (CRM-08 UX, "Attio-style")
**What:** `useState` + `useMemo` filter over the full dataset, no server round-trip.
**Example:**
```tsx
// Source: src/app/admin/catalog/CatalogSearch.tsx (Phase 11, shipped) — port to leads
const filtered = useMemo(() => {
const q = query.trim().toLowerCase();
if (!q) return leads;
return leads.filter((l) =>
l.name.toLowerCase().includes(q) ||
l.email?.toLowerCase().includes(q) ||
l.company?.toLowerCase().includes(q) ||
l.status.toLowerCase().includes(q) ||
l.tags.some((t) => t.toLowerCase().includes(q))
);
}, [leads, query]);
```
### Pattern 6: react-hook-form with Zod-Inferred Types (CRM-11 fix)
**What:** Replace `useForm<any>()` with `useForm<CreateLeadInput>()` (or the existing exported type from `lead-validators.ts`), and remove `as any` / `defaultValue={field.value}` workarounds where `field.value` is `string | undefined` vs the `Input`'s expected `string`.
**Current problem in `LeadForm.tsx`:**
```tsx
// CURRENT (CRM-11 violation)
const form = useForm<any>({
resolver: zodResolver(createLeadSchema),
defaultValues: { ... },
});
// ...
status: lead.status as any,
```
**Fixed pattern:**
```tsx
// Source: lead-validators.ts already exports CreateLeadInput = z.infer<typeof createLeadSchema>
const form = useForm<CreateLeadInput>({
resolver: zodResolver(createLeadSchema),
defaultValues: {
name: lead.name,
email: lead.email ?? "",
phone: lead.phone ?? "",
company: lead.company ?? "",
status: lead.status as CreateLeadInput["status"], // narrows from `string` (DB column) to the literal union — still a cast, but type-safe at the schema boundary, not an `any` escape hatch
notes: lead.notes ?? "",
},
});
```
**Note:** `lead.status` from Drizzle's `Lead` type is `string` (the `status` column is `text()`, not a pg enum), while `createLeadSchema.status` is `z.enum(LEAD_STAGES)`. A cast from `string` to the literal union is unavoidable here UNLESS the Drizzle schema itself uses a typed enum — that's a schema-level change out of scope for this phase. The CRM-11 fix should focus on removing `useForm<any>` and `data: any` in `onSubmit`, and ensure the one remaining narrowing cast (`lead.status as CreateLeadInput["status"]`) is the *only* type assertion, replacing the current blanket `as any` on the whole form generic.
### Anti-Patterns to Avoid
- **`useForm<any>()`:** Defeats the entire purpose of `zodResolver` — no compile-time field-name or type checking. CRM-11 requires removing this.
- **Reintroducing a `lead_tags` table:** The polymorphic `tags` table was explicitly designed (see migration 0006 comments) to cover this. A separate table would fragment the tag UX between catalog and CRM.
- **Server-side search/filter for the leads table:** Contradicts the "Attio-style, instant" requirement and Phase 11 precedent (`CatalogSearch.tsx` is 100% client-side).
- **Keeping `STAGE_COLOR` as a separate hardcoded map AND introducing `getOptionColor`:** Pick one color strategy for `status` badges — don't run two parallel color systems for the same field (see Open Questions).
## Don't Hand-Roll
| Problem | Don't Build | Use Instead | Why |
|---------|-------------|-------------|-----|
| Inline-editable table cell (click → edit → save) | New cell component | `EditableCell` (`src/components/ui/editable-cell.tsx`) | Already handles focus management, Enter/Escape/blur commit, required-field validation, React Compiler / hooks-lint compliant (per Phase 11 fix history) |
| Single-select dropdown with create-on-the-fly + rename | New dropdown | `OptionSelect` (`src/components/ui/option-select.tsx`) | Handles search/filter, badge coloring, click-outside, keyboard nav |
| Multi-select tag pills with create/remove/rename | New tag picker | `OptionMultiSelect` (`src/components/ui/option-multi-select.tsx`) | Same as above, plus pill rendering with remove (×) buttons |
| Badge color assignment for tags/status values | Hardcoded color map per value | `getOptionColor` (`src/components/ui/option-colors.ts`) | Deterministic hash → 7-color pastel palette, AA-contrast; used automatically by `OptionSelect`/`OptionMultiSelect` |
| Polymorphic tag storage/CRUD | New `lead_tags` table + migration | Existing `tags` table with `entity_type = "leads"` | Table and unique index already exist (migration 0006); zero migration needed |
| Client-side instant search/filter | Server action + reload, or a search library | `useMemo` filter pattern from `CatalogSearch.tsx` | Proven pattern, zero new deps, matches "Attio-style instant" requirement |
**Key insight:** Every UI primitive this phase needs was built in Phase 11 specifically so Phase 14 wouldn't have to rebuild them — the migration 0006 code comment ("services now, leads in Phase 14") is essentially a forward-reference left for this research to find. Treat any deviation from the `ServiceTable.tsx`/`CatalogSearch.tsx`/catalog-actions pattern as a red flag requiring justification.
## Common Pitfalls
### Pitfall 1: Missing `requireAdmin()` on new lead server actions
**What goes wrong:** New actions (`updateLeadField`, `addLeadTag`, etc.) ship without an auth check, while the catalog actions they're modeled on (`updateServiceField`, `addServiceOption`) all call `await requireAdmin()` first.
**Why it happens:** The *existing* `src/app/admin/leads/actions.ts` (`createLead`, `updateLead`, `deleteLead`, `assignQuoteToLead`) has no `requireAdmin()` calls — copying that file's existing style instead of the catalog file's style propagates the gap.
**How to avoid:** New actions in `src/app/admin/leads/actions.ts` should import `getServerSession`/`authOptions` and call `requireAdmin()` (copy from `src/app/admin/catalog/actions.ts` lines 8, 18-21), matching the more rigorous Phase 11 convention. Whether to *also* retrofit the pre-existing actions is a scope decision — flagged in Open Questions, not required by CRM-08..12 directly but worth a one-line task if low-risk.
### Pitfall 2: `status` field type mismatch between Drizzle `Lead.status: string` and Zod `z.enum(LEAD_STAGES)`
**What goes wrong:** `OptionSelect`'s `value: string | null` and `onChange: (value: string | null) => void` accept any string, including values not in `LEAD_STAGES` (e.g., a typo'd custom status). If a user "creates" a new status value via `OptionSelect`'s create-on-the-fly UX (same as tags), it would write an arbitrary string into `leads.status`, breaking `STAGE_COLOR`/`LEAD_STAGES`-based logic elsewhere (`LeadDetail.tsx`, `FollowUpWidget`, lead-service queries that filter `eq(leads.status, stage)`).
**Why it happens:** `OptionSelect` was designed for `category`/`fase` — genuinely open-ended Notion-style properties. `status` is a closed enum (`LEAD_STAGES`, 6 fixed pipeline stages) — a different semantic.
**How to avoid:** For the `status` column, either (a) use `OptionSelect` but pass `options={LEAD_STAGES}` as a *fixed, non-extensible* list and don't wire an `onRename`/create handler (i.e., the dropdown only lets you pick from the 6 stages, no "Crea «...»" option) — or (b) build a small dedicated `<select>`/`OptionSelect`-lite specifically for closed enums. Validate in `updateLeadField`'s `status` branch that the incoming value is one of `LEAD_STAGES` regardless of UI (defense in depth, per Pattern 2 example).
**Warning signs:** A lead row shows a status badge with an unexpected color/label not in `STAGE_COLOR`, or pipeline-stage-based dashboards/queries silently exclude leads with a "custom" status string.
### Pitfall 3: FollowUpWidget i18n — string source location may include both the component AND `lead-service.ts`
**What goes wrong:** Translating only the JSX strings in `FollowUpWidget.tsx` (`"Require Follow-up"`, `"to contact"`, `"Contact"`, `"View All"`, `"All leads are up to date!"`) misses any English strings returned from `getLeadsNeedingFollowUp` or related helper functions, if any exist.
**Why it happens:** i18n bugs are often scattered across the component AND its data-fetching helper.
**How to avoid:** Grep for English words across `FollowUpWidget.tsx` AND `lead-service.ts`'s `getLeadsNeedingFollowUp`. From research, `getLeadsNeedingFollowUp` (lines 34-48 of `lead-service.ts`) returns raw `Lead[]` with no English strings — confirmed the fix is scoped to `FollowUpWidget.tsx` only. Six strings identified: `"Require Follow-up"`, `"lead"/"leads"` pluralization + `"to contact"`, `"Contact"` (button), `"View All"`, `"All leads are up to date!"`.
**Warning signs:** Visual QA on the dashboard widget — any remaining English text.
### Pitfall 4: SendQuoteModal's dead branch — fixing it might change UX, not just code structure
**What goes wrong:** The `if (tab === "new")` branch inside `onSubmit` (line 51-54 of `SendQuoteModal.tsx`) is unreachable because the "new" tab's UI (`TabsContent value="new"`) has its own standalone `<Button onClick={...}>` that does `window.location.href = ...` directly — it never calls `form.handleSubmit(onSubmit)`. Simply deleting the dead `if` block is correct, but a naive "make it reachable instead" fix (e.g., wrapping the "new" tab's button in the form) would change behavior (introduce a submit-triggered validation pass on a form whose schema doesn't apply to the "new" flow) without being asked.
**Why it happens:** The form's `assignQuoteSchema` validates `lead_id`/`quote_token`/`generate_new` — none of which are meaningful for the "open quote builder" redirect flow.
**How to avoid:** CRM-12 says "non contiene rami di codice irraggiungibili" (doesn't contain unreachable branches) — the minimal, correct fix is **removing the dead `if (tab === "new")` block from `onSubmit`** (since that code path can never execute) and potentially simplifying `onSubmit` to only handle the "existing quote" case, since the "new" tab is already self-contained via its own button. Also consider: the `generate_new` field in `assignQuoteSchema`/`defaultValues` is never meaningfully used (always `false`, and the one `parsed.data.generate_new` check in `actions.ts`'s `assignQuoteToLead` is itself a no-op early-return) — this is a second unreachable/dead-value chain worth flagging together.
**Warning signs:** TypeScript/ESLint `no-unreachable` won't catch this (it's not syntactically unreachable, just logically — `tab` state can theoretically be `"new"` when `onSubmit` fires, but the UI never lets that happen since the "new" tab doesn't render a submit button inside the `<Form>`). Manual review of `tab` state transitions vs. form submission triggers is required.
### Pitfall 5: `revalidatePath` granularity — both list and detail pages
**What goes wrong:** New `updateLeadField`/`addLeadTag`/etc. actions only call `revalidatePath("/admin/leads")`, but the lead detail page (`/admin/leads/[id]`) shows the same `status`/tags data and goes stale.
**Why it happens:** Easy to copy only the list-page revalidation from `updateServiceField` (catalog has no per-item detail page to worry about).
**How to avoid:** Follow `updateLead`'s existing pattern in `src/app/admin/leads/actions.ts` (lines 62-63), which calls both `revalidatePath("/admin/leads")` AND `revalidatePath(`/admin/leads/${id}`)`. New inline-edit/tag actions should do the same.
### Pitfall 6: `Lead.status` typed as plain `string` makes `STAGE_COLOR` lookups silently fall through
**What goes wrong:** `STAGE_COLOR[lead.status as keyof typeof STAGE_COLOR] || ""` already has a fallback (`|| ""`) for unknown statuses — so this isn't a crash risk, but if `OptionSelect` is used for `status` with `getOptionColor()` instead of `STAGE_COLOR`, the "won"/"lost" semantic colors (green/red) are lost in favor of hash-derived pastels, which may look *less* meaningful in an Attio-style pipeline view where color-coding the funnel stage is a core UX signal.
**How to avoid:** This is a design decision, not a bug — see Open Questions. Recommend preserving `STAGE_COLOR`'s semantic green/red/etc. for `status`, either via a custom render (not `OptionSelect`'s default `getOptionColor`) or by extending `OptionSelect` with an optional `colorMap` prop.
## Code Examples
### Existing `Lead` / `Activity` / `Reminder` types (from `src/db/schema.ts`)
```ts
// Source: src/db/schema.ts lines 396-417 (leads table)
export const leads = pgTable("leads", {
id: text("id").primaryKey().$defaultFn(() => nanoid()),
name: text("name").notNull(),
email: text("email"),
phone: text("phone"),
company: text("company"),
status: text("status").notNull().default("contacted"), // contacted | qualified | proposal_sent | negotiating | won | lost
last_contact_date: timestamp("last_contact_date", { withTimezone: true }),
next_action: text("next_action"),
next_action_date: timestamp("next_action_date", { withTimezone: true }),
notes: text("notes"),
created_at: timestamp("created_at", { withTimezone: true }).notNull().defaultNow(),
updated_at: timestamp("updated_at", { withTimezone: true }).notNull().defaultNow(),
});
```
### Existing polymorphic `tags` table (from `src/db/schema.ts`, migration 0006 — already in prod per memory)
```ts
// Source: src/db/schema.ts lines 119-140
export const tags = pgTable("tags", {
id: text("id").primaryKey().$defaultFn(() => nanoid()),
entity_type: text("entity_type").notNull(), // "services" | "leads" (Phase 14)
entity_id: text("entity_id").notNull(),
name: text("name").notNull(),
created_at: timestamp("created_at", { withTimezone: true }).notNull().defaultNow(),
}, (t) => ({
entityTagUnique: uniqueIndex("tags_entity_name_unique").on(t.entity_type, t.entity_id, t.name),
entityIdx: index("tags_entity_idx").on(t.entity_type, t.entity_id),
}));
```
This table is **already deployed to production** (per `MEMORY.md`: "Tags Table and Legacy Data Consolidation Deployed to Production", 2026-06-13). CRM-09 needs zero migration — just new rows with `entity_type = "leads"`.
## State of the Art
| Old Approach | Current Approach | When Changed | Impact |
|--------------|------------------|---------------|--------|
| `LeadTable.tsx` uses shadcn `<Table>`/`<TableRow>`/`<TableCell>` wrapper components, read-only, modal-based editing via `EditLeadModal` | `ServiceTable.tsx` uses raw `<table>`/`<tr>`/`<td>` with Tailwind classes, inline-editable via `EditableCell`/`OptionSelect`/`OptionMultiSelect`, no modals for field edits | Phase 11 (2026-06-13) | Phase 14 ports this same shift to leads — `LeadTable.tsx` should drop the shadcn `Table` import entirely in favor of the raw-table pattern, for visual + interaction consistency with `/admin/catalog` |
| Lead creation/edit via `CreateLeadModal`/`EditLeadModal` dialogs (`LeadForm.tsx`) | Catalog has both: quick-add inline row (`QuickAddRow` in `ServiceTable.tsx`) for new items + inline edit for existing | Phase 11 | **Not explicitly required by CRM-08..12**`CreateLeadModal` can stay as the "new lead" entry point (CRM-08 only requires inline edit of *existing* lead fields, not a new quick-add row). Adding a `QuickAddRow`-equivalent for leads is a "nice to have" / Claude's-discretion item, not a stated requirement — flagged in Open Questions |
| Lead `status` shown via hardcoded `STAGE_COLOR` record + shadcn `<Badge>` | Catalog `category`/`fase`/`tags` shown via `OptionSelect`/`OptionMultiSelect` + `getOptionColor` | Phase 11 | See Pitfall 6 — recommend hybrid: keep semantic stage colors, adopt `OptionSelect`'s interaction model |
**Deprecated/outdated:** None — no library deprecations relevant to this phase. The "old approach" column above refers to pre-Phase-11 UI patterns within this codebase, not external library deprecations.
## Assumptions Log
| # | Claim | Section | Risk if Wrong |
|---|-------|---------|---------------|
| A1 | The 6 fields needing inline edit per CRM-08 ("status, next_action, ecc.") should include `name`, `email`, `phone`, `company` in addition to `status`/`next_action` — i.e., "tutti i campi principali" maps to the full set of scalar columns shown in the current `LeadTable.tsx` header row (Nome, Email, Azienda, Stato, Ultimo Contatto, Prossima Azione) | Architecture Patterns Pattern 1/2, Recommended Project Structure | If the user only wants `status`+`next_action` inline-editable and the rest left read-only/modal-edited, the planner would over-scope `updateLeadField`'s `EDITABLE_FIELDS` allowlist — low risk since extra editable fields are additive and harmless, but worth confirming during planning/discuss-phase |
| A2 | `Ultimo Contatto` (`last_contact_date`) stays read-only/computed (auto-set by `createActivity`, per `lead-service.ts` lines 102-106) and is NOT inline-editable | Recommended Project Structure | Low risk — `last_contact_date` is a derived/audit field; making it editable would conflict with the auto-update-on-activity logic. If wrong, planner adds one more `EditableCell` for a date field (more complex type="date" handling not yet in `EditableCell`'s `type` union) |
| A3 | For `status` (CRM-08), the existing `STAGE_COLOR` semantic color map should be preserved rather than switched to `getOptionColor()`'s hash-based palette (Pitfall 6) | Pitfalls Pitfall 6, Anti-Patterns | Low/cosmetic risk — if wrong, `won`/`lost` lose their green/red semantic colors in favor of hash-derived pastels; purely visual, easy to adjust in a follow-up |
| A4 | No new lead "quick-add row" (mirroring catalog's `QuickAddRow`) is required — `CreateLeadModal` remains the lead-creation entry point | State of the Art row 2, Open Questions | Low risk — CRM-08..12 requirements don't mention lead creation UX; if the user actually wants Attio-style quick-add-row creation too, it's an additive enhancement, not a blocker |
| A5 | New lead-scoped server actions (`updateLeadField`, `addLeadTag`, etc.) should include `requireAdmin()` checks per the catalog-actions convention, even though the pre-existing `leads/actions.ts` functions lack this check | Pitfalls Pitfall 1 | Medium — if the planner skips this, new mutating endpoints would be unauthenticated (matching the pre-existing gap, but inconsistent with Phase 11's stricter convention); retrofitting the *existing* actions is explicitly out of scope unless the user requests it |
**Validation needed:** A1, A3, A4 are UX/scope judgment calls best confirmed during `/gsd-plan-phase` or a quick `/gsd-discuss-phase` pass, since the user has strong opinions about the CRM's look-and-feel ("tutti i miei amici fanno robe fighe e complesse"). A5 is a security-adjacent decision (Pitfall 1 / Security Domain V4) that should be called out explicitly to the user given CLAUDE.md's emphasis on confirming destructive/security-relevant changes — though adding an auth check is additive/protective, not destructive, so it likely doesn't need explicit pre-approval, just documentation in the plan.
## Open Questions
1. **Should `status` use `OptionSelect` (Notion-style, hash colors, user-extensible) or a closed-enum dropdown preserving `STAGE_COLOR`'s semantic colors?**
- What we know: `LEAD_STAGES` is a fixed 6-value enum (`contacted | qualified | proposal_sent | negotiating | won | lost`) with meaningful semantic colors already defined in `STAGE_COLOR` (green=won, red=lost, etc.). `OptionSelect` is designed for open-ended, user-extensible pools (category/fase/tags) with hash-based colors.
- What's unclear: Whether "Attio/Pipedrive style" for the planner means visual consistency with the tags column (same `OptionSelect` widget, hash colors) or semantic pipeline-stage coloring (Pipedrive's actual stage pills ARE color-coded by stage meaning, often user-configurable per stage — closer to `STAGE_COLOR`).
- Recommendation: Use `OptionSelect` for interaction consistency (click-to-open dropdown, same as tags) but pass a fixed `options={LEAD_STAGES}` list (no create-on-the-fly) and either (a) extend `OptionSelect` with an optional color-override map, or (b) render the closed/non-extensible dropdown with `STAGE_COLOR`-based badges directly (simpler, less reuse). Either is a small, low-risk implementation choice — flag for the planner to pick one explicitly rather than leaving ambiguous.
2. **Does CRM-08's "campi principali" include adding a quick-add row for new leads (Attio/Pipedrive "+ Add record" inline row)?**
- What we know: CRM-08 requirement text says "La tabella lead supporta inline editing dei campi principali ... senza apertura modale" — this is about *editing*, not *creation*. `CreateLeadModal` already exists for creation.
- What's unclear: Whether the Pipedrive/Attio reference implies replacing modal-based creation with an inline quick-add row too (Phase 11 did both for catalog).
- Recommendation: Treat as out-of-scope/additive (A4) unless `/gsd-discuss-phase` or the planner surfaces it — CRM-08..12 requirements don't list it, and `CreateLeadModal` is functional. If the planner wants full UX parity with Phase 11's catalog, a `QuickAddRow`-equivalent for leads (name + email + Enter) is a natural, low-effort addition using the same pattern.
3. **Should `LeadDetail.tsx` (the `/admin/leads/[id]` detail page) also show the new lead tags?**
- What we know: CRM-09 says "L'utente assegna tag multi-select ai lead" — doesn't specify whether tags are visible/editable only in the table or also on the detail page. The phase description's "compact detail panels" UI hint suggests Attio-style detail views also show tags/properties.
- What's unclear: Whether `LeadDetail.tsx` needs an `OptionMultiSelect` for tags too, or if the table is the sole tag-management surface.
- Recommendation: Low-cost addition — if `getLeadsWithTags`/`addLeadTag`/etc. are built for the table, surfacing the same `OptionMultiSelect` in `LeadDetail.tsx`'s "Profilo" card costs little extra and improves consistency. Mark as Claude's discretion / nice-to-have within CRM-09's scope.
4. **Pre-existing `requireAdmin()` gap in `src/app/admin/leads/actions.ts` — retrofit existing actions too, or only guard new ones?**
- What we know: `createLead`, `updateLead`, `deleteLead`, `logActivity`, `assignQuoteToLead` (all pre-existing) have no `requireAdmin()` check, unlike `src/app/admin/catalog/actions.ts`'s Phase-11-era actions.
- What's unclear: Whether this is in scope for Phase 14 at all — none of CRM-08..12 mention auth.
- Recommendation: Out of scope for CRM-08..12 directly. New actions (CRM-08/09) should include `requireAdmin()` per A5. Retrofitting the 5 pre-existing actions is a separate, small security-hardening task that could be mentioned to the user as a "spotted while researching" item, but shouldn't block or expand this phase's plan unless the user asks.
## Environment Availability
Skipped — this phase is purely code/UI changes against the existing Next.js app and already-provisioned Postgres database (no new external tools, services, or runtimes required). All necessary npm packages are already installed (verified above).
## Security Domain
### Applicable ASVS Categories
| ASVS Category | Applies | Standard Control |
|---------------|---------|-------------------|
| V2 Authentication | Indirect | `/admin/*` routes protected by Auth.js v4 session (per CLAUDE.md architecture constraint #4) — new server actions should call `requireAdmin()` (see Pitfall 1 / A5) |
| V3 Session Management | No | No session-handling changes in this phase |
| V4 Access Control | Yes | All new mutating server actions (`updateLeadField`, `addLeadTag`, `removeLeadTag`, `renameLeadTag`) must be reachable only from authenticated admin context — follow `requireAdmin()` pattern from `src/app/admin/catalog/actions.ts` |
| V5 Input Validation | Yes | `updateLeadField`'s per-field validation (Pattern 2) — especially `status` must be constrained to `LEAD_STAGES` enum values regardless of client-side `OptionSelect` configuration (defense in depth, Pitfall 2). Tag names should be trimmed/non-empty (mirrors `addServiceOption`) |
| V6 Cryptography | No | No crypto/secrets involved in this phase |
### Known Threat Patterns for this stack
| Pattern | STRIDE | Standard Mitigation |
|---------|--------|----------------------|
| Unauthenticated server action invocation (Next.js Server Actions are callable directly if not guarded) | Elevation of Privilege | `requireAdmin()` session check at the top of every new mutating action (Pattern 2/3 examples include this) |
| Arbitrary `status` value injection via `OptionSelect`'s create-on-the-fly UX if misapplied to the closed `status` enum | Tampering | Server-side allowlist check against `LEAD_STAGES` in `updateLeadField` (Pattern 2), independent of client UI configuration (Pitfall 2) |
| SQL injection via raw string interpolation | Tampering | N/A — Drizzle ORM parameterized queries used throughout; no raw SQL introduced by this phase |
## Sources
### Primary (HIGH confidence)
- `/Users/simonecavalli/Vault/IAMCAVALLI/src/components/admin/catalog/ServiceTable.tsx` — Phase 11 database-view table pattern (shipped, code-complete)
- `/Users/simonecavalli/Vault/IAMCAVALLI/src/components/ui/editable-cell.tsx` — inline-edit primitive
- `/Users/simonecavalli/Vault/IAMCAVALLI/src/components/ui/option-select.tsx` — single-select primitive
- `/Users/simonecavalli/Vault/IAMCAVALLI/src/components/ui/option-multi-select.tsx` — multi-select primitive
- `/Users/simonecavalli/Vault/IAMCAVALLI/src/components/ui/option-colors.ts` — color derivation
- `/Users/simonecavalli/Vault/IAMCAVALLI/src/app/admin/catalog/actions.ts` — server action patterns (updateServiceField, addServiceOption/removeServiceOption/renameServiceOption, requireAdmin)
- `/Users/simonecavalli/Vault/IAMCAVALLI/src/app/admin/catalog/CatalogSearch.tsx` — client-side instant filter pattern
- `/Users/simonecavalli/Vault/IAMCAVALLI/src/lib/admin-queries.ts` (lines 355-430) — getAllServices/getCatalogFieldOptions tags-join pattern
- `/Users/simonecavalli/Vault/IAMCAVALLI/src/db/schema.ts` — leads, activities, reminders, tags, quotes table definitions
- `/Users/simonecavalli/Vault/IAMCAVALLI/src/db/migrations/0006_add_tags_table.sql` — confirms `tags` table already deployed, polymorphic, no migration needed for CRM-09
- `/Users/simonecavalli/Vault/IAMCAVALLI/src/components/admin/leads/LeadTable.tsx`, `LeadForm.tsx`, `SendQuoteModal.tsx`, `LeadDetail.tsx`, `LogActivityModal.tsx` — current CRM code under fix/redesign
- `/Users/simonecavalli/Vault/IAMCAVALLI/src/components/admin/dashboard/FollowUpWidget.tsx` — CRM-10 target
- `/Users/simonecavalli/Vault/IAMCAVALLI/src/lib/lead-validators.ts`, `lead-service.ts` — Zod schemas and query/mutation helpers
- npm registry — `npm view next/react-hook-form/zod/@hookform/resolvers version` [VERIFIED: npm registry, 2026-06-13]
### Secondary (MEDIUM confidence)
- None used — all findings verified directly against the codebase (HIGH confidence, codebase is ground truth for an internal refactor/redesign phase)
### Tertiary (LOW confidence)
- None — no external/ecosystem research was needed since this phase reuses 100% internal, already-shipped primitives
## Metadata
**Confidence breakdown:**
- Standard Stack: HIGH — zero new dependencies; all versions verified against npm registry and the project's own `package.json`
- Architecture: HIGH — directly ported from Phase 11's shipped, verified, lint-passing code (`ServiceTable.tsx`, catalog actions, admin-queries.ts)
- Pitfalls: HIGH — all six pitfalls derived from direct code reading of the actual files this phase will modify, not speculation
**Research date:** 2026-06-13
**Valid until:** 30 days (stable internal codebase pattern; revisit if Phase 11 patterns change before Phase 14 executes, or if Phase 12/13 modify shared primitives)