[WEB-2114]: Chore: project cycle optimization #5430
Conversation
…/makeplane/plane into chore/project-cycle-optimization
…ne/plane into chore/project-cycle-optimization
…/makeplane/plane into chore/project-cycle-optimization
WalkthroughThe changes introduced multiple new API endpoints for cycle progress and analytics, streamlining data retrieval and management. A custom hook was created to simplify cycle detail fetching, while existing components were refactored to enhance state management and improve code clarity. Additionally, type definitions were refined for better type safety, and new methods were added to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CycleDetailPage
participant useCyclesDetails
participant CycleService
participant CycleStore
User->>CycleDetailPage: Request Cycle Details
CycleDetailPage->>useCyclesDetails: Fetch Cycle Data
useCyclesDetails->>CycleService: Call API for Cycle Data
CycleService-->>useCyclesDetails: Return Cycle Data
useCyclesDetails-->>CycleDetailPage: Provide Cycle Data
CycleDetailPage-->>User: Display Cycle Details
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://github.com/avatars/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
web/core/components/cycles/active-cycle/productivity.tsx (1)
Line range hint
1-49: Consider re-adding loading feedback.The removal of the
useStatehook andfetchCycleDetailsfunction simplifies the component but may impact user experience by not providing loading feedback during data fetching. Consider re-adding some form of loading feedback to enhance user experience.Tools
Biome
[error] 40-40: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- apiserver/plane/app/urls/cycle.py (2 hunks)
- apiserver/plane/app/views/init.py (1 hunks)
- apiserver/plane/app/views/cycle/base.py (11 hunks)
- web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/[cycleId]/page.tsx (3 hunks)
- web/core/components/cycles/active-cycle/cycle-stats.tsx (7 hunks)
- web/core/components/cycles/active-cycle/productivity.tsx (4 hunks)
- web/core/components/cycles/active-cycle/progress.tsx (2 hunks)
- web/core/components/cycles/active-cycle/root.tsx (4 hunks)
- web/core/components/cycles/active-cycle/use-cycles-details.ts (1 hunks)
- web/core/services/cycle.service.ts (3 hunks)
- web/core/store/cycle.store.ts (7 hunks)
Additional context used
Ruff
apiserver/plane/app/views/__init__.py
101-101:
.cycle.base.CycleAnalyticsEndpointimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
102-102:
.cycle.base.CycleProgressEndpointimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
Additional comments not posted (38)
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/[cycleId]/page.tsx (3)
9-9: LGTM!The new import statement for
useCyclesDetailsis correct.The code changes are approved.
33-37: LGTM!The custom hook
useCyclesDetailsis correctly invoked with the necessary parameters.The code changes are approved.
54-54: LGTM!The conditional rendering logic is correctly implemented and improves the readability of the component.
The code changes are approved.
apiserver/plane/app/urls/cycle.py (3)
9-10: LGTM!The new import statements for
CycleProgressEndpointandCycleAnalyticsEndpointare correct.The code changes are approved.
111-115: LGTM!The new route for
CycleProgressEndpointis correctly defined.The code changes are approved.
116-120: LGTM!The new route for
CycleAnalyticsEndpointis correctly defined.The code changes are approved.
web/core/components/cycles/active-cycle/root.tsx (4)
19-19: LGTM!The new import statement for
useCyclesDetailsis correct.The code changes are approved.
31-34: LGTM!The custom hook
useCyclesDetailsis correctly invoked with the necessary parameters.The code changes are approved.
37-37: LGTM!The conditional rendering logic is correctly implemented and improves the readability of the component.
The code changes are approved.
Line range hint
68-85: LGTM!The updated props for
ActiveCycleProgress,ActiveCycleProductivity, andActiveCycleStatsare correctly passed.The code changes are approved.
web/core/components/cycles/active-cycle/use-cycles-details.ts (8)
1-8: Import statements look good.The imports are well-organized and necessary for the functionality of the hook.
10-14: Interface definition is clear.The
IActiveCycleDetailsinterface is well-defined and provides a clear structure for the expected props.
16-25: Destructuring props and initializing hooks.The props are correctly destructured, and necessary hooks are initialized. Ensure that
useIssuesanduseCyclehooks are correctly implemented and imported.
27-29: Fetching cycle details using SWR.The
useSWRhooks are used effectively to fetch cycle details. Ensure thatfetchActiveCycleProgress,fetchActiveCycleAnalytics, andfetchActiveCycleIssuesfunctions handle errors appropriately.Verify that the functions handle errors appropriately and return meaningful error messages.
31-56: Fetching cycle details using SWR.The
useSWRhooks are used effectively to fetch cycle details. Ensure thatfetchActiveCycleProgress,fetchActiveCycleAnalytics, andfetchActiveCycleIssuesfunctions handle errors appropriately.Verify that the functions handle errors appropriately and return meaningful error messages.
58-58: Fetching cycle issue details.The
getActiveCycleByIdFromIssuefunction is used to fetch cycle issue details. Ensure that this function is correctly implemented and handles errors appropriately.Verify that the function handles errors appropriately and returns meaningful error messages.
60-84: Callback function for updating filters.The
handleFiltersUpdatefunction is well-implemented and usesuseCallbackfor optimization. Ensure thatupdateFiltersfunction is correctly implemented and handles errors appropriately.Verify that the
updateFiltersfunction handles errors appropriately and returns meaningful error messages.
85-92: Returning hook values.The hook returns necessary values and functions, making it easy to use in components.
web/core/components/cycles/active-cycle/progress.tsx (9)
Line range hint
4-21: Import statements and prop types look good.The imports are well-organized and necessary for the functionality of the component. The prop types are well-defined and provide a clear structure for the expected props.
26-29: Destructuring props and initializing hooks.The props are correctly destructured, and necessary hooks are initialized. Ensure that
useProjectStatehook is correctly implemented and imported.
30-45: Derived values and grouped issues.The derived values and grouped issues are correctly calculated. Ensure that
PROGRESS_STATE_GROUPS_DETAILSis correctly defined and imported.
46-46: Component rendering logic.The component rendering logic is well-implemented and uses
isEmptyto handle empty cycle data. This ensures robustness and prevents runtime errors.
47-47: Progress section rendering.The progress section is correctly rendered based on the cycle data. Ensure that
LinearProgressIndicatoris correctly implemented and imported.
48-48: Grouped issues rendering.The grouped issues are correctly rendered based on the cycle data. Ensure that
groupedProjectStatesis correctly defined and imported.
49-49: Cancelled issues rendering.The cancelled issues are correctly rendered based on the cycle data.
Line range hint
50-50: Empty state rendering.The empty state is correctly rendered when there are no issues. Ensure that
EmptyStateandEmptyStateTypeare correctly implemented and imported.
Line range hint
51-51: Loader rendering.The loader is correctly rendered when the cycle data is empty. Ensure that
Loaderis correctly implemented and imported.apiserver/plane/app/views/__init__.py (1)
Line range hint
1-100: Import statements look good.The imports are well-organized and necessary for the functionality of the file.
Tools
Ruff
98-98:
.cycle.base.CycleUserPropertiesEndpointimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
99-99: Redefinition of unused
CycleViewSetfrom line 94Remove definition:
CycleViewSet(F811)
99-99:
.cycle.base.CycleViewSetimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
100-100: Redefinition of unused
TransferCycleIssueEndpointfrom line 97Remove definition:
TransferCycleIssueEndpoint(F811)
100-100:
.cycle.base.TransferCycleIssueEndpointimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
101-101:
.cycle.base.CycleAnalyticsEndpointimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
102-102:
.cycle.base.CycleProgressEndpointimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
105-105:
.cycle.issue.CycleIssueViewSetimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
web/core/services/cycle.service.ts (2)
Line range hint
24-28: LGTM!The function is correctly implemented with proper error handling. The updated return type enhances type safety.
The code changes are approved.
Line range hint
38-42: LGTM!The function is correctly implemented with proper error handling. The updated return type enhances type safety.
The code changes are approved.
web/core/components/cycles/active-cycle/cycle-stats.tsx (1)
Line range hint
4-302: LGTM!The addition of the
cycleIssueDetailsprop enhances the component's functionality. The removal of theuseSWRhook and streamlined logic improve performance. The adjusted conditional rendering improves robustness.The code changes are approved.
web/core/store/cycle.store.ts (3)
65-71: LGTM!The new methods
fetchActiveCycleProgressandfetchActiveCycleAnalyticsare correctly added to theICycleStoreinterface.The code changes are approved.
110-110: Verify the impact of changingplotTypeto a standard observable.Changing
plotTypefromobservable.reftoobservablewill affect how changes to this property are tracked and reacted to within the MobX state management framework. Ensure this change doesn't introduce any unintended side effects.
130-131: LGTM!The new methods
fetchActiveCycleProgressandfetchActiveCycleAnalyticsare correctly implemented in theCycleStoreclass. They include logic to update thecycleMapwith fetched data, ensuring the state reflects the most current information regarding cycles.The code changes are approved.
Also applies to: 486-492, 501-514
apiserver/plane/app/views/cycle/base.py (4)
30-30: Verify the impact of reducing complexity inget_queryset.The reduction in complexity simplifies the logic and reduces potential performance overhead. Ensure that the removal of detailed estimates does not affect any critical functionality.
1008-1143: LGTM!The new class
CycleProgressEndpointis correctly implemented and enhances the functionality by allowing users to access summarized estimate data directly related to a specific cycle.The code changes are approved.
1144-1362: LGTM!The new class
CycleAnalyticsEndpointis correctly implemented and enhances the functionality by allowing users to retrieve analytics based on either points or issues.The code changes are approved.
412-418: Verify the impact of new annotations inretrievemethod.The new annotations provide additional information about the issues associated with a cycle. Ensure that these changes do not introduce any performance overhead.
| CycleAnalyticsEndpoint, | ||
| CycleProgressEndpoint, |
There was a problem hiding this comment.
Unused imports.
The CycleAnalyticsEndpoint and CycleProgressEndpoint are imported but not used in the file. Consider removing them or adding them to __all__.
Apply this diff to remove the unused imports:
- CycleAnalyticsEndpoint,
- CycleProgressEndpoint,Alternatively, if these endpoints are intended to be used elsewhere, add them to __all__:
+__all__ = [
+ ...
+ "CycleAnalyticsEndpoint",
+ "CycleProgressEndpoint",
+]Tools
Ruff
101-101:
.cycle.base.CycleAnalyticsEndpointimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
102-102:
.cycle.base.CycleProgressEndpointimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- web/core/store/cycle.store.ts (6 hunks)
Files skipped from review as they are similar to previous changes (1)
- web/core/store/cycle.store.ts
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- web/core/components/cycles/active-cycle/root.tsx (4 hunks)
Additional comments not posted (8)
web/core/components/cycles/active-cycle/root.tsx (8)
18-18: LGTM!The import of
ActiveCycleIssueDetailsis necessary for type definition.The code changes are approved.
19-19: LGTM!The import of
useCyclesDetailsis necessary for the new state management logic.The code changes are approved.
29-29: LGTM!The
useCyclehook is correctly used to manage the current project active cycle and loader state.The code changes are approved.
31-34: LGTM!The
useCyclesDetailscustom hook is correctly used to manage the cycle details.The code changes are approved.
37-40: LGTM!The loader state is correctly checked to show a loader if the active cycle is loading.
The code changes are approved.
68-73: LGTM!The
ActiveCycleProgresscomponent props are correctly updated to enhance integration with the overall state management.The code changes are approved.
85-85: LGTM!The
ActiveCycleStatscomponent prop is correctly updated to enhance integration with the overall state management.The code changes are approved.
85-85: LGTM!The type casting ensures type safety for the
cycleIssueDetailsprop.The code changes are approved.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (6)
web/core/components/cycles/active-cycle/root.tsx (2)
16-17: Remove unused importActiveCycleIssueDetails.The
ActiveCycleIssueDetailsimport is not used in this file and can be removed to clean up the code.-import { ActiveCycleIssueDetails } from "@/store/issue/cycle";
75-75: Remove unnecessary type assertion forcycleIssueDetails.The type assertion
as ActiveCycleIssueDetailsis unnecessary if the type is already correctly inferred. Remove it to clean up the code.- cycleIssueDetails={cycleIssueDetails as ActiveCycleIssueDetails} + cycleIssueDetails={cycleIssueDetails}web/core/components/cycles/active-cycle/progress.tsx (3)
25-25: DestructureworkspaceSlugandprojectIdprops for consistency.Destructure
workspaceSlugandprojectIdprops along withhandleFiltersUpdateandcyclefor consistency.- const { handleFiltersUpdate, cycle } = props; + const { handleFiltersUpdate, cycle, workspaceSlug, projectId } = props;
29-29: Use optional chaining forcycle.total_issues.Use optional chaining to safely access
cycle.total_issuesto prevent potential runtime errors.- value: cycle && cycle.total_issues > 0 ? (cycle[group.key as keyof ICycle] as number) : 0, + value: cycle?.total_issues > 0 ? (cycle[group.key as keyof ICycle] as number) : 0,
38-41: Use optional chaining forcycleproperties.Use optional chaining to safely access
cycleproperties to prevent potential runtime errors.- completed: cycle?.completed_issues, - started: cycle?.started_issues, - unstarted: cycle?.unstarted_issues, - backlog: cycle?.backlog_issues, + completed: cycle?.completed_issues ?? 0, + started: cycle?.started_issues ?? 0, + unstarted: cycle?.unstarted_issues ?? 0, + backlog: cycle?.backlog_issues ?? 0,web/core/components/cycles/analytics-sidebar/issue-progress.tsx (1)
11-11: Remove unused importSpinner.The
Spinnerimport is not used in this file and can be removed to clean up the code.- import { CustomSelect, Loader, Spinner } from "@plane/ui"; + import { CustomSelect, Loader } from "@plane/ui";
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- web/core/components/cycles/active-cycle/productivity.tsx (4 hunks)
- web/core/components/cycles/active-cycle/progress.tsx (1 hunks)
- web/core/components/cycles/active-cycle/root.tsx (4 hunks)
- web/core/components/cycles/active-cycle/use-cycles-details.ts (1 hunks)
- web/core/components/cycles/analytics-sidebar/issue-progress.tsx (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- web/core/components/cycles/active-cycle/productivity.tsx
- web/core/components/cycles/active-cycle/use-cycles-details.ts
Additional context used
Biome
web/core/components/cycles/active-cycle/progress.tsx
[error] 45-45: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 45-45: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
Additional comments not posted (6)
web/core/components/cycles/active-cycle/root.tsx (3)
27-27: Ensure proper error handling foruseCyclehook.The
useCyclehook is used to getcurrentProjectActiveCycleandcurrentProjectActiveCycleId. Ensure that the hook handles errors properly and returns default values if needed.Ensure that the
useCyclehook handles errors and returns default values if needed.
29-32: Ensure proper error handling foruseCyclesDetailshook.The
useCyclesDetailshook is used to gethandleFiltersUpdate,activeCycle, andcycleIssueDetails. Ensure that the hook handles errors properly and returns default values if needed.Ensure that the
useCyclesDetailshook handles errors and returns default values if needed.
58-63: EnsureActiveCycleProgresscomponent handles null or undefinedcycleprop.The
ActiveCycleProgresscomponent receivescycleas a prop. Ensure that the component handles cases wherecycleis null or undefined to prevent runtime errors.Ensure that the
ActiveCycleProgresscomponent handles null or undefinedcycleprop.web/core/components/cycles/active-cycle/progress.tsx (2)
19-20: EnsureworkspaceSlugandprojectIdprops are always provided.The component now accepts
workspaceSlugandprojectIdas props. Ensure that these props are always provided to prevent runtime errors.Ensure that
workspaceSlugandprojectIdprops are always provided.
Line range hint
234-257: EnsureLoadercomponent handles loading state properly.The
Loadercomponent is used as a fallback whencompletionChartDistributionDatais not available. Ensure that theLoadercomponent handles the loading state properly and provides a smooth user experience.Ensure that the
Loadercomponent handles the loading state properly.Tools
Biome
[error] 45-45: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 45-45: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
web/core/components/cycles/analytics-sidebar/issue-progress.tsx (1)
Line range hint
234-257: EnsureLoadercomponent handles loading state properly.The
Loadercomponent is used as a fallback whencompletionChartDistributionDatais not available. Ensure that theLoadercomponent handles the loading state properly and provides a smooth user experience.Ensure that the
Loadercomponent handles the loading state properly.
| } | ||
| : {}; | ||
|
|
||
| return cycle ? ( | ||
| return cycle && cycle.hasOwnProperty("started_issues") ? ( |
There was a problem hiding this comment.
Use optional chaining and Object.hasOwn for property check.
Replace hasOwnProperty with Object.hasOwn and use optional chaining for safer property access.
- return cycle && cycle.hasOwnProperty("started_issues") ? (
+ return cycle?.hasOwnProperty("started_issues") ? (- return cycle && cycle.hasOwnProperty("started_issues") ? (
+ return cycle && Object.hasOwn(cycle, "started_issues") ? (Tools
Biome
[error] 45-45: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 45-45: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
There was a problem hiding this comment.
Shouldn't this be in hooks?
Summary
Backend Change: Split the cycles api into /progress and /analytics
Frontend Change: Made integrational changes to incorporate the backend changes
[WEB-2114]
Summary by CodeRabbit
New Features
Improvements
Bug Fixes