[Analytics] Format organisation dropdown text to uppercase #2308
Conversation
|
Warning Rate limit exceeded@Codebmk has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 22 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve significant updates to the Changes
Possibly related PRs
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: 2
🧹 Outside diff range and nitpick comments (3)
platform/src/common/components/Dropdowns/OrganizationDropdown.jsx (1)
Line range hint
21-29: Align formatting logic across utility functionsThe
getAbbreviationfunction still maintains special case handling for "airqo" whileformatGroupNamedoesn't. Consider standardizing the approach across all text formatting utilities.Consider updating the abbreviation logic to match the new formatting approach:
const getAbbreviation = (name) => { if (!name || typeof name !== 'string') return 'NA'; const trimmedName = name.trim(); if (!trimmedName) return 'NA'; - return trimmedName.toLowerCase() === 'airqo' - ? 'AQ' - : trimmedName.slice(0, 2).toUpperCase(); + return trimmedName.slice(0, 2).toUpperCase(); };platform/src/common/components/Modal/dataDownload/components/CustomFields.jsx (2)
11-17: Improve readability of the formatName functionWhile the logic is correct, we can make it more maintainable by reducing nesting and avoiding code duplication.
const formatName = (name, textFormat = 'lowercase') => { if (!name) return ''; - return typeof name === 'string' - ? textFormat === 'uppercase' - ? name.toUpperCase().replace(/_/g, ' ').replace(/-/g, ' ') - : name.replace(/_/g, ' ').replace(/-/g, ' ') - : capitalize(name); + if (typeof name !== 'string') return capitalize(name); + + const formatted = name.replace(/[_-]/g, ' '); + return textFormat === 'uppercase' ? formatted.toUpperCase() : formatted; };
75-78: Consider memoizing formatted text valuesSince formatName is called multiple times with the same inputs, consider memoizing the results to optimize performance.
+const formattedBtnText = useMemo(() => { + return formatName(btnText || selectedOption.name, textFormat); +}, [btnText, selectedOption.name, textFormat]); +const formattedOptionName = useCallback((name) => { + return formatName(name, textFormat); +}, [textFormat]); -btnText={formatName(btnText, textFormat) || formatName(selectedOption.name, textFormat)} +btnText={formattedBtnText} -<span>{formatName(option.name, textFormat)}</span> +<span>{formattedOptionName(option.name)}</span>Also applies to: 91-92
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
platform/src/common/components/Dropdowns/OrganizationDropdown.jsx(1 hunks)platform/src/common/components/Modal/dataDownload/components/CustomFields.jsx(4 hunks)platform/src/common/components/Modal/dataDownload/modules/DataDownload.jsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- platform/src/common/components/Modal/dataDownload/modules/DataDownload.jsx
🔇 Additional comments (3)
platform/src/common/components/Dropdowns/OrganizationDropdown.jsx (2)
17-18: Consider preserving brand name formatting
While the simplified uppercase conversion makes the code cleaner, it removes special case handling for brand names like "AirQo". Consider maintaining brand-specific formatting to ensure consistent brand representation across the platform.
Let's check if there are any brand-specific formatting requirements:
17-18: Verify text formatting consistency
The uppercase transformation should align with the new textFormat prop in CustomFields component mentioned in the PR description.
Let's verify the text formatting implementation across components:
platform/src/common/components/Modal/dataDownload/components/CustomFields.jsx (1)
11-17: Verify consistent text formatting across the application
Let's ensure the new text formatting capabilities are used consistently across other components that display organization names.
Also applies to: 31-31
✅ Verification successful
The previous attempt failed because the --type jsx flag wasn't supported. Let's try without the file type restriction and use AST-based search to find similar formatting functions.
Let's try a more focused search for text formatting functions and organization name handling, specifically in JavaScript/TypeScript files. The previous search was too broad and included vendor files.
Let's narrow our search to the platform/src directory where the file under review is located, and remove the file type restrictions since they're not supported. This should help us find relevant text formatting patterns.
Text formatting is consistently implemented across the application
The codebase analysis shows that text formatting is already handled consistently:
- The
formatNamefunction in CustomFields.jsx is the primary text formatter for organization names and similar data - Other components that need text formatting either:
- Use this component directly
- Follow the same pattern (e.g., OrganizationDropdown.jsx uses identical replace patterns)
- Use standard capitalization for UI elements (first letter uppercase)
No inconsistencies were found in the text formatting implementation across the application.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other components that might need similar text formatting
rg -l "organization.*name" --type jsx
# Look for potential inconsistencies in text casing
rg "toUpperCase|toLowerCase" --type jsx
Length of output: 137
Script:
#!/bin/bash
# Search for other components that might need similar text formatting
rg -l "organization.*name"
# Look for potential inconsistencies in text casing
rg "toUpperCase|toLowerCase"
# Search for similar formatting functions
ast-grep --pattern 'const formatName = ($$$) => { $$$ }'
Length of output: 80466
Script:
#!/bin/bash
# Search for text formatting functions and their usage
rg -A 2 "formatName|toUpperCase|toLowerCase" --type js --type jsx --type ts --type tsx
# Search for organization name formatting
rg -A 2 "organization.*name.*\.(toUpperCase|toLowerCase|replace)" --type js --type jsx --type ts --type tsx
Length of output: 254
Script:
#!/bin/bash
# Search for text formatting functions and their usage without file type restrictions
rg -A 2 "formatName|toUpperCase|toLowerCase" platform/src/
# Search for organization name formatting
rg -A 2 "organization.*name.*\.(toUpperCase|toLowerCase|replace)" platform/src/
# Look for similar text formatting functions
ast-grep --pattern 'const formatName = ($$$) => { $$$ }' platform/src/
Length of output: 14826
| const capitalize = (name) => { | ||
| return name.charAt(0).toUpperCase() + name.slice(1); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to the capitalize function
The function should handle edge cases like null, undefined, or empty strings to prevent runtime errors.
const capitalize = (name) => {
+ if (!name) return '';
+ if (typeof name !== 'string') return name;
return name.charAt(0).toUpperCase() + name.slice(1);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const capitalize = (name) => { | |
| return name.charAt(0).toUpperCase() + name.slice(1); | |
| }; | |
| const capitalize = (name) => { | |
| if (!name) return ''; | |
| if (typeof name !== 'string') return name; | |
| return name.charAt(0).toUpperCase() + name.slice(1); | |
| }; |
| @@ -22,6 +28,7 @@ | |||
| useCalendar = false, | |||
| handleOptionSelect, | |||
| defaultOption, | |||
| textFormat = 'lowercase', | |||
There was a problem hiding this comment.
Add textFormat to PropTypes
The new textFormat prop should be documented in the component's PropTypes for better maintainability and type checking.
CustomFields.propTypes = {
field: PropTypes.bool,
title: PropTypes.string,
options: PropTypes.array,
id: PropTypes.string,
icon: PropTypes.node,
btnText: PropTypes.string,
edit: PropTypes.bool,
useCalendar: PropTypes.bool,
handleOptionSelect: PropTypes.func,
defaultOption: PropTypes.object,
+ textFormat: PropTypes.oneOf(['uppercase', 'lowercase']),
};Committable suggestion skipped: line range outside the PR's diff.
|
New next-platform changes available for preview here |
|
New next-platform changes available for preview here |
1 similar comment
|
New next-platform changes available for preview here |
Summary of Changes (What does this PR do?)
This pull request includes changes to the
platform/src/common/components/Modal/dataDownload/components/CustomFields.jsxandplatform/src/common/components/Dropdowns/OrganizationDropdown.jsxfiles to improve the formatting of names and provide more flexibility in text formatting. The most important changes include modifying theformatNamefunction to support different text formats, updating theCustomFieldscomponent to accept atextFormatprop, and simplifying theformatGroupNamefunction.Improvements to name formatting:
platform/src/common/components/Dropdowns/OrganizationDropdown.jsx: Simplified theformatGroupNamefunction to make the name uppercase and replace underscores or hyphens with spaces.platform/src/common/components/Modal/dataDownload/components/CustomFields.jsx: Modified theformatNamefunction to support different text formats (uppercase and lowercase) and added acapitalizehelper function.Enhancements to
CustomFieldscomponent:platform/src/common/components/Modal/dataDownload/components/CustomFields.jsx: Updated theCustomFieldscomponent to accept atextFormatprop, allowing for dynamic text formatting based on the provided format. [1] [2] [3]platform/src/common/components/Modal/dataDownload/modules/DataDownload.jsx: Set thetextFormatprop to "uppercase" for theCustomFieldscomponent in theDataDownloadmodule.Status of maturity (all need to be checked before merging):
How should this be manually tested?
What are the relevant tickets?
Screenshots (optional)
Summary by CodeRabbit
New Features
CustomFieldscomponent, allowing names to be displayed in uppercase or lowercase.textFormatproperty for the "Network" field in theDataDownloadcomponent.Bug Fixes
formatNamefunction to prevent errors.Documentation
textFormatparameter inCustomFields.