docs(16-02): complete service layer userId scoping plan
- SUMMARY.md documents 7 service files updated with userId parameter - STATE.md advanced to plan 2 of 4 in phase 16 - ROADMAP.md updated with plan progress - Requirements MULTI-01, MULTI-02, MULTI-03, MULTI-06 marked complete
This commit is contained in:
146
.planning/phases/16-multi-user-data-model/16-02-SUMMARY.md
Normal file
146
.planning/phases/16-multi-user-data-model/16-02-SUMMARY.md
Normal file
@@ -0,0 +1,146 @@
|
||||
---
|
||||
phase: 16-multi-user-data-model
|
||||
plan: 02
|
||||
subsystem: api
|
||||
tags: [drizzle, postgres, multi-user, data-isolation, services]
|
||||
|
||||
requires:
|
||||
- phase: 16-multi-user-data-model (plan 01)
|
||||
provides: schema with userId columns, users table, auth middleware resolving userId
|
||||
provides:
|
||||
- User-scoped service layer — all 7 service files accept userId and filter queries
|
||||
- Cross-user data isolation via and(eq(id), eq(userId)) on all get/update/delete
|
||||
- Ownership validation on thread resolution, setup item sync, candidate operations
|
||||
affects: [16-03 route handlers, 16-04 MCP tools, tests]
|
||||
|
||||
tech-stack:
|
||||
added: []
|
||||
patterns: [userId-as-second-param, and(eq) isolation, ownership-validation-before-mutation]
|
||||
|
||||
key-files:
|
||||
created: []
|
||||
modified:
|
||||
- src/server/services/item.service.ts
|
||||
- src/server/services/category.service.ts
|
||||
- src/server/services/thread.service.ts
|
||||
- src/server/services/setup.service.ts
|
||||
- src/server/services/totals.service.ts
|
||||
- src/server/services/csv.service.ts
|
||||
- src/server/services/auth.service.ts
|
||||
|
||||
key-decisions:
|
||||
- "Category deletion uses dynamic getOrCreateUncategorized(db, userId) instead of hardcoded ID 1"
|
||||
- "Candidate operations verify parent thread ownership before proceeding (not just candidate existence)"
|
||||
- "syncSetupItems validates both setup ownership and item ownership via inArray"
|
||||
- "resolveThread verifies category belongs to user with fallback to getOrCreateUncategorized"
|
||||
- "createApiKey param order changed to (db, userId, name) for consistency with other services"
|
||||
|
||||
patterns-established:
|
||||
- "userId-second-param: all service functions use signature (db, userId, ...rest)"
|
||||
- "composite-where: get/update/delete by ID always use and(eq(table.id, id), eq(table.userId, userId))"
|
||||
- "ownership-chain: candidate ops verify parent thread ownership, setup sync verifies item ownership"
|
||||
|
||||
requirements-completed: [MULTI-01, MULTI-02, MULTI-03, MULTI-06]
|
||||
|
||||
duration: 4min
|
||||
completed: 2026-04-05
|
||||
---
|
||||
|
||||
# Phase 16 Plan 02: Service Layer userId Scoping Summary
|
||||
|
||||
**All 7 service files accept userId parameter with and(eq) isolation on every query — no unscoped reads or writes remain**
|
||||
|
||||
## Performance
|
||||
|
||||
- **Duration:** 4 min
|
||||
- **Started:** 2026-04-05T08:40:21Z
|
||||
- **Completed:** 2026-04-05T08:43:55Z
|
||||
- **Tasks:** 2
|
||||
- **Files modified:** 7
|
||||
|
||||
## Accomplishments
|
||||
- Every exported service function now accepts userId as second parameter with no default db values
|
||||
- All get-by-id, update, and delete operations use and(eq(id), eq(userId)) for cross-user isolation
|
||||
- Thread resolution includes userId on new item creation and verifies category ownership
|
||||
- Setup item sync validates both setup and item ownership before inserting
|
||||
- Candidate operations (add, update, remove) verify parent thread belongs to user
|
||||
- CSV import scopes category lookup/creation and item insertion to the importing user
|
||||
- Category deletion uses dynamic Uncategorized lookup per user instead of hardcoded ID
|
||||
|
||||
## Task Commits
|
||||
|
||||
Each task was committed atomically:
|
||||
|
||||
1. **Task 1: Update item, category, totals, and CSV services** - `8d85d28` (feat)
|
||||
2. **Task 2: Update thread, setup, and auth services** - `242cace` (feat)
|
||||
|
||||
## Files Created/Modified
|
||||
- `src/server/services/item.service.ts` - userId on all 6 functions, and() isolation on get/update/delete/duplicate
|
||||
- `src/server/services/category.service.ts` - userId on all functions, async Postgres patterns, dynamic Uncategorized lookup on delete
|
||||
- `src/server/services/totals.service.ts` - userId filtering on category and global totals
|
||||
- `src/server/services/csv.service.ts` - userId on import/export, user-scoped category cache, userId on item inserts
|
||||
- `src/server/services/thread.service.ts` - userId on all 9 functions, ownership verification on candidate ops, userId on resolveThread item insert
|
||||
- `src/server/services/setup.service.ts` - userId on all 7 functions, inArray item ownership validation in syncSetupItems
|
||||
- `src/server/services/auth.service.ts` - removed prodDb defaults, reordered createApiKey params
|
||||
|
||||
## Decisions Made
|
||||
- Category deletion now uses `getOrCreateUncategorized(db, userId)` instead of hardcoded category ID 1, supporting multi-user where each user has their own Uncategorized category
|
||||
- Candidate operations verify parent thread ownership (not just candidate existence) to prevent cross-user manipulation via candidate ID guessing
|
||||
- `syncSetupItems` validates item ownership via `inArray` query before inserting, silently filtering out items that don't belong to the user
|
||||
- `resolveThread` verifies the candidate's category belongs to the user, falling back to `getOrCreateUncategorized` if not
|
||||
- `createApiKey` parameter order changed from `(db, name, userId)` to `(db, userId, name)` for consistency with the userId-second-param pattern
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
### Auto-fixed Issues
|
||||
|
||||
**1. [Rule 1 - Bug] Category service sync SQLite to async Postgres patterns**
|
||||
- **Found during:** Task 1 (category.service.ts)
|
||||
- **Issue:** Category service still used synchronous SQLite patterns (.get(), .all(), .run()) that don't work with Postgres driver
|
||||
- **Fix:** Converted all functions to async with await and array destructuring, consistent with other services
|
||||
- **Files modified:** src/server/services/category.service.ts
|
||||
- **Verification:** All functions now use async/await patterns
|
||||
- **Committed in:** 8d85d28 (Task 1 commit)
|
||||
|
||||
**2. [Rule 2 - Missing Critical] Added getCategoryById function**
|
||||
- **Found during:** Task 1 (category.service.ts)
|
||||
- **Issue:** Category service had no getCategoryById function — needed for userId-scoped lookups
|
||||
- **Fix:** Added getCategoryById(db, userId, id) with and(eq) isolation
|
||||
- **Files modified:** src/server/services/category.service.ts
|
||||
- **Verification:** Function exists with proper userId scoping
|
||||
- **Committed in:** 8d85d28 (Task 1 commit)
|
||||
|
||||
**3. [Rule 2 - Missing Critical] Setup operations verify ownership before mutations**
|
||||
- **Found during:** Task 2 (setup.service.ts)
|
||||
- **Issue:** updateItemClassification and removeSetupItem had no ownership checks — raw SQL conditions without setup ownership validation
|
||||
- **Fix:** Added setup ownership verification before both operations
|
||||
- **Files modified:** src/server/services/setup.service.ts
|
||||
- **Verification:** Both functions check setup belongs to user before proceeding
|
||||
- **Committed in:** 242cace (Task 2 commit)
|
||||
|
||||
---
|
||||
|
||||
**Total deviations:** 3 auto-fixed (1 bug, 2 missing critical)
|
||||
**Impact on plan:** All auto-fixes necessary for correctness and security. No scope creep.
|
||||
|
||||
## Issues Encountered
|
||||
None
|
||||
|
||||
## User Setup Required
|
||||
None - no external service configuration required.
|
||||
|
||||
## Known Stubs
|
||||
None - all service functions are fully implemented with proper userId scoping.
|
||||
|
||||
## Next Phase Readiness
|
||||
- All services now accept userId — route handlers (Plan 03) can pass c.get("userId") from auth middleware
|
||||
- MCP tools (Plan 04) can pass userId from MCP auth context
|
||||
- Tests will need updating to pass userId to all service calls
|
||||
|
||||
## Self-Check: PASSED
|
||||
|
||||
All 7 modified service files exist. Both task commits (8d85d28, 242cace) verified in git log.
|
||||
|
||||
---
|
||||
*Phase: 16-multi-user-data-model*
|
||||
*Completed: 2026-04-05*
|
||||
Reference in New Issue
Block a user