fix: show correct unit type for manage projects list screen #2344
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
npm warn config production Use WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (3)
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 (
|
| Number(project.countPlanted), | ||
| 1 | ||
| )}{' '} | ||
| {tCommon('tree', { count: Number(project.countPlanted) })} | ||
| {project.unitType === 'm2' | ||
| ? tCommon(project.unitType) | ||
| : tCommon('tree', { count: Number(project.countPlanted) })} | ||
| </p> |
There was a problem hiding this comment.
-
I suggest simply using something like
tCommon(project.unitType, {formattedCount, count})
instead of handling different unitTypes in different ways. Make sure to not affect places that use the existing translation keys - you probably need new keys. -
API needs to be updated to return
countPlanted/areaConservedetc. forunitType = 'm2'. This should consider conservation projects as well. Discuss this with @jmiridis -
Show the total units value if unitType is 'trees' or 'm2'.
-
Update the type of project to reflect the API response. Be specific about the values
unitTypecan take.
- Simplified translation key structure for unit types (tree, m2). - Updated `TreeProperties` and `ConservProperties` interfaces to improve type safety. - Refined count and formatted count logic using `useMemo` and conditional rendering. - Enhanced translation key lookup for unit types using dynamic `unitTypes` object.
![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
🧹 Nitpick comments (1)
src/features/common/types/project.d.ts (1)
121-133: Consider reusing the Units type for ConservProperties.The separation of tree and conservation properties is well-structured. However, the
ConservPropertiesinterface could potentially reuse theUnitstype to maintain consistency and DRY principle.Consider this alternative for
ConservProperties:export interface ConservProperties extends Properties { - unitsContributed: { - m2: number; - }; - unitsTargeted: { - m2: number; - }; + unitsContributed: Pick<Units, 'm2'>; + unitsTargeted: Pick<Units, 'm2'>; }This approach:
- Maintains consistency with
TreeProperties- Reuses the existing
Unitstype- Still enforces the m² requirement through
Pick
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/static/locales/en/common.jsonis excluded by!**/*.json
📒 Files selected for processing (2)
src/features/common/types/project.d.ts(2 hunks)src/features/user/ManageProjects/ProjectsContainer.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/features/user/ManageProjects/ProjectsContainer.tsx
🔇 Additional comments (1)
src/features/common/types/project.d.ts (1)
104-104: LGTM! Verify the impact of this type restriction.The change from
stringto literal union type improves type safety by restrictingunitTypeto valid values.Let's verify all usages of this property to ensure compatibility:
✅ Verification successful
Type restriction is safe and well-aligned with existing usage
The change from
stringto'tree' | 'm2'is safe as all implementations already use these specific literals. The restriction improves type safety without breaking existing code:
- Form controls in
BasicDetails.tsxenforce the same literalsProjectSnippetcomponent uses compatible type definitions- Type definitions across the codebase are consistent with this change
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for unitType usage in the codebase rg "unitType" --type ts --type tsxLength of output: 62
Script:
#!/bin/bash # Search for unitType usage without file type restrictions rg "unitType" -l # Use ast-grep to find property access and type annotations ast-grep --pattern 'unitType:' ast-grep --pattern '.unitType'Length of output: 2848
Script:
#!/bin/bash # Check type definitions in related .d.ts files cat src/features/common/types/myForest.d.ts src/features/common/types/user.d.ts src/features/common/types/payments.d.ts src/features/common/types/map.d.ts src/features/common/types/redeem.d.ts # Check implementation in key components cat src/features/user/ManageProjects/components/BasicDetails.tsx src/features/projectsV2/ProjectSnippet/index.tsxLength of output: 54332
#FIxes -
This PR ensures the correct unit type is used for restoration projects (unit type: m²; purpose: trees) in the "Manage Project" section.