-
Couldn't load subscription status.
- Fork 20
fix: add validations to rangeDate filter #806
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: add validations to rangeDate filter #806
Conversation
Signed-off-by: Carlos Feria <[email protected]>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR enhances the DateRangeFilter components by integrating validation logic into the PatternFly DatePicker inputs to disable invalid date selections, enforce that the end date follows the start date, and provide consistent error messaging. Sequence diagram for date range filter validation and error handlingsequenceDiagram
actor User
participant "DateRangeFilter Component"
participant "DatePicker"
User->>"DateRangeFilter Component": Selects "From" date
"DateRangeFilter Component"->>"DatePicker": Updates "From" date
User->>"DateRangeFilter Component": Selects "To" date
"DateRangeFilter Component"->>"DatePicker": Applies toValidator
"DatePicker"->>"DateRangeFilter Component": Returns validation result
alt Invalid "To" date
"DateRangeFilter Component"->>User: Shows error 'The "to" date must be after the "from" date'
else Valid dates
"DateRangeFilter Component"->>User: Applies filter
end
Class diagram for updated DateRangeFilter componentclassDiagram
class DateRangeFilter {
- from: Date
- to: Date
+ onFromDateChange(date: Date)
+ onToDateChange(date: Date)
+ toValidator(date: Date): string
}
class DatePicker {
+ invalidFormatText: string
+ validators: [function]
}
DateRangeFilter "1" --|> "2" DatePicker: uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Extract the toValidator into a shared util to avoid duplicating validation logic across FilterPanel and FilterToolbar.
- Consider implementing a complementary ‘from’ date validator to ensure the from date is always before the selected to date.
- Avoid hard-coding the error messages (‘Invalid date’ and the validator text); extract them into shared constants or i18n resources.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Extract the toValidator into a shared util to avoid duplicating validation logic across FilterPanel and FilterToolbar.
- Consider implementing a complementary ‘from’ date validator to ensure the from date is always before the selected to date.
- Avoid hard-coding the error messages (‘Invalid date’ and the validator text); extract them into shared constants or i18n resources.
## Individual Comments
### Comment 1
<location> `client/src/app/components/FilterPanel/DateRangeFilter.tsx:76-79` </location>
<code_context>
}
};
+ const toValidator = (date: Date) =>
+ from && isValidDate(from) && date >= from
+ ? ""
+ : 'The "to" date must be after the "from" date';
+
return (
</code_context>
<issue_to_address>
**suggestion:** Consider handling cases where 'from' is not a valid date in the validator.
If 'from' is missing or invalid, consider skipping validation or returning an empty string to avoid unnecessary error messages.
```suggestion
const toValidator = (date: Date) => {
if (!from || !isValidDate(from)) {
// Skip validation if 'from' is missing or invalid
return "";
}
return date >= from
? ""
: 'The "to" date must be after the "from" date';
};
```
</issue_to_address>
### Comment 2
<location> `client/src/app/components/FilterToolbar/DateRangeFilter.tsx:119-122` </location>
<code_context>
}
};
+ const toValidator = (date: Date) =>
+ from && isValidDate(from) && date >= from
+ ? ""
+ : 'The "to" date must be after the "from" date';
+
return (
</code_context>
<issue_to_address>
**suggestion:** Validator logic may not handle cases where 'from' is missing or invalid.
Consider skipping validation or returning an empty string when 'from' is missing or invalid to avoid confusing error messages for users.
```suggestion
const toValidator = (date: Date) => {
if (!from || !isValidDate(from)) {
// Skip validation if 'from' is missing or invalid
return "";
}
return date >= from
? ""
: 'The "to" date must be after the "from" date';
};
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Carlos Feria <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #806 +/- ##
==========================================
- Coverage 57.76% 57.72% -0.05%
==========================================
Files 163 163
Lines 2872 2874 +2
Branches 654 656 +2
==========================================
Hits 1659 1659
- Misses 976 977 +1
- Partials 237 238 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fixes
Fixes: #393, #394, and:
JIRA fixes:
What changes
validatorsto each of the date picker components. That should avoid the user to select dates that wrong.Screencast.From.2025-10-20.15-13-17.mp4
Summary by Sourcery
Add date format validation and range enforcement to the DateRangeFilter components in both the filter panel and toolbar.
Enhancements: