Jean-Luc Makiola
884bec0b35
- 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
147 lines
7.0 KiB
Markdown
147 lines
7.0 KiB
Markdown
---
|
|
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*
|