fix: cycles import issues for ee #5732
Conversation
WalkthroughThe pull request introduces several modifications to the cycle management components, specifically improving the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
web/ee/components/cycles/active-cycle/index.ts (1)
1-1: LGTM! This change effectively addresses the import cycle issue.The addition of this re-export statement aligns well with the PR objectives by resolving import issues in the EE folder. It provides a clean way to make CE components available in the EE context, promoting modularity and separation of concerns between CE and EE.
For future consideration: If only specific components from the CE active-cycle module are needed in the EE context, you might want to consider explicitly naming the exports. This could potentially improve performance and make the dependencies clearer. For example:
export { ComponentA, ComponentB } from "ce/components/cycles/active-cycle";However, this is just a suggestion for future optimization and is not necessary for the current implementation.
web/ee/components/cycles/analytics-sidebar/index.ts (1)
1-1: Approved: Good solution for resolving import cyclesThis change aligns well with the PR objectives by creating a mock file in the EE folder to resolve import cycle issues. Re-exporting the CE components allows for easy extension or overriding in the EE context if needed in the future.
To further improve this solution:
- Consider adding a comment explaining the purpose of this file and the pattern being used.
- Ensure this pattern is consistently applied to other components that may face similar import cycle issues.
- Update the project documentation to explain this architectural decision for future developers.
Would you like assistance in drafting a comment for this file or updating the project documentation?
web/core/components/cycles/analytics-sidebar/issue-progress.tsx (2)
Line range hint
106-120: RefactorgroupedIssuesto reduce repetition and enhance maintainability.The current implementation of
groupedIssuescontains repetitive code when assigning values based onestimateType. Refactoring this logic can simplify the code and make it more maintainable.Consider applying the following refactor:
-const groupedIssues = useMemo( - () => ({ - backlog: - estimateType === "points" ? cycleDetails?.backlog_estimate_points || 0 : cycleDetails?.backlog_issues || 0, - unstarted: - estimateType === "points" ? cycleDetails?.unstarted_estimate_points || 0 : cycleDetails?.unstarted_issues || 0, - started: - estimateType === "points" ? cycleDetails?.started_estimate_points || 0 : cycleDetails?.started_issues || 0, - completed: - estimateType === "points" ? cycleDetails?.completed_estimate_points || 0 : cycleDetails?.completed_issues || 0, - cancelled: - estimateType === "points" ? cycleDetails?.cancelled_estimate_points || 0 : cycleDetails?.cancelled_issues || 0, - }), - [estimateType, cycleDetails] -); +const groupedIssues = useMemo(() => { + const statuses = ["backlog", "unstarted", "started", "completed", "cancelled"]; + const result: { [key: string]: number } = {}; + statuses.forEach((status) => { + const pointsKey = `${status}_estimate_points` as keyof ICycle; + const issuesKey = `${status}_issues` as keyof ICycle; + result[status] = estimateType === "points" + ? cycleDetails?.[pointsKey] || 0 + : cycleDetails?.[issuesKey] || 0; + }); + return result; +}, [estimateType, cycleDetails]);This refactor reduces duplication and makes it easier to manage if additional statuses are added in the future.
Line range hint
160-162: Provide feedback or loading state whencycleDetailsis not available.Currently, if
cycleDetailsis null or undefined, the component returns an empty fragment (<></>). Consider enhancing the user experience by displaying a loading indicator or an informative message in this scenario.For example, you could modify the code to display a loader:
-if (!cycleDetails) return <></>; +if (!cycleDetails) return <Spinner />; // Assuming Spinner is a loading indicator componentOr display a message:
-if (!cycleDetails) return <></>; +if (!cycleDetails) return <p>Loading cycle details...</p>;This provides users with visual feedback that data is being fetched.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- web/core/components/cycles/analytics-sidebar/issue-progress.tsx (1 hunks)
- web/ee/components/cycles/active-cycle/index.ts (1 hunks)
- web/ee/components/cycles/analytics-sidebar/index.ts (1 hunks)
- web/ee/components/cycles/index.ts (1 hunks)
🔇 Additional comments (2)
web/ee/components/cycles/index.ts (1)
1-2: LGTM! Verify the existence of imported modules.The changes look good and align with the PR objectives. The index file is correctly set up to re-export all entities from the "active-cycle" and "analytics-sidebar" modules, which should help resolve the import issues mentioned in the PR description.
To ensure the imported modules exist, please run the following script:
This script will help confirm that the imported modules exist in the expected location or elsewhere in the web/ee directory.
web/core/components/cycles/analytics-sidebar/issue-progress.tsx (1)
21-21: Verify the updated import path forSidebarBaseChart.The import path for
SidebarBaseCharthas been changed to"@/plane-web/components/cycles/analytics-sidebar". Please ensure that this new path is correct and does not introduce any import cycle issues or module resolution problems.To verify the correctness of the new import path and check for potential import cycles, you can run the following script:
Summary
Added mock files in EE folder to fix imports
Summary by CodeRabbit
New Features
Bug Fixes