-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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-2007] fix: cycles loading optimization #5194
Conversation
WalkthroughThe recent updates enhance the handling of optional cycle data across multiple components within the application. By allowing the Changes
Poem
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 (
|
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- web/core/components/cycles/active-cycle/cycle-stats.tsx (3 hunks)
- web/core/components/cycles/active-cycle/productivity.tsx (4 hunks)
- web/core/components/cycles/active-cycle/progress.tsx (4 hunks)
- web/core/components/cycles/active-cycle/root.tsx (4 hunks)
- web/core/store/cycle.store.ts (4 hunks)
Additional comments not posted (16)
web/core/components/cycles/active-cycle/root.tsx (3)
31-31
: LGTM!The introduction of
currentProjectActiveCycle
enhances clarity by directly referencing the project-specific active cycle.
41-41
: LGTM!The update to use
currentProjectActiveCycle
in the loading state check ensures accurate management of the loading state.
Line range hint
57-83
:
LGTM!The updates to use
currentProjectActiveCycle
andcurrentProjectActiveCycleId
enhance the specificity and context-awareness of the data. The additional propcycleId
inActiveCycleStats
improves data handling.web/core/components/cycles/active-cycle/progress.tsx (3)
8-8
: LGTM!The import of the
Loader
component is necessary for the updated rendering logic that includes a loading state whencycle
is null.
18-18
: LGTM!Changing the
cycle
prop's type toICycle | null
allows the component to handle scenarios where thecycle
data might be absent, improving robustness.
Line range hint
27-102
:
LGTM!The conditional checks for the
cycle
prop and the updated rendering logic ensure that operations dependent oncycle
are only executed when it is not null, preventing potential runtime errors and improving user experience.web/core/components/cycles/active-cycle/productivity.tsx (3)
5-5
: LGTM!The import of the
Loader
component is necessary for the updated rendering logic that includes a loading state whencycle
is null.
18-18
: LGTM!Changing the
cycle
prop's type toICycle | null
allows the component to handle scenarios where thecycle
data might be absent, improving robustness.
Line range hint
54-142
:
LGTM!The conditional checks for the
cycle
prop and the updated rendering logic ensure that operations dependent oncycle
are only executed when it is not null, preventing potential runtime errors and improving user experience.web/core/components/cycles/active-cycle/cycle-stats.tsx (5)
38-38
: LGTM!The destructuring of props to include
cycleId
is correct and necessary for accessing the new prop.
67-68
: LGTM!The useSWR hook's condition to check for
cycleId
before fetching data is a good practice to prevent potential errors from undefined values.
75-78
: LGTM!The
loadMoreIssues
function's early return ifcycleId
is not provided is a good practice to prevent potential errors from undefined values.
Line range hint
82-322
:
LGTM!The conditional rendering based on the presence of both
cycle
andcycleId
enhances user experience by providing feedback when the data is loading or unavailable.
33-34
: LGTM! Ensure consistent usage of nullable and optional props.The changes to make
cycle
nullable and introduce an optionalcycleId
prop are appropriate for improved error handling and flexibility.However, ensure that all usages of these props throughout the codebase handle their nullability and optionality correctly.
web/core/store/cycle.store.ts (2)
35-36
: LGTM!The addition of the
currentProjectActiveCycle
property to theICycleStore
interface enhances its capability to manage project cycles by tracking the currently active cycle.
105-106
: LGTM!The implementation of the
currentProjectActiveCycle
computed property in theCycleStore
class streamlines the process of obtaining cycle information related to the current project.Also applies to: 235-239
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- web/core/components/cycles/active-cycle/cycle-stats.tsx (4 hunks)
Additional comments not posted (5)
web/core/components/cycles/active-cycle/cycle-stats.tsx (5)
33-34
: LGTM! Improved flexibility and error handling.The changes to the
cycle
prop and the addition of thecycleId
prop enhance the component's flexibility and error handling.
38-38
: LGTM! Destructuring props updated.The destructuring of props now includes
cycleId
, ensuring correct operation even if thecycle
object is null.
67-68
: LGTM! useSWR hook updated.The
useSWR
hook now checks forcycleId
instead ofcycle.id
, ensuring correct data fetching even if thecycle
object is null.
72-72
: LGTM! cycleIssueDetails updated.The
cycleIssueDetails
variable now checks forcycleId
, ensuring correct handling of the absence of acycle
object.
82-82
: LGTM! Improved conditional rendering.The return statement now conditionally renders either the main content or a loader based on the presence of both
cycle
andcycleId
, improving user experience.Also applies to: 319-322
There was a problem hiding this 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
Outside diff range, codebase verification and nitpick comments (1)
packages/types/src/workspace.d.ts (1)
222-223
: Consider defining a more specific type forcompletion_chart
.The
completion_chart
property is currently typed asany
, which may lead to type safety issues. Consider defining a more specific type.- completion_chart: any; + completion_chart: TCompletionChart; // Define TCompletionChart appropriately
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- packages/types/src/cycle/cycle.d.ts (1 hunks)
- packages/types/src/workspace.d.ts (1 hunks)
- web/core/services/cycle.service.ts (1 hunks)
Additional comments not posted (7)
packages/types/src/cycle/cycle.d.ts (2)
64-66
: LGTM!The new
IProjectDetails
interface is straightforward and correctly defined.
67-92
: LGTM! But verify the interface usage in the codebase.The addition of
project_detail
to theICycle
interface is approved.However, ensure that all usages of
ICycle
handle the newproject_detail
property.packages/types/src/workspace.d.ts (2)
202-210
: LGTM!The
IWorkspaceActiveCyclesResponse
interface is well-defined and includes all necessary properties for pagination and cycle management.
213-220
: LGTM!The
IWorkspaceProgressResponse
interface is well-defined and includes all necessary properties for tracking progress metrics.web/core/services/cycle.service.ts (3)
45-60
: LGTM! But verify the API endpoint.The
workspaceActiveCycles
method is well-defined and includes error handling.However, ensure that the API endpoint and parameters are correct.
Verification successful
Let's correct the file type specification in the
rg
command and re-run the script to verify the API endpoint and parameters.
Verified: The API endpoint and parameters are correct.
The
workspaceActiveCycles
method correctly defines the API endpoint/api/workspaces/${workspaceSlug}/active-cycles/
and includes the necessary parameterscursor
andper_page
.
- web/core/services/cycle.service.ts: Lines 45-60
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the API endpoint and parameters for `workspaceActiveCycles` method. # Test: Search for the method usage. Expect: Correct API endpoint and parameters. rg --type typescript -A 5 $'workspaceActiveCycles'Length of output: 84
Script:
#!/bin/bash # Description: Verify the API endpoint and parameters for `workspaceActiveCycles` method. # Test: Search for the method usage. Expect: Correct API endpoint and parameters. rg --glob '*.ts' -A 5 $'workspaceActiveCycles'Length of output: 1350
18-31
: LGTM! But verify the API endpoint.The
workspaceActiveCyclesAnalytics
method is well-defined and includes error handling.However, ensure that the API endpoint and parameters are correct.
33-43
: LGTM! But verify the API endpoint.The
workspaceActiveCyclesProgress
method is well-defined and includes error handling.However, ensure that the API endpoint and parameters are correct.
Verification successful
The API endpoint structure appears consistent with other similar methods in the same service file.
The
workspaceActiveCyclesProgress
method's API endpoint and parameters align with the patterns used in other methods withincycle.service.ts
. No issues were found in the endpoint structure.
cycle.service.ts
: Consistent API endpoint structure.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the API endpoint and parameters for `workspaceActiveCyclesProgress` method. # Test: Search for the method usage. Expect: Correct API endpoint and parameters. rg --type typescript -A 5 $'workspaceActiveCyclesProgress'Length of output: 92
Script:
#!/bin/bash # Description: Verify the API endpoint and parameters for `workspaceActiveCyclesProgress` method. # Test: Search for the method usage. Expect: Correct API endpoint and parameters. rg -A 10 'workspaceActiveCyclesProgress'Length of output: 788
Script:
#!/bin/bash # Description: Verify the API endpoint and parameters for `workspaceActiveCyclesProgress` method. # Test: Search for the method usage across the entire codebase. Expect: Correct API endpoint and parameters. rg 'workspaceActiveCyclesProgress'Length of output: 108
Script:
#!/bin/bash # Description: Verify the API endpoint structure in `cycle.service.ts`. # Test: Search for other methods in `cycle.service.ts` that interact with similar API endpoints. rg 'get\(`/api/workspaces/' web/core/services/cycle.service.tsLength of output: 492
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/types/src/cycle/cycle.d.ts (4 hunks)
- packages/types/src/workspace.d.ts (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- packages/types/src/cycle/cycle.d.ts
- packages/types/src/workspace.d.ts
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- web/core/components/cycles/active-cycle/cycle-stats.tsx (5 hunks)
- web/core/components/cycles/active-cycle/root.tsx (4 hunks)
Files skipped from review as they are similar to previous changes (2)
- web/core/components/cycles/active-cycle/cycle-stats.tsx
- web/core/components/cycles/active-cycle/root.tsx
Summary
Fixed the loading state on active cycle to unblock the independent components
[WEB-2007]
Summary by CodeRabbit
New Features
Bug Fixes
Documentation