✨ Add date filter and replace dateRange #655
Conversation
Signed-off-by: Carlos Feria <[email protected]>
Reviewer's GuideIntroduce a new "date" filter type and replace all existing dateRange filters with separate before/after date filters across list contexts, update request parameter handling to support the new filter type, and add dedicated DateFilter components and rendering logic. Sequence diagram for applying a date filter in the filter toolbarsequenceDiagram
participant User as actor User
participant FilterToolbar
participant DateFilter
participant getFilterHubRequestParams
User->>FilterToolbar: Selects a date filter
FilterToolbar->>DateFilter: Renders DatePicker
User->>DateFilter: Picks a date
DateFilter->>FilterToolbar: Updates filter value
FilterToolbar->>getFilterHubRequestParams: Prepares request params with date filter
getFilterHubRequestParams->>FilterToolbar: Returns updated params
Class diagram for new and updated filter componentsclassDiagram
class FilterType {
<<enum>>
multiselect
search
numsearch
date
dateRange
autocompleteLabel
}
class DateFilter {
+category
+filterValue
+setFilterValue
+showToolbarItem
+isDisabled
+onDateChange()
}
class DateRangeFilter {
+category
+filterValue
+setFilterValue
+showToolbarItem
+isDisabled
+onDateChange()
}
FilterType <|-- DateFilter
FilterType <|-- DateRangeFilter
class FilterControl {
+category
+props
+render()
}
FilterControl --> DateFilter : uses
FilterControl --> DateRangeFilter : uses
Class diagram for updated filter context interfacesclassDiagram
class IAdvisorySearchContext {
+modifiedBefore
+modifiedAfter
}
class ISbomSearchContext {
+publishedBefore
+publishedAfter
}
class IVulnerabilitySearchContext {
+publishedBefore
+publishedAfter
}
IAdvisorySearchContext o-- FilterType
ISbomSearchContext o-- FilterType
IVulnerabilitySearchContext o-- FilterType
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The two DateFilter implementations in FilterToolbar and FilterPanel are nearly identical—consider consolidating them into a single reusable component to reduce duplication and ensure consistency.
- The date parsing/formatting utilities are duplicated under both FilterToolbar and FilterPanel—extract them into a shared module to avoid code duplication and divergence.
- In getFilterHubRequestParams, add a return or else branch after handling FilterType.date to prevent falling through and unintentionally processing the dateRange logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The two DateFilter implementations in FilterToolbar and FilterPanel are nearly identical—consider consolidating them into a single reusable component to reduce duplication and ensure consistency.
- The date parsing/formatting utilities are duplicated under both FilterToolbar and FilterPanel—extract them into a shared module to avoid code duplication and divergence.
- In getFilterHubRequestParams, add a return or else branch after handling FilterType.date to prevent falling through and unintentionally processing the dateRange logic.
## Individual Comments
### Comment 1
<location> `client/src/app/hooks/table-controls/filtering/getFilterHubRequestParams.ts:146` </location>
<code_context>
},
});
}
+ if (filterCategory.type === "date") {
+ const date = parseAmericanDate(serverFilterValue[0]);
+ pushOrMergeFilter(filters, {
+ field: serverFilterField,
+ operator: filterCategory.operator ?? "=",
</code_context>
<issue_to_address>
Date filter assumes serverFilterValue[0] is always present and valid.
Add a check to ensure serverFilterValue[0] exists and is a valid date before calling parseAmericanDate to prevent downstream errors.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
client/src/app/hooks/table-controls/filtering/getFilterHubRequestParams.ts
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #655 +/- ##
==========================================
- Coverage 57.28% 55.95% -1.33%
==========================================
Files 146 148 +2
Lines 2430 2475 +45
Branches 552 565 +13
==========================================
- Hits 1392 1385 -7
- Misses 833 884 +51
- Partials 205 206 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
carlosthe19916
changed the title
carlosthe19916
changed the title
carlosthe19916
changed the title
|
@carlosthe19916, I just added #712 which we need to support. I think this is a good place to get started to avoid accumulating technical debt. |
|
@gildub yes, good point about the dates. It is an important piece of work and it deserves its own PR |
|
In a separate PR we can revisit/replace |
Signed-off-by: Carlos Feria <[email protected]>
carlosthe19916
force-pushed
the
fix/filter-date
branch
from
e84f150 to
14a6594
Compare
|
@gildub - Gilles can you review this PR please? As this one PR addresses the 3 bugs (Carlos listed above). |
|
The filter date fields for the "Search Page" make sense this way with "Created before" and "Created after" titles. Meanwhile it seems issue https://issues.redhat.com/browse/TC-2402 has not been fully addressed. Per UXD recommendation we should stick to Patternfly standards : 
|
@gildub I think it is important to make a difference between a
Date range filter (main branch): Date filter (this PR): Note: notice that the screenshots above were taken from the List Pages. Both "Search Page" and "SBOM|Advisory" List page render the same filter but in a different layout. IMHO, the Search page breaks all filtering guidelines from Patternfly but it is something we carry from Trustify V1 so we keep it. There were many JIRAS + Github Issues that complained about the Date Range Component TC-2402, TC-2422, TC-2393, #393, #394 Date Range FilterProblems reported on the jiras and issues above:
Advantages:
Date FilterAs a response to all JIRAS and issues, this PR was raised to replace the Date Range Filter and replace it by 2 independent Filters "From date" and "To date" Problems:
Advantages:
What to do?I don't think neither the By choosing one option or the other, we also need to mindful of its limitations as a component. Some options (feel free to suggest other options):
What do you think? |
|
I understand it's currently impossible with built-in filter in the table hook to have two independent filters to share data and be conditionally dependent on each others. This is a limitation seen in Tackle-ui which is using that framework. That's where a Range filter becomes handy and should work with the table hook. Now if the requirements have evolved to give to the user only Single Date filter then need to make that more explicit to the user that the date filters I think there might be a third option, with a custom Date filter with a before or after option. But they would be exclusive. But overall the requirements of what type filters are needed on which page and how they're expected to behave should be decided by the design. |
If I understand correctly, that would mean that either we choose
But how would I select documents that are greater than X date but less than Y date? Seems like it won't be possible. (scenario 3) Somehow this looks similar to what this PR is proposing, the difference is that
I think that is a fair point. If needed we can put on hold this and ask UXD to design it and wait until there is a design. However, it wouldn't hurt give them some concrete proposals. |
|
Yes, I think we need to circle that back with UXD. From a functional viewpoint, we have :
Use case 1 could be implemented in different ways and that's where all the troubles came from. The requirements must decide when we need either use cases. |
|
@carlosthe19916, noted, thanks for the follow-up. |
3 participants
Fixes: #393, #394, and:
JIRA fixes:
Changes:
date.The image below is an example of how the date filter is visulized:
Summary by Sourcery
Add a new date filter type and migrate all dateRange filters to separate before/after date filters across multiple list contexts, updating the UI components and request parameter handling to fix related issues
New Features:
Bug Fixes:
Enhancements: