Analytics updates #2326
Analytics updates #2326
Conversation
📝 WalkthroughWalkthroughThe pull request introduces significant updates to the Changes
Possibly related PRs
Suggested labels
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: 1
🧹 Outside diff range and nitpick comments (3)
platform/src/common/components/Modal/dataDownload/components/CustomFields.jsx (3)
7-17: Consider adding input validation for textFormat parameterThe
formatNamefunction looks solid, but we could make it more robust with a few tweaks.Consider this enhanced implementation:
const formatName = (name, textFormat = 'lowercase') => { if (typeof name !== 'string' || !name) return name; + if (!['uppercase', 'lowercase'].includes(textFormat)) { + console.warn(`Invalid textFormat: ${textFormat}. Defaulting to lowercase.`); + textFormat = 'lowercase'; + } const formatted = name.replace(/[_-]/g, ' '); return textFormat === 'uppercase' ? formatted.toUpperCase() : formatted; };
44-61: Consider making the formatting rules configurableThe hardcoded check for
id !== 'organization'makes the component less reusable. Consider making this more flexible.A few approaches to consider:
- Add a
skipFormattingprop to specify fields that should skip formatting- Add a
formattingRulesprop to define custom formatting per field typeExample implementation:
// In props skipFormatting: PropTypes.arrayOf(PropTypes.string) // In handleSelect const shouldFormat = !skipFormatting?.includes(id);
131-147: Excellent PropTypes enhancement!The improved type definitions make the component more reliable and self-documenting. 👍
Consider adding defaultProps for optional values:
CustomFields.defaultProps = { field: false, options: [], edit: false, useCalendar: false, textFormat: 'lowercase' };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
platform/src/common/components/Modal/dataDownload/components/CustomFields.jsx(3 hunks)
🔇 Additional comments (1)
platform/src/common/components/Modal/dataDownload/components/CustomFields.jsx (1)
52-56: Verify fix for organization selection issue
The special handling for organization names looks correct, but let's verify the fix.
✅ Verification successful
Let me search for more context about the CustomFields component and organization name handling to verify this change properly.
I need to see the full implementation of these components to understand how the organization field is being handled and verify if skipping the formatting for organization names is the correct approach.
Special handling for organization names is correctly implemented
The code correctly handles organization names by skipping the formatting for the 'organization' field. This is the right approach because:
- Organization names are used as identifiers in API calls (
fetchSitesSummaryand download data requests) - The original organization names are preserved in both the dropdown and form data
- The special case is consistently applied in both the
handleSelectfunction and the input field rendering
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for related organization field implementations
rg -l "CustomFields.*organization" --type js
# Look for any existing organization name formatting
rg "formatName.*organization" --type js
Length of output: 88
Script:
#!/bin/bash
# Search for CustomFields component and its usage
fd -e jsx -e js | xargs rg -l "CustomFields"
# Look for organization-related code in the modal components
fd -e jsx -e js | xargs rg "organization.*name" -A 2 -B 2
# Search for formatName function implementation
ast-grep --pattern 'function formatName($_) { $$$ }'
Length of output: 3450
Script:
#!/bin/bash
# Let's look at the CustomFields component implementation
cat platform/src/common/components/Modal/dataDownload/components/CustomFields.jsx
# And check the DataDownload component to understand the context
cat platform/src/common/components/Modal/dataDownload/modules/DataDownload.jsx
Length of output: 18418
| value={ | ||
| id === 'organization' | ||
| ? selectedOption.name | ||
| : formatName(selectedOption.name, textFormat) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract repeated formatting logic to avoid duplication
The formatting condition is duplicated across the component. Let's make it DRY.
Consider extracting the formatting logic:
+ const formatFieldValue = (value, fieldId) => {
+ return fieldId === 'organization' ? value : formatName(value, textFormat);
+ };
// Then use it like:
value={formatFieldValue(selectedOption.name, id)}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
- please take note of this review comment.
- But also please ensure that you maintain our earlier UPPER CASE only organisation name implementation. Please do NOT change it.
cc: @Codebmk
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Upper casing is still maintained @Baalmart
There was a problem hiding this comment.
will push a commit on this comment
|
New next-platform changes available for preview here |
OchiengPaul442
added
work in progress
ready for review
and removed
work in progress
labels
Summary of Changes (What does this PR do?)
Summary by CodeRabbit
New Features
formatNamefunction for improved name formatting.Bug Fixes
Documentation