changed latitude longitude type to a number in facility create form #10147
Conversation
WalkthroughThe pull request introduces type changes for latitude and longitude across multiple files, transitioning from string to number representations. This modification affects the validation functions, facility form schema, and facility model interface. The changes aim to improve type consistency and ensure that geographical coordinates are handled as numeric values throughout the application. Changes
Possibly related PRs
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 (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Deploying care-fe with
|
| Latest commit: |
b6cfe78
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e3d73638.care-fe.pages.dev |
| Branch Preview URL: | https://khavinshankar-fix-facility-l.care-fe.pages.dev |
![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 (2)
src/common/validation.tsx (2)
9-11: Avoid redundant numeric conversion.
Since the parameter is already typed asnumber, theNumber(latitude)call may be redundant. Also consider how the function should handle potentialNaNorundefinedvalues if this function is used in a broader context.
13-15: Mirror change for longitude.
As with latitude, removing theNumber(longitude)call and adding explicit handling forNaNorundefinedcould make the validation more robust.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/common/validation.tsx(1 hunks)src/components/Facility/FacilityForm.tsx(4 hunks)src/components/Facility/models.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (6)
src/components/Facility/models.tsx (1)
21-22: Latitude/longitude changed to optional numbers.
These type changes consistently align with the rest of your PR. Good job ensuring these fields match their numeric usage.src/components/Facility/FacilityForm.tsx (5)
78-80: Updated schema for latitude.
Switching fromz.string()toz.number()with an optional constraint is appropriate. This ensures the schema truly reflects numeric data.
82-84: Updated schema for longitude.
Same reasoning as latitude—this maintains consistency in your form handling for numeric coordinates.
101-102: Default values changed toundefined.
This is a sensible revision—usingundefinedrather than an empty string will help avoid accidental string-number conflicts in your form.
166-167: Setting coordinates from geolocation.
Storingposition.coords.latitudeandposition.coords.longitudeas numbers aligns with your updated schema. Consider verifying they fall within expected bounds or handle edge cases (e.g. missing coords).
230-231: Assigning facility data.
Loadinglatitudeandlongitudeas numbers preserves type consistency throughout the component. No issues spotted.
CARE
|
||||||||||||||||||||||||||||
| Project |
CARE
|
| Branch Review |
khavinshankar/fix/facility-lat-lon-type-issue
|
| Run status |
|
| Run duration | 03m 20s |
| Commit |
|
| Committer | Khavin Shankar |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
6
|
| View all changes introduced in this branch ↗︎ | |
|
When you type 12.14701 as latitude, it fails and error message is not proper |
|
Reminder: To add the "needs testing" label, comment "ready for testing" on this PR. |
|
@Nihal fixed |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Facility/FacilityForm.tsx(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (4)
src/components/Facility/FacilityForm.tsx (4)
101-102: LGTM! Appropriate default values for optional number fields.The change from empty strings to
undefinedaligns well with the number type and optional nature of these fields.
166-167: LGTM! Proper handling of geolocation coordinates.The values from
position.coordsare already numbers, making this a clean implementation.
78-84: Verify validation functions handle decimal precision correctly.Given the reported issue with the value "12.14701", ensure that the validation functions in
@/common/validation.tsxproperly handle decimal precision for geographical coordinates.✅ Verification successful
Validation functions handle decimal precision correctly
The validation implementation uses pure numerical comparisons without any rounding or truncation, ensuring precise handling of decimal coordinates like "12.14701". The redundant
Number()conversion can be safely removed since the schema now provides number types.
src/common/validation.tsx: Consider removing redundantNumber()calls invalidateLatitudeandvalidateLongitudefunctions🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check validation function implementation for decimal handling # Search for validation function implementations echo "Searching for latitude/longitude validation functions..." rg -A 10 "export const validate(Latitude|Longitude)"Length of output: 1018
Script:
#!/bin/bash # Search for valueIsBetween implementation echo "Searching for valueIsBetween implementation..." rg "valueIsBetween.*=" -A 5Length of output: 486
230-231: Verify API response types for geographical coordinates.Ensure that the API consistently returns numbers for latitude and longitude fields to maintain type safety.
✅ Verification successful
Latitude and longitude types are properly handled throughout the codebase
The implementation maintains type safety through:
- Number type definitions in the facility interface
- Explicit number validation in the form schema
- Range validation (-90/90 for latitude, -180/180 for longitude)
- Proper type conversion in form handlers
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check API response type consistency # Search for facility type definitions and API response handling echo "Searching for facility interface definitions..." rg -t typescript "interface.*Facility.*{" -A 15 echo "Searching for API response handling..." rg -t typescript "getPermittedFacility.*" -A 5Length of output: 433
Script:
#!/bin/bash # Search for facility-related types and interfaces echo "=== Searching for facility type definitions ===" rg -t ts -t tsx "interface.*Facility" -A 15 echo -e "\n=== Searching for types with geographical coordinates ===" rg -t ts -t tsx "latitude|longitude" -B 2 -A 2 echo -e "\n=== Searching for API calls and response handling ===" rg -t ts -t tsx "(get|fetch).*Facility" -B 2 -A 5Length of output: 589
Script:
#!/bin/bash # Search for TypeScript interfaces and types echo "=== Searching for facility interfaces with ast-grep ===" ast-grep --pattern 'interface $_ { $$$ latitude$$$ longitude$$$ $$$ }' echo -e "\n=== Searching for coordinate-related code ===" rg "latitude|longitude" --no-ignore-vcs -B 2 -A 2 echo -e "\n=== Looking at the specific component file ===" find . -name "FacilityForm.tsx" -exec cat {} \;Length of output: 41178
| type="number" | ||
| onChange={(e) => { | ||
| form.setValue("latitude", Number(e.target.value)); | ||
| }} |
There was a problem hiding this comment.
Fix decimal precision handling in coordinate inputs.
The current implementation using type="number" might cause issues with decimal precision (like the reported issue with "12.14701"). Consider these improvements:
Apply this diff to fix the precision issues:
- type="number"
+ type="number"
+ step="any"
onChange={(e) => {
- form.setValue("latitude", Number(e.target.value));
+ const value = e.target.value;
+ form.setValue(
+ "latitude",
+ value ? parseFloat(value) : undefined
+ );
}}Apply the same changes to the longitude input.
Alternatively, consider using type="text" with a pattern for stricter validation:
- type="number"
+ type="text"
+ pattern="^-?\d*\.?\d*$"
onChange={(e) => {
- form.setValue("latitude", Number(e.target.value));
+ const value = e.target.value;
+ form.setValue(
+ "latitude",
+ value ? parseFloat(value) : undefined
+ );
}}Also applies to: 495-498
|
LGTM |
rithviknishad
deleted the
khavinshankar/fix/facility-lat-lon-type-issue
branch
|
@khavinshankar Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Summary by CodeRabbit