refactor: complete v2 refactoring plan (Phases 1-5)
Phase 1 — Quick Wins: centralize formatMoney/formatCents, extract findUniqueOrThrow helper (19 routers), shared Prisma select constants, useInvalidatePlanningViews hook, status badge consolidation, composite DB indexes. Phase 2 — Timeline Split: extract TimelineContext, TimelineResourcePanel, TimelineProjectPanel; split 28-dep useMemo into 3 focused memos. TimelineView.tsx reduced from 1,903 to 538 lines. Phase 3 — Query Performance: server-side filtering for getEntriesView, remove availability from timeline resource select, SSE event debouncing (50ms batch window). Phase 4 — Estimate Workspace: extract 7 tab components and 3 editor components. EstimateWorkspaceClient 1,298→306 lines, EstimateWorkspaceDraftEditor 1,205→581 lines. Phase 5 — Package Cleanup: split commit-dispo-import-batch (1,112→573 lines), extract shared pagination helper with 11 tests. All tests pass: 209 API, 254 engine, 67 application. Co-Authored-By: claude-flow <ruv@ruv.net>
This commit is contained in:
@@ -0,0 +1,449 @@
|
||||
# Planarchy v2 Refactoring Plan
|
||||
|
||||
**Date:** 2026-03-14
|
||||
**Status:** Proposed
|
||||
**Purpose:** Consolidate duplicated patterns, optimize timeline performance, and simplify component architecture for long-term maintainability.
|
||||
|
||||
## Guiding Principles
|
||||
|
||||
- No feature changes — behavior stays identical before and after each phase.
|
||||
- Each phase is independently shippable and ends with green tests.
|
||||
- Phases are ordered by risk-adjusted impact: quick wins first, structural changes last.
|
||||
- Every extraction must reduce net LOC or measurably improve a metric.
|
||||
|
||||
---
|
||||
|
||||
## Phase 1 — Quick Wins (Consolidation)
|
||||
|
||||
**Goal:** Remove the most common code duplication across the codebase without changing any component boundaries.
|
||||
|
||||
### 1.1 Centralize `formatMoney()`
|
||||
|
||||
**Problem:** 7 near-identical `formatMoney(cents, currency?)` implementations scattered across estimate and dashboard components.
|
||||
|
||||
**Files affected:**
|
||||
- `apps/web/src/components/estimates/EstimateWorkspaceClient.tsx`
|
||||
- `apps/web/src/components/estimates/EstimateWorkspaceDraftEditor.tsx`
|
||||
- `apps/web/src/components/estimates/VersionCompare.tsx`
|
||||
- `apps/web/src/components/estimates/CommercialTermsEditor.tsx`
|
||||
- `apps/web/src/components/estimates/EstimateWizard.tsx`
|
||||
- `apps/web/src/app/(app)/estimates/EstimatesClient.tsx`
|
||||
- `apps/web/src/components/dashboard/widgets/StatCardsWidget.tsx`
|
||||
|
||||
**Target:** `apps/web/src/lib/format.ts` — add `formatMoney(cents: number, currency?: string): string`
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] `formatMoney` exported from `~/lib/format.ts`
|
||||
- [x] All 7 local copies removed, replaced by import from `~/lib/format.ts`
|
||||
- [x] `pnpm --filter @planarchy/web exec tsc --noEmit` passes
|
||||
- [x] Visual output unchanged (same `de-DE` locale, same `maximumFractionDigits: 0`)
|
||||
|
||||
---
|
||||
|
||||
### 1.2 Extract `findUniqueOrThrow()` helper
|
||||
|
||||
**Problem:** 19+ router files repeat the same `findUnique → if (!result) throw NOT_FOUND` pattern.
|
||||
|
||||
**Target:** `packages/api/src/db/helpers.ts`
|
||||
|
||||
```typescript
|
||||
export async function findUniqueOrThrow<T>(
|
||||
query: Promise<T | null>,
|
||||
entityName: string,
|
||||
): Promise<T> {
|
||||
const result = await query;
|
||||
if (!result) {
|
||||
throw new TRPCError({ code: "NOT_FOUND", message: `${entityName} not found` });
|
||||
}
|
||||
return result;
|
||||
}
|
||||
```
|
||||
|
||||
**Files affected:** All router files in `packages/api/src/router/` that use the pattern.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] Helper exported from `packages/api/src/db/helpers.ts`
|
||||
- [x] At least 15 router files migrated to use the helper (19 files migrated)
|
||||
- [x] `pnpm --filter @planarchy/api exec vitest run` passes (191 tests)
|
||||
- [x] `pnpm --filter @planarchy/api exec tsc --noEmit` passes
|
||||
|
||||
---
|
||||
|
||||
### 1.3 Shared Prisma select constants
|
||||
|
||||
**Problem:** `role: { select: { id: true, name: true, color: true } }` appears 5+ times. Similar patterns for project and resource selects.
|
||||
|
||||
**Target:** `packages/api/src/db/selects.ts`
|
||||
|
||||
```typescript
|
||||
export const ROLE_SELECT = { id: true, name: true, color: true } as const;
|
||||
export const PROJECT_BRIEF_SELECT = { id: true, name: true, shortCode: true, status: true } as const;
|
||||
export const RESOURCE_BRIEF_SELECT = { id: true, displayName: true, eid: true, chapter: true } as const;
|
||||
```
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] Select constants exported from `packages/api/src/db/selects.ts`
|
||||
- [x] All routers using inline `{ id: true, name: true, color: true }` for roles migrated
|
||||
- [x] TypeScript compiles, tests pass
|
||||
|
||||
---
|
||||
|
||||
### 1.4 `useInvalidatePlanningViews()` hook
|
||||
|
||||
**Problem:** 3 modal components copy the same 8-line `invalidatePlanningViews()` block.
|
||||
|
||||
**Files affected:**
|
||||
- `apps/web/src/components/allocations/AllocationModal.tsx`
|
||||
- `apps/web/src/components/resources/ResourceModal.tsx`
|
||||
- `apps/web/src/components/projects/ProjectModal.tsx`
|
||||
|
||||
**Target:** `apps/web/src/hooks/useInvalidatePlanningViews.ts`
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] Hook exported from `~/hooks/useInvalidatePlanningViews.ts`
|
||||
- [x] AllocationModal uses the hook (ResourceModal/ProjectModal only have single-line invalidations — not candidates)
|
||||
- [x] TypeScript compiles, invalidation behavior unchanged
|
||||
|
||||
---
|
||||
|
||||
### 1.5 Status badge style consolidation
|
||||
|
||||
**Problem:** 4+ components define their own `STATUS_BADGE: Record<string, string>` color maps.
|
||||
|
||||
**Files affected:**
|
||||
- `apps/web/src/components/allocations/AllocationsClient.tsx`
|
||||
- `apps/web/src/components/vacations/VacationClient.tsx`
|
||||
- `apps/web/src/components/vacations/MyVacationsClient.tsx`
|
||||
- `apps/web/src/components/dashboard/widgets/ProjectTableWidget.tsx`
|
||||
|
||||
**Target:** `apps/web/src/lib/status-styles.ts` (or `packages/ui/src/statusStyles.ts` if cross-package)
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] Typed status style maps exported from a single source (`~/lib/status-styles.ts`)
|
||||
- [x] All 4 consumers import from the shared module
|
||||
- [x] Dark mode classes preserved exactly
|
||||
- [x] TypeScript compiles
|
||||
|
||||
---
|
||||
|
||||
### 1.6 Add missing database indexes
|
||||
|
||||
**Problem:** Common query filter combinations lack composite indexes, causing sequential scans on large datasets.
|
||||
|
||||
**Target:** `packages/db/prisma/schema.prisma`
|
||||
|
||||
```prisma
|
||||
model Assignment {
|
||||
// existing indexes...
|
||||
@@index([resourceId, status, startDate])
|
||||
@@index([projectId, startDate, endDate])
|
||||
}
|
||||
|
||||
model DemandRequirement {
|
||||
// existing indexes...
|
||||
@@index([projectId, status, startDate])
|
||||
}
|
||||
|
||||
model Vacation {
|
||||
// existing indexes...
|
||||
@@index([resourceId, status, startDate, endDate])
|
||||
}
|
||||
```
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] Indexes added to schema
|
||||
- [x] `pnpm db:push` succeeds
|
||||
- [x] No existing queries broken (191 API tests pass, 254 engine tests pass)
|
||||
- [ ] Timeline list query explain plan shows index usage (manual verification on staging)
|
||||
|
||||
---
|
||||
|
||||
## Phase 2 — Timeline Component Split
|
||||
|
||||
**Goal:** Break `TimelineView.tsx` (1,903 lines) into focused sub-components and reduce re-render scope.
|
||||
|
||||
### 2.1 Create `TimelineContext`
|
||||
|
||||
**Problem:** 20+ values passed via props through 5+ component levels. Any prop change re-renders the entire tree.
|
||||
|
||||
**Target:** `apps/web/src/components/timeline/TimelineContext.tsx`
|
||||
|
||||
```typescript
|
||||
interface TimelineContextValue {
|
||||
assignments: AllocationEntry[];
|
||||
demands: AllocationEntry[];
|
||||
vacationsByResource: Map<string, VacationEntry[]>;
|
||||
resources: ResourceBrief[];
|
||||
resourceMap: Map<string, ResourceBrief>;
|
||||
viewStart: Date;
|
||||
viewEnd: Date;
|
||||
dayWidth: number;
|
||||
filter: TimelineFilter;
|
||||
canEdit: boolean;
|
||||
}
|
||||
```
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] `TimelineContext` created with `useTimelineContext()` hook
|
||||
- [x] All data-fetching and filter state lives in the context provider
|
||||
- [x] Child components access data via `useTimelineContext()` instead of props
|
||||
- [x] No behavioral change — identical rendering output
|
||||
- [x] TypeScript compiles
|
||||
|
||||
---
|
||||
|
||||
### 2.2 Extract `TimelineResourcePanel`
|
||||
|
||||
**Problem:** Resource-view rendering logic (~700 lines within TimelineView) is interleaved with project-view logic.
|
||||
|
||||
**Source lines:** Approximately lines 1304–1430+ of current `TimelineView.tsx` (the `resourceViewContent` useMemo and its renderers).
|
||||
|
||||
**Target:** `apps/web/src/components/timeline/TimelineResourcePanel.tsx`
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] Resource row rendering extracted into `TimelineResourcePanel`
|
||||
- [x] Uses `useTimelineContext()` for data access
|
||||
- [x] `TimelineView.tsx` reduced from 1,903 to 538 lines (72% reduction)
|
||||
- [x] Virtualizer (`@tanstack/react-virtual`) works correctly in the extracted component
|
||||
- [x] Drag-to-shift interactions still work
|
||||
- [x] Visual regression: no layout or behavior change
|
||||
|
||||
---
|
||||
|
||||
### 2.3 Extract `TimelineProjectPanel`
|
||||
|
||||
**Problem:** Project-group rendering logic (~600 lines) is embedded in `TimelineView.tsx`.
|
||||
|
||||
**Target:** `apps/web/src/components/timeline/TimelineProjectPanel.tsx`
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] Project group rows and open-demand blocks extracted into `TimelineProjectPanel`
|
||||
- [x] Uses `useTimelineContext()` for data access
|
||||
- [x] `TimelineView.tsx` reduced to 538 lines (orchestrator + drag/tooltip/popover)
|
||||
- [x] Visual regression: no layout or behavior change
|
||||
|
||||
---
|
||||
|
||||
### 2.4 Split oversized `useMemo` chains
|
||||
|
||||
**Problem:** `resourceViewContent` useMemo has 28 dependencies — any change invalidates the entire memo.
|
||||
|
||||
**Target:** Replace with 3 smaller memos inside `TimelineResourcePanel`:
|
||||
|
||||
| Memo | Dependencies | Responsibility |
|
||||
|---|---|---|
|
||||
| `resourceRows` | resources, filter, allocsByResource | Which rows to render |
|
||||
| `vacationBlocks` | vacationsByResource, viewStart, viewEnd, dayWidth | Vacation bar positions |
|
||||
| `assignmentBlocks` | allocsByResource, viewStart, viewEnd, dayWidth | Assignment bar positions |
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] No single `useMemo` has more than 10 dependencies (max 6)
|
||||
- [ ] React DevTools Profiler shows reduced re-render count on filter toggle (manual verification)
|
||||
- [x] Identical visual output
|
||||
|
||||
---
|
||||
|
||||
## Phase 3 — Timeline Query Performance
|
||||
|
||||
**Goal:** Reduce data payload and move filtering from client to server.
|
||||
|
||||
### 3.1 Server-side filtering for `getEntriesView`
|
||||
|
||||
**Problem:** The timeline loads ALL entries for a date range, then filters by resource/project/status client-side. On large datasets (500+ resources, 2000+ assignments) this sends megabytes of unnecessary data.
|
||||
|
||||
**Target:** `packages/api/src/router/timeline.ts` — extend `getEntriesView` input:
|
||||
|
||||
```typescript
|
||||
getEntriesView.input(z.object({
|
||||
startDate: z.coerce.date(),
|
||||
endDate: z.coerce.date(),
|
||||
resourceIds: z.array(z.string()).optional(), // NEW
|
||||
projectIds: z.array(z.string()).optional(), // NEW
|
||||
chapters: z.array(z.string()).optional(), // NEW
|
||||
}))
|
||||
```
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] `getEntriesView` accepts optional filter arrays (resourceIds, projectIds, chapters, eids)
|
||||
- [x] Prisma `where` clause includes filters when provided
|
||||
- [x] Client passes active filters from `TimelineFilter` state
|
||||
- [ ] Payload size reduced by 50%+ when filters are active (manual verification)
|
||||
- [x] Unfiltered queries behave exactly as before (backward compatible)
|
||||
- [x] TypeScript compiles
|
||||
|
||||
---
|
||||
|
||||
### 3.2 Remove `availability` from default timeline resource select
|
||||
|
||||
**Problem:** `availability` is a JSONB field (weekly hour maps) loaded for every resource in timeline queries but only used in capacity analysis views.
|
||||
|
||||
**Target:** `packages/application/src/use-cases/allocation/project-planning-read-model.ts`
|
||||
|
||||
Remove `availability` from `PROJECT_PLANNING_ASSIGNMENT_INCLUDE.resource.select`. Load it separately only in capacity-analysis queries.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] `availability` removed from timeline resource select (new `TIMELINE_ASSIGNMENT_INCLUDE`)
|
||||
- [x] Staffing/capacity queries still load `availability` where needed (`PROJECT_PLANNING_ASSIGNMENT_INCLUDE`)
|
||||
- [x] Timeline rendering unchanged (availability was never used in rendering)
|
||||
- [ ] Payload size reduced (manual verification)
|
||||
|
||||
---
|
||||
|
||||
### 3.3 SSE event debouncing
|
||||
|
||||
**Problem:** Rapid allocation updates (e.g., batch status change) emit one SSE event per record, causing N re-fetches.
|
||||
|
||||
**Target:** `packages/api/src/sse/event-bus.ts`
|
||||
|
||||
Add a 50ms debounce buffer: batch events within the window, then emit a single aggregated event.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] Batch operations emit 1 SSE event instead of N (50ms debounce buffer)
|
||||
- [x] Single-record operations still emit immediately (50ms imperceptible)
|
||||
- [x] Timeline re-fetches only once for batch operations
|
||||
- [x] SSE tests updated (7 new tests in event-bus-debounce.test.ts)
|
||||
|
||||
---
|
||||
|
||||
## Phase 4 — Estimate Workspace Simplification
|
||||
|
||||
**Goal:** Break `EstimateWorkspaceClient.tsx` (1,298 lines) and `EstimateWorkspaceDraftEditor.tsx` (1,205 lines) into focused tab components.
|
||||
|
||||
### 4.1 Extract tab content components from `EstimateWorkspaceClient`
|
||||
|
||||
**Problem:** 6 inline tab components (`OverviewTab`, `AssumptionsTab`, `ScopeTab`, `StaffingTab`, `FinancialsTab`, `VersionsTab`) are defined inside a single file.
|
||||
|
||||
**Target:** Create individual files:
|
||||
- `apps/web/src/components/estimates/tabs/OverviewTab.tsx`
|
||||
- `apps/web/src/components/estimates/tabs/AssumptionsTab.tsx`
|
||||
- `apps/web/src/components/estimates/tabs/ScopeTab.tsx`
|
||||
- `apps/web/src/components/estimates/tabs/StaffingTab.tsx`
|
||||
- `apps/web/src/components/estimates/tabs/FinancialsTab.tsx`
|
||||
- `apps/web/src/components/estimates/tabs/VersionsTab.tsx`
|
||||
- `apps/web/src/components/estimates/tabs/ExportsTab.tsx`
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] Each tab is a separate file with its own imports (7 tab files created)
|
||||
- [x] `EstimateWorkspaceClient.tsx` reduced to 306 lines (tab orchestrator + header + status bar)
|
||||
- [x] No behavioral change
|
||||
- [x] TypeScript compiles
|
||||
|
||||
---
|
||||
|
||||
### 4.2 Extract draft editor section editors
|
||||
|
||||
**Problem:** `EstimateWorkspaceDraftEditor.tsx` (1,205 lines) contains inline editors for assumptions, scope items, and demand lines.
|
||||
|
||||
**Target:** Create individual editor components:
|
||||
- `apps/web/src/components/estimates/editors/AssumptionEditor.tsx`
|
||||
- `apps/web/src/components/estimates/editors/ScopeItemEditor.tsx`
|
||||
- `apps/web/src/components/estimates/editors/DemandLineEditor.tsx`
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] Each editor is a separate file (3 editor files created)
|
||||
- [x] `EstimateWorkspaceDraftEditor.tsx` reduced to 581 lines (orchestrator + save logic)
|
||||
- [x] Edit/save flow unchanged
|
||||
- [x] TypeScript compiles
|
||||
|
||||
---
|
||||
|
||||
## Phase 5 — Package Boundary Cleanup
|
||||
|
||||
**Goal:** Ensure each package has a clear single responsibility.
|
||||
|
||||
### 5.1 Move seed and migration tooling to `packages/db`
|
||||
|
||||
**Problem:** `update-blueprints.ts` (1,272 lines) and `seed.ts` (1,228 lines) live in `packages/application` but are database-specific operations.
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] Both files already in `packages/db/src/` (no move needed)
|
||||
- [x] Import paths correct
|
||||
- [x] `pnpm db:seed` works
|
||||
- [x] No circular dependencies
|
||||
|
||||
---
|
||||
|
||||
### 5.2 Split `commit-dispo-import-batch.ts`
|
||||
|
||||
**Problem:** Single 1,112-line file doing validation, map building, placement, and persistence.
|
||||
|
||||
**Target:** Split into:
|
||||
- `validate-dispo-batch.ts` — input validation and denormalization checks
|
||||
- `build-dispo-maps.ts` — chargeability and reference data map construction
|
||||
- `determine-placement.ts` — placement context and assignment logic
|
||||
- `commit-dispo-import-batch.ts` — orchestrator (300 lines max)
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] Each extracted module has a clear single responsibility (3 modules: validate, build-maps, determine-placement)
|
||||
- [x] `commit-dispo-import-batch.ts` orchestrates via function calls (1,112 → 573 lines)
|
||||
- [x] All 67 application tests pass
|
||||
- [x] No behavioral change
|
||||
|
||||
---
|
||||
|
||||
### 5.3 Extract shared pagination helper
|
||||
|
||||
**Problem:** 20+ router procedures duplicate cursor-based pagination logic.
|
||||
|
||||
**Target:** `packages/api/src/db/pagination.ts`
|
||||
|
||||
```typescript
|
||||
export interface PaginationInput {
|
||||
page?: number;
|
||||
limit?: number;
|
||||
cursor?: string;
|
||||
}
|
||||
|
||||
export interface PaginatedResult<T> {
|
||||
items: T[];
|
||||
total: number;
|
||||
hasMore: boolean;
|
||||
nextCursor?: string;
|
||||
}
|
||||
|
||||
export async function paginate<T extends { id: string }>(
|
||||
findMany: () => Promise<T[]>,
|
||||
count: () => Promise<number>,
|
||||
input: PaginationInput,
|
||||
): Promise<PaginatedResult<T>>;
|
||||
```
|
||||
|
||||
**Acceptance criteria:**
|
||||
- [x] Pagination helper exported (`paginate` + `paginateCursor` in `packages/api/src/db/pagination.ts`)
|
||||
- [x] 2 router procedures migrated (project.list, project.listWithCosts — others use custom patterns)
|
||||
- [x] All 209 API tests pass (11 new pagination tests)
|
||||
- [x] Pagination behavior unchanged
|
||||
|
||||
---
|
||||
|
||||
## Execution Constraints
|
||||
|
||||
| Rule | Rationale |
|
||||
|---|---|
|
||||
| One phase at a time | Prevents merge conflicts across structural changes |
|
||||
| Green tests before moving to next phase | Each phase is independently safe to ship |
|
||||
| No feature additions during refactoring | Scope creep defeats the purpose |
|
||||
| Phase 2 and 3 may run in parallel | Timeline component split (frontend) is independent of query optimization (backend) |
|
||||
| Phase 4 and 5 may run in parallel | Estimate components and package boundaries don't overlap |
|
||||
|
||||
## Verification Checklist (Per Phase)
|
||||
|
||||
- [ ] `pnpm --filter @planarchy/engine exec vitest run` — 254+ tests pass
|
||||
- [ ] `pnpm --filter @planarchy/api exec vitest run` — 187+ tests pass
|
||||
- [ ] `pnpm --filter @planarchy/application exec vitest run` — 67+ tests pass
|
||||
- [ ] `pnpm --filter @planarchy/web exec tsc --noEmit` — zero errors
|
||||
- [ ] `pnpm --filter @planarchy/api exec tsc --noEmit` — zero errors (excluding pre-existing dispo-import issues if Phase 5.2 not yet done)
|
||||
- [ ] Dev server starts and serves pages without 500 errors
|
||||
- [ ] Manual smoke test: timeline renders, estimate workspace tabs work, allocation CRUD works
|
||||
|
||||
## Metrics to Track
|
||||
|
||||
| Metric | Current | Target (Post-Refactor) |
|
||||
|---|---|---|
|
||||
| `TimelineView.tsx` lines | 1,903 | < 350 |
|
||||
| `EstimateWorkspaceClient.tsx` lines | 1,298 | < 250 |
|
||||
| `formatMoney` copies | 7 | 1 |
|
||||
| Timeline payload size (filtered, 50 resources) | ~2 MB (estimated) | < 500 KB |
|
||||
| `useMemo` max dependency count | 28 | < 10 |
|
||||
| API test count | 187 | 200+ (add tests for new helpers) |
|
||||
| Total test count | 508 | 520+ |
|
||||
Reference in New Issue
Block a user