feat(tasks): add Tasks sidebar button and scoped task views #206
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughAdds a Tasks feature: a GET /api/tasks endpoint with auth, authorization, validation, filtering, enrichment and pagination; comprehensive contract tests; user and drive task pages; a TasksDashboard UI with filters and grouping; task types; and a permission helper to list drives accessible to a user. Changes
Sequence Diagram(s)sequenceDiagram
actor Browser
participant Page as Tasks Page
participant API as /api/tasks
participant Auth as Auth Service
participant Perm as Permissions (getDriveIdsForUser)
participant DB as Database
participant UI as TasksDashboard
Browser->>Page: Navigate to tasks page
Page->>API: GET /api/tasks?context=...
API->>Auth: Validate token (JWT/MCP)
Auth-->>API: Authenticated (userId)
API->>Perm: getDriveIdsForUser(userId)
Perm->>DB: Query drives, driveMembers, pagePermissions
DB-->>Perm: driveId list
Perm-->>API: accessible driveIds
API->>DB: Fetch task list pages for drives
DB-->>API: pages & taskLists
API->>DB: Query tasks with relations + count (filters, limit/offset)
DB-->>API: tasks + total
API->>API: Enrich tasks with drive/page titles
API-->>Page: { tasks, total, limit, offset, hasMore }
Page->>UI: Render tasks with grouping, filters, pagination
UI->>Browser: Display tasks
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (5)apps/web/src/app/**/route.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,json}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/web/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (3)📚 Learning: 2025-12-23T18:49:41.966ZApplied to files:
📚 Learning: 2025-12-22T20:04:40.910ZApplied to files:
📚 Learning: 2025-12-14T14:54:45.713ZApplied to files:
🔇 Additional comments (8)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔬 Code Review SummaryReviewer: Claude Code ✅ Strengths
🟠 Important Issues (Should Address)
🟡 Minor Issues
Security Review (OWASP Top 10) ✅
RecommendationApproved with conditions - Merge after:
Detailed comments added below on specific files. |
🟠 Important: Missing Test CoverageFile: This new 254-line API endpoint has no test coverage. For a data-access endpoint handling task information across drives, tests are critical. Recommended test cases: describe('GET /api/tasks', () => {
// Authorization
it('should return 403 for inaccessible drive');
it('should return tasks only from user-accessible drives');
// Pagination
it('should respect limit and offset parameters');
it('should return hasMore=true when more tasks exist');
// Filters
it('should filter by status');
it('should filter by priority');
it('should combine multiple filters');
// Edge cases
it('should return empty array when no tasks exist');
it('should exclude tasks from trashed pages');
}); |
Code reviewFound 2 issues:
PageSpace/apps/web/src/app/api/tasks/route.ts Lines 216 to 234 in c85fb62
PageSpace/apps/web/src/components/tasks/TasksDashboard.tsx Lines 454 to 456 in c85fb62 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
🟠 Important: Inconsistent Tool Name SanitizationFiles affected:
The chat route correctly sanitizes MCP tool names for Gemini: // chat/route.ts - Lines 579-594
if (currentProvider === 'google' && filteredTools) {
const sanitizedTools: Record<string, unknown> = {};
for (const [originalName, tool] of Object.entries(filteredTools)) {
const sanitizedName = originalName.replace(/:/g, '__');
sanitizedTools[sanitizedName] = tool;
}
filteredTools = sanitizedTools as any;
}But the global assistant route is missing this: // global/[id]/messages/route.ts - Lines 670-671
// Merge MCP tools with PageSpace tools (type assertion safe since MCP tools match AI SDK format)
finalTools = { ...finalTools, ...mcpToolsWithExecute } as typeof finalTools;
// ❌ No Gemini sanitization hereImpact: MCP tools with colons (e.g., Suggestion: Extract sanitization to a shared utility or apply the same logic here. |
|
Review comments:
|
🟡 Minor: Potential N+1 Query PatternFile: The endpoint performs 4 sequential database queries:
// Query 1: Get pages
const taskListPages = await db.query.pages.findMany({ ... });
// Query 2: Get task lists
const taskListsData = await db.query.taskLists.findMany({ ... });
// Query 3: Get tasks with relations
const tasks = await db.query.taskItems.findMany({
with: { assignee, assigneeAgent, user, page, taskList },
});
// Query 4: Get count
const [countResult] = await db.select({ total: count() })...Not blocking - This is fine for initial implementation. If performance issues arise with many tasks/drives, consider:
|
🟡 Minor: Consider Extracting Sanitization UtilityFile: The Gemini tool name sanitization is implemented inline. Since this logic:
Consider extracting it to a shared utility: // Suggested: apps/web/src/lib/ai/core/tool-utils.ts
export function sanitizeToolNamesForProvider(
tools: Record<string, unknown>,
provider: string
): Record<string, unknown> {
// Only Gemini requires sanitization
if (provider !== 'google') return tools;
const sanitized: Record<string, unknown> = {};
for (const [name, tool] of Object.entries(tools)) {
sanitized[name.replace(/:/g, '__')] = tool;
}
return sanitized;
}Not blocking - This is a code quality improvement that can be done later. |
✅ Excellent: WebSocket Security ImplementationFiles:
The WebSocket auth migration is well-implemented with defense-in-depth:
Test coverage for this is also excellent in Great work on this security refactor! 👏 |
Code Review ResponseThanks for the thorough review! I've addressed the feedback: ✅ TypeScript Test Fixtures (f013ee3)Fixed the missing type properties in test mock objects by:
✅ Pagination/Trashed Page Filtering (fd5400c - already fixed)The pagination issues were already addressed in a previous commit:
✅ Gemini Tool Name Sanitization (d039f57)Added tool name sanitization to global assistant messages route to match the chat route:
Regarding N+1 Query PatternThe 4 sequential queries in the tasks route are intentional:
These cannot be easily parallelized due to dependencies between them. The queries are simple index-based lookups and the total overhead is minimal. |
Add a Tasks button to the left sidebar that shows all tasks assigned to the current user, with scoping similar to activity feeds: - Dashboard level: Shows tasks across all drives - Drive level: Shows tasks within that specific drive Changes: - Add Tasks button to left sidebar (next to Activity) - Create /api/tasks endpoint for scoped task queries - Add TasksDashboard component with filtering by status/priority/drive - Add getDriveIdsForUser permission helper function - Create /dashboard/tasks and /dashboard/[driveId]/tasks pages
- Fix pagination count mismatch: moved isTrashed filtering to SQL level so count query matches filtered results - Fix potential malformed URL: validate driveId exists before constructing navigation links in TasksDashboard - Add comprehensive tests for /api/tasks endpoint (18 test cases) covering auth, authorization, validation, pagination, and errors
Add createPageFixture and createTaskListFixture helpers with all required properties to satisfy Drizzle schema types. Use 'as const' assertions for enum-like fields (type, status, priority) to preserve literal types. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add tool name sanitization for Gemini provider compatibility in the global assistant messages route. Converts colons in MCP tool names to double underscores (e.g., mcp:server:tool → mcp__server__tool) since Gemini doesn't allow multiple colons in function names. This matches the existing sanitization in the chat route. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2witstudios
force-pushed
the
claude/add-tasks-sidebar-button-Sl1l5
branch
from
4d7e490 to
25035ee
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/app/api/tasks/route.ts`:
- Around line 228-236: The enrichment step using
taskListToPageMap.get(task.taskListId) can return undefined and produce
enrichedTasks with driveId undefined; update the logic in the enrichedTasks
creation to skip or handle orphaned tasks by filtering out tasks where pageInfo
is falsy (i.e., ensure pageInfo exists before mapping) or provide a safe
fallback (e.g., omit driveId/taskListPageId/taskListPageTitle or set a sentinel)
and optionally log a warning for missing pageInfo so frontend URLs like
/dashboard/{driveId}/page/{id} are never built with undefined values.
🧹 Nitpick comments (6)
packages/lib/src/permissions/permissions.ts (1)
10-47: Consider parallelizing independent DB queries for better performance.The three queries (owned drives, member drives, page permissions) are independent and could execute concurrently using
Promise.all. This would reduce latency, especially when the user has access through multiple sources.♻️ Optional: Parallelize queries
export async function getDriveIdsForUser(userId: string): Promise<string[]> { const driveIdSet = new Set<string>(); - // 1. Get drives owned by user - const ownedDrives = await db.select({ id: drives.id }) + // Execute all queries in parallel + const [ownedDrives, memberDrives, pageDrives] = await Promise.all([ + db.select({ id: drives.id }) .from(drives) - .where(eq(drives.ownerId, userId)); - - for (const drive of ownedDrives) { - driveIdSet.add(drive.id); - } - - // 2. Get drives where user is a member - const memberDrives = await db.select({ driveId: driveMembers.driveId }) + .where(eq(drives.ownerId, userId)), + db.select({ driveId: driveMembers.driveId }) .from(driveMembers) - .where(eq(driveMembers.userId, userId)); - - for (const membership of memberDrives) { - driveIdSet.add(membership.driveId); - } - - // 3. Get drives where user has page permissions - const pageDrives = await db.select({ driveId: pages.driveId }) + .where(eq(driveMembers.userId, userId)), + db.select({ driveId: pages.driveId }) .from(pagePermissions) .leftJoin(pages, eq(pagePermissions.pageId, pages.id)) .where(and( eq(pagePermissions.userId, userId), eq(pagePermissions.canView, true) - )); + )), + ]); + for (const drive of ownedDrives) { + driveIdSet.add(drive.id); + } + for (const membership of memberDrives) { + driveIdSet.add(membership.driveId); + } for (const page of pageDrives) { if (page.driveId) { driveIdSet.add(page.driveId); } } return Array.from(driveIdSet); }apps/web/src/app/api/tasks/route.ts (1)
168-173: Trashed pages query fetches globally instead of scoped to accessible drives.This query retrieves all trashed pages in the system regardless of the user's accessible drives. For users with limited access, this is inefficient and retrieves unnecessary data. Consider scoping the query to only pages within
driveIds.♻️ Scope trashed pages query to accessible drives
// Get trashed page IDs to exclude tasks referencing them // This ensures pagination counts match the actual filtered results const trashedPages = await db.select({ id: pages.id }) .from(pages) - .where(eq(pages.isTrashed, true)); + .where(and( + eq(pages.isTrashed, true), + inArray(pages.driveId, driveIds) + )); const trashedPageIds = trashedPages.map(p => p.id);apps/web/src/app/api/tasks/__tests__/route.test.ts (1)
401-421: Consider adding assertions to verify filter conditions are applied.The filter tests verify that
db.query.taskItems.findManyis called but don't assert that the correct filter conditions were passed. While deep mocking is complex, you could capture and verify thewhereargument structure.// Example: Capture and verify filter was applied it('should filter by status', async () => { const request = new Request('https://example.com/api/tasks?context=user&status=completed'); await GET(request); expect(db.query.taskItems.findMany).toHaveBeenCalledWith( expect.objectContaining({ where: expect.anything(), // Could use a custom matcher }) ); // Verify eq() was called with status filter expect(eq).toHaveBeenCalledWith(expect.anything(), 'completed'); });apps/web/src/components/tasks/TasksDashboard.tsx (1)
106-157: Consider using SWR for server state management.Based on project learnings, SWR is the recommended approach for server state and caching. The current implementation uses manual fetch with useState, which works but doesn't provide automatic revalidation, caching, or deduplication benefits.
This is a nice-to-have refactor that could be addressed in a follow-up.
Based on learnings, the project prefers SWR for server state management and caching. Consider refactoring to use
useSWRoruseSWRInfinitefor the tasks fetching, which would provide automatic revalidation and better caching behavior.apps/web/src/app/dashboard/[driveId]/tasks/page.tsx (2)
10-11: Consider safer type handling for route params.The
as stringcast is generally safe for single-segment dynamic routes like[driveId], but for stricter type safety, consider validating the param type.🔧 Suggested improvement
const params = useParams(); - const driveId = params.driveId as string; + const driveId = typeof params.driveId === 'string' ? params.driveId : params.driveId?.[0]; + + if (!driveId) { + return ( + <div className="flex items-center justify-center h-full"> + <p className="text-muted-foreground">Invalid drive ID</p> + </div> + ); + }
20-32: Extract skeleton fallback to reduce duplication.The loading skeleton UI is duplicated between
DriveTasksContent(lines 21-31) and theSuspensefallback inDriveTasksPage(lines 55-63). Consider extracting to a shared component.♻️ Suggested refactor
+function TasksPageSkeleton() { + return ( + <div className="h-full overflow-y-auto"> + <div className="container mx-auto px-4 py-10 sm:px-6 lg:px-10 max-w-5xl"> + <div className="space-y-6"> + <Skeleton className="h-8 w-48" /> + <Skeleton className="h-10 w-full" /> + <Skeleton className="h-96" /> + </div> + </div> + </div> + ); +} function DriveTasksContent() { // ... if (isLoading) { - return ( - <div className="h-full overflow-y-auto"> - <div className="container mx-auto px-4 py-10 sm:px-6 lg:px-10 max-w-5xl"> - <div className="space-y-6"> - <Skeleton className="h-8 w-48" /> - <Skeleton className="h-10 w-full" /> - <Skeleton className="h-96" /> - </div> - </div> - </div> - ); + return <TasksPageSkeleton />; } // ... } export default function DriveTasksPage() { return ( - <Suspense - fallback={ - <div className="h-full overflow-y-auto"> - ... - </div> - } - > + <Suspense fallback={<TasksPageSkeleton />}> <DriveTasksContent /> </Suspense> ); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/web/src/app/api/tasks/__tests__/route.test.tsapps/web/src/app/api/tasks/route.tsapps/web/src/app/dashboard/[driveId]/tasks/page.tsxapps/web/src/app/dashboard/tasks/page.tsxapps/web/src/components/layout/left-sidebar/index.tsxapps/web/src/components/tasks/TasksDashboard.tsxapps/web/src/components/tasks/index.tsapps/web/src/components/tasks/types.tspackages/lib/src/index.tspackages/lib/src/permissions/index.tspackages/lib/src/permissions/permissions.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/web/src/components/tasks/index.ts
- packages/lib/src/permissions/index.ts
- apps/web/src/app/dashboard/tasks/page.tsx
- packages/lib/src/index.ts
- apps/web/src/components/tasks/types.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Never useanytypes - always use proper TypeScript types
Use camelCase for variable and function names
Use UPPER_SNAKE_CASE for constants
Use PascalCase for type and enum names
Use kebab-case for filenames, except React hooks (camelCase withuseprefix), Zustand stores (camelCase withuseprefix), and React components (PascalCase)
Lint with Next/ESLint as configured inapps/web/eslint.config.mjs
Message content should always use the message parts structure with{ parts: [{ type: 'text', text: '...' }] }
Use centralized permission functions from@pagespace/lib/permissions(e.g.,getUserAccessLevel,canUserEditPage) instead of implementing permission logic locally
Always use Drizzle client from@pagespace/dbpackage for database access
Use ESM modules throughout the codebase
**/*.{ts,tsx}: Never useanytypes - always use proper TypeScript types
Write code that is explicit over implicit and self-documenting
Files:
packages/lib/src/permissions/permissions.tsapps/web/src/components/tasks/TasksDashboard.tsxapps/web/src/app/api/tasks/__tests__/route.test.tsapps/web/src/components/layout/left-sidebar/index.tsxapps/web/src/app/dashboard/[driveId]/tasks/page.tsxapps/web/src/app/api/tasks/route.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: React hook files should use camelCase matching the exported hook name (e.g.,useAuth.ts)
Zustand store files should use camelCase withuseprefix (e.g.,useAuthStore.ts)
Files:
packages/lib/src/permissions/permissions.tsapps/web/src/app/api/tasks/__tests__/route.test.tsapps/web/src/app/api/tasks/route.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier
Files:
packages/lib/src/permissions/permissions.tsapps/web/src/components/tasks/TasksDashboard.tsxapps/web/src/app/api/tasks/__tests__/route.test.tsapps/web/src/components/layout/left-sidebar/index.tsxapps/web/src/app/dashboard/[driveId]/tasks/page.tsxapps/web/src/app/api/tasks/route.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tsx: React component files should use PascalCase (e.g.,UserProfile.tsx)
Use@dnd-kitfor drag-and-drop functionality
Use Zustand for client state management
Use SWR for server state management and caching
Files:
apps/web/src/components/tasks/TasksDashboard.tsxapps/web/src/components/layout/left-sidebar/index.tsxapps/web/src/app/dashboard/[driveId]/tasks/page.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS and shadcn/ui components for styling and UI
Files:
apps/web/src/components/tasks/TasksDashboard.tsxapps/web/src/components/layout/left-sidebar/index.tsxapps/web/src/app/dashboard/[driveId]/tasks/page.tsx
apps/web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/**/*.{ts,tsx}: Use message parts structure for message content:{ parts: [{ type: 'text', text: '...' }] }
For database access, always use Drizzle client from@pagespace/db:import { db, pages } from '@pagespace/db';
Use centralized Drizzle ORM with PostgreSQL for all database operations - no direct SQL or other ORMs
Use Socket.IO for real-time collaboration features - imported from the realtime service at port 3001
Use Vercel AI SDK with async/await for all AI operations and streaming
Use Next.js 15 App Router and TypeScript for all routes and components
Files:
apps/web/src/components/tasks/TasksDashboard.tsxapps/web/src/app/api/tasks/__tests__/route.test.tsapps/web/src/components/layout/left-sidebar/index.tsxapps/web/src/app/dashboard/[driveId]/tasks/page.tsxapps/web/src/app/api/tasks/route.ts
apps/web/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/components/**/*.{ts,tsx}: When document editing, register editing state withuseEditingStore.getState().startEditing()to prevent UI refreshes, and clean up in return statement
When AI is streaming, register streaming state withuseEditingStore.getState().startStreaming()to prevent UI refreshes, and clean up in return statement
For SWR data fetching with editing protection, useisPaused: () => hasLoadedRef.current && isEditingActive()to allow initial fetch and only pause after, withrevalidateOnFocus: false
Use Zustand for client-side state management as the primary state solution
Use SWR for server state and caching with proper configuration includingrevalidateOnFocus: falsefor editing protection
Use TipTap rich text editor with markdown support for document editing
Use Monaco Editor for code editing features
Use@dnd-kitfor drag-and-drop functionality instead of other libraries
Use Tailwind CSS with shadcn/ui components for all UI styling and components
Files:
apps/web/src/components/tasks/TasksDashboard.tsxapps/web/src/components/layout/left-sidebar/index.tsx
apps/web/src/app/**/route.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/src/app/**/route.{ts,tsx}: In Next.js 15 route handlers,paramsin dynamic routes are Promise objects and MUST be awaited before destructuring
UseResponse.json()orNextResponse.json()for returning JSON from route handlers
Get request body usingconst body = await request.json();
Get search parameters usingconst { searchParams } = new URL(request.url);
apps/web/src/app/**/route.{ts,tsx}: In Next.js 15 dynamic routes,paramsare Promise objects and MUST be awaited before destructuring:const { id } = await context.params;
In Route Handlers, get request body withconst body = await request.json();
In Route Handlers, get search parameters withconst { searchParams } = new URL(request.url);
In Route Handlers, return JSON usingResponse.json(data)orNextResponse.json(data)
For permission logic, use centralized functions from@pagespace/lib/permissions:getUserAccessLevel(),canUserEditPage()
Files:
apps/web/src/app/api/tasks/route.ts
🧠 Learnings (10)
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to apps/processor/**/*.test.ts : Write unit tests for the processor service with test files named `*.test.ts` alongside source or in `__tests__/` directory
Applied to files:
apps/web/src/app/api/tasks/__tests__/route.test.ts
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to packages/lib/**/*.test.ts : Write unit tests for shared utilities in `packages/lib` with test files named `*.test.ts` alongside source or in `__tests__/` directory
Applied to files:
apps/web/src/app/api/tasks/__tests__/route.test.ts
📚 Learning: 2025-12-23T18:49:41.966Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T18:49:41.966Z
Learning: Applies to apps/web/src/components/**/*.{ts,tsx} : Use SWR for server state and caching with proper configuration including `revalidateOnFocus: false` for editing protection
Applied to files:
apps/web/src/components/layout/left-sidebar/index.tsx
📚 Learning: 2025-12-22T20:04:40.910Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.910Z
Learning: Applies to **/*.tsx : Use SWR for server state management and caching
Applied to files:
apps/web/src/components/layout/left-sidebar/index.tsx
📚 Learning: 2025-12-23T18:49:41.966Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T18:49:41.966Z
Learning: Applies to apps/web/src/app/**/route.{ts,tsx} : In Route Handlers, get search parameters with `const { searchParams } = new URL(request.url);`
Applied to files:
apps/web/src/app/api/tasks/route.ts
📚 Learning: 2025-12-23T18:49:41.966Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T18:49:41.966Z
Learning: Applies to apps/web/src/app/**/route.{ts,tsx} : For permission logic, use centralized functions from `pagespace/lib/permissions`: `getUserAccessLevel()`, `canUserEditPage()`
Applied to files:
apps/web/src/app/api/tasks/route.ts
📚 Learning: 2025-12-23T18:49:41.966Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-23T18:49:41.966Z
Learning: Applies to apps/web/src/app/**/route.{ts,tsx} : In Route Handlers, get request body with `const body = await request.json();`
Applied to files:
apps/web/src/app/api/tasks/route.ts
📚 Learning: 2025-12-22T20:04:40.910Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.910Z
Learning: Applies to apps/web/src/app/**/route.{ts,tsx} : In Next.js 15 route handlers, `params` in dynamic routes are Promise objects and MUST be awaited before destructuring
Applied to files:
apps/web/src/app/api/tasks/route.ts
📚 Learning: 2025-12-22T20:04:40.910Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.910Z
Learning: Applies to apps/web/src/app/**/route.{ts,tsx} : Get search parameters using `const { searchParams } = new URL(request.url);`
Applied to files:
apps/web/src/app/api/tasks/route.ts
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to app/api/**/*.{ts,tsx} : Use `const body = await request.json();` to extract request bodies, `const { searchParams } = new URL(request.url);` for query parameters, and `return Response.json(data)` or `return NextResponse.json(data)` for responses
Applied to files:
apps/web/src/app/api/tasks/route.ts
🧬 Code graph analysis (3)
apps/web/src/app/api/tasks/__tests__/route.test.ts (5)
apps/web/src/lib/auth/index.ts (3)
WebAuthResult(26-29)AuthError(33-35)authenticateRequestWithOptions(221-291)packages/db/src/index.ts (1)
db(20-20)apps/web/src/app/api/tasks/route.ts (1)
GET(31-262)apps/processor/src/logger.ts (1)
error(57-63)packages/lib/src/logging/logger-config.ts (1)
loggers(8-18)
apps/web/src/app/dashboard/[driveId]/tasks/page.tsx (3)
apps/web/src/hooks/useDrive.ts (1)
useDriveStore(21-74)apps/web/src/components/ui/skeleton.tsx (1)
Skeleton(145-145)apps/web/src/components/tasks/TasksDashboard.tsx (1)
TasksDashboard(43-410)
apps/web/src/app/api/tasks/route.ts (3)
apps/web/src/lib/auth/index.ts (1)
authenticateRequestWithOptions(221-291)packages/lib/src/permissions/index.ts (2)
isUserDriveMember(15-15)getDriveIdsForUser(11-11)packages/db/src/schema/tasks.ts (2)
taskLists(11-28)taskItems(40-65)
🔇 Additional comments (17)
apps/web/src/components/layout/left-sidebar/index.tsx (1)
152-158: LGTM!The Tasks navigation item follows the same conditional URL pattern as the Activity link above, maintaining consistency in the sidebar navigation. The styling and structure are consistent with the other sidebar items.
apps/web/src/app/api/tasks/route.ts (3)
1-6: LGTM on imports and authentication setup.The imports follow project conventions, authentication options correctly allow jwt/mcp without CSRF (appropriate for a read-only GET endpoint), and the centralized permission functions from
@pagespace/libare used as per coding guidelines.
37-51: Good use of URL search params extraction.The search parameters extraction follows the coding guidelines using
new URL(request.url)and handles nullable params correctly with the nullish coalescing operator.
62-109: Drive access authorization is properly implemented.The authorization flow correctly validates drive access for both contexts:
- Drive context: Uses
isUserDriveMemberto verify access- User context: Uses
getDriveIdsForUserand validates any optional driveId filter against accessible drivesReturns appropriate 403 responses for unauthorized access and handles empty drive lists gracefully.
apps/web/src/app/api/tasks/__tests__/route.test.ts (5)
1-5: LGTM on test setup and imports.The eslint-disable for
@typescript-eslint/no-explicit-anyis acceptable in test files where mocking often requires type flexibility. The imports are correctly structured with mocks defined before usage.
85-180: Well-structured test fixtures with proper TypeScript typing.The fixture helpers (
createPageFixture,createTaskListFixture,createTaskFixture) usePartial<>for overrides and provide sensible defaults. This makes tests readable and maintainable.
209-231: Good authentication test coverage.Tests correctly verify 401 on auth failure and validate that
authenticateRequestWithOptionsis called with the expected auth options.
233-274: Authorization tests cover key scenarios.Tests cover 403 for inaccessible drives in both drive and user contexts, and verify that
getDriveIdsForUseris called with the correct user ID.
448-469: Error handling tests are comprehensive.Tests verify both the 500 response and that the error is logged via
loggers.api.error. Good coverage of the error path.apps/web/src/components/tasks/TasksDashboard.tsx (6)
1-22: LGTM on imports and type usage.The component correctly uses
'use client'directive, imports follow project conventions, and types are properly imported from the local./typesmodule.
43-61: State initialization from URL search params is well implemented.The component correctly initializes filter state from URL search params with proper type casting. The pattern of using a function initializer for
useStateprevents unnecessary recomputation.
88-104: Intentional eslint-disable for exhaustive-deps is acceptable here.The comment explains that
selectedDriveIdis only checked, not derived from, which justifies excluding it from the dependency array. This prevents an infinite loop where settingselectedDriveIdwould trigger a refetch which would set it again.
454-459: URL construction correctly guards against undefined values.The Link href condition
task.pageId && task.driveIdproperly prevents malformed URLs when either value is undefined. This addresses the PR comment concern about URLs like/dashboard/undefined/page/123.
465-476: Task list link also has proper guards.The condition
task.taskListPageTitle && task.driveId && task.taskListPageIdensures all required values are present before rendering the link. Good defensive coding.
487-503: Date handling is correct and user-friendly.The due date display logic properly handles overdue, due today, and future due dates with appropriate styling. Using
date-fnsfunctions (isPast,isToday,format,formatDistanceToNow) is idiomatic.apps/web/src/app/dashboard/[driveId]/tasks/page.tsx (2)
1-8: LGTM!Imports are correctly structured with proper module sources. The
'use client'directive is appropriately placed for this interactive component.
51-69: LGTM!The
Suspenseboundary provides a good pattern for consistency and future-proofing, even though the current client-side data fetching viauseDriveStorewon't suspend. The actual loading state is correctly handled by theisLoadingcheck inDriveTasksContent.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Skip tasks where taskListToPageMap lookup returns undefined to prevent
enrichedTasks having driveId: undefined. This ensures frontend URLs
like /dashboard/{driveId}/page/{id} are never built with undefined values.
Added warning log for orphaned tasks to help identify data integrity issues.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add a Tasks button to the left sidebar that shows all tasks assigned to the current user, with scoping similar to activity feeds:
Changes:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.