fix: This adds the ability to send nested filters with the same field name#3386
fix: This adds the ability to send nested filters with the same field name#3386lholmquist wants to merge 5 commits into
Conversation
Changed Packages
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3386 +/- ##
==========================================
+ Coverage 54.02% 54.05% +0.02%
==========================================
Files 2409 2409
Lines 87733 87763 +30
Branches 24306 24299 -7
==========================================
+ Hits 47400 47436 +36
+ Misses 38812 38806 -6
Partials 1521 1521
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
lholmquist
requested review from
a team,
christoph-jerolimov,
karthikjeeyar and
lokanandaprabhu
as code owners
ciiay
left a comment
ciiay
left a comment
There was a problem hiding this comment.
Thanks for the PR 👏
Good direction on supporting nested filters with the same field name. The handleNestedFilter array handling looks right. Just left a comment about a possible regression in processFilters function.
| if ('field' in item) { | ||
| acc[item.field] ??= []; | ||
| acc[item.field].push(item); | ||
| } |
There was a problem hiding this comment.
Looks like the grouping logic only keeps children that have a field property which could cause a regression issue. Children that are LogicalFilter (no field) are never added to grouped and are silently dropped from the output.
In router.ts (~897–901), when a non-admin calls getInstances, the backend combines filters like this:
filters = {
operator: 'AND',
filters: [nestedVariablesFilter, requestFilters], // requestFilters is often a LogicalFilter
};
From the frontend (WorkflowRunsTabContent.tsx), when the user applies table filters (status, date, etc.) alongside an entity-scoped targetEntity filter, requestFilters is wrapped as:
{
operator: 'AND',
filters: [
{ field: 'variables', nested: { field: 'targetEntity', ... } },
{ field: 'processId', ... },
{ field: 'state', ... },
],
}
So the full filter tree becomes:
AND [
{ field: 'variables', nested: initiatorEntity },
AND [
{ field: 'variables', nested: targetEntity },
{ field: 'processId', ... },
{ field: 'state', ... },
],
]
After processFilters, only the initiatorEntity filter survives, the inner AND (and with it targetEntity, processId, state) is lost.
On upstream/main, this nested-AND shape actually worked because duplicate variables keys were at different nesting levels:
and: {
variables: { initiatorEntity: ... },
and: { variables: { targetEntity: ... }, processId: ..., state: ... }
}
There was a problem hiding this comment.
Thanks for the review. I'll be honest, i was never really thrilled with using that group by to process the filters.
I think it would probably be better if the payload already had the nested parameter as an array so we wouldn't have to do all that group by stuff. I think to do that, i need to update the API a little to allow for that since when i first tried it, i was getting errors on the router.
So i'm thinking that the payload sent to the backend might look something like this then:
I updated the function that processes the filter first, i think this should take care of the payload you mentioned
lholmquist
force-pushed
the
RHIDP-14337-filter-on-variables-entities
branch
from
ca9b8ba to
80cb2cf
Compare
Assisted-by: cursor
lholmquist
force-pushed
the
RHIDP-14337-filter-on-variables-entities
branch
from
78d8daf to
c8941d2
Compare
|
2 participants
This is the backend change that addresses https://redhat.atlassian.net/browse/RHIDP-14337 when the user wants to create a filter using either the
variables.initiatorEntityorvariables.targetEntityThese values are "nested" values and should use the NestedFilter format, similar to how it is done in the router here: https://github.com/redhat-developer/rhdh-plugins/blob/main/workspaces/orchestrator/plugins/orchestrator-backend/src/service/router.ts#L884-L892
The payload should look something like this if using just one filter:
If using multiple filters, the payload might look something like this:
The code will now group up the field paramter in a nested query if the field is defined multiple times and will then format the query correctly.
✔️ Checklist