sha174n commented on code in PR #40607:
URL: https://github.com/apache/superset/pull/40607#discussion_r3341280531
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/agGridFilterConverter.test.ts:
##########
@@ -284,6 +284,38 @@ describe('agGridFilterConverter', () => {
val: 18,
});
});
+
+ test('should emit a numeric BETWEEN clause for a metric range filter', ()
=> {
+ const filterModel: AgGridFilterModel = {
+ revenue: {
+ filterType: 'number',
+ type: 'inRange',
+ filter: 10,
+ filterTo: 20,
+ },
+ };
+
+ // revenue is a metric, so the range filter renders as a HAVING clause
+ const result = convertAgGridFiltersToSQL(filterModel, ['revenue']);
+
+ expect(result.havingClause).toContain('BETWEEN 10 AND 20');
+ });
+
+ test('should drop a metric range filter whose bounds are not numeric', ()
=> {
+ const filterModel = {
+ revenue: {
+ filterType: 'number',
+ type: 'inRange',
+ filter: '0',
+ filterTo: '100 OR 1=1',
+ },
+ } as unknown as AgGridFilterModel;
+
+ const result = convertAgGridFiltersToSQL(filterModel, ['revenue']);
+
+ // a non-numeric bound must never be interpolated into the clause
+ expect(result.havingClause).toBeUndefined();
+ });
Review Comment:
Agreed, worth a couple more regressions. Three cases to cover: `filter: ''`,
`filter: null`, `filterTo: undefined`. Each should produce no clause. Up to you
on naming/structure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
