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-3102]fix: transfer issue count #6379

Closed
wants to merge 6 commits into from

Conversation

vamsikrishnamathala
Copy link
Collaborator

@vamsikrishnamathala vamsikrishnamathala commented Jan 10, 2025

Description

Corrected issues count in then Transfer issues button in cycles list.

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

References

WEB-3102

Summary by CodeRabbit

Release Notes

  • New Features

    • Added a pending_issues count to cycle details, providing more granular insight into issue status.
  • Improvements

    • Updated cycle-related components to utilize pending_issues instead of calculating transferable issues manually.
    • Enhanced data representation for cycle progress tracking.
  • Technical Updates

    • Modified type definitions and API responses to include the pending_issues property.
    • Improved handling of pending_issues during issue transfers in the cycle management logic.

Copy link
Contributor

coderabbitai bot commented Jan 10, 2025

Walkthrough

The pull request introduces a new pending_issues annotation for cycles, which counts issues in "backlog", "unstarted", or "started" states. This change spans multiple files across the application, updating the data structure and components to incorporate the new pending_issues field. The modification replaces the previous transferrableIssuesCount with a more precise representation of issues in progress, affecting both backend API and frontend components.

Changes

File Change Summary
apiserver/plane/app/views/cycle/base.py Added pending_issues annotation to get_queryset method, counting distinct issues in specific states; included pending_issues in the response of list method.
packages/types/src/cycle/cycle.d.ts Added pending_issues: number to TProgressSnapshot type, minor formatting update to SelectCycleType.
web/ce/components/cycles/end-cycle/modal.tsx Replaced transferrableIssuesCount with pendingIssues in Props interface.
web/core/components/cycles/list/cycle-list-item-action.tsx Updated logic to use pending_issues for displaying transferable issues.
web/core/components/cycles/quick-actions.tsx Updated EndCycleModal prop from transferrableIssuesCount to pendingIssues.
web/core/store/issue/cycle/issue.store.ts Enhanced transferIssuesFromCycle method to update pending_issues count in the cycleMap after transferring issues.

Sequence Diagram

sequenceDiagram
    participant API as CycleViewSet
    participant Type as TProgressSnapshot
    participant Frontend as CycleComponents
    
    API->>API: Count pending issues
    API-->>Type: Add pending_issues field
    Type-->>Frontend: Update cycle data structure
    Frontend->>Frontend: Render cycle list with pending issues
Loading

Possibly related PRs

Suggested reviewers

  • SatishGandham
  • rahulramesha
  • gurusainath

Poem

🐰 Cycles spin, issues in flight,
Pending tasks now shine so bright!
From backlog to started, we track with glee,
A rabbit's count of productivity!
Hop along, progress takes its stride! 🚀


📜 Recent review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 2d77b39 and 78fcadb.

📒 Files selected for processing (1)
  • web/core/store/issue/cycle/issue.store.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • web/core/store/issue/cycle/issue.store.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: lint-apiserver
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Analyze (python)

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.

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/views/cycle/base.py (2)

135-146: Implementation of pending_issues annotation looks good!

The annotation correctly counts distinct issues in transferable states (backlog, unstarted, started) while maintaining consistency with existing filters for archived, draft, and deleted items.

Consider breaking the long line into multiple lines for better readability:

-                        issue_cycle__issue__state__group__in=["backlog", "unstarted", "started"],
+                        issue_cycle__issue__state__group__in=[
+                            "backlog",
+                            "unstarted",
+                            "started"
+                        ],
🧰 Tools
🪛 Ruff (0.8.2)

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

(E501)


140-140: Fix line length issues.

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

  • Line 140: 97 characters
  • Line 586: 146 characters

Apply these changes to improve readability:

# For line 140
-                        issue_cycle__issue__state__group__in=["backlog", "unstarted", "started"],
+                        issue_cycle__issue__state__group__in=[
+                            "backlog",
+                            "unstarted",
+                            "started"
+                        ],

# For line 586
-                    avatar_url=Case(When(assignees__avatar_asset__isnull=False, then=Concat(Value("/api/assets/v2/static/"), "assignees__avatar_asset", Value("/"),)),
+                    avatar_url=Case(
+                        When(
+                            assignees__avatar_asset__isnull=False,
+                            then=Concat(
+                                Value("/api/assets/v2/static/"),
+                                "assignees__avatar_asset",
+                                Value("/"),
+                            ),
+                        ),

Also applies to: 586-586

🧰 Tools
🪛 Ruff (0.8.2)

140-140: Line too long (97 > 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 42e9281 and 78f9643.

📒 Files selected for processing (5)
  • apiserver/plane/app/views/cycle/base.py (3 hunks)
  • packages/types/src/cycle/cycle.d.ts (2 hunks)
  • web/ce/components/cycles/end-cycle/modal.tsx (1 hunks)
  • web/core/components/cycles/list/cycle-list-item-action.tsx (2 hunks)
  • web/core/components/cycles/quick-actions.tsx (1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/app/views/cycle/base.py

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

(E501)

🪛 GitHub Actions: Build and Lint on Pull Request
apiserver/plane/app/views/cycle/base.py

[error] 586-586: Line too long (146 > 88 characters). Consider breaking down the line or using line continuation.

🔇 Additional comments (7)
web/ce/components/cycles/end-cycle/modal.tsx (1)

9-9: LGTM! Interface update aligns with the new pending issues count.

The property rename from transferrableIssuesCount to pendingIssues is consistent with the bug fix objectives.

packages/types/src/cycle/cycle.d.ts (2)

61-61: LGTM! Type definition properly placed.

The pending_issues field is correctly added to the TProgressSnapshot type, maintaining consistency with other issue count fields.


124-124: LGTM! Formatting improvement.

The SelectCycleType formatting change improves readability without affecting functionality.

web/core/components/cycles/list/cycle-list-item-action.tsx (2)

256-256: LGTM! Updated text display.

The transfer issues text now correctly displays the count from pending_issues.


84-84: Verify completion state handling.

The condition correctly uses pending_issues > 0 for completed cycles. However, let's verify that this handles all cycle states correctly.

✅ Verification successful

Completion state handling is correctly implemented

The cycleStatus === "completed" check is consistently implemented across all cycle-related components, and the additional pending_issues > 0 condition is appropriately specific to the transfer functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for all cycle status checks to ensure consistent handling
rg "cycleStatus.*=== .*completed" web/

Length of output: 980

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

229-229: LGTM!

The pending_issues field is correctly included in the response data structure.


261-261: LGTM!

The pending_issues field is correctly included in the values list.

web/core/components/cycles/quick-actions.tsx Show resolved Hide resolved
@vamsikrishnamathala vamsikrishnamathala marked this pull request as draft January 10, 2025 15:03
@vamsikrishnamathala vamsikrishnamathala marked this pull request as ready for review January 13, 2025 04:44
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

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between ff0a22a and 2d77b39.

📒 Files selected for processing (1)
  • web/core/store/issue/cycle/issue.store.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: lint-apiserver
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
web/core/store/issue/cycle/issue.store.ts (1)

311-318: Verify the impact on UI components.

The changes to pending_issues count might affect UI components that rely on this value. Let's verify the usage across the codebase.

✅ Verification successful

The pending_issues changes are safe for UI components

The modification to set pending_issues to 0 is handled appropriately by all dependent UI components:

  • Cycle actions properly handle the zero state for transfer options
  • Dashboard statistics and profile views safely calculate and display the count
  • No potential rendering or calculation issues were found
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for UI components using pending_issues
echo "Searching for UI components using pending_issues..."
rg -l "pending_issues" --type=typescript --type=tsx

# Search for specific usage patterns
echo "\nSearching for specific usage patterns..."
ast-grep --pattern 'pending_issues'

Length of output: 1941


Script:

#!/bin/bash
# Search with explicit file extensions
echo "Searching TypeScript/TSX files..."
rg -l "pending_issues" --glob "*.ts" --glob "*.tsx"

# Search for cycle store usage
echo "\nSearching for cycle store imports and usage..."
rg -l "cycleStore|CycleStore" --glob "*.tsx"

# Search for specific pending_issues access pattern
echo "\nSearching for pending_issues access patterns..."
ast-grep --pattern 'pending_issues'

Length of output: 2515


Script:

#!/bin/bash
# Check implementation of components using pending_issues
echo "Checking quick-actions implementation..."
rg -A 5 -B 5 "pending_issues" "web/core/components/cycles/quick-actions.tsx"

echo "\nChecking cycle-list-item-action implementation..."
rg -A 5 -B 5 "pending_issues" "web/core/components/cycles/list/cycle-list-item-action.tsx"

echo "\nChecking overview-stats implementation..."
rg -A 5 -B 5 "pending_issues" "web/core/components/dashboard/widgets/overview-stats.tsx"

echo "\nChecking profile sidebar implementation..."
rg -A 5 -B 5 "pending_issues" "web/core/components/profile/sidebar.tsx"

Length of output: 4209

Comment on lines 311 to 318
// update pending count
runInAction(() => {
if (this.rootIssueStore.cycleMap)
set(this.rootIssueStore.cycleMap[cycleId], "pending_issues", {
...this.rootIssueStore.cycleMap[cycleId],
pending_issues: 0,
});
});
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

Simplify the object structure and improve type safety.

The current implementation has a few issues that could be improved:

  1. The object spread is redundant as you're setting a new value directly.
  2. The type safety could be improved with proper null checks.
  3. Error handling could be added for edge cases.

Here's the suggested implementation:

    // update pending count
    runInAction(() => {
-      if (this.rootIssueStore.cycleMap)
-        set(this.rootIssueStore.cycleMap[cycleId], "pending_issues", {
-          ...this.rootIssueStore.cycleMap[cycleId],
-          pending_issues: 0,
-        });
+      try {
+        const cycleMap = this.rootIssueStore.cycleMap;
+        if (!cycleMap?.[cycleId]) return;
+        
+        set(cycleMap, [cycleId, "pending_issues"], 0);
+      } catch (error) {
+        console.error("Failed to update pending issues count:", error);
+      }
    });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// update pending count
runInAction(() => {
if (this.rootIssueStore.cycleMap)
set(this.rootIssueStore.cycleMap[cycleId], "pending_issues", {
...this.rootIssueStore.cycleMap[cycleId],
pending_issues: 0,
});
});
// update pending count
runInAction(() => {
try {
const cycleMap = this.rootIssueStore.cycleMap;
if (!cycleMap?.[cycleId]) return;
set(cycleMap, [cycleId, "pending_issues"], 0);
} catch (error) {
console.error("Failed to update pending issues count:", error);
}
});

@vamsikrishnamathala vamsikrishnamathala marked this pull request as draft January 13, 2025 05:19
@vamsikrishnamathala vamsikrishnamathala deleted the fix-transfer_issue_count branch January 13, 2025 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug Something isn't working 🌐frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants