[WIKI-855] feat: pagination, filters, and ordering for project pages #8466
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds backend page_filters and moves PageViewSet.list to paginated, ordered responses; introduces frontend cursor-based, per-tab pagination with infinite-scroll, filter-aware SWR fetching, updated service/store signatures and a list skeleton loader. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant UI as Frontend UI
participant IO as IntersectionObserver
participant Store as ProjectPageStore
participant Service as ProjectPageService
participant API as Backend API
User->>UI: Open pages list (workspace, project, pageType)
UI->>Store: fetchPagesList(workspace, project, pageType)
Store->>Service: fetchAll(workspace, project, queries?, config?)
Service->>API: GET /pages?filters&order_by&cursor
API->>API: page_filters(query_params) -> queryset
API->>API: paginate(queryset)
API-->>Service: TPagePaginatedResponse { results, paginationInfo }
Service-->>Store: paginated response
Store->>Store: update paginationInfo[pageType], append filteredPageIds
Store-->>UI: updated items, hasNextPage
UI->>IO: render intersection trigger
Note over User,IO: user scrolls
IO->>UI: intersection triggered
UI->>Store: fetchPagesList(..., cursor)
Store->>Service: fetchAll(..., { cursor })
Service->>API: GET /pages?cursor&filters
API-->>Service: next page response
Store->>Store: append results, update paginationInfo
Store-->>UI: new items
UI->>UI: render appended items / loader
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels🌐frontend, 🌟improvement Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used📓 Path-based instructions (4)**/*.{ts,tsx,mts,cts}📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{js,jsx,ts,tsx,json,css}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-10-21T17:22:05.204ZApplied to files:
📚 Learning: 2025-12-23T14:18:32.899ZApplied to files:
🧬 Code graph analysis (1)apps/api/plane/app/views/page/base.py (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (31)
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 |
im-vipin
requested review from
NarayanBavisetti,
aaryan610 and
prateekshourya29
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/core/store/pages/project-page.store.ts (2)
560-580: Deleted pages not removed from filtered ID arrays.Similar to
createPage,removePageonly removes fromthis.databut doesn't clean upfilteredPublicPageIds,filteredPrivatePageIds, orfilteredArchivedPageIds. This could cause rendering issues when the component tries to render a deleted page.🔎 Proposed fix
await this.service.remove(workspaceSlug, projectId, pageId); runInAction(() => { unset(this.data, [pageId]); + // Remove from filtered arrays + this.filteredPublicPageIds = this.filteredPublicPageIds.filter((id) => id !== pageId); + this.filteredPrivatePageIds = this.filteredPrivatePageIds.filter((id) => id !== pageId); + this.filteredArchivedPageIds = this.filteredArchivedPageIds.filter((id) => id !== pageId); if (this.rootStore.favorite.entityMap[pageId]) this.rootStore.favorite.removeFavoriteFromStore(pageId); });
527-554: Newly created pages do not appear in filtered lists until refetch.The
createPagemethod adds the page tothis.databut doesn't updatefilteredPublicPageIdsorfilteredPrivatePageIds. These filtered arrays are populated only byfetchPagesList, so users won't see newly created pages in the filtered list until they refresh or filters are changed.Consider either adding the new page ID to the appropriate filtered array based on its visibility type, or triggering a refetch after creation.
🧹 Nitpick comments (3)
apps/api/plane/utils/page_filters.py (1)
28-51: Month calculation uses 30-day approximation.The month calculation uses
timedelta(days=duration * 30)which is an approximation. This may lead to slight inaccuracies for date filters (e.g., 3 months = 90 days instead of actual calendar months). This is acceptable for most use cases but worth noting for precision-sensitive scenarios.apps/web/core/components/pages/pages-list-view.tsx (1)
24-36: SWR key construction correctly triggers refetch on filter changes.The key includes all relevant filter state to ensure data refreshes when filters change. However,
revalidateOnFocus: truecombined with complex filter state may cause frequent refetches when users switch tabs/windows.Consider setting
revalidateOnFocus: falseif users report excessive loading states when returning to the page.apps/web/core/components/pages/list/root.tsx (1)
59-65: Intersection observer rootMargin may trigger loading too early.The rootMargin
100% 0% 100% 0%expands the root bounds by 100% of its height both top and bottom. This means the intersection callback could fire when the element is still a full viewport away. Consider using a smaller pixel-based margin like100px 0px 100px 0pxfor more predictable loading behavior.🔎 Proposed fix
useIntersectionObserver( containerRef, isFetchingNextPage ? null : intersectionElement, fetchNextPage, - `100% 0% 100% 0%` + `100px 0% 100px 0%` );
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/api/plane/app/views/page/base.pyapps/api/plane/utils/page_filters.pyapps/web/core/components/pages/list/loader.tsxapps/web/core/components/pages/list/root.tsxapps/web/core/components/pages/pages-list-main-content.tsxapps/web/core/components/pages/pages-list-view.tsxapps/web/core/services/page/project-page.service.tsapps/web/core/store/pages/project-page.store.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/core/components/pages/pages-list-main-content.tsxapps/web/core/components/pages/list/loader.tsxapps/web/core/components/pages/list/root.tsxapps/web/core/components/pages/pages-list-view.tsxapps/web/core/services/page/project-page.service.tsapps/web/core/store/pages/project-page.store.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Enable TypeScript strict mode and ensure all files are fully typed
Files:
apps/web/core/components/pages/pages-list-main-content.tsxapps/web/core/components/pages/list/loader.tsxapps/web/core/components/pages/list/root.tsxapps/web/core/components/pages/pages-list-view.tsxapps/web/core/services/page/project-page.service.tsapps/web/core/store/pages/project-page.store.ts
**/*.{js,jsx,ts,tsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with Tailwind plugin for code formatting, run
pnpm fix:format
Files:
apps/web/core/components/pages/pages-list-main-content.tsxapps/web/core/components/pages/list/loader.tsxapps/web/core/components/pages/list/root.tsxapps/web/core/components/pages/pages-list-view.tsxapps/web/core/services/page/project-page.service.tsapps/web/core/store/pages/project-page.store.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Use ESLint with shared config across packages, adhering to max warnings limits per package
Use camelCase for variable and function names, PascalCase for components and types
Use try-catch with proper error types and log errors appropriately
Files:
apps/web/core/components/pages/pages-list-main-content.tsxapps/web/core/components/pages/list/loader.tsxapps/web/core/components/pages/list/root.tsxapps/web/core/components/pages/pages-list-view.tsxapps/web/core/services/page/project-page.service.tsapps/web/core/store/pages/project-page.store.ts
🧠 Learnings (2)
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/core/components/pages/list/root.tsx
📚 Learning: 2025-12-23T14:18:32.899Z
Learnt from: dheeru0198
Repo: makeplane/plane PR: 8339
File: apps/api/plane/db/models/api.py:35-35
Timestamp: 2025-12-23T14:18:32.899Z
Learning: Django REST Framework rate limit strings are flexible: only the first character of the time unit matters. Acceptable formats include: "60/s", "60/sec", "60/second" (all equivalent), "60/m", "60/min", "60/minute" (all equivalent), "60/h", "60/hr", "60/hour" (all equivalent), and "60/d", "60/day" (all equivalent). Abbreviations like "min" are valid and do not need to be changed to "minute". Apply this guidance to any Python files in the project that configure DRF throttling rules.
Applied to files:
apps/api/plane/utils/page_filters.pyapps/api/plane/app/views/page/base.py
🧬 Code graph analysis (5)
apps/web/core/components/pages/pages-list-main-content.tsx (1)
apps/web/core/components/pages/loaders/page-loader.tsx (1)
PageLoader(4-31)
apps/web/core/components/pages/list/loader.tsx (1)
apps/web/core/components/pages/loaders/page-loader.tsx (1)
PageLoader(4-31)
apps/web/core/components/pages/list/root.tsx (4)
apps/space/app/compat/next/navigation.ts (1)
useParams(34-36)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)apps/web/core/components/pages/list/block.tsx (1)
PageListBlock(21-54)apps/web/core/components/pages/list/loader.tsx (1)
PageListLoader(3-15)
apps/web/core/components/pages/pages-list-view.tsx (1)
apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
apps/web/core/store/pages/project-page.store.ts (1)
packages/types/src/page/core.ts (3)
TPageNavigationTabs(28-28)TPageFilters(41-46)TPage(5-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Lint API
- GitHub Check: Build packages
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Agent
- GitHub Check: Analyze (javascript)
🔇 Additional comments (7)
apps/web/core/services/page/project-page.service.ts (1)
23-40: LGTM on the paginated fetch implementation.The updated
fetchAllmethod correctly supports query parameters for pagination and filtering. The type definitionTPagePaginatedResponseproperly combines pagination info with results.One minor note: the
configparameter has an implicitanytype. Consider adding an explicit type annotation for stricter typing, though this follows the existing pattern in the codebase.apps/api/plane/app/views/page/base.py (1)
286-314: List method pagination implementation looks well-structured.The filtering, ordering, and pagination flow is logically organized. The
paginatehelper withdefault_per_page=20andmax_per_page=100provides sensible defaults and limits.apps/web/core/components/pages/pages-list-main-content.tsx (1)
159-162: Good addition for handling mutation loading state.This correctly shows a loading indicator when filters are being applied and no results are available yet, preventing a flash of the empty state before results arrive.
apps/web/core/components/pages/list/root.tsx (1)
46-57: Well-implemented pagination fetch with proper guard conditions.The
fetchNextPagecallback correctly guards against missing params, absence of next page, and concurrent fetches. The dependency array is complete.apps/web/core/store/pages/project-page.store.ts (3)
26-40: Well-designed pagination state types.The
TPagePaginationInfotype andDEFAULT_PAGINATION_INFOconstant provide a clean structure for managing cursor-based pagination state. The per-page count of 20 aligns with the backend'sdefault_per_page.
182-207: Comprehensive filter detection logic.The
hasActiveFilterscomputed property thoroughly checks search query and various filter types (arrays, booleans, etc.). This enables proper UI state management for filter indicators.
299-384: Robust pagination implementation with proper state guards.The
fetchPagesListmethod correctly:
- Guards against concurrent fetches and invalid pagination states
- Builds query parameters from filter state
- Differentiates between first-page and subsequent-page fetches
- Resets pagination state on first page load
The backward-compatible response parsing (lines 396-406) is a good defensive pattern.
There was a problem hiding this comment.
Pull request overview
This PR implements backend-driven pagination, filtering, and ordering for project pages, moving the logic from client-side to server-side for improved scalability and consistency. The implementation introduces cursor-based pagination with infinite scroll support and centralizes filtering logic in reusable backend utilities.
Key Changes:
- Implemented cursor-based pagination with backend filtering and ordering for page lists
- Added comprehensive page filter utilities (search, favorites, created_by, created_at) on the backend
- Updated frontend store and components to support paginated fetching with infinite scroll
- Changed default sort order from "updated_at" to "created_at"
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/core/store/pages/project-page.store.ts | Adds pagination state management, filtered page ID arrays per tab, and refactors fetchPagesList to support cursor-based pagination with backend filtering |
| apps/web/core/services/page/project-page.service.ts | Updates fetchAll method signature to accept query parameters and return paginated response type |
| apps/web/core/components/pages/pages-list-view.tsx | Modifies SWR key to include filter/sort parameters for proper cache invalidation |
| apps/web/core/components/pages/pages-list-main-content.tsx | Adds additional loader check for mutation-loader state with empty results |
| apps/web/core/components/pages/list/root.tsx | Implements infinite scroll with intersection observer for loading paginated results |
| apps/web/core/components/pages/list/loader.tsx | New loader component for pagination loading states |
| apps/api/plane/utils/page_filters.py | New utility module providing reusable filter functions for pages (search, favorites, created_by, created_at) |
| apps/api/plane/app/views/page/base.py | Integrates page filters and pagination in the list endpoint, removes duplicate order_by calls from queryset |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else: | ||
| page_filter[f"{date_term}__lte"] = date_query[0] | ||
| else: | ||
| page_filter[f"{date_term}__contains"] = date_query[0] |
There was a problem hiding this comment.
Missing return statement in the date filter logic. The function modifies page_filter in place but doesn't explicitly return it, though Python returns None implicitly. For consistency with other filter functions (filter_created_by, filter_created_at, filter_favorites, filter_search) which all return page_filter, this function should also return page_filter.
| page_filter[f"{date_term}__contains"] = date_query[0] | |
| page_filter[f"{date_term}__contains"] = date_query[0] | |
| return page_filter |
| paginationData = { | ||
| nextCursor: response.next_cursor || null, | ||
| hasNextPage: response.next_page_results ?? false, | ||
| totalResults: response.total_results ?? 0, | ||
| }; |
There was a problem hiding this comment.
Type mismatch with TPaginationInfo. According to the TPaginationInfo type definition, next_cursor is a non-nullable string, but the code treats it as potentially falsy (using || null). The local TPagePaginationInfo type defines nextCursor as "string | null" which conflicts with the backend type. Either the backend type definition needs to be updated to allow null, or the frontend should handle empty strings differently.
| import { useParams } from "next/navigation"; | ||
| // types | ||
| import type { TPageNavigationTabs } from "@plane/types"; | ||
| import { Loader } from "@plane/ui"; |
There was a problem hiding this comment.
Unused import Loader.
| import { Loader } from "@plane/ui"; |
im-vipin
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/web/core/components/pages/list/root.tsx (1)
6-6: Unused import:LoaderThe
Loadercomponent is imported but never used in this file.PageListLoaderis used instead.Suggested fix
-import { Loader } from "@plane/ui";apps/web/core/components/pages/pages-list-main-content.tsx (1)
159-161: Mutation loader guard is reasonable, but verify error handling.This condition shows a loader during filter/sort mutations when no filtered results exist yet. The store implementation clears
loaderon error (line 392 in store), so this shouldn't show a loader indefinitely on error. However, consider adding defensive error state handling if the store gains error states that could coexist withmutation-loader.apps/web/core/store/pages/project-page.store.ts (1)
96-96: Default sort key changed fromupdated_attocreated_at.This changes the default ordering for all users. Existing users accustomed to seeing recently updated pages first will now see pages ordered by creation date. Consider whether this is intentional or if backward compatibility should be preserved.
🧹 Nitpick comments (2)
apps/web/core/store/pages/project-page.store.ts (2)
371-376: Consider deduplicating page IDs when appending paginated results.If the backend returns overlapping results (e.g., due to data changes between fetches or cursor edge cases), duplicate page IDs could appear in
filteredPageIds, causing duplicate React keys and redundant renders.Suggested fix using Set for deduplication
// Update filtered page IDs - this.filteredPageIds = isFirstPage ? responsePageIds : [...this.filteredPageIds, ...responsePageIds]; + this.filteredPageIds = isFirstPage + ? responsePageIds + : Array.from(new Set([...this.filteredPageIds, ...responsePageIds]));
298-303: Redundant null check fornextCursor.At line 266, you already verified
this.paginationInfo?.nextCursorexists before reaching this point (sinceisFirstPageis false). The innerif (nextCursor)check at line 300 is redundant.Simplified version
// Add pagination cursor if (!isFirstPage) { - const nextCursor = this.paginationInfo.nextCursor; - if (nextCursor) { - queries.cursor = nextCursor; - } + queries.cursor = this.paginationInfo.nextCursor!; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/core/components/pages/list/root.tsxapps/web/core/components/pages/pages-list-main-content.tsxapps/web/core/store/pages/project-page.store.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/core/store/pages/project-page.store.tsapps/web/core/components/pages/pages-list-main-content.tsxapps/web/core/components/pages/list/root.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Enable TypeScript strict mode and ensure all files are fully typed
Files:
apps/web/core/store/pages/project-page.store.tsapps/web/core/components/pages/pages-list-main-content.tsxapps/web/core/components/pages/list/root.tsx
**/*.{js,jsx,ts,tsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with Tailwind plugin for code formatting, run
pnpm fix:format
Files:
apps/web/core/store/pages/project-page.store.tsapps/web/core/components/pages/pages-list-main-content.tsxapps/web/core/components/pages/list/root.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Use ESLint with shared config across packages, adhering to max warnings limits per package
Use camelCase for variable and function names, PascalCase for components and types
Use try-catch with proper error types and log errors appropriately
Files:
apps/web/core/store/pages/project-page.store.tsapps/web/core/components/pages/pages-list-main-content.tsxapps/web/core/components/pages/list/root.tsx
🧠 Learnings (2)
📚 Learning: 2025-06-18T09:46:08.566Z
Learnt from: prateekshourya29
Repo: makeplane/plane PR: 7188
File: web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(list)/header.tsx:40-45
Timestamp: 2025-06-18T09:46:08.566Z
Learning: When reviewing breadcrumb components that accept projectId or similar props, check if empty strings are being passed during loading states, which can result in invalid URLs. The preferred approach is to handle these loading states internally within the component rather than requiring each consumer to manage the loading logic.
Applied to files:
apps/web/core/components/pages/pages-list-main-content.tsx
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/core/components/pages/list/root.tsx
🧬 Code graph analysis (3)
apps/web/core/store/pages/project-page.store.ts (1)
packages/types/src/page/core.ts (3)
TPageNavigationTabs(28-28)TPageFilters(41-46)TPage(5-25)
apps/web/core/components/pages/pages-list-main-content.tsx (1)
apps/web/core/components/pages/loaders/page-loader.tsx (1)
PageLoader(4-31)
apps/web/core/components/pages/list/root.tsx (4)
apps/space/app/compat/next/navigation.ts (1)
useParams(34-36)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)apps/web/core/components/pages/list/block.tsx (1)
PageListBlock(21-54)apps/web/core/components/pages/list/loader.tsx (1)
PageListLoader(3-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check:types
- GitHub Check: check:lint
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
apps/web/core/components/pages/list/root.tsx (1)
67-74: Intersection observer pattern looks correct.The implementation correctly renders the intersection target
divwheneverhasNextPageis true, ensuring the observer can detect when the user scrolls near the bottom. ThePageListLoaderis conditionally shown only during fetch. This addresses the infinite scroll trigger concern from the past review since thedivwithrefis always present when there are more pages.apps/web/core/components/pages/pages-list-main-content.tsx (1)
36-46: Store hook usage updated correctly.The destructured values align with the new store interface (
isAnyPageAvailable,getCurrentProjectPageIdsByTab,getFilteredPageIds,loader), andgetFilteredPageIds()is correctly called without arguments per the new signature.apps/web/core/store/pages/project-page.store.ts (4)
165-190:hasActiveFilterscomputed getter is well-implemented.The logic correctly identifies active filters by checking search query presence and iterating over filter values with appropriate type handling for arrays, booleans, and other values.
261-274: Guard clauses for pagination state are thorough.The early returns prevent redundant fetches when there's no next page, no cursor, or an ongoing pagination request. This is good defensive programming for infinite scroll scenarios.
344-354: Backward compatibility for non-paginated responses is handled.The code gracefully handles both array responses (legacy) and paginated response objects, ensuring smooth migration. The
next_cursor || nullandnext_page_results ?? falsefallbacks are appropriate for type safety.
218-223:getFilteredPageIdsimplementation is minimal but effective.The computed function simply returns the stored
filteredPageIds, which is populated byfetchPagesList. This aligns with the backend-driven filtering approach where the store trusts the API response ordering.
im-vipin
force-pushed
the
feat-pagination_filter_orderby_project_pages
branch
from
35e6d9c to
5093e03
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/api/plane/app/views/page/base.py (1)
302-309: Validatepage_typeparameter against allowed values.The
page_typeparameter is taken from the request without validation. While a default of "public" is provided, invalid values (e.g., "foo") will fall through to the "public" case, potentially causing confusion or unexpected behavior.🔎 Proposed fix
+ ALLOWED_PAGE_TYPES = ["public", "private", "archived"] + # Filter by page type (public, private, archived) page_type = request.GET.get("type", "public") + if page_type not in ALLOWED_PAGE_TYPES: + page_type = "public" + if page_type == "private": queryset = queryset.filter(access=1, archived_at__isnull=True) elif page_type == "archived": queryset = queryset.filter(archived_at__isnull=False) elif page_type == "public": queryset = queryset.filter(access=0, archived_at__isnull=True)apps/web/core/store/pages/project-page.store.ts (1)
333-337: Consider edge cases in sorting parameter handling.The code checks
if (this.filters.sortKey && this.filters.sortBy)but doesn't handle cases where only one is set. While the filters are initialized with both values, a partial update could leave them inconsistent.🔎 Suggested enhancement
// Add sorting parameters // Convert sortKey and sortBy to Django order_by format - if (this.filters.sortKey && this.filters.sortBy) { + const sortKey = this.filters.sortKey || "created_at"; + const sortBy = this.filters.sortBy || "desc"; + if (sortKey && sortBy) { - const orderByField = this.filters.sortBy === "desc" ? `-${this.filters.sortKey}` : this.filters.sortKey; + const orderByField = sortBy === "desc" ? `-${sortKey}` : sortKey; queries.order_by = orderByField; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api/plane/app/views/page/base.pyapps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/layout.tsxapps/web/core/store/pages/project-page.store.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/layout.tsxapps/web/core/store/pages/project-page.store.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Enable TypeScript strict mode and ensure all files are fully typed
Files:
apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/layout.tsxapps/web/core/store/pages/project-page.store.ts
**/*.{js,jsx,ts,tsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with Tailwind plugin for code formatting, run
pnpm fix:format
Files:
apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/layout.tsxapps/web/core/store/pages/project-page.store.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Use ESLint with shared config across packages, adhering to max warnings limits per package
Use camelCase for variable and function names, PascalCase for components and types
Use try-catch with proper error types and log errors appropriately
Files:
apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/layout.tsxapps/web/core/store/pages/project-page.store.ts
🧠 Learnings (3)
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/layout.tsx
📚 Learning: 2025-12-23T14:18:32.899Z
Learnt from: dheeru0198
Repo: makeplane/plane PR: 8339
File: apps/api/plane/db/models/api.py:35-35
Timestamp: 2025-12-23T14:18:32.899Z
Learning: Django REST Framework rate limit strings are flexible: only the first character of the time unit matters. Acceptable formats include: "60/s", "60/sec", "60/second" (all equivalent), "60/m", "60/min", "60/minute" (all equivalent), "60/h", "60/hr", "60/hour" (all equivalent), and "60/d", "60/day" (all equivalent). Abbreviations like "min" are valid and do not need to be changed to "minute". Apply this guidance to any Python files in the project that configure DRF throttling rules.
Applied to files:
apps/api/plane/app/views/page/base.py
📚 Learning: 2025-06-16T07:23:39.497Z
Learnt from: vamsikrishnamathala
Repo: makeplane/plane PR: 7214
File: web/core/store/issue/helpers/base-issues.store.ts:117-117
Timestamp: 2025-06-16T07:23:39.497Z
Learning: In the updateIssueDates method of BaseIssuesStore (web/core/store/issue/helpers/base-issues.store.ts), the projectId parameter is intentionally made optional to support override implementations in subclasses. The base implementation requires projectId and includes an early return check, but making it optional allows derived classes to override the method with different parameter requirements.
Applied to files:
apps/web/core/store/pages/project-page.store.ts
🧬 Code graph analysis (1)
apps/web/core/store/pages/project-page.store.ts (1)
packages/types/src/page/core.ts (3)
TPageNavigationTabs(28-28)TPageFilters(41-46)TPage(5-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check:lint
- GitHub Check: check:types
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/web/core/store/pages/project-page.store.ts (1)
328-330: No action required—axios automatically handles URL encoding.Query parameters passed via the
paramsoption in axios (as is done here) are automatically URL-encoded by the library. This is the correct and standard approach; explicit encoding is neither needed nor recommended, as it would result in double-encoding and actually create problems. The code pattern is consistent with other query parameter usage throughout the service layer.Likely an incorrect or invalid review comment.
im-vipin
force-pushed
the
feat-pagination_filter_orderby_project_pages
branch
from
f5413a2 to
622290c
Compare
im-vipin
force-pushed
the
feat-pagination_filter_orderby_project_pages
branch
from
e9b3f0e to
520a48c
Compare
Description
This PR adds backend-driven pagination, filtering, and ordering for project pages to ensure consistent and scalable page listing.
order_bylogic fully to the backendType of Change
Screenshots and Media (if applicable)
Screen.Recording.2025-12-29.at.5.14.02.PM.mov
Summary by CodeRabbit
New Features
Bug Fixes / Stability
✏️ Tip: You can customize this high-level summary in your review settings.