From bb23780660ed77cebb5e1240de2bfc6df23feb07 Mon Sep 17 00:00:00 2001 From: Tai Dupree Date: Thu, 24 Sep 2020 21:11:51 -0700 Subject: [PATCH 1/2] feat(charts): modify custom api filter to include more fields --- .../src/components/ListView/types.ts | 3 +- .../src/views/CRUD/chart/ChartList.tsx | 2 +- superset/charts/api.py | 4 +- superset/charts/filters.py | 11 ++--- tests/charts/api_tests.py | 44 ++++++++++++------- 5 files changed, 40 insertions(+), 24 deletions(-) diff --git a/superset-frontend/src/components/ListView/types.ts b/superset-frontend/src/components/ListView/types.ts index 634631a1833ec..8c9a577f8ee3e 100644 --- a/superset-frontend/src/components/ListView/types.ts +++ b/superset-frontend/src/components/ListView/types.ts @@ -54,7 +54,8 @@ export interface Filter { | 'rel_o_m' | 'title_or_slug' | 'name_or_description' - | 'all_text'; + | 'all_text' + | 'all_text_chart'; input?: 'text' | 'textarea' | 'select' | 'checkbox' | 'search'; unfilteredLabel?: string; selects?: SelectOption[]; diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index 79f136fb0fd82..30b642583b556 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -364,7 +364,7 @@ function ChartList(props: ChartListProps) { Header: t('Search'), id: 'slice_name', input: 'search', - operator: 'name_or_description', + operator: 'all_text_chart', }, ]; diff --git a/superset/charts/api.py b/superset/charts/api.py index dfeea038df385..fc65275673db1 100644 --- a/superset/charts/api.py +++ b/superset/charts/api.py @@ -41,7 +41,7 @@ ChartUpdateFailedError, ) from superset.charts.commands.update import UpdateChartCommand -from superset.charts.filters import ChartFilter, ChartNameOrDescriptionFilter +from superset.charts.filters import ChartAllTextFilter, ChartFilter from superset.charts.schemas import ( CHART_SCHEMAS, ChartDataQueryContextSchema, @@ -145,7 +145,7 @@ class ChartRestApi(BaseSupersetModelRestApi): ] base_order = ("changed_on", "desc") base_filters = [["id", ChartFilter, lambda: []]] - search_filters = {"slice_name": [ChartNameOrDescriptionFilter]} + search_filters = {"slice_name": [ChartAllTextFilter]} # Will just affect _info endpoint edit_columns = ["slice_name"] diff --git a/superset/charts/filters.py b/superset/charts/filters.py index 2659c6ce688fc..2e2a9598a239f 100644 --- a/superset/charts/filters.py +++ b/superset/charts/filters.py @@ -21,15 +21,14 @@ from sqlalchemy.orm.query import Query from superset import security_manager +from superset.connectors.sqla.models import SqlaTable from superset.models.slice import Slice from superset.views.base import BaseFilter -class ChartNameOrDescriptionFilter( - BaseFilter -): # pylint: disable=too-few-public-methods - name = _("Name or Description") - arg_name = "name_or_description" +class ChartAllTextFilter(BaseFilter): # pylint: disable=too-few-public-methods + name = _("All Text") + arg_name = "all_text_chart" def apply(self, query: Query, value: Any) -> Query: if not value: @@ -39,6 +38,8 @@ def apply(self, query: Query, value: Any) -> Query: or_( Slice.slice_name.ilike(ilike_value), Slice.description.ilike(ilike_value), + Slice.viz_type.ilike(ilike_value), + SqlaTable.table_name.ilike(ilike_value), ) ) diff --git a/tests/charts/api_tests.py b/tests/charts/api_tests.py index 75485e3f4bbfd..feece09db8398 100644 --- a/tests/charts/api_tests.py +++ b/tests/charts/api_tests.py @@ -515,7 +515,7 @@ def test_get_chart(self): def test_get_chart_not_found(self): """ - Chart API: Test get chart not found + Chart API: Test get chart not found """ chart_id = 1000 self.login(username="admin") @@ -525,7 +525,7 @@ def test_get_chart_not_found(self): def test_get_chart_no_data_access(self): """ - Chart API: Test get chart without data access + Chart API: Test get chart without data access """ self.login(username="gamma") chart_no_access = ( @@ -596,34 +596,49 @@ def test_get_charts_custom_filter(self): chart1 = self.insert_chart("foo_a", [admin.id], 1, description="ZY_bar") chart2 = self.insert_chart("zy_foo", [admin.id], 1, description="desc1") chart3 = self.insert_chart("foo_b", [admin.id], 1, description="desc1zy_") - chart4 = self.insert_chart("bar", [admin.id], 1, description="foo") + chart4 = self.insert_chart("foo_c", [admin.id], 1, viz_type="viz_zy_") + chart5 = self.insert_chart("bar", [admin.id], 1, description="foo") arguments = { - "filters": [ - {"col": "slice_name", "opr": "name_or_description", "value": "zy_"} - ], + "filters": [{"col": "slice_name", "opr": "all_text_chart", "value": "zy_"}], "order_column": "slice_name", "order_direction": "asc", "keys": ["none"], - "columns": ["slice_name", "description"], + "columns": ["slice_name", "description", "viz_type"], } self.login(username="admin") uri = f"api/v1/chart/?q={prison.dumps(arguments)}" rv = self.get_assert_metric(uri, "get_list") self.assertEqual(rv.status_code, 200) data = json.loads(rv.data.decode("utf-8")) - self.assertEqual(data["count"], 3) + self.assertEqual(data["count"], 4) expected_response = [ - {"description": "ZY_bar", "slice_name": "foo_a",}, - {"description": "desc1zy_", "slice_name": "foo_b",}, - {"description": "desc1", "slice_name": "zy_foo",}, + {"description": "ZY_bar", "slice_name": "foo_a", "viz_type": None}, + {"description": "desc1zy_", "slice_name": "foo_b", "viz_type": None}, + {"description": None, "slice_name": "foo_c", "viz_type": "viz_zy_"}, + {"description": "desc1", "slice_name": "zy_foo", "viz_type": None}, ] for index, item in enumerate(data["result"]): self.assertEqual( item["description"], expected_response[index]["description"] ) self.assertEqual(item["slice_name"], expected_response[index]["slice_name"]) + self.assertEqual(item["viz_type"], expected_response[index]["viz_type"]) + + # test filtering on datasource_name + arguments = { + "filters": [ + {"col": "slice_name", "opr": "all_text_chart", "value": "energy",} + ], + "keys": ["none"], + "columns": ["slice_name"], + } + uri = f"api/v1/chart/?q={prison.dumps(arguments)}" + rv = self.get_assert_metric(uri, "get_list") + self.assertEqual(rv.status_code, 200) + data = json.loads(rv.data.decode("utf-8")) + self.assertEqual(data["count"], 8) self.logout() self.login(username="gamma") @@ -638,6 +653,7 @@ def test_get_charts_custom_filter(self): db.session.delete(chart2) db.session.delete(chart3) db.session.delete(chart4) + db.session.delete(chart5) db.session.commit() def test_get_charts_page(self): @@ -870,8 +886,7 @@ def test_chart_data_incorrect_request(self): self.assertEqual(rv.status_code, 400) def test_chart_data_with_invalid_datasource(self): - """Chart data API: Test chart data query with invalid schema - """ + """Chart data API: Test chart data query with invalid schema""" self.login(username="admin") table = self.get_table_by_name("birth_names") payload = get_query_context(table.name, table.id, table.type) @@ -880,8 +895,7 @@ def test_chart_data_with_invalid_datasource(self): self.assertEqual(rv.status_code, 400) def test_chart_data_with_invalid_enum_value(self): - """Chart data API: Test chart data query with invalid enum value - """ + """Chart data API: Test chart data query with invalid enum value""" self.login(username="admin") table = self.get_table_by_name("birth_names") payload = get_query_context(table.name, table.id, table.type) From 9ed9b069d3e5bc897fcbf7744bef57e64ee815ea Mon Sep 17 00:00:00 2001 From: Tai Dupree Date: Thu, 24 Sep 2020 23:17:10 -0700 Subject: [PATCH 2/2] all_text_chart -> chart_all_text --- superset-frontend/src/components/ListView/types.ts | 2 +- superset-frontend/src/views/CRUD/chart/ChartList.tsx | 2 +- superset/charts/filters.py | 2 +- tests/charts/api_tests.py | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/components/ListView/types.ts b/superset-frontend/src/components/ListView/types.ts index 8c9a577f8ee3e..cd955f6616d8c 100644 --- a/superset-frontend/src/components/ListView/types.ts +++ b/superset-frontend/src/components/ListView/types.ts @@ -55,7 +55,7 @@ export interface Filter { | 'title_or_slug' | 'name_or_description' | 'all_text' - | 'all_text_chart'; + | 'chart_all_text'; input?: 'text' | 'textarea' | 'select' | 'checkbox' | 'search'; unfilteredLabel?: string; selects?: SelectOption[]; diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index 30b642583b556..352aece9897c0 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -364,7 +364,7 @@ function ChartList(props: ChartListProps) { Header: t('Search'), id: 'slice_name', input: 'search', - operator: 'all_text_chart', + operator: 'chart_all_text', }, ]; diff --git a/superset/charts/filters.py b/superset/charts/filters.py index 2e2a9598a239f..c352b01d6d57c 100644 --- a/superset/charts/filters.py +++ b/superset/charts/filters.py @@ -28,7 +28,7 @@ class ChartAllTextFilter(BaseFilter): # pylint: disable=too-few-public-methods name = _("All Text") - arg_name = "all_text_chart" + arg_name = "chart_all_text" def apply(self, query: Query, value: Any) -> Query: if not value: diff --git a/tests/charts/api_tests.py b/tests/charts/api_tests.py index feece09db8398..a3873e84935ce 100644 --- a/tests/charts/api_tests.py +++ b/tests/charts/api_tests.py @@ -600,7 +600,7 @@ def test_get_charts_custom_filter(self): chart5 = self.insert_chart("bar", [admin.id], 1, description="foo") arguments = { - "filters": [{"col": "slice_name", "opr": "all_text_chart", "value": "zy_"}], + "filters": [{"col": "slice_name", "opr": "chart_all_text", "value": "zy_"}], "order_column": "slice_name", "order_direction": "asc", "keys": ["none"], @@ -629,7 +629,7 @@ def test_get_charts_custom_filter(self): # test filtering on datasource_name arguments = { "filters": [ - {"col": "slice_name", "opr": "all_text_chart", "value": "energy",} + {"col": "slice_name", "opr": "chart_all_text", "value": "energy",} ], "keys": ["none"], "columns": ["slice_name"],