-
-
Notifications
You must be signed in to change notification settings - Fork 18
Add IN filter support for multi-value filtering across all database d… #1700
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
Changes from 2 commits
d75e120
fc0e92d
8318246
3ceb9b8
82e46b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,7 @@ export class FilteringFieldsDs { | |
| criteria: FilterCriteriaEnum; | ||
|
|
||
| @ApiProperty() | ||
| value: string; | ||
| value: unknown; | ||
|
Comment on lines
12
to
+16
|
||
| } | ||
|
|
||
| export class ForeignKeyDSInfo { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -87,6 +87,32 @@ export function findFilteringFieldsUtil( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value: filters[`f_${fieldname}__empty`], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (isObjectPropertyExists(filters, `f_${fieldname}__in`)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const rawValue = filters[`f_${fieldname}__in`]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| filteringItems.push({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| field: fieldname, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| criteria: FilterCriteriaEnum.in, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value: Array.isArray(rawValue) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? rawValue | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : String(rawValue) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .split(',') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map((v) => v.trim()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (isObjectPropertyExists(filters, `f_${fieldname}__between`)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const rawValue = filters[`f_${fieldname}__between`]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| filteringItems.push({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| field: fieldname, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| criteria: FilterCriteriaEnum.between, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value: Array.isArray(rawValue) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? rawValue | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : String(rawValue) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .split(',') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map((v) => v.trim()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+93
to
+114
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| filteringItems.push({ | |
| field: fieldname, | |
| criteria: FilterCriteriaEnum.in, | |
| value: Array.isArray(rawValue) | |
| ? rawValue | |
| : String(rawValue) | |
| .split(',') | |
| .map((v) => v.trim()), | |
| }); | |
| const inValues = (Array.isArray(rawValue) | |
| ? rawValue.map((value) => (typeof value === 'string' ? value.trim() : value)) | |
| : String(rawValue) | |
| .split(',') | |
| .map((value) => value.trim()) | |
| ).filter((value) => value !== ''); | |
| if (inValues.length > 0) { | |
| filteringItems.push({ | |
| field: fieldname, | |
| criteria: FilterCriteriaEnum.in, | |
| value: inValues, | |
| }); | |
| } |
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.
Validate __in / __between values before storing filter criteria.
Line 91-115 accepts empty tokens and arbitrary between arity. That can propagate malformed filters and break DAO query generation (IN () / undefined upper bound).
Suggested fix
if (isObjectPropertyExists(filters, `f_${fieldname}__in`)) {
const rawValue = filters[`f_${fieldname}__in`];
+ const inValues = (Array.isArray(rawValue) ? rawValue : String(rawValue).split(','))
+ .map((v) => String(v).trim())
+ .filter((v) => v.length > 0);
+ if (inValues.length === 0) {
+ continue;
+ }
filteringItems.push({
field: fieldname,
criteria: FilterCriteriaEnum.in,
- value: Array.isArray(rawValue)
- ? rawValue
- : String(rawValue)
- .split(',')
- .map((v) => v.trim()),
+ value: inValues,
});
}
if (isObjectPropertyExists(filters, `f_${fieldname}__between`)) {
const rawValue = filters[`f_${fieldname}__between`];
+ const betweenValues = (Array.isArray(rawValue) ? rawValue : String(rawValue).split(','))
+ .map((v) => String(v).trim())
+ .filter((v) => v.length > 0);
+ if (betweenValues.length !== 2) {
+ throw new Error(`Invalid between filter for "${fieldname}". Expected exactly 2 values.`);
+ }
filteringItems.push({
field: fieldname,
criteria: FilterCriteriaEnum.between,
- value: Array.isArray(rawValue)
- ? rawValue
- : String(rawValue)
- .split(',')
- .map((v) => v.trim()),
+ value: betweenValues,
});
}📝 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 (isObjectPropertyExists(filters, `f_${fieldname}__in`)) { | |
| const rawValue = filters[`f_${fieldname}__in`]; | |
| filteringItems.push({ | |
| field: fieldname, | |
| criteria: FilterCriteriaEnum.in, | |
| value: Array.isArray(rawValue) | |
| ? rawValue | |
| : String(rawValue) | |
| .split(',') | |
| .map((v) => v.trim()), | |
| }); | |
| } | |
| if (isObjectPropertyExists(filters, `f_${fieldname}__between`)) { | |
| const rawValue = filters[`f_${fieldname}__between`]; | |
| filteringItems.push({ | |
| field: fieldname, | |
| criteria: FilterCriteriaEnum.between, | |
| value: Array.isArray(rawValue) | |
| ? rawValue | |
| : String(rawValue) | |
| .split(',') | |
| .map((v) => v.trim()), | |
| }); | |
| } | |
| if (isObjectPropertyExists(filters, `f_${fieldname}__in`)) { | |
| const rawValue = filters[`f_${fieldname}__in`]; | |
| const inValues = (Array.isArray(rawValue) ? rawValue : String(rawValue).split(',')) | |
| .map((v) => String(v).trim()) | |
| .filter((v) => v.length > 0); | |
| if (inValues.length === 0) { | |
| continue; | |
| } | |
| filteringItems.push({ | |
| field: fieldname, | |
| criteria: FilterCriteriaEnum.in, | |
| value: inValues, | |
| }); | |
| } | |
| if (isObjectPropertyExists(filters, `f_${fieldname}__between`)) { | |
| const rawValue = filters[`f_${fieldname}__between`]; | |
| const betweenValues = (Array.isArray(rawValue) ? rawValue : String(rawValue).split(',')) | |
| .map((v) => String(v).trim()) | |
| .filter((v) => v.length > 0); | |
| if (betweenValues.length !== 2) { | |
| throw new Error(`Invalid between filter for "${fieldname}". Expected exactly 2 values.`); | |
| } | |
| filteringItems.push({ | |
| field: fieldname, | |
| criteria: FilterCriteriaEnum.between, | |
| value: betweenValues, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/entities/table/utils/find-filtering-fields.util.ts` around lines
91 - 115, The current handling of filters[`f_${fieldname}__in`] and
filters[`f_${fieldname}__between`] can produce empty tokens or wrong arity;
before pushing into filteringItems (for FilterCriteriaEnum.in and
FilterCriteriaEnum.between) validate and normalize rawValue: convert non-array
to String(...).split(',').map(v=>v.trim()).filter(Boolean) and for __in only
push if the resulting array.length > 0; for __between ensure the resulting
array.length === 2 and both values are non-empty (or otherwise skip/throw),
using the existing symbols filters, fieldname, filteringItems,
FilterCriteriaEnum.in and FilterCriteriaEnum.between to locate and update the
logic.
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.
Guard body filter payload shape before iterating.
Line 128 assumes filtersDataFromBody[rowName] is an object. If it is null, array, or primitive, iteration can throw or produce invalid criteria parsing.
Suggested fix
- const filterData = filtersDataFromBody[rowName] as Record<string, unknown>;
+ const rawFilterData = filtersDataFromBody[rowName];
+ if (!rawFilterData || typeof rawFilterData !== 'object' || Array.isArray(rawFilterData)) {
+ throw new Error(`Invalid filters payload for "${rowName}".`);
+ }
+ const filterData = rawFilterData as Record<string, unknown>;📝 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.
| const filterData = filtersDataFromBody[rowName] as Record<string, unknown>; | |
| for (const key in filterData) { | |
| const rawFilterData = filtersDataFromBody[rowName]; | |
| if (!rawFilterData || typeof rawFilterData !== 'object' || Array.isArray(rawFilterData)) { | |
| throw new Error(`Invalid filters payload for "${rowName}".`); | |
| } | |
| const filterData = rawFilterData as Record<string, unknown>; | |
| for (const key in filterData) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/entities/table/utils/find-filtering-fields.util.ts` around lines
128 - 129, Guard against non-object payloads before iterating
filtersDataFromBody[rowName]: ensure the value (assigned to filterData) is a
non-null plain object (typeof === 'object' && filterData !== null &&
!Array.isArray(filterData)) and bail/continue if not, so iteration over
filterData in the loop does not throw or produce invalid criteria; apply this
check around where filtersDataFromBody, rowName, and filterData are used (in the
function in find-filtering-fields.util.ts) and handle invalid shapes by skipping
or returning an appropriate default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,13 @@ | ||
| export enum FilterCriteriaEnum { | ||
| startswith = 'startswith', | ||
| endswith = 'endswith', | ||
| gt = 'gt', | ||
| lt = 'lt', | ||
| lte = 'lte', | ||
| gte = 'gte', | ||
| contains = 'contains', | ||
| icontains = 'icontains', | ||
| eq = 'eq', | ||
| empty = 'empty', | ||
| startswith = 'startswith', | ||
| endswith = 'endswith', | ||
| gt = 'gt', | ||
| lt = 'lt', | ||
| lte = 'lte', | ||
| gte = 'gte', | ||
| contains = 'contains', | ||
| icontains = 'icontains', | ||
| eq = 'eq', | ||
| empty = 'empty', | ||
| in = 'in', | ||
| } |
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.
FilteringFieldsDs.valueis nowunknown, which weakens the generated Swagger schema for this response DTO. Consider using a narrower union type and@ApiProperty({ oneOf: ... })/isArraymetadata so clients can understand expected scalar vs array shapes forinfilters.