[WEB-2681] fix: module progress indicator#5842
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes involve the Changes
Suggested labels
Suggested reviewers
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 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: 1
🧹 Outside diff range and nitpick comments (1)
web/core/components/modules/module-list-item.tsx (1)
42-43: Suggestion: Add explanatory comment for progress calculationWhile the new calculation is straightforward, it would be beneficial to add a brief comment explaining the rationale behind including both completed and cancelled issues in the progress calculation. This will enhance code maintainability and clarify the business logic for future developers.
Consider adding a comment like this:
// Calculate completion percentage based on completed and cancelled issues // relative to total issues, as per updated business requirements const completionPercentage = ((moduleDetails.completed_issues + moduleDetails.cancelled_issues) / moduleDetails.total_issues) * 100;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- web/core/components/modules/module-list-item.tsx (2 hunks)
🧰 Additional context used
🔇 Additional comments (2)
web/core/components/modules/module-list-item.tsx (2)
16-17: LGTM: Import removal aligns with PR objectiveThe removal of the
useProjectEstimatesimport is consistent with the PR's goal of calculating progress based on issue state rather than estimate points. This change simplifies the component's dependencies and aligns with the new implementation.
Line range hint
1-114: Summary: Changes successfully implement PR objectivesThe modifications to the
ModuleListItemcomponent successfully implement the PR objectives by:
- Removing the dependency on estimate points for progress calculation.
- Simplifying the completion percentage calculation to use issue states.
- Maintaining the overall structure and functionality of the component.
These changes improve code maintainability and align the progress calculation with the intended design. The component should now accurately reflect module progress based on issue states rather than estimate points.
🧰 Tools
🪛 Biome
[error] 45-45: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
| const completionPercentage = | ||
| ((moduleDetails.completed_issues + moduleDetails.cancelled_issues) / moduleDetails.total_issues) * 100; |
There was a problem hiding this comment.
💡 Codebase verification
Verification Completed: Simplified Calculation Consistent Across Codebase
The updated completion percentage calculation using completed_issues, cancelled_issues, and total_issues is consistently applied across the codebase, aligning with the PR objective to simplify and enhance maintainability.
However, there are still several references to estimate_point and related variables. Please ensure that no progress calculations are still dependent on estimate points to maintain uniform behavior.
- Review instances of
estimate_pointto confirm they do not impact progress calculations. - If estimate-based calculations are no longer needed, consider removing or refactoring them accordingly.
🔗 Analysis chain
LGTM: Simplified completion percentage calculation
The new calculation for completionPercentage correctly implements the PR objective by basing progress on issue state (completed and cancelled issues) rather than estimate points. This change simplifies the logic and improves maintainability.
To ensure consistency across the codebase, please run the following script:
This will help identify any other locations where similar calculations might need to be updated for consistency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other occurrences of module progress calculation
# Search for similar progress calculations in other files
rg --type typescript --type javascript -e 'completed_issues' -e 'cancelled_issues' -e 'total_issues' --glob '!**/module-list-item.tsx'
# Search for any remaining uses of estimate points for progress calculation
rg --type typescript --type javascript -e 'estimate_point' -e 'useProjectEstimates'
Length of output: 282
Script:
#!/bin/bash
# Description: Check for other occurrences of module progress calculation with corrected file types
# Search for similar progress calculations in other files
rg --type ts --type tsx --type js --type jsx -e 'completed_issues' -e 'cancelled_issues' -e 'total_issues' --glob '!**/module-list-item.tsx'
# Search for any remaining uses of estimate points for progress calculation
rg --type ts --type tsx --type js --type jsx -e 'estimate_point' -e 'useProjectEstimates'
Length of output: 280
Script:
#!/bin/bash
# Description: Check for other occurrences of module progress calculation using file extensions
# Search for similar progress calculations in other files
rg -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' 'completed_issues|cancelled_issues|total_issues' --glob '!**/module-list-item.tsx'
# Search for any remaining uses of estimate points for progress calculation
rg -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' 'estimate_point|useProjectEstimates'
Length of output: 29494
4 participants
Changes:
This PR fixes the module progress indicator. Previously, when the estimate points were enabled, the progress was being calculated based on those points. I have made the necessary changes to ensure that progress is now calculated based on the issue state as intended.
Reference:
[WEB-2681]
Summary by CodeRabbit
New Features
Bug Fixes