14 KiB
14 KiB
phase, plan, type, wave, depends_on, files_modified, autonomous, requirements, must_haves
| phase | plan | type | wave | depends_on | files_modified | autonomous | requirements | must_haves | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 16-multi-user-data-model | 04 | execute | 3 |
|
|
true |
|
|
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.
<execution_context> @$HOME/.claude/get-shit-done/workflows/execute-plan.md @$HOME/.claude/get-shit-done/templates/summary.md </execution_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.tsTest update pattern for service tests:
// 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:
// 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:
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");
});
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.
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
```
<success_criteria>
- Full test suite passes (
bun testreturns 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>