Fix: Prevent negative number input in the form fields of "Add Triage" #9433
Conversation
WalkthroughThe pull request introduces a validation enhancement in the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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. |
![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 (2)
src/components/Facility/TriageForm.tsx (2)
Line range hint
52-90: Consider consolidating validation logicThe form currently has validation in both
handleFormFieldChangeandvalidateForm. Consider extracting common validation logic into a shared utility function to maintain consistency and avoid duplication.const validateNumericField = (value: string, fieldName: string): string | null => { if (value === "") return null; if (!/^\d*$/.test(value)) return "Only non-negative whole numbers are allowed"; if (Number(value) < 0) return "Value must be greater than or equal to 0"; return null; };🧰 Tools
🪛 eslint
[error] 192-193: Delete
⏎····(prettier/prettier)
Line range hint
280-332: Add aria-label attributes for better accessibilityThe numeric input fields should have appropriate aria-labels to improve accessibility for screen readers.
<TextFormField name="num_patients_visited" type="number" + aria-label="Number of patients visited in triage" label="Patients Visited in Triage" value={state.form.num_patients_visited} onChange={handleFormFieldChange} error={state.errors.num_patients_visited} />🧰 Tools
🪛 eslint
[error] 192-193: Delete
⏎····(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Facility/TriageForm.tsx(1 hunks)
🧰 Additional context used
🪛 eslint
src/components/Facility/TriageForm.tsx
[error] 192-193: Delete ⏎····
(prettier/prettier)
|
|
||
| if (!/^\d*$/.test(event.value as string) && event.name !== "entry_date") | ||
| return; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enhancing the input validation feedback
While the regex validation successfully prevents negative numbers, it silently fails without informing users why their input was rejected. Consider adding immediate visual feedback.
const handleFormFieldChange = (event: FieldChangeEvent<unknown>) => {
-
- if (!/^\d*$/.test(event.value as string) && event.name !== "entry_date")
- return;
+ if (event.name !== "entry_date") {
+ if (!/^\d*$/.test(event.value as string)) {
+ dispatch({
+ type: "set_error",
+ errors: {
+ ...state.errors,
+ [event.name]: "Only non-negative whole numbers are allowed"
+ }
+ });
+ return;
+ }
+ // Clear error when valid input is entered
+ dispatch({
+ type: "set_error",
+ errors: {
+ ...state.errors,
+ [event.name]: ""
+ }
+ });
+ }📝 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.
| if (!/^\d*$/.test(event.value as string) && event.name !== "entry_date") | |
| return; | |
| if (event.name !== "entry_date") { | |
| if (!/^\d*$/.test(event.value as string)) { | |
| dispatch({ | |
| type: "set_error", | |
| errors: { | |
| ...state.errors, | |
| [event.name]: "Only non-negative whole numbers are allowed" | |
| } | |
| }); | |
| return; | |
| } | |
| // Clear error when valid input is entered | |
| dispatch({ | |
| type: "set_error", | |
| errors: { | |
| ...state.errors, | |
| [event.name]: "" | |
| } | |
| }); | |
| } |
🧰 Tools
🪛 eslint
[error] 192-193: Delete ⏎····
(prettier/prettier)
💡 Codebase verification
Numeric validation is incomplete for form fields
The current validation /^\d*$/ is applied conditionally to all fields except "entry_date", but the form contains multiple numeric fields that should be explicitly validated:
- num_patients_visited
- num_patients_home_quarantine
- num_patients_isolation
- num_patient_referred
- num_patient_confirmed_positive
The current implementation might allow invalid input in these numeric fields since the validation is tied to the event handler rather than being specific to numeric fields.
🔗 Analysis chain
Verify the validation coverage
Let's ensure the validation is properly applied to all numeric fields in the form.
Let me gather more information about the form fields to better understand the validation context.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all numeric input fields in the component
ast-grep --pattern 'TextFormField
name="num_$_"
type="number"'
# Check for any other numeric validation patterns
rg "test\(.*value.*\)" --type ts
Length of output: 420
Script:
#!/bin/bash
# Find all form fields in TriageForm.tsx
ast-grep --pattern 'TextFormField {
$$$
}' src/components/Facility/TriageForm.tsx
# Also check for any field type definitions or interfaces
ast-grep --pattern 'interface $_ {
$$$
}' src/components/Facility/TriageForm.tsx
# Look for form field declarations
rg "name=" src/components/Facility/TriageForm.tsx
Length of output: 518
🧰 Tools
🪛 eslint
[error] 192-193: Delete ⏎····
(prettier/prettier)
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Screencast from 2024-12-15 01-27-09.webm
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes