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-2693] chore: removed the deleted cycles from the issue list #5868

Merged
merged 3 commits into from
Oct 18, 2024

Conversation

NarayanBavisetti
Copy link
Collaborator

@NarayanBavisetti NarayanBavisetti commented Oct 18, 2024

chore:

  • removed the deleted cycles from the issue list

Issue Link: WEB-2693

Summary by CodeRabbit

  • New Features

    • Enhanced logic for retrieving valid cycles across various views, ensuring only active cycles are considered.
    • Improved handling of issue priority and state updates in the inbox view.
  • Bug Fixes

    • Resolved potential issues with cycle ID annotations, ensuring consistent behavior across multiple endpoints.
  • Documentation

    • Updated comments and annotations for clarity in the CustomImageBlock component.
  • Chores

    • Cleaned up import statements and refactored existing code for better readability and maintainability.

Copy link
Contributor

coderabbitai bot commented Oct 18, 2024

Walkthrough

The pull request introduces modifications across multiple classes related to issue and cycle management within the API. Key changes involve the implementation of conditional annotations for the cycle_id field using Case statements, ensuring that only valid cycles are considered based on the deleted_at status. Additionally, several method signatures have been updated to reflect these changes, along with enhancements in validation and permission checks in various view sets.

Changes

File Path Change Summary
apiserver/plane/api/views/issue.py Updated get and get_queryset methods in IssueAPIEndpoint for cycle_id annotations using Case.
apiserver/plane/app/views/cycle/issue.py Modified list method in CycleIssueViewSet to include conditional annotations for cycle_id.
apiserver/plane/app/views/inbox/base.py Enhanced get_queryset, create, and partial_update methods in InboxIssueViewSet with new annotations and checks.
apiserver/plane/app/views/issue/archive.py Updated get_queryset in IssueArchiveViewSet for conditional cycle_id annotation.
apiserver/plane/app/views/issue/base.py Replaced direct annotations for cycle_id in IssueListEndpoint, IssueViewSet, and IssuePaginatedViewSet.
apiserver/plane/app/views/issue/relation.py Enhanced list method in IssueRelationViewSet with conditional cycle_id annotations.
apiserver/plane/app/views/issue/sub_issue.py Updated get method in SubIssuesEndpoint for cycle_id annotation.
apiserver/plane/app/views/module/issue.py Modified get_queryset in ModuleIssueViewSet for cycle_id annotation.
apiserver/plane/app/views/view/base.py Updated get_queryset and list methods in WorkspaceViewIssuesViewSet and perform_create in WorkspaceViewViewSet.
apiserver/plane/app/views/workspace/draft.py Modified get_queryset in WorkspaceDraftIssueViewSet for cycle_id annotation.
apiserver/plane/app/views/workspace/user.py Updated get method in WorkspaceUserProfileIssuesEndpoint for cycle_id annotation.
apiserver/plane/space/views/issue.py Updated get methods in ProjectIssuesPublicEndpoint and IssueRetrievePublicEndpoint for cycle_id annotations.
packages/editor/src/core/extensions/custom-image/components/image-block.tsx Commented out src destructuring and updated visibility logic in CustomImageBlock.

Possibly related PRs

Suggested labels

⚙️backend, 🧹chore

Suggested reviewers

  • sriramveeraghanta
  • pablohashescobar

Poem

🐇 In the garden of code, where cycles bloom,
We tidy up issues, dispelling the gloom.
With Case and When, we make things right,
Ensuring our cycles are clear in the light.
So hop along, friends, let’s code and create,
For each little change makes our project first-rate! 🌼


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 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.

@pushya22 pushya22 merged commit db91942 into preview Oct 18, 2024
11 of 12 checks passed
@pushya22 pushya22 deleted the chore/cycle-deletion branch October 18, 2024 10:18
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: 4

🧹 Outside diff range and nitpick comments (10)
apiserver/plane/app/views/workspace/draft.py (1)

