[WEB-2115] chore: implemented global paginator and handled project issues pagination v1 #5432
Conversation
WalkthroughThe updates introduce a new paginated endpoint for retrieving issues associated with specific projects in the API. A new view set, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API
participant IssuePaginatedViewSet
participant Database
User->>API: Request paginated issues
API->>IssuePaginatedViewSet: Forward request
IssuePaginatedViewSet->>Database: Query for issues
Database-->>IssuePaginatedViewSet: Return issues
IssuePaginatedViewSet-->>API: Return paginated response
API-->>User: Send paginated issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://github.com/avatars/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
apiserver/plane/utils/global_paginator.py (1)
28-78: Consider adding comments for clarity.The function
paginateis correctly implemented but could benefit from additional comments to explain the logic, especially around cursor validation, index calculation, and pagination info creation.Consider adding comments like this:
def paginate(base_queryset, queryset, cursor, on_result): # Validate cursor if cursor is None: cursor_object = PaginateCursor(PAGINATOR_MAX_LIMIT, 0, 0) else: cursor_object = PaginateCursor.from_string(cursor) # Get the total number of results total_results = base_queryset.count() page_size = min(cursor_object.current_page_size, PAGINATOR_MAX_LIMIT) # Calculate start and end indices for paginated data start_index = 0 if cursor_object.current_page > 0: start_index = cursor_object.current_page * page_size end_index = min(start_index + page_size, total_results) # Retrieve paginated data paginated_data = queryset[start_index:end_index] # Create pagination info prev_cursor = f"{page_size}:{cursor_object.current_page-1}:0" cursor = f"{page_size}:{cursor_object.current_page}:0" next_cursor = None if end_index < total_results: next_cursor = f"{page_size}:{cursor_object.current_page+1}:0" prev_page_results = cursor_object.current_page > 0 next_page_results = next_cursor is not None if on_result: paginated_data = on_result(paginated_data) # Return paginated data and pagination info return { "prev_cursor": prev_cursor, "cursor": cursor, "next_cursor": next_cursor, "prev_page_results": prev_page_results, "next_page_results": next_page_results, "page_count": len(paginated_data), "total_results": total_results, "results": paginated_data, }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- apiserver/plane/app/urls/issue.py (3 hunks)
- apiserver/plane/app/views/init.py (1 hunks)
- apiserver/plane/app/views/issue/base.py (4 hunks)
- apiserver/plane/utils/global_paginator.py (1 hunks)
Additional context used
Ruff
apiserver/plane/app/views/__init__.py
115-115:
.issue.base.IssuePaginatedViewSetimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
Additional comments not posted (6)
apiserver/plane/utils/global_paginator.py (2)
1-2: LGTM!The constant
PAGINATOR_MAX_LIMITis correctly defined.The code changes are approved.
5-25: LGTM!The class
PaginateCursoris correctly defined and provides necessary functionality for pagination cursor management.The code changes are approved.
apiserver/plane/app/urls/issue.py (1)
42-47: LGTM!The new endpoint for paginated issue retrieval is correctly defined and enhances the functionality of the API by providing a more efficient way to retrieve large sets of issues.
The code changes are approved.
apiserver/plane/app/views/issue/base.py (3)
656-690: Ensure the query is optimized.The
get_querysetmethod constructs a complex query to retrieve issue data. Ensure that the query is optimized for performance, especially when dealing with large datasets.Use Django's
QuerySet.explainmethod to analyze the query execution plan and identify potential performance bottlenecks.
692-701: LGTM!The
process_paginated_resultmethod correctly processes paginated data and converts datetime fields to the user's timezone.The code changes are approved.
703-791: Ensure proper error handling for pagination.The
listmethod handles GET requests to retrieve a list of issues with cursor-based pagination. Ensure that proper error handling is in place for potential issues such as invalid cursor values or database errors.Test the pagination functionality with various edge cases to ensure robustness.
gurusainath
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
apiserver/plane/app/views/issue/base.py (2)
657-690: Add comments to improve readability.The
get_querysetmethod is correctly implemented but would benefit from additional comments to explain the logic and purpose of each step.Consider adding comments like this:
def get_queryset(self): workspace_slug = self.kwargs.get("slug") project_id = self.kwargs.get("project_id") + # Construct the base queryset with necessary filters return ( Issue.issue_objects.filter( workspace__slug=workspace_slug, project_id=project_id ) .select_related("workspace", "project", "state", "parent") .prefetch_related("assignees", "labels", "issue_module__module") .annotate(cycle_id=F("issue_cycle__cycle_id")) .annotate( link_count=IssueLink.objects.filter(issue=OuterRef("id")) .order_by() .annotate(count=Func(F("id"), function="Count")) .values("count") ) .annotate( attachment_count=IssueAttachment.objects.filter( issue=OuterRef("id") ) .order_by() .annotate(count=Func(F("id"), function="Count")) .values("count") ) .annotate( sub_issues_count=Issue.issue_objects.filter( parent=OuterRef("id") ) .order_by() .annotate(count=Func(F("id"), function="Count")) .values("count") ) ).distinct()
703-791: Add comments to improve readability.The
listmethod is correctly implemented but would benefit from additional comments to explain the logic and purpose of each step.Consider adding comments like this:
@allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.VIEWER, ROLE.GUEST]) def list(self, request, slug, project_id): cursor = request.GET.get("cursor", None) is_description_required = request.GET.get("description", False) updated_at = request.GET.get("updated_at", False) + # Define the required fields for the response required_fields = [ "id", "name", "state_id", "sort_order", "completed_at", "estimate_point", "priority", "start_date", "target_date", "sequence_id", "project_id", "parent_id", "cycle_id", "created_at", "updated_at", "created_by", "updated_by", "is_draft", "archived_at", "deleted_at", "module_ids", "label_ids", "assignee_ids", "link_count", "attachment_count", "sub_issues_count", ] if is_description_required: required_fields.append("description_html") + # Query issues based on workspace slug and project ID base_queryset = Issue.issue_objects.filter( workspace__slug=slug, project_id=project_id ).order_by("updated_at") queryset = self.get_queryset().order_by("updated_at") + # Filter issues by updated_at if provided if updated_at: base_queryset = base_queryset.filter(updated_at__gte=updated_at) queryset = queryset.filter(updated_at__gte=updated_at) + # Annotate the queryset with additional fields queryset = queryset.annotate( label_ids=Coalesce( ArrayAgg( "labels__id", distinct=True, filter=~Q(labels__id__isnull=True), ), Value([], output_field=ArrayField(UUIDField())), ), assignee_ids=Coalesce( ArrayAgg( "assignees__id", distinct=True, filter=~Q(assignees__id__isnull=True) & Q(assignees__member_project__is_active=True), ), Value([], output_field=ArrayField(UUIDField())), ), module_ids=Coalesce( ArrayAgg( "issue_module__module_id", distinct=True, filter=~Q(issue_module__module_id__isnull=True) & Q(issue_module__module__archived_at__isnull=True), ), Value([], output_field=ArrayField(UUIDField())), ), ) + # Paginate the data and process the results paginated_data = paginate( base_queryset=base_queryset, queryset=queryset, cursor=cursor, on_result=lambda results: self.process_paginated_result( required_fields, results, request.user.user_timezone ), ) return Response(paginated_data, status=status.HTTP_200_OK)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- apiserver/plane/app/urls/issue.py (3 hunks)
- apiserver/plane/app/views/issue/base.py (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- apiserver/plane/app/urls/issue.py
Additional comments not posted (1)
apiserver/plane/app/views/issue/base.py (1)
692-701: LGTM!The
process_paginated_resultmethod is correctly implemented.The code changes are approved.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apiserver/plane/app/views/issue/base.py (4 hunks)
Additional comments not posted (3)
apiserver/plane/app/views/issue/base.py (3)
657-690: LGTM!The method is correctly implemented with appropriate ORM optimizations.
The code changes are approved.
692-701: LGTM!The method is correctly implemented and enhances usability for clients interacting with the API.
The code changes are approved.
703-791: LGTM!The method is correctly implemented and enhances the API's capability to manage large datasets efficiently.
The code changes are approved.
Summary
Plane Issue:
[WEB-2115]
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes