From 4dee6c03175bcd71d856c191758102306cdf0794 Mon Sep 17 00:00:00 2001 From: "Hugh A. Miles II" Date: Thu, 19 May 2022 21:31:22 +0000 Subject: [PATCH 1/5] proto --- superset/datasets/api.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/superset/datasets/api.py b/superset/datasets/api.py index 268337c090fee..24cc311bcd281 100644 --- a/superset/datasets/api.py +++ b/superset/datasets/api.py @@ -27,6 +27,7 @@ from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_babel import ngettext from marshmallow import ValidationError +from sqlalchemy import not_ from superset import event_logger, is_feature_enabled from superset.commands.importers.exceptions import NoValidFilesFoundError @@ -73,6 +74,24 @@ logger = logging.getLogger(__name__) +from flask_babel import lazy_gettext as _ +from sqlalchemy.orm.query import Query + +from superset.views.base import BaseFilter + + +class DatasetCertifiedFilter(BaseFilter): + name = _("Is certified") + arg_name = "dataset_is_certified" + + def apply(self, query: Query, value: Any) -> Query: + breakpoint() + if value is True: + return query.filter(SqlaTable.extra.ilike("%certification%")) + if value is False: + return query.exclude(SqlaTable.extra.ilike("%certification%")) + return query + class DatasetRestApi(BaseSupersetModelRestApi): datamodel = SQLAInterface(SqlaTable) @@ -195,7 +214,10 @@ class DatasetRestApi(BaseSupersetModelRestApi): "owners": RelatedFieldFilter("first_name", FilterRelatedOwners), "database": "database_name", } - search_filters = {"sql": [DatasetIsNullOrEmptyFilter]} + search_filters = { + "sql": [DatasetIsNullOrEmptyFilter], + "sql": [DatasetCertifiedFilter], + } filter_rel_fields = {"database": [["id", DatabaseFilter, lambda: []]]} allowed_rel_fields = {"database", "owners"} allowed_distinct_fields = {"schema"} From 166b35264a0e9a642b7c67560eba31e6a7bea661 Mon Sep 17 00:00:00 2001 From: "Hugh A. Miles II" Date: Thu, 19 May 2022 22:28:21 +0000 Subject: [PATCH 2/5] add certified --- .../views/CRUD/data/dataset/DatasetList.tsx | 13 ++++++++++ superset/datasets/api.py | 24 +++---------------- superset/datasets/filters.py | 19 +++++++++++++++ 3 files changed, 35 insertions(+), 21 deletions(-) diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx index 563a3bce1fc4c..e11705ffd36cf 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx @@ -258,6 +258,7 @@ const DatasetList: FunctionComponent = ({ accessor: 'kind_icon', disableSortBy: true, size: 'xs', + id: 'id', }, { Cell: ({ @@ -506,6 +507,18 @@ const DatasetList: FunctionComponent = ({ { label: 'Physical', value: true }, ], }, + { + Header: t('Certified'), + id: 'id', + urlDisplay: 'certified', + input: 'select', + operator: 'dataset_is_certified', + unfilteredLabel: t('Any'), + selects: [ + { label: t('Yes'), value: true }, + { label: t('No'), value: false }, + ], + }, { Header: t('Search'), id: 'table_name', diff --git a/superset/datasets/api.py b/superset/datasets/api.py index 24cc311bcd281..8c8d1797a4a1a 100644 --- a/superset/datasets/api.py +++ b/superset/datasets/api.py @@ -27,7 +27,6 @@ from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_babel import ngettext from marshmallow import ValidationError -from sqlalchemy import not_ from superset import event_logger, is_feature_enabled from superset.commands.importers.exceptions import NoValidFilesFoundError @@ -53,7 +52,7 @@ from superset.datasets.commands.refresh import RefreshDatasetCommand from superset.datasets.commands.update import UpdateDatasetCommand from superset.datasets.dao import DatasetDAO -from superset.datasets.filters import DatasetIsNullOrEmptyFilter +from superset.datasets.filters import DatasetCertifiedFilter, DatasetIsNullOrEmptyFilter from superset.datasets.schemas import ( DatasetPostSchema, DatasetPutSchema, @@ -74,24 +73,6 @@ logger = logging.getLogger(__name__) -from flask_babel import lazy_gettext as _ -from sqlalchemy.orm.query import Query - -from superset.views.base import BaseFilter - - -class DatasetCertifiedFilter(BaseFilter): - name = _("Is certified") - arg_name = "dataset_is_certified" - - def apply(self, query: Query, value: Any) -> Query: - breakpoint() - if value is True: - return query.filter(SqlaTable.extra.ilike("%certification%")) - if value is False: - return query.exclude(SqlaTable.extra.ilike("%certification%")) - return query - class DatasetRestApi(BaseSupersetModelRestApi): datamodel = SQLAInterface(SqlaTable) @@ -216,8 +197,9 @@ class DatasetRestApi(BaseSupersetModelRestApi): } search_filters = { "sql": [DatasetIsNullOrEmptyFilter], - "sql": [DatasetCertifiedFilter], + "id": [DatasetCertifiedFilter], } + search_columns = ["id", "sql"] filter_rel_fields = {"database": [["id", DatabaseFilter, lambda: []]]} allowed_rel_fields = {"database", "owners"} allowed_distinct_fields = {"schema"} diff --git a/superset/datasets/filters.py b/superset/datasets/filters.py index 4bbe80fd4e4f1..6694d954020b0 100644 --- a/superset/datasets/filters.py +++ b/superset/datasets/filters.py @@ -14,6 +14,8 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from typing import Any + from flask_babel import lazy_gettext as _ from sqlalchemy import not_, or_ from sqlalchemy.orm.query import Query @@ -33,3 +35,20 @@ def apply(self, query: Query, value: bool) -> Query: filter_clause = not_(filter_clause) return query.filter(filter_clause) + + +class DatasetCertifiedFilter(BaseFilter): # pylint: disable=too-few-public-methods + name = _("Is certified") + arg_name = "dataset_is_certified" + + def apply(self, query: Query, value: Any) -> Query: + if value is True: + return query.filter(SqlaTable.extra.ilike("%certification%")) + if value is False: + return query.filter( + or_( + SqlaTable.extra.notlike("%certification%"), + SqlaTable.extra.is_(None), + ) + ) + return query From 5c365b85108b8b60246c81acb637c92fd11fb21f Mon Sep 17 00:00:00 2001 From: "Hugh A. Miles II" Date: Mon, 23 May 2022 15:43:41 +0000 Subject: [PATCH 3/5] fix lint --- superset-frontend/src/components/ListView/types.ts | 1 + superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/components/ListView/types.ts b/superset-frontend/src/components/ListView/types.ts index 53710b84d271d..f8bff90f0ee95 100644 --- a/superset-frontend/src/components/ListView/types.ts +++ b/superset-frontend/src/components/ListView/types.ts @@ -113,4 +113,5 @@ export enum FilterOperator { chartIsFav = 'chart_is_favorite', chartIsCertified = 'chart_is_certified', dashboardIsCertified = 'dashboard_is_certified', + datasetIsCertified = 'dataset_is_certified', } diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx index e11705ffd36cf..a56a69b346c11 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx @@ -512,7 +512,7 @@ const DatasetList: FunctionComponent = ({ id: 'id', urlDisplay: 'certified', input: 'select', - operator: 'dataset_is_certified', + operator: FilterOperator.datasetIsCertified, unfilteredLabel: t('Any'), selects: [ { label: t('Yes'), value: true }, From 9a368cb6abf2f61286ca39dcd8dcb35a50bd3fd9 Mon Sep 17 00:00:00 2001 From: "Hugh A. Miles II" Date: Mon, 23 May 2022 16:11:54 +0000 Subject: [PATCH 4/5] added test --- superset/datasets/filters.py | 5 ++- tests/integration_tests/datasets/api_tests.py | 42 +++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/superset/datasets/filters.py b/superset/datasets/filters.py index 6694d954020b0..d90d66ca3b288 100644 --- a/superset/datasets/filters.py +++ b/superset/datasets/filters.py @@ -42,12 +42,13 @@ class DatasetCertifiedFilter(BaseFilter): # pylint: disable=too-few-public-meth arg_name = "dataset_is_certified" def apply(self, query: Query, value: Any) -> Query: + check_value = '%"certification":%' if value is True: - return query.filter(SqlaTable.extra.ilike("%certification%")) + return query.filter(SqlaTable.extra.ilike(check_value)) if value is False: return query.filter( or_( - SqlaTable.extra.notlike("%certification%"), + SqlaTable.extra.notlike(check_value), SqlaTable.extra.is_(None), ) ) diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index 87d0da3cad827..f8a0f3495dcec 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -128,6 +128,7 @@ def create_datasets(self): main_db = get_main_database() for tables_name in self.fixture_tables_names: datasets.append(self.insert_dataset(tables_name, [admin.id], main_db)) + yield datasets # rollback changes @@ -1811,3 +1812,44 @@ def test_import_dataset_invalid_v0_validation(self): } ] } + + @pytest.mark.usefixtures("create_datasets", "create_virtual_datasets") + def test_get_datasets_is_certified_filter(self): + """ + Dataset API: Test custom dataset_is_certified filter + """ + arguments = { + "filters": [{"col": "id", "opr": "dataset_is_certified", "value": False}] + } + self.login(username="admin") + uri = f"api/v1/dataset/?q={prison.dumps(arguments)}" + rv = self.client.get(uri) + + assert rv.status_code == 200 + response = json.loads(rv.data.decode("utf-8")) + assert response.get("count") == 5 + + table_w_certification = SqlaTable( + table_name="foo", + schema=None, + owners=[], + database=get_main_database(), + sql=None, + extra='{"certification": 1}', + ) + db.session.add(table_w_certification) + db.session.commit() + + arguments = { + "filters": [{"col": "id", "opr": "dataset_is_certified", "value": True}] + } + self.login(username="admin") + uri = f"api/v1/dataset/?q={prison.dumps(arguments)}" + rv = self.client.get(uri) + + assert rv.status_code == 200 + response = json.loads(rv.data.decode("utf-8")) + assert response.get("count") == 1 + + db.session.delete(table_w_certification) + db.session.commit() From 169022a28aa17a3b44c17145686ba05b7cbd5192 Mon Sep 17 00:00:00 2001 From: "Hugh A. Miles II" Date: Mon, 23 May 2022 18:22:01 +0000 Subject: [PATCH 5/5] working fix --- superset/datasets/api.py | 2 +- superset/datasets/filters.py | 4 +--- tests/integration_tests/datasets/api_tests.py | 13 +------------ 3 files changed, 3 insertions(+), 16 deletions(-) diff --git a/superset/datasets/api.py b/superset/datasets/api.py index 8c8d1797a4a1a..fb01b6ee8c9cc 100644 --- a/superset/datasets/api.py +++ b/superset/datasets/api.py @@ -199,7 +199,7 @@ class DatasetRestApi(BaseSupersetModelRestApi): "sql": [DatasetIsNullOrEmptyFilter], "id": [DatasetCertifiedFilter], } - search_columns = ["id", "sql"] + search_columns = ["id", "database", "owners", "sql", "table_name"] filter_rel_fields = {"database": [["id", DatabaseFilter, lambda: []]]} allowed_rel_fields = {"database", "owners"} allowed_distinct_fields = {"schema"} diff --git a/superset/datasets/filters.py b/superset/datasets/filters.py index d90d66ca3b288..e72eb281818ae 100644 --- a/superset/datasets/filters.py +++ b/superset/datasets/filters.py @@ -14,8 +14,6 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from typing import Any - from flask_babel import lazy_gettext as _ from sqlalchemy import not_, or_ from sqlalchemy.orm.query import Query @@ -41,7 +39,7 @@ class DatasetCertifiedFilter(BaseFilter): # pylint: disable=too-few-public-meth name = _("Is certified") arg_name = "dataset_is_certified" - def apply(self, query: Query, value: Any) -> Query: + def apply(self, query: Query, value: bool) -> Query: check_value = '%"certification":%' if value is True: return query.filter(SqlaTable.extra.ilike(check_value)) diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index f8a0f3495dcec..781ae929b743c 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -1813,22 +1813,11 @@ def test_import_dataset_invalid_v0_validation(self): ] } - @pytest.mark.usefixtures("create_datasets", "create_virtual_datasets") + @pytest.mark.usefixtures("create_datasets") def test_get_datasets_is_certified_filter(self): """ Dataset API: Test custom dataset_is_certified filter """ - arguments = { - "filters": [{"col": "id", "opr": "dataset_is_certified", "value": False}] - } - self.login(username="admin") - uri = f"api/v1/dataset/?q={prison.dumps(arguments)}" - rv = self.client.get(uri) - - assert rv.status_code == 200 - response = json.loads(rv.data.decode("utf-8")) - assert response.get("count") == 5 - table_w_certification = SqlaTable( table_name="foo", schema=None,