Line range hint 60-100: LGTM: Improved queryset annotations.

The changes in the get_queryset method enhance data integrity and readability:

  1. The new cycle_id annotation ensures that only non-deleted cycles are considered, which is a good practice for data consistency.

  2. The module_ids annotation has been reformatted for better readability, and additional conditions have been added to filter out archived and deleted modules.

These changes improve the overall quality of the queryset and the data it returns.

Consider adding a comment explaining the purpose of the cycle_id annotation for better code documentation:

# Annotate cycle_id, ensuring only non-deleted cycles are considered
.annotate(
    cycle_id=Case(
        When(
            issue_cycle__cycle__deleted_at__isnull=True,
            then=F("issue_cycle__cycle_id"),
        ),
        default=None,
    )
)
apiserver/plane/app/views/module/issue.py (1)

9-10: LGTM! Consider a minor readability improvement.

The changes to the get_queryset method effectively implement the removal of deleted cycles from the issue list. The use of Case and When for conditional annotation is a robust approach.

To slightly improve readability, consider extracting the Case statement into a separate variable:

cycle_id_annotation = Case(
    When(
        issue_cycle__cycle__deleted_at__isnull=True,
        then=F("issue_cycle__cycle_id"),
    ),
    default=None,
)

# Then in the annotate call:
.annotate(cycle_id=cycle_id_annotation)

This separation can make the query structure clearer, especially if you need to reuse or modify the annotation in the future.

Also applies to: 70-78

apiserver/plane/app/views/cycle/issue.py (1)

105-113: LGTM: Annotation logic for cycle_id improved.

The new annotation for cycle_id using Case and When effectively filters out deleted cycles, aligning with the PR objective. This change ensures that only active cycles are considered when listing issues.

For improved readability, consider adding a brief comment explaining the purpose of this annotation:

# Annotate cycle_id, setting it to None for deleted cycles
cycle_id=Case(
    When(
        issue_cycle__cycle__deleted_at__isnull=True,
        then=F("issue_cycle__cycle_id"),
    ),
    default=None,
)

This minor addition would make the intention clearer for future maintainers.

apiserver/plane/app/views/issue/archive.py (1)

67-75: Approve the changes with a minor suggestion.

The modification to the cycle_id annotation is a good improvement. It ensures that only non-deleted cycles are considered when annotating cycle_id, which enhances data consistency in the issue list.

Consider adding a comment explaining the purpose of this conditional annotation, as it might not be immediately clear to other developers why this change was necessary. For example:

# Annotate cycle_id only for non-deleted cycles to maintain consistency in the issue list
cycle_id=Case(
    When(
        issue_cycle__cycle__deleted_at__isnull=True,
        then=F("issue_cycle__cycle_id"),
    ),
    default=None,
)

This comment will help future maintainers understand the rationale behind this specific implementation.

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

123-131: Improved cycle_id annotation logic

The changes to the cycle_id annotation are well-implemented and align with the PR objective. By using a Case statement to check if the related cycle's deleted_at is null, the code now ensures that only valid (non-deleted) cycle IDs are returned. This improvement helps maintain data integrity and prevents issues associated with deleted cycles from appearing in the list.

Consider adding a comment explaining the purpose of this conditional annotation to improve code readability and maintainability for future developers.

apiserver/plane/app/views/issue/base.py (1)

778-786: Approved: Consistent implementation of cycle_id annotation.

The conditional annotation for cycle_id is correctly implemented here, consistent with the previous occurrence. This ensures uniform behavior across different parts of the code.

Consider refactoring this repeated annotation into a reusable method or queryset annotation to reduce code duplication. This would improve maintainability and ensure consistency if changes are needed in the future.

For example, you could create a method like:

def annotate_cycle_id(queryset):
    return queryset.annotate(
        cycle_id=Case(
            When(
                issue_cycle__cycle__deleted_at__isnull=True,
                then=F("issue_cycle__cycle_id"),
            ),
            default=None,
        )
    )

Then use it in both places:

queryset = annotate_cycle_id(queryset)

This would centralize the logic and make it easier to maintain.

apiserver/plane/app/views/inbox/base.py (2)

Line range hint 225-233: Handle potential NoneType error when retrieving inbox_id

The inbox_id obtained with .first() may be None if no matching Inbox is found. Passing None to InboxIssue.objects.get() could raise an error.

Consider adding a check to ensure inbox_id is not None before proceeding. If it is None, return an appropriate error response.

Apply this diff to handle the potential error:

 def partial_update(self, request, slug, project_id, pk):
     inbox_id = Inbox.objects.filter(
         workspace__slug=slug, project_id=project_id
     ).first()
+    if inbox_id is None:
+        return Response(
+            {"error": "Inbox not found"},
+            status=status.HTTP_400_BAD_REQUEST,
+        )
     inbox_issue = InboxIssue.objects.get(
         issue_id=pk,
         workspace__slug=slug,

Line range hint 228-233: Ensure correct assignment of inbox_id when fetching InboxIssue

In the InboxIssue.objects.get() call, ensure that inbox_id is correctly referenced using inbox_id.id instead of the entire object.

Update the inbox_id parameter to use the ID attribute:

     inbox_issue = InboxIssue.objects.get(
         issue_id=pk,
         workspace__slug=slug,
         project_id=project_id,
-        inbox_id=inbox_id,
+        inbox_id=inbox_id.id,
     )
apiserver/plane/app/views/view/base.py (2)

Line range hint 138-144: Duplicate Annotation of sub_issues_count

The sub_issues_count annotation is defined twice in the get_queryset method—once at lines 138-144 and again at lines 225-231. This duplication is unnecessary and may affect query efficiency.

Remove the duplicate annotation of sub_issues_count to streamline the queryset.

Also applies to: 225-231


Line range hint 153-165: Simplify Permission Checks in the list Method

The permission checks within the list method of WorkspaceViewViewSet involve nested if statements, which can be simplified for better readability and maintainability.

Consider refactoring the permission logic to reduce nesting and improve clarity. For example, you can combine conditions or use early returns to make the code more concise.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 2982cd4 and 0dc2b25.

📒 Files selected for processing (13)
  • apiserver/plane/api/views/issue.py (1 hunks)
  • apiserver/plane/app/views/cycle/issue.py (2 hunks)
  • apiserver/plane/app/views/inbox/base.py (2 hunks)
  • apiserver/plane/app/views/issue/archive.py (2 hunks)
  • apiserver/plane/app/views/issue/base.py (4 hunks)
  • apiserver/plane/app/views/issue/relation.py (2 hunks)
  • apiserver/plane/app/views/issue/sub_issue.py (2 hunks)
  • apiserver/plane/app/views/module/issue.py (2 hunks)
  • apiserver/plane/app/views/view/base.py (3 hunks)
  • apiserver/plane/app/views/workspace/draft.py (3 hunks)
  • apiserver/plane/app/views/workspace/user.py (1 hunks)
  • apiserver/plane/space/views/issue.py (2 hunks)
  • packages/editor/src/core/extensions/custom-image/components/image-block.tsx (1 hunks)
🧰 Additional context used
🔇 Additional comments (23)
apiserver/plane/app/views/issue/sub_issue.py (2)

13-14: LGTM: New imports for Case statement.

The addition of Case and When imports from django.db.models is appropriate for the new cycle_id annotation logic.


53-61: Excellent: Improved cycle_id annotation logic.

The updated cycle_id annotation using a Case statement effectively addresses the PR objective of removing deleted cycles from the issue list. This change ensures that:

  1. cycle_id is set to None if the associated cycle has been deleted.
  2. The filtering of deleted cycles is handled efficiently at the database level.

This implementation maintains the existing functionality while improving data consistency.

apiserver/plane/app/views/issue/relation.py (2)

6-16: LGTM: Import statements updated correctly.

The addition of Case and When imports from django.db.models is appropriate for the new functionality implemented in the list method.


96-104: LGTM: Cycle annotation updated to exclude deleted cycles.

The new annotation for cycle_id effectively addresses the PR objective of removing deleted cycles from the issue list. The implementation correctly uses Case and When to set the cycle_id only when the cycle is not deleted, defaulting to None otherwise. This change ensures that only valid cycles are considered in the issue list.

packages/editor/src/core/extensions/custom-image/components/image-block.tsx (3)

59-59: Approve changes in image source handling and UI conditionals.

The refactoring to use remoteImageSrc consistently throughout the component is a positive change. It simplifies the source of truth for the image source and updates the conditions for showing the image loader, utils, and resizer accordingly. This change improves the overall clarity and maintainability of the component.

These changes look good and align well with the PR objectives of removing deleted cycles from the issue list. The consistent use of remoteImageSrc should make the code more robust and easier to maintain.

Also applies to: 201-213


Line range hint 1-324: Summary of changes and their impact.

The modifications in this file are focused on simplifying the handling of the image source in the CustomImageBlock component. The main changes include:

  1. Removal of the src prop from the component's props.
  2. Consistent use of remoteImageSrc derived from node.attrs.src.
  3. Updated conditions for rendering image-related UI elements.

These changes align with the PR objectives and the AI-generated summary. They improve the component's consistency and maintainability without introducing new issues or altering its core functionality.

The changes in this file are well-implemented and contribute positively to the codebase. They simplify the component's logic and improve its overall structure.


59-59: Verify the removal of src prop and its impact.

The src prop has been commented out in the destructuring. This change aligns with the removal of the src property from CustomImageBlockProps as mentioned in the AI-generated summary. Now, remoteImageSrc is derived solely from node.attrs.src.

Please ensure that:

  1. This change is intentional and aligns with the PR objectives.
  2. All places where CustomImageBlock is used have been updated to remove the src prop.
  3. The logic depending on remoteImageSrc throughout the component still works as expected.

You can use the following script to check for any remaining usage of the src prop with CustomImageBlock:

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

15-16: LGTM: New imports for conditional annotation.

The addition of Case and When imports from django.db.models is appropriate for the new conditional annotation in the get_queryset method.


Line range hint 1-100: Summary: Improved queryset handling for draft issues.

The changes in this file focus on enhancing the get_queryset method of the WorkspaceDraftIssueViewSet class. The main improvements are:

  1. Addition of a conditional annotation for cycle_id to ensure only non-deleted cycles are considered.
  2. Refinement of the module_ids annotation to exclude archived and deleted modules.

These changes contribute to better data integrity and accuracy in the draft issue queryset. The overall structure and functionality of the class remain unchanged, maintaining consistency with the existing codebase.

apiserver/plane/app/views/module/issue.py (1)

Line range hint 1-378: Overall impact assessment: Changes are well-contained and aligned with PR objectives.

The modifications to the get_queryset method effectively implement the removal of deleted cycles from the issue list without affecting other parts of the ModuleIssueViewSet class. The changes are well-contained and don't require alterations to other methods or class structure.

The new imports (Case and When) are correctly placed and used appropriately. This implementation aligns well with the PR objective of removing deleted cycles from the issue list, improving data consistency in the application.

apiserver/plane/app/views/cycle/issue.py (2)

6-6: LGTM: Import statement updated correctly.

The import statement has been appropriately updated to include Case and When from Django's ORM, which are used in the new annotation logic for cycle_id.


Line range hint 1-359: Summary: Effective implementation of cycle filtering.

The changes in this file successfully implement the filtering of deleted cycles from the issue list. The new annotation logic for cycle_id ensures that only active cycles are considered when listing issues, which aligns perfectly with the PR objective.

Key points:

  1. The import statement has been updated correctly to include the necessary Django ORM functions.
  2. The list method now includes a Case statement to handle deleted cycles appropriately.
  3. The changes are focused and don't introduce any apparent side effects to other parts of the class.

These modifications should improve the accuracy of the issue list by excluding issues associated with deleted cycles.

apiserver/plane/app/views/issue/archive.py (1)

Line range hint 1-375: Summary of changes and potential impact

The main change in this file is the modification of the cycle_id annotation in the get_queryset method of the IssueArchiveViewSet class. This change improves data consistency by excluding deleted cycles from the cycle_id annotation.

Key points:

  1. The change ensures that only non-deleted cycles are considered when annotating cycle_id.
  2. This modification enhances data consistency in the issue list.
  3. Other methods in this file are not directly affected by this change.

To ensure a smooth integration of this change:

  1. Review the results of the verification script provided earlier.
  2. Update any code that relies on the presence of cycle_id for deleted cycles.
  3. Test thoroughly to confirm that this change doesn't introduce unexpected behavior in other parts of the application that interact with archived issues or cycles.

These steps will help maintain the integrity of the application while benefiting from the improved data consistency introduced by this change.

apiserver/plane/app/views/issue/base.py (3)

17-18: LGTM: New imports for conditional annotations.

The addition of When and Case imports from django.db.models is appropriate for implementing the conditional annotations for the cycle_id field.


88-96: Excellent: Conditional annotation for cycle_id.

This change effectively implements the PR objective of removing deleted cycles from the issue list. The use of Case and When ensures that cycle_id is only set when the associated cycle is not deleted (deleted_at is null). This is a robust way to handle soft-deleted cycles in the issue list.


Line range hint 1-896: Summary: Effective implementation of cycle removal logic.

The changes in this file successfully address the PR objective of removing deleted cycles from the issue list. The consistent implementation of the conditional annotation for cycle_id ensures that deleted cycles are not included in issue queries. This improves data integrity and user experience by preventing issues from being associated with non-existent cycles.

Key points:

  1. Appropriate imports have been added for the new functionality.
  2. The conditional annotation is correctly implemented using Case and When.
  3. The changes are consistent across different methods in the file.

While the implementation is solid, consider the refactoring suggestion to reduce code duplication and improve maintainability.

Overall, these changes represent a significant improvement in handling deleted cycles within the issue management system.

apiserver/plane/space/views/issue.py (2)

109-117: LGTM! This change effectively filters out deleted cycles.

The introduction of the Case statement for cycle_id annotation is a good improvement. It ensures that only cycles that haven't been deleted (i.e., deleted_at is null) are associated with the issues. This aligns well with the PR objective of removing deleted cycles from the issue list.


706-714: LGTM! Consistent implementation for single issue retrieval.

This change mirrors the modification made in the ProjectIssuesPublicEndpoint class. By applying the same Case statement for cycle_id annotation, it ensures consistency in how deleted cycles are handled across both bulk issue listing and single issue retrieval. This is a good practice that maintains code coherence and prevents potential discrepancies in behavior.

apiserver/plane/app/views/inbox/base.py (2)

6-6: Import 'Case' and 'When' for conditional annotations

The addition of Case and When to the imports is appropriate, as they are used for conditional annotations in the queryset to handle conditional logic.


115-123: Conditional annotation of cycle_id to exclude deleted cycles

The annotate method correctly uses Case and When to set cycle_id only if the related cycle is not deleted (deleted_at__isnull=True). This ensures that issues associated with deleted cycles do not incorrectly retain a cycle_id, aligning with the PR objective to remove deleted cycles from the issue list.

apiserver/plane/api/views/issue.py (1)

205-213: Correctly excludes deleted cycles from the issue list

The updated annotation for cycle_id ensures that only cycles that are not deleted (deleted_at__isnull=True) are associated with issues. This change aligns with the PR objective of removing deleted cycles from the issue list.

apiserver/plane/app/views/view/base.py (2)

12-13: Approved: Added necessary imports for conditional annotations

The imports of Case and When are required for the conditional annotations used later in the code.


210-218: Ensure Consistent Handling of Deleted Cycles

The conditional annotation for cycle_id using Case and When is added in both get_queryset and list methods. While addressing deleted cycles is important, ensure that this logic is consistently applied and tested across all relevant querysets.

Run the following script to check for other querysets where cycle_id should be conditionally annotated:

Also applies to: 288-296

✅ Verification successful

Consistent Handling of Deleted Cycles Verified

The conditional annotation for cycle_id using Case and When is consistently applied only within the get_queryset and list methods as intended. No additional querysets requiring this annotation were found.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all occurrences of `cycle_id` annotation in the codebase.

# Search for annotations of `cycle_id` using `Case` and `When`.
rg --type python 'annotate\(\s*cycle_id=Case\(' -A 5

Length of output: 85


Script:

#!/bin/bash
# Description: Find all occurrences of `cycle_id` annotation in Python files.

# Search for annotations of `cycle_id` using `Case` and `When` in .py files.
rg --glob "*.py" 'annotate\(\s*cycle_id=Case\(' -A 5

Length of output: 54


Script:

#!/bin/bash
# Description: Find all occurrences of `cycle_id` annotation using `Case` and `When` in Python files with ast-grep.

# Use ast-grep to search for `cycle_id=Case(` patterns in Python files.
ast-grep --lang python --pattern '_.annotate(cycle_id=Case($_, $_))'

Length of output: 70

Comment on lines +67 to +75
.annotate(
cycle_id=Case(
When(
issue_cycle__cycle__deleted_at__isnull=True,
then=F("issue_cycle__cycle_id"),
),
default=None,
)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Issues Found with cycle_id Handling.

The recent change allowing cycle_id to be None has introduced potential issues in both serializers and query filters. The following areas need attention to ensure proper handling of None values for cycle_id:

  • Serializers:

    • apiserver/plane/app/serializers/draft.py:
      cycle_id = serializers.PrimaryKeyRelatedField(read_only=True)
    • apiserver/plane/app/serializers/issue.py:
      cycle_id = serializers.PrimaryKeyRelatedField(read_only=True)

    Action: Add allow_null=True to the cycle_id fields to permit None values.

  • Query Filters:

    • apiserver/plane/bgtasks/issue_activities_task.py:
      cycle = Cycle.objects.filter(pk=cycle_id).first()
    • apiserver/plane/api/views/cycle.py:
      .filter(cycle_id=self.kwargs.get("cycle_id"))
      Issue.issue_objects.filter(issue_cycle__cycle_id=cycle_id)
    • apiserver/plane/app/views/cycle/issue.py:
      .filter(cycle_id=self.kwargs.get("cycle_id"))
      Issue.issue_objects.filter(issue_cycle__cycle_id=cycle_id)

    Action: Update these query filters to handle cases where cycle_id is None. For example, include checks like cycle_id__isnull=True where appropriate.

Please address these issues to ensure that the application correctly handles scenarios where cycle_id can be None, thereby maintaining data consistency and preventing potential runtime errors.

🔗 Analysis chain

Verify the impact of the cycle_id annotation change.

While the change in the cycle_id annotation improves data consistency, it's important to ensure that this modification doesn't unintentionally affect other parts of the codebase that might rely on the cycle_id field, especially for deleted cycles.

Please run the following script to identify any potential issues:

This script will help identify areas of the codebase that might need attention due to the cycle_id annotation change. Please review the results and update any affected code as necessary.


Further verification needed for cycle_id null handling.

To ensure that the recent change allowing cycle_id to be None does not introduce issues, please run the following scripts to verify that serializers and query filters handle cycle_id appropriately.

This script performs the following checks:

  1. Serializers Null Handling:

    • Searches for cycle_id fields in serializers.
    • Ensures that allow_null=True is set, indicating that the field can accept None values.
  2. Query Filters Null Handling:

    • Looks for query filters that involve cycle_id.
    • Checks whether these filters include conditions to handle None values, such as cycle_id__isnull=True.
  3. Direct Access Checks (Optional):

    • Identifies places where cycle_id is accessed directly.
    • Ensures that there are checks in place to handle scenarios where cycle_id might be None.

Please review the results of these scripts and update the serializers or query conditions as necessary to handle cycle_id being None.

```markdown
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for usages of cycle_id that might be affected by the new annotation logic

# Search for cycle_id usages in Python files
echo "Searching for cycle_id usages:"
rg "cycle_id" --type py -g "!*/migrations/*"

# Search for potential filter conditions on cycle_id that might need updating
echo "\nSearching for potential cycle_id filters:"
rg "filter\(.*cycle_id" --type py -g "!*/migrations/*"

# Search for serializers that include cycle_id to ensure they handle None values
echo "\nSearching for serializers including cycle_id:"
rg "class.*Serializer" -A 10 -g "!*/migrations/*" | rg "cycle_id"

Length of output: 21795


Script:

#!/bin/bash
# Description: Verify serializers allow null values for cycle_id and query filters handle None appropriately

# Check serializers for cycle_id fields allowing nulls
echo "Checking serializers for cycle_id fields allowing null values:"
rg 'cycle_id\s*=\s*serializers\.PrimaryKeyRelatedField' --type py -g "!*/migrations/*" | grep -v 'allow_null=True'

# Check query filters using cycle_id without handling None
echo "\nChecking query filters for cycle_id without handling nulls:"
rg '\.filter\([^)]*cycle_id[^)]*\)' --type py -g "!*/migrations/*" | grep -v 'cycle_id__isnull'

# Optionally, check for direct accesses to cycle_id that might not account for None
echo "\nChecking for direct cycle_id accesses without None checks:"
rg 'cycle_id' --type py -g "!*/migrations/*" | grep -E 'if\s+cycle_id\s+|cycle_id\s*='

Length of output: 10160

Comment on lines +109 to +117
.annotate(
cycle_id=Case(
When(
issue_cycle__cycle__deleted_at__isnull=True,
then=F("issue_cycle__cycle_id"),
),
default=None,
)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider refactoring to reduce code duplication.

The Case statement for cycle_id annotation is identical in both ProjectIssuesPublicEndpoint and IssueRetrievePublicEndpoint classes. To improve maintainability and reduce duplication, consider extracting this annotation logic into a separate method or utility function. This would make it easier to update the logic in the future if needed and ensure consistency across the codebase.

Example refactoring:

def annotate_cycle_id(queryset):
    return queryset.annotate(
        cycle_id=Case(
            When(
                issue_cycle__cycle__deleted_at__isnull=True,
                then=F("issue_cycle__cycle_id"),
            ),
            default=None,
        )
    )

# Then in both classes:
issue_queryset = annotate_cycle_id(issue_queryset)

This refactoring would make the code more DRY (Don't Repeat Yourself) and easier to maintain.

Also applies to: 706-714

Comment on lines +210 to +218
.annotate(
cycle_id=Case(
When(
issue_cycle__cycle__deleted_at__isnull=True,
then=F("issue_cycle__cycle_id"),
),
default=None,
)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Duplicate Annotation of cycle_id in get_queryset

The cycle_id annotation is being added in the get_queryset method. However, the same annotation is also present in the list method starting at line 288. This duplication may lead to redundant computations and could impact performance.

Consider removing the redundant annotation from either get_queryset or list to optimize the query performance.

Comment on lines +288 to +296
.annotate(
cycle_id=Case(
When(
issue_cycle__cycle__deleted_at__isnull=True,
then=F("issue_cycle__cycle_id"),
),
default=None,
)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Redundant Annotation of cycle_id in list Method

The cycle_id annotation is being reapplied in the list method even though it was already annotated in get_queryset. Reapplying annotations can cause unnecessary overhead.

Since get_queryset already includes the cycle_id annotation, you can remove it from the list method to avoid redundancy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants