321 lines
14 KiB
Markdown
321 lines
14 KiB
Markdown
---
|
|
phase: 16-multi-user-data-model
|
|
plan: 04
|
|
type: execute
|
|
wave: 3
|
|
depends_on: ["16-01", "16-02"]
|
|
files_modified:
|
|
- tests/services/item.service.test.ts
|
|
- tests/services/category.service.test.ts
|
|
- tests/services/thread.service.test.ts
|
|
- tests/services/setup.service.test.ts
|
|
- tests/services/totals.test.ts
|
|
- tests/services/csv.service.test.ts
|
|
- tests/services/auth.service.test.ts
|
|
- tests/services/oauth.service.test.ts
|
|
- tests/routes/items.test.ts
|
|
- tests/routes/categories.test.ts
|
|
- tests/routes/threads.test.ts
|
|
- tests/routes/setups.test.ts
|
|
- tests/routes/auth.test.ts
|
|
- tests/routes/images.test.ts
|
|
- tests/routes/oauth.test.ts
|
|
- tests/routes/params.test.ts
|
|
- tests/mcp/tools.test.ts
|
|
autonomous: true
|
|
requirements:
|
|
- MULTI-02
|
|
- MULTI-04
|
|
- MULTI-05
|
|
|
|
must_haves:
|
|
truths:
|
|
- "All existing tests pass after updating to use { db, userId } from createTestDb"
|
|
- "Service tests pass userId to every service function call"
|
|
- "Route tests set userId on the test app context"
|
|
- "MCP tests pass userId to MCP server creation"
|
|
- "At least one cross-user isolation test exists proving User A cannot see User B's data"
|
|
artifacts:
|
|
- path: "tests/services/item.service.test.ts"
|
|
provides: "User-scoped item service tests"
|
|
contains: "userId"
|
|
- path: "tests/mcp/tools.test.ts"
|
|
provides: "User-scoped MCP tool tests"
|
|
contains: "userId"
|
|
key_links:
|
|
- from: "tests/helpers/db.ts"
|
|
to: "tests/services/*.test.ts"
|
|
via: "createTestDb returns { db, userId }"
|
|
pattern: "const \\{ db, userId \\}"
|
|
- from: "tests/services/item.service.test.ts"
|
|
to: "src/server/services/item.service.ts"
|
|
via: "passes userId to all service calls"
|
|
pattern: "getAllItems.*db.*userId"
|
|
---
|
|
|
|
<objective>
|
|
Update all test files to work with the new multi-user data model: destructure { db, userId } from createTestDb(), pass userId to all service calls, set userId in route test contexts, and add cross-user isolation tests.
|
|
|
|
Purpose: Tests validate that multi-user isolation works correctly. Without updated tests, we cannot verify that MULTI-02 (cross-user isolation) is enforced. The test suite must pass green before the phase is complete.
|
|
|
|
Output: All 17 test files updated, cross-user isolation tests added, full test suite passes.
|
|
</objective>
|
|
|
|
<execution_context>
|
|
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
|
|
@$HOME/.claude/get-shit-done/templates/summary.md
|
|
</execution_context>
|
|
|
|
<context>
|
|
@.planning/PROJECT.md
|
|
@.planning/ROADMAP.md
|
|
@.planning/phases/16-multi-user-data-model/16-CONTEXT.md
|
|
@.planning/phases/16-multi-user-data-model/16-RESEARCH.md
|
|
@.planning/phases/16-multi-user-data-model/16-01-SUMMARY.md
|
|
@.planning/phases/16-multi-user-data-model/16-02-SUMMARY.md
|
|
@tests/helpers/db.ts
|
|
@tests/services/item.service.test.ts
|
|
@tests/services/category.service.test.ts
|
|
@tests/routes/items.test.ts
|
|
@tests/mcp/tools.test.ts
|
|
|
|
<interfaces>
|
|
<!-- From Plan 01: createTestDb() now returns { db, userId } -->
|
|
<!-- From Plan 01: createSecondTestUser(db) creates user 2 for isolation tests -->
|
|
<!-- From Plan 02: all service functions now accept (db, userId, ...) -->
|
|
<!-- From Plan 03: routes expect userId on context, MCP expects userId -->
|
|
|
|
Test update pattern for service tests:
|
|
```typescript
|
|
// Before:
|
|
let db: any;
|
|
beforeEach(async () => { db = await createTestDb(); });
|
|
// ...
|
|
const items = await getAllItems(db);
|
|
|
|
// After:
|
|
let db: any;
|
|
let userId: number;
|
|
beforeEach(async () => { ({ db, userId } = await createTestDb()); });
|
|
// ...
|
|
const items = await getAllItems(db, userId);
|
|
```
|
|
|
|
Test update pattern for route tests:
|
|
```typescript
|
|
// Before:
|
|
const testApp = new Hono();
|
|
testApp.use("*", async (c, next) => { c.set("db", db); await next(); });
|
|
|
|
// After:
|
|
const testApp = new Hono();
|
|
testApp.use("*", async (c, next) => { c.set("db", db); c.set("userId", userId); await next(); });
|
|
```
|
|
|
|
Cross-user isolation test pattern:
|
|
```typescript
|
|
import { createSecondTestUser } from "../helpers/db";
|
|
|
|
test("user cannot see other user's items", async () => {
|
|
const userId2 = await createSecondTestUser(db);
|
|
await createItem(db, userId, { name: "User 1 Item", ... });
|
|
await createItem(db, userId2, { name: "User 2 Item", ... });
|
|
|
|
const user1Items = await getAllItems(db, userId);
|
|
const user2Items = await getAllItems(db, userId2);
|
|
|
|
expect(user1Items).toHaveLength(1);
|
|
expect(user1Items[0].name).toBe("User 1 Item");
|
|
expect(user2Items).toHaveLength(1);
|
|
expect(user2Items[0].name).toBe("User 2 Item");
|
|
});
|
|
```
|
|
</interfaces>
|
|
</context>
|
|
|
|
<tasks>
|
|
|
|
<task type="auto">
|
|
<name>Task 1: Update all service test files to pass userId</name>
|
|
<files>tests/services/item.service.test.ts, tests/services/category.service.test.ts, tests/services/thread.service.test.ts, tests/services/setup.service.test.ts, tests/services/totals.test.ts, tests/services/csv.service.test.ts, tests/services/auth.service.test.ts, tests/services/oauth.service.test.ts</files>
|
|
<read_first>tests/services/item.service.test.ts, tests/services/category.service.test.ts, tests/services/thread.service.test.ts, tests/services/setup.service.test.ts, tests/services/totals.test.ts, tests/services/csv.service.test.ts, tests/services/auth.service.test.ts, tests/services/oauth.service.test.ts, tests/helpers/db.ts</read_first>
|
|
<action>
|
|
For EACH of the 8 service test files, apply this systematic transformation:
|
|
|
|
1. **Change createTestDb destructuring** from `db = await createTestDb()` to `({ db, userId } = await createTestDb())`. Add `userId` variable declaration alongside `db`.
|
|
|
|
2. **Add userId to every service function call** as the second argument after `db`. Go through every call to any service function in each test and add `userId`.
|
|
|
|
3. **Fix category references**: Tests that reference "Uncategorized" by hardcoded ID 1 must instead look up the category by name+userId or use the ID returned from the seeded data. The test helper already seeds Uncategorized with the test user's ID.
|
|
|
|
4. **Specific file notes:**
|
|
|
|
**item.service.test.ts** (~160 lines):
|
|
- Destructure `{ db, userId }` from createTestDb
|
|
- Every `getAllItems(db)` becomes `getAllItems(db, userId)`
|
|
- Every `getItemById(db, id)` becomes `getItemById(db, userId, id)`
|
|
- Every `createItem(db, data)` becomes `createItem(db, userId, data)`
|
|
- Every `updateItem(db, id, data)` becomes `updateItem(db, userId, id, data)`
|
|
- Every `deleteItem(db, id)` becomes `deleteItem(db, userId, id)`
|
|
- ADD a cross-user isolation test: create a second user with `createSecondTestUser(db)`, create items for each user, verify each user only sees their own items, and verify getItemById returns null for another user's item ID
|
|
|
|
**category.service.test.ts** (~97 lines):
|
|
- Same destructuring pattern
|
|
- Add userId to all category service calls
|
|
- ADD a test for composite unique constraint: two users can have categories with the same name
|
|
- Fix any hardcoded "Uncategorized" ID references
|
|
|
|
**thread.service.test.ts** (~523 lines):
|
|
- Same destructuring pattern
|
|
- Add userId to ALL thread, candidate, and resolve calls
|
|
- Candidate operations need userId (for parent thread verification)
|
|
- Thread resolution test: verify the created item has the correct userId
|
|
- ADD a cross-user isolation test for threads
|
|
|
|
**setup.service.test.ts** (~293 lines):
|
|
- Same destructuring pattern
|
|
- Add userId to all setup service calls
|
|
- syncSetupItems calls need userId
|
|
- ADD a test that verifies a user cannot add another user's items to their setup
|
|
|
|
**totals.test.ts** (~79 lines):
|
|
- Same destructuring pattern
|
|
- Add userId to getTotals calls
|
|
|
|
**csv.service.test.ts** (~196 lines):
|
|
- Same destructuring pattern
|
|
- Add userId to import/export calls
|
|
- CSV import must create categories with userId
|
|
|
|
**auth.service.test.ts** (~68 lines):
|
|
- Same destructuring pattern
|
|
- Add userId to createApiKey, listApiKeys, deleteApiKey calls
|
|
- Update verifyApiKey assertions to check for `{ userId }` return instead of boolean `true`
|
|
|
|
**oauth.service.test.ts** (~290 lines):
|
|
- Same destructuring pattern
|
|
- Add userId where needed (createTokens, verifyAccessToken)
|
|
- Update verifyAccessToken assertions to check for `{ userId }` return instead of boolean
|
|
|
|
5. **Import `createSecondTestUser`** from helpers/db.ts in files that add isolation tests (at minimum: item, category, thread, setup test files).
|
|
|
|
After all files are updated, run `bun test` to verify the full suite passes.
|
|
</action>
|
|
<verify>
|
|
<automated>bun test tests/services/ 2>&1 | tail -5</automated>
|
|
</verify>
|
|
<acceptance_criteria>
|
|
- All 8 service test files use `{ db, userId } = await createTestDb()`
|
|
- Every service function call in tests includes userId argument
|
|
- `item.service.test.ts` has a cross-user isolation test using `createSecondTestUser`
|
|
- `category.service.test.ts` has a composite unique constraint test (same name, different users)
|
|
- `auth.service.test.ts` checks `verifyApiKey` returns `{ userId }` not boolean
|
|
- `oauth.service.test.ts` checks `verifyAccessToken` returns `{ userId }` not boolean
|
|
- `bun test tests/services/` passes all tests
|
|
</acceptance_criteria>
|
|
<done>All 8 service test files updated with userId, cross-user isolation tests added, all service tests pass</done>
|
|
</task>
|
|
|
|
<task type="auto">
|
|
<name>Task 2: Update route tests, MCP tests, and run full suite</name>
|
|
<files>tests/routes/items.test.ts, tests/routes/categories.test.ts, tests/routes/threads.test.ts, tests/routes/setups.test.ts, tests/routes/auth.test.ts, tests/routes/images.test.ts, tests/routes/oauth.test.ts, tests/routes/params.test.ts, tests/mcp/tools.test.ts</files>
|
|
<read_first>tests/routes/items.test.ts, tests/routes/categories.test.ts, tests/routes/threads.test.ts, tests/routes/setups.test.ts, tests/routes/auth.test.ts, tests/routes/images.test.ts, tests/routes/oauth.test.ts, tests/routes/params.test.ts, tests/mcp/tools.test.ts</read_first>
|
|
<action>
|
|
**Route tests** (7 files):
|
|
|
|
Route tests create a test Hono app with middleware that sets `db` on context. They need TWO changes:
|
|
|
|
1. **Destructure `{ db, userId }`** from createTestDb
|
|
2. **Set userId on the test app context** in the middleware setup:
|
|
```typescript
|
|
testApp.use("*", async (c, next) => {
|
|
c.set("db", db);
|
|
c.set("userId", userId);
|
|
await next();
|
|
});
|
|
```
|
|
|
|
For each route test file, read the file first to understand its middleware setup pattern, then add `c.set("userId", userId)` to the middleware. The route handlers will then pick it up via `c.get("userId")`.
|
|
|
|
**Specific notes per file:**
|
|
|
|
**items.test.ts** (~207 lines): Set userId in context middleware. Tests should work as-is since routes now pass userId to services.
|
|
|
|
**categories.test.ts** (~91 lines): Set userId in context middleware.
|
|
|
|
**threads.test.ts** (~413 lines): Set userId in context middleware. Thread tests may reference hardcoded Uncategorized category ID -- fix these.
|
|
|
|
**setups.test.ts** (~305 lines): Set userId in context middleware.
|
|
|
|
**auth.test.ts** (~139 lines): Set userId in context middleware for API key management routes. Auth-specific routes (login, setup) may not need userId.
|
|
|
|
**images.test.ts** (~26 lines): Set userId in context middleware if needed.
|
|
|
|
**oauth.test.ts** (~443 lines): May not need userId for OAuth flow tests, but set it for consistency. OAuth token creation may need userId.
|
|
|
|
**params.test.ts** (~81 lines): Set userId in context middleware.
|
|
|
|
**MCP tests** (1 file):
|
|
|
|
**tools.test.ts** (~253 lines):
|
|
- Destructure `{ db, userId }` from createTestDb
|
|
- Update `createMcpServer(db)` calls to `createMcpServer(db, userId)`
|
|
- MCP tool calls should then work since tools now receive userId from the server
|
|
- ADD an isolation test: create data as user 1, verify MCP tools as user 2 don't return user 1's data
|
|
|
|
After ALL test files are updated, run the FULL test suite:
|
|
```bash
|
|
bun test
|
|
```
|
|
|
|
Fix any remaining failures. Common issues to watch for:
|
|
- Missing userId argument (TypeScript will flag this)
|
|
- Hardcoded category ID 1 (use the seeded category from createTestDb)
|
|
- Boolean vs object return type from verifyApiKey/verifyAccessToken
|
|
- Settings tests using single key lookup instead of composite
|
|
- Route tests not setting userId in context
|
|
|
|
Also run lint to ensure no formatting issues:
|
|
```bash
|
|
bun run lint
|
|
```
|
|
</action>
|
|
<verify>
|
|
<automated>bun test 2>&1 | tail -10</automated>
|
|
</verify>
|
|
<acceptance_criteria>
|
|
- All route test files set `c.set("userId", userId)` in test middleware
|
|
- All route test files use `{ db, userId } = await createTestDb()`
|
|
- MCP tools.test.ts passes userId to createMcpServer
|
|
- MCP tools.test.ts has a cross-user isolation test
|
|
- `bun test` passes ALL tests (0 failures)
|
|
- `bun run lint` passes
|
|
</acceptance_criteria>
|
|
<done>All 17 test files updated, full test suite passes green, lint passes, cross-user isolation verified by tests</done>
|
|
</task>
|
|
|
|
</tasks>
|
|
|
|
<verification>
|
|
After all tasks complete:
|
|
1. `bun test` exits with 0 failures
|
|
2. `bun run lint` passes
|
|
3. `grep -r "createSecondTestUser" tests/` shows isolation tests exist
|
|
4. `grep -r 'c.set("userId"' tests/routes/` shows userId set in all route test middleware
|
|
5. `grep "createMcpServer(db, userId)" tests/mcp/tools.test.ts` confirms MCP test update
|
|
</verification>
|
|
|
|
<success_criteria>
|
|
- Full test suite passes (`bun test` returns 0 failures)
|
|
- All service tests pass userId to every service call
|
|
- All route tests set userId in test app context
|
|
- MCP tests pass userId to createMcpServer
|
|
- Cross-user isolation tests exist for items, categories, threads, setups
|
|
- Composite unique constraint test exists for categories
|
|
- Lint passes
|
|
</success_criteria>
|
|
|
|
<output>
|
|
After completion, create `.planning/phases/16-multi-user-data-model/16-04-SUMMARY.md`
|
|
</output>
|