Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WEB-2937] feat: home recent activies list endpoint #6295

Merged
merged 18 commits into from
Jan 3, 2025

Conversation

sangeethailango
Copy link
Collaborator

@sangeethailango sangeethailango commented Dec 31, 2024

Description

This PR contains the list API endpoint to list recent activities in pages, projects, and issues of a workspace.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Improvement (change that would cause existing functionality to not work as expected)
  • Code refactoring
  • Performance improvements
  • Documentation update

Screenshots and Media (if applicable)

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Added workspace quick links management.
    • Introduced user recent visits tracking for workspaces, issues, projects, and pages.
    • Enhanced serialization for workspace-related entities.
    • Added new view sets for managing quick links and recent visits.
  • Improvements

    • Added URL validation for workspace user links.
    • Implemented dynamic entity model and serializer retrieval.
    • Expanded functionality with new serializers for recent visits and quick links.

Copy link
Contributor

coderabbitai bot commented Dec 31, 2024

Walkthrough

This pull request introduces new serializers and view sets for workspace-related functionalities, focusing on quick links and recent visits. The changes expand the API capabilities by adding endpoints for managing workspace user links, tracking recent visits to issues, projects, and pages, and providing methods to retrieve and interact with these entities. The modifications enhance the workspace management system by introducing more granular tracking and interaction features.

Changes

File Change Summary
apiserver/plane/app/serializers/__init__.py Added imports for WorkspaceUserLinkSerializer and WorkspaceRecentVisitSerializer
apiserver/plane/app/serializers/workspace.py Introduced multiple new serializers:
- WorkspaceUserLinkSerializer for URL validation
- IssueRecentVisitSerializer
- ProjectRecentVisitSerializer
- PageRecentVisitSerializer
- WorkspaceRecentVisitSerializer
apiserver/plane/app/urls/workspace.py Added paths for:
- Quick links management
- Recent visits listing
apiserver/plane/app/views/__init__.py Imported UserRecentVisitViewSet and QuickLinkViewSet
apiserver/plane/app/views/workspace/recent_visit.py Created UserRecentVisitViewSet for handling recent visits
apiserver/plane/app/serializers/favorite.py Added get_entity_model_and_serializer method

Possibly related PRs

Suggested labels

🌟enhancement, 🌐frontend

Suggested reviewers

  • sriramveeraghanta
  • pablohashescobar

Poem

🐰 Hop, hop, through workspace's lane,
Quick links and visits, a digital refrain!
Serializers dance, views take flight,
Tracking journeys with rabbit's delight!
Code grows smarter, connections bloom 🌈


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@sangeethailango sangeethailango marked this pull request as draft December 31, 2024 12:14
@sangeethailango sangeethailango changed the title [WEB-2937] - feat: home recent activies list endpoint [WEB-2937] feat: home recent activies list endpoint Dec 31, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (6)
apiserver/plane/app/serializers/workspace.py (3)

3-4: Remove unused imports.

These imports appear unused and can safely be removed to keep the codebase clean and aligned with best practices.

- from rest_framework import status
- from rest_framework.response import Response
🧰 Tools
🪛 Ruff (0.8.2)

3-3: rest_framework.status imported but unused

Remove unused import: rest_framework.status

(F401)


4-4: rest_framework.response.Response imported but unused

Remove unused import: rest_framework.response.Response

(F401)


155-167: ProjectRecentVisitSerializer effectively gathers member data.

Retrieving project members with get_project_members is intuitive. Consider verifying performance on large projects.


181-188: get_entity_model_and_serializer is practical.

Defining this lookup map is a neat way to centralize entity-type handling. If new entity types are frequently added, consider a more scalable approach or a registry.

apiserver/plane/app/views/workspace/quick_link.py (1)

29-37: Line-length warnings from static analysis.

Lines exceed 88 characters. If your style guidelines enforce strict line-length limits, consider wrapping them.

- quick_link = WorkspaceUserLink.objects.filter(pk=pk, workspace__slug=slug).first()
+ quick_link = WorkspaceUserLink.objects.filter(
+     pk=pk, 
+     workspace__slug=slug
+ ).first()

Also applies to: 56-61

🧰 Tools
🪛 Ruff (0.8.2)

29-29: Line too long (90 > 88)

(E501)


32-32: Line too long (97 > 88)

(E501)


36-36: Line too long (90 > 88)

(E501)


37-37: Line too long (94 > 88)

(E501)

apiserver/plane/app/urls/workspace.py (2)

220-224: Remove trailing space from the 'name' attribute.
There is an unintended trailing space in the name="workspace-quick-links " attribute on line 223, which might cause confusion.

-        name="workspace-quick-links "
+        name="workspace-quick-links"

225-229: Line exceeds maximum recommended length (E501).
Static analysis points out that line 227 is longer than 88 characters. Splitting the dictionary into multiple lines helps readability and avoids lint warnings.

-        QuickLinkViewSet.as_view({"get": "retrieve", "patch": "partial_update", "delete": "destroy"}), 
+        QuickLinkViewSet.as_view({
+            "get": "retrieve",
+            "patch": "partial_update",
+            "delete": "destroy"
+        }),
🧰 Tools
🪛 Ruff (0.8.2)

227-227: Line too long (103 > 88)

(E501)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c7ebaf6 and bf36eb0.

📒 Files selected for processing (6)
  • apiserver/plane/app/serializers/__init__.py (1 hunks)
  • apiserver/plane/app/serializers/workspace.py (3 hunks)
  • apiserver/plane/app/urls/workspace.py (2 hunks)
  • apiserver/plane/app/views/__init__.py (2 hunks)
  • apiserver/plane/app/views/workspace/quick_link.py (1 hunks)
  • apiserver/plane/app/views/workspace/recent_visit.py (1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/app/urls/workspace.py

227-227: Line too long (103 > 88)

(E501)

apiserver/plane/app/serializers/workspace.py

3-3: rest_framework.status imported but unused

Remove unused import: rest_framework.status

(F401)


4-4: rest_framework.response.Response imported but unused

Remove unused import: rest_framework.response.Response

(F401)

apiserver/plane/app/views/workspace/quick_link.py

29-29: Line too long (90 > 88)

(E501)


32-32: Line too long (97 > 88)

(E501)


36-36: Line too long (90 > 88)

(E501)


37-37: Line too long (94 > 88)

(E501)


56-56: Line too long (90 > 88)

(E501)


61-61: Line too long (94 > 88)

(E501)

🔇 Additional comments (13)
apiserver/plane/app/serializers/workspace.py (5)

121-142: New WorkspaceUserLinkSerializer class looks solid.

The design aligns well with BaseSerializer. Manually prefixing "http://" when no scheme is provided is a reasonable approach. Ensure that this meets the team’s requirements, especially for enforcing HTTPS in production if needed.


143-147: IssueRecentVisitSerializer is straightforward.

This serializer simply exposes the required fields. Everything looks good here.


148-154: ProjectMemberSerializer is clear.

It wraps a UserLiteSerializer for the member detail. This is adequately structured.


168-180: PageRecentVisitSerializer approach is fine.

Deriving project_id from the first associated project is acceptable. Confirm that a single page cannot belong to multiple critical projects simultaneously if that’s a concern.


189-216: WorkspaceRecentVisitSerializer guards against missing entities well.

Gracefully returning None for nonexistent entities is user-friendly. Confirm if additional logging is needed when an entity is missing.

apiserver/plane/app/views/workspace/recent_visit.py (2)

1-3: Imports used for responses and statuses are valid.

No issues here. The usage at line 23 is appropriate.


19-23: Validate listing all recent visits.

Currently, this fetches every recent visit for the workspace. If needed, consider filtering by the requesting user or other criteria. Otherwise, this is functionally correct.

apiserver/plane/app/serializers/__init__.py (1)

22-23: Added imports for new serializers look good.

These new serializers (WorkspaceUserLinkSerializer, WorkspaceRecentVisitSerializer) integrate seamlessly into the package.

apiserver/plane/app/views/workspace/quick_link.py (2)

1-4: Imports appear necessary and are in use.

They are used for HTTP status codes, responses, and model/serializer references.


11-68: Overall CRUD logic is concise and consistent.

  • create properly validates and saves the new link.
  • partial_update gracefully handles updates with robust error responses.
  • retrieve is straightforward and includes a try-except for 404 handling.
  • destroy checks existence first, then deletes.
  • list fetches all quick links in the workspace.
🧰 Tools
🪛 Ruff (0.8.2)

29-29: Line too long (90 > 88)

(E501)


32-32: Line too long (97 > 88)

(E501)


36-36: Line too long (90 > 88)

(E501)


37-37: Line too long (94 > 88)

(E501)


56-56: Line too long (90 > 88)

(E501)


61-61: Line too long (94 > 88)

(E501)

apiserver/plane/app/views/__init__.py (1)

48-48: New view sets imported successfully.

UserRecentVisitViewSet and QuickLinkViewSet are clearly identified and integrated.

Also applies to: 76-76

apiserver/plane/app/urls/workspace.py (2)

218-219: No issues found in this comment block.
These lines merely add a succinct comment for "quick link" functionality, which helps clarify intent.


230-234: Recent visits endpoint looks good.
The read-only endpoint for listing recent visits is aptly named and clearly defined.

@sangeethailango sangeethailango marked this pull request as ready for review December 31, 2024 12:44
@sangeethailango sangeethailango self-assigned this Dec 31, 2024
@sangeethailango sangeethailango marked this pull request as draft December 31, 2024 12:48
@sangeethailango sangeethailango marked this pull request as ready for review December 31, 2024 14:52
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
apiserver/plane/app/urls/workspace.py (2)

227-231: Fix indentation for consistency.

The indentation of the view methods dictionary is inconsistent with the rest of the file.

Apply this diff to maintain consistent indentation:

     "workspaces/<str:slug>/quick-links/<uuid:pk>/", 
-        QuickLinkViewSet.as_view({
-            "get": "retrieve", 
-            "patch": "partial_update", 
-            "delete": "destroy"
-        }), 
+    QuickLinkViewSet.as_view({
+        "get": "retrieve", 
+        "patch": "partial_update", 
+        "delete": "destroy"
+    }), 

235-237: Consider pagination for recent visits.

Since this endpoint returns a list of recent visits across pages, projects, and issues, it would be beneficial to ensure that pagination is implemented in the view set to handle potentially large result sets efficiently.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3c5c892 and 40a18f7.

📒 Files selected for processing (1)
  • apiserver/plane/app/urls/workspace.py (2 hunks)
🔇 Additional comments (2)
apiserver/plane/app/urls/workspace.py (2)

30-31: LGTM! Clean import additions.

The new view set imports are properly integrated into the existing imports list.


222-223: LGTM! Well-structured RESTful endpoint.

The quick links list endpoint follows RESTful conventions and maintains consistency with other workspace endpoints.

apiserver/plane/app/urls/workspace.py Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (4)
apiserver/plane/app/views/workspace/recent_visit.py (1)

12-24: Consider adding filtering capabilities.

The endpoint could benefit from additional filtering options such as:

  • Date range for visits
  • Specific entity types
  • User-specific visits (for admins)

Consider implementing Django Filter backends:

from django_filters import rest_framework as filters

class UserRecentVisitFilter(filters.FilterSet):
    entity_name = filters.ChoiceFilter(choices=[
        ('issue', 'Issue'),
        ('page', 'Page'),
        ('project', 'Project'),
    ])
    visited_after = filters.DateTimeFilter(field_name="visited_at", lookup_expr="gte")
    visited_before = filters.DateTimeFilter(field_name="visited_at", lookup_expr="lte")

    class Meta:
        model = UserRecentVisit
        fields = ['entity_name', 'visited_after', 'visited_before']
🧰 Tools
🪛 Ruff (0.8.2)

20-20: Line too long (132 > 88)

(E501)

apiserver/plane/app/serializers/workspace.py (3)

3-4: Remove unused imports.

The following imports are not used in this file:

  • rest_framework.status
  • rest_framework.response.Response
🧰 Tools
🪛 Ruff (0.8.2)

3-3: rest_framework.status imported but unused

Remove unused import: rest_framework.status

(F401)


4-4: rest_framework.response.Response imported but unused

Remove unused import: rest_framework.response.Response

(F401)


204-217: Enhance error handling in get_entity_data.

The current implementation silently returns None for any error. Consider:

  1. Adding logging for debugging purposes
  2. Distinguishing between different error cases
  3. Implementing bulk data retrieval for better performance
     def get_entity_data(self, obj):
         entity_name = obj.entity_name
         entity_identifier = obj.entity_identifier
 
         entity_model, entity_serializer = get_entity_model_and_serializer(entity_name)
 
         if entity_model and entity_serializer:
             try:
                 entity = entity_model.objects.get(pk=entity_identifier)
                 return entity_serializer(entity).data
             except entity_model.DoesNotExist:
+                logger.warning(
+                    f"Entity not found: {entity_name} with id {entity_identifier}"
+                )
                 return None
+            except Exception as e:
+                logger.error(
+                    f"Error retrieving entity data: {str(e)}"
+                )
+                return None
         return None

182-188: Consider making entity mapping more maintainable.

The current hardcoded mapping could be made more maintainable and extensible.

Consider moving this to a configuration file or using a registry pattern:

from enum import Enum
from typing import Tuple, Type

class EntityType(Enum):
    ISSUE = "issue"
    PAGE = "page"
    PROJECT = "project"

ENTITY_MAPPING = {
    EntityType.ISSUE: (Issue, IssueRecentVisitSerializer),
    EntityType.PAGE: (Page, PageRecentVisitSerializer),
    EntityType.PROJECT: (Project, ProjectRecentVisitSerializer),
}

def get_entity_model_and_serializer(entity_type: str) -> Tuple[Type, Type]:
    try:
        return ENTITY_MAPPING[EntityType(entity_type)]
    except ValueError:
        return None, None
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 40a18f7 and 6fbb36c.

📒 Files selected for processing (2)
  • apiserver/plane/app/serializers/workspace.py (2 hunks)
  • apiserver/plane/app/views/workspace/recent_visit.py (1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/app/serializers/workspace.py

3-3: rest_framework.status imported but unused

Remove unused import: rest_framework.status

(F401)


4-4: rest_framework.response.Response imported but unused

Remove unused import: rest_framework.response.Response

(F401)

apiserver/plane/app/views/workspace/recent_visit.py

20-20: Line too long (132 > 88)

(E501)

apiserver/plane/app/views/workspace/recent_visit.py Outdated Show resolved Hide resolved
apiserver/plane/app/serializers/workspace.py Outdated Show resolved Hide resolved
apiserver/plane/app/serializers/workspace.py Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (4)
apiserver/plane/app/serializers/workspace.py (4)

3-4: Remove unused imports

The following imports are not used in the code:

  • rest_framework.status
  • rest_framework.response.Response
- from rest_framework import status
- from rest_framework.response import Response
🧰 Tools
🪛 Ruff (0.8.2)

3-3: rest_framework.status imported but unused

Remove unused import: rest_framework.status

(F401)


4-4: rest_framework.response.Response imported but unused

Remove unused import: rest_framework.response.Response

(F401)


Line range hint 127-142: Enhance URL validation and normalization

The URL handling implementation is good but could be improved:

  1. Provide more specific error messages to help users understand what's wrong
  2. Handle edge cases in URL normalization (e.g., multiple schemes, trailing spaces)
     def validate_url(self, value):
         url_validator = URLValidator()
         try:
             url_validator(value)
         except ValidationError:
-            raise serializers.ValidationError({"error": "Invalid URL format."})
+            raise serializers.ValidationError({
+                "error": "Invalid URL format. URL must start with http:// or https:// and contain a valid domain."
+            })
         return value

     def to_internal_value(self, data):
         url = data.get("url", "")
+        url = url.strip()
         if url and not url.startswith(("http://", "https://")):
+            if url.startswith("//"):
+                url = "http:" + url
+            elif url.startswith("www."):
+                url = "http://" + url
+            else:
                 url = "http://" + url
         data["url"] = url
         return super().to_internal_value(data)
🧰 Tools
🪛 Ruff (0.8.2)

149-149: Line too long (126 > 88)

(E501)


183-183: Line too long (93 > 88)

(E501)


219-232: Optimize entity data retrieval

The current implementation makes individual queries for each entity. Consider using bulk operations when retrieving multiple entities.

     def get_entity_data(self, obj):
         entity_name = obj.entity_name
         entity_identifier = obj.entity_identifier
 
         entity_model, entity_serializer = get_entity_model_and_serializer(entity_name)
 
         if entity_model and entity_serializer:
             try:
-                entity = entity_model.objects.get(pk=entity_identifier)
+                # Use select_related or prefetch_related based on the entity relationships
+                entity = entity_model.objects.select_related(
+                    'project',  # for issues
+                    'workspace'  # common relationship
+                ).get(pk=entity_identifier)
 
                 return entity_serializer(entity).data
             except entity_model.DoesNotExist:
                 return None
         return None

149-149: Fix line length violations

The following lines exceed the maximum length of 88 characters:

  • Line 149: Fields list in IssueRecentVisitSerializer
  • Line 183: Fields list in PageRecentVisitSerializer
-        fields = ["name", "state", "priority", "assignees", "type", "sequence_id", "project_id", "project_identifier"]        
+        fields = [
+            "name", "state", "priority", "assignees", "type",
+            "sequence_id", "project_id", "project_identifier"
+        ]

-        fields = ["id", "name", "logo_props", "project_id", "owned_by", "project_identifier"]
+        fields = [
+            "id", "name", "logo_props", "project_id",
+            "owned_by", "project_identifier"
+        ]

Also applies to: 183-183

🧰 Tools
🪛 Ruff (0.8.2)

149-149: Line too long (126 > 88)

(E501)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6fbb36c and 3f70338.

📒 Files selected for processing (2)
  • apiserver/plane/app/serializers/workspace.py (2 hunks)
  • apiserver/plane/app/views/workspace/recent_visit.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apiserver/plane/app/views/workspace/recent_visit.py
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/app/serializers/workspace.py

3-3: rest_framework.status imported but unused

Remove unused import: rest_framework.status

(F401)


4-4: rest_framework.response.Response imported but unused

Remove unused import: rest_framework.response.Response

(F401)


149-149: Line too long (126 > 88)

(E501)


183-183: Line too long (93 > 88)

(E501)

🔇 Additional comments (2)
apiserver/plane/app/serializers/workspace.py (2)

171-175: Optimize database queries in get_project_members

The current implementation could lead to N+1 queries when serializing multiple projects.


185-195: ⚠️ Potential issue

Fix project retrieval logic and optimize queries

The current implementation has several issues:

  1. Potential race condition with first()
  2. Redundant database query
  3. No error handling for missing project
     def get_project_id(self, obj):
-        project = (
-            obj.projects.first()
-        )
-        return project.id if project else None
+        return obj.projects.values_list('id', flat=True).first()
     
     def get_project_identifier(self, obj):
-        project = obj.projects.first()
-
-        project = Project.objects.get(id=project.id)
-        return project.identifier if project else None
+        return obj.projects.values_list('identifier', flat=True).first()

Likely invalid or redundant comment.

apiserver/plane/app/serializers/workspace.py Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
apiserver/plane/app/serializers/workspace.py (3)

3-4: Remove unused imports.

The following imports are not used in this file:

-from rest_framework import status
-from rest_framework.response import Response
🧰 Tools
🪛 Ruff (0.8.2)

3-3: rest_framework.status imported but unused

Remove unused import: rest_framework.status

(F401)


4-4: rest_framework.response.Response imported but unused

Remove unused import: rest_framework.response.Response

(F401)


149-149: Fix line length violations.

The following lines exceed the maximum line length of 88 characters:

  • Line 149: Fields declaration in IssueRecentVisitSerializer
  • Line 171: ProjectMember query in get_project_members
  • Line 182: Fields declaration in PageRecentVisitSerializer

Consider breaking these lines for better readability.

Also applies to: 171-171, 182-182

🧰 Tools
🪛 Ruff (0.8.2)

149-149: Line too long (126 > 88)

(E501)


214-227: Consider adding error logging in get_entity_data.

While the error handling is good, it would be beneficial to log these errors for monitoring and debugging purposes.

    def get_entity_data(self, obj):
        entity_name = obj.entity_name
        entity_identifier = obj.entity_identifier

        entity_model, entity_serializer = get_entity_model_and_serializer(entity_name)

        if entity_model and entity_serializer:
            try:
                entity = entity_model.objects.get(pk=entity_identifier)

                return entity_serializer(entity).data
            except entity_model.DoesNotExist:
+               import logging
+               logger = logging.getLogger(__name__)
+               logger.warning(
+                   f"Entity not found: {entity_name} with id {entity_identifier}"
+               )
                return None
        return None
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bd2b140 and 53d521f.

📒 Files selected for processing (1)
  • apiserver/plane/app/serializers/workspace.py (2 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/app/serializers/workspace.py

3-3: rest_framework.status imported but unused

Remove unused import: rest_framework.status

(F401)


4-4: rest_framework.response.Response imported but unused

Remove unused import: rest_framework.response.Response

(F401)


149-149: Line too long (126 > 88)

(E501)


171-171: Line too long (90 > 88)

(E501)


182-182: Line too long (93 > 88)

(E501)


185-185: Line too long (114 > 88)

(E501)

🔇 Additional comments (3)
apiserver/plane/app/serializers/workspace.py (3)

187-190: Optimize project retrieval in get_project_identifier.

The current implementation could lead to N+1 queries. Consider using a similar optimization as in get_project_id.

    def get_project_identifier(self, obj):
-        project = obj.projects.first()
-
-        return project.identifier if project else None
+        return obj.projects.values_list('identifier', flat=True).first()

192-198: LGTM! Clean and maintainable implementation.

The helper function provides a clear mapping between entity types and their corresponding models/serializers, with proper handling of unknown entity types.


144-227: Verify API response structure meets frontend requirements.

The serializers implement comprehensive support for the workspace recent activities list endpoint. However, it would be good to verify:

  1. The response structure matches frontend expectations
  2. Performance implications of retrieving entity data for large activity lists
  3. Adequate test coverage for different entity types and error scenarios
🧰 Tools
🪛 Ruff (0.8.2)

149-149: Line too long (126 > 88)

(E501)


171-171: Line too long (90 > 88)

(E501)


182-182: Line too long (93 > 88)

(E501)


185-185: Line too long (114 > 88)

(E501)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
apiserver/plane/app/views/workspace/recent_visit.py (1)

29-30: ⚠️ Potential issue

Replace array slicing with proper pagination and ordering.

Using array slicing for pagination is inefficient as it still retrieves all records from the database. Additionally, without ordering, results may be inconsistent.

-            serializer = WorkspaceRecentVisitSerializer(user_recent_visits[:20], many=True)    
-            return Response(serializer.data, status=status.HTTP_200_OK)
+            user_recent_visits = user_recent_visits.order_by("-visited_at")
+            page = self.paginate_queryset(user_recent_visits)
+            serializer = WorkspaceRecentVisitSerializer(page, many=True)
+            return self.get_paginated_response(serializer.data)
🧰 Tools
🪛 Ruff (0.8.2)

29-29: Line too long (95 > 88)

(E501)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 53d521f and 9ebd6f8.

📒 Files selected for processing (1)
  • apiserver/plane/app/views/workspace/recent_visit.py (1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/app/views/workspace/recent_visit.py

27-27: Line too long (102 > 88)

(E501)


29-29: Line too long (95 > 88)

(E501)

🔇 Additional comments (2)
apiserver/plane/app/views/workspace/recent_visit.py (2)

1-14: LGTM! Clean class setup with well-organized imports.

The class is properly structured following Django REST framework conventions with clear model assignment.


15-17: LGTM! Straightforward serializer class selection.

The serializer method is appropriately implemented for the current use case.

@sriramveeraghanta sriramveeraghanta merged commit 870ca17 into preview Jan 3, 2025
12 of 14 checks passed
@sriramveeraghanta sriramveeraghanta deleted the feat-home-recents branch January 3, 2025 09:39
@coderabbitai coderabbitai bot mentioned this pull request Jan 3, 2025
6 tasks
sangeethailango added a commit that referenced this pull request Jan 3, 2025
* Crud for wuick links

* Validate quick link existence

* Add custom method for destroy and retrieve

* Add List method

* Remove print statements

* List all the workspace quick links

* feat: endpoint to get recently active items

* Resolve conflicts

* Resolve conflicts

* Add filter to only list required entities

* Return required fields

* Add filter

* Add filter
sriramveeraghanta added a commit that referenced this pull request Jan 6, 2025
* WIP

* WIP

* WIP

* WIP

* Create home preference if not exist

* chore: handled the unique state name validation (#6299)

* fix: changed the response structure (#6301)

* [WEB-1964]chore: cycles actions restructuring (#6298)

* chore: cycles quick actions restructuring

* chore: added additional actions to cycle list actions

* chore: cycle quick action structure

* chore: added additional actions to cycle list actions

* chore: added end cycle hook

* fix: updated end cycle export

---------

Co-authored-by: gurusinath <[email protected]>

* fix: active cycle graph tooltip and endpoint validation (#6306)

* [WEB-2870]feat: language support (#6215)

* fix: adding language support package

* fix: language support implementation using mobx

* fix: adding more languages for support

* fix: profile settings translations

* feat: added language support for sidebar and user settings

* feat: added language support for deactivation modal

* fix: added project sync after transfer issues (#6200)

* code refactor and improvement (#6203)

* chore: package code refactoring

* chore: component restructuring and refactor

* chore: comment create improvement

* refactor: enhance workspace and project wrapper modularity (#6207)

* [WEB-2678]feat: added functionality to add labels directly from dropdown (#6211)

* enhancement:added functionality to add features directly from dropdown

* fix: fixed import order

* fix: fixed lint errors

* chore: added common component for project activity (#6212)

* chore: added common component for project activity

* fix: added enum

* fix: added enum for initiatives

* - Do not clear temp files that are locked. (#6214)

- Handle edge cases in sync workspace

* fix: labels empty state for drop down (#6216)

* refactor: remove cn helper function from the editor package (#6217)

* * feat: added language support to issue create modal in sidebar
* fix: project activity type

* * fix: added missing translations
* fix: modified translation for plurals

* fix: fixed spanish translation

* dev: language type error in space user profile types

* fix: type fixes

* chore: added alpha tag

---------

Co-authored-by: sriram veeraghanta <[email protected]>
Co-authored-by: Anmol Singh Bhatia <[email protected]>
Co-authored-by: Prateek Shourya <[email protected]>
Co-authored-by: Akshita Goyal <[email protected]>
Co-authored-by: Satish Gandham <[email protected]>
Co-authored-by: Aaryan Khandelwal <[email protected]>
Co-authored-by: gurusinath <[email protected]>

* feat: introduced stacked bar chart and tree map chart. (#6305)

* feat: add issue attachment external endpoint (#6307)

* [PE-97] chore: re-order pages options (#6303)

* chore: re-order pages dropdown options

* chore: re-order pages dropdown options

* fix: remove localdb tracing

* [WEB-2937] feat: home recent activies list endpoint (#6295)

* Crud for wuick links

* Validate quick link existence

* Add custom method for destroy and retrieve

* Add List method

* Remove print statements

* List all the workspace quick links

* feat: endpoint to get recently active items

* Resolve conflicts

* Resolve conflicts

* Add filter to only list required entities

* Return required fields

* Add filter

* Add filter

* fix: remove emoji edit for uneditable pages (#6304)

* Removed duplicate imports

* feat: patch api

* Enable sort order to be updatable

* Return key name
only insert missing keys
use serializer to return data

* Remove random generation of sort_order

* Remove name field
Remove random generation of sort_order

---------

Co-authored-by: Bavisetti Narayan <[email protected]>
Co-authored-by: Vamsi Krishna <[email protected]>
Co-authored-by: gurusinath <[email protected]>
Co-authored-by: Anmol Singh Bhatia <[email protected]>
Co-authored-by: sriram veeraghanta <[email protected]>
Co-authored-by: Prateek Shourya <[email protected]>
Co-authored-by: Akshita Goyal <[email protected]>
Co-authored-by: Satish Gandham <[email protected]>
Co-authored-by: Aaryan Khandelwal <[email protected]>
Co-authored-by: Nikhil <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants