From 1bed57ebfda910e38c5f81719aef950aaa2ca943 Mon Sep 17 00:00:00 2001 From: Antonio Rivero Date: Mon, 17 Apr 2023 16:30:39 -0400 Subject: [PATCH 1/3] Filters: - Consider cases where translateToSql doesn't return a string when getting the default label of a filter - Add tests for changes --- .../AdhocFilter/AdhocFilter.test.js | 32 +++++++++++++++++++ .../FilterControl/AdhocFilter/index.js | 4 +++ 2 files changed, 36 insertions(+) diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/AdhocFilter.test.js b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/AdhocFilter.test.js index 20f9576a47aa2..19cc2f783e765 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/AdhocFilter.test.js +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/AdhocFilter.test.js @@ -247,4 +247,36 @@ describe('AdhocFilter', () => { }); expect(adhocFilter2.comparator).toBe(null); }); + it('sets the label properly if subject is a string', () => { + const adhocFilter2 = new AdhocFilter({ + expressionType: EXPRESSION_TYPES.SIMPLE, + subject: 'order_date', + }); + expect(adhocFilter2.getDefaultLabel()).toBe('order_date'); + }); + it('sets the label properly if subject is an object with the column_date property', () => { + const adhocFilter2 = new AdhocFilter({ + expressionType: EXPRESSION_TYPES.SIMPLE, + subject: { + column_name: 'year', + }, + }); + expect(adhocFilter2.getDefaultLabel()).toBe('year'); + }); + it('sets the label to empty is there is no column_name in the object', () => { + const adhocFilter2 = new AdhocFilter({ + expressionType: EXPRESSION_TYPES.SIMPLE, + subject: { + unknown: 'year', + }, + }); + expect(adhocFilter2.getDefaultLabel()).toBe(''); + }); + it('sets the label to empty is there is no subject', () => { + const adhocFilter2 = new AdhocFilter({ + expressionType: EXPRESSION_TYPES.SIMPLE, + subject: undefined, + }); + expect(adhocFilter2.getDefaultLabel()).toBe(''); + }); }); diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js index f00491b89fa53..8e453e06de957 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js @@ -135,6 +135,10 @@ export default class AdhocFilter { getDefaultLabel() { const label = this.translateToSql(); + // If returned value is an object after changing dataset + if (typeof label === 'object') { + return label.column_name ?? ''; + } return label.length < 43 ? label : `${label.substring(0, 40)}...`; } From 699870ad0f703e626ea8bb4049dfe139f6f11c45 Mon Sep 17 00:00:00 2001 From: Antonio Rivero Date: Mon, 8 May 2023 13:51:42 -0400 Subject: [PATCH 2/3] Filters: - Moving logic to extract colum_name to getSimpleSQLExpression instead --- .../components/controls/FilterControl/AdhocFilter/index.js | 4 ---- superset-frontend/src/explore/exploreUtils/index.js | 4 +++- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js index 8e453e06de957..f00491b89fa53 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js @@ -135,10 +135,6 @@ export default class AdhocFilter { getDefaultLabel() { const label = this.translateToSql(); - // If returned value is an object after changing dataset - if (typeof label === 'object') { - return label.column_name ?? ''; - } return label.length < 43 ? label : `${label.substring(0, 40)}...`; } diff --git a/superset-frontend/src/explore/exploreUtils/index.js b/superset-frontend/src/explore/exploreUtils/index.js index 1d678427ff3cf..b79ba93892aea 100644 --- a/superset-frontend/src/explore/exploreUtils/index.js +++ b/superset-frontend/src/explore/exploreUtils/index.js @@ -295,7 +295,9 @@ export const getSimpleSQLExpression = (subject, operator, comparator) => { [...MULTI_OPERATORS] .map(op => OPERATOR_ENUM_TO_OPERATOR_TYPE[op].operation) .indexOf(operator) >= 0; - let expression = subject ?? ''; + // If returned value is an object after changing dataset + let expression = + typeof subject === 'object' ? subject.column_name ?? '' : subject ?? ''; if (subject && operator) { expression += ` ${operator}`; const firstValue = From 41c4a07e47c4f4b977271309b04efa9da7851c58 Mon Sep 17 00:00:00 2001 From: Antonio Rivero Date: Mon, 8 May 2023 14:56:25 -0400 Subject: [PATCH 3/3] Filters: - Optional chaining when accessing column_name --- superset-frontend/src/explore/exploreUtils/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/explore/exploreUtils/index.js b/superset-frontend/src/explore/exploreUtils/index.js index b79ba93892aea..e3e0b4f8ff0ad 100644 --- a/superset-frontend/src/explore/exploreUtils/index.js +++ b/superset-frontend/src/explore/exploreUtils/index.js @@ -297,7 +297,7 @@ export const getSimpleSQLExpression = (subject, operator, comparator) => { .indexOf(operator) >= 0; // If returned value is an object after changing dataset let expression = - typeof subject === 'object' ? subject.column_name ?? '' : subject ?? ''; + typeof subject === 'object' ? subject?.column_name ?? '' : subject ?? ''; if (subject && operator) { expression += ` ${operator}`; const firstValue =