From cfdf5639f54ae97ae82e8d0303e7f3d2af9e6618 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 21 Nov 2022 12:03:19 +0000 Subject: [PATCH 1/9] chore: remove annotation layer FAB CRUD model view --- .../AnnotationLayer.jsx | 2 +- .../AnnotationLayer.test.tsx | 2 +- superset/views/annotations.py | 130 ------------------ 3 files changed, 2 insertions(+), 132 deletions(-) delete mode 100644 superset/views/annotations.py diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx index 64d09364f6134..359e50510616c 100644 --- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx +++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx @@ -300,7 +300,7 @@ class AnnotationLayer extends React.PureComponent { if (isLoadingOptions) { if (sourceType === ANNOTATION_SOURCE_TYPES.NATIVE) { SupersetClient.get({ - endpoint: '/annotationlayermodelview/api/read?', + endpoint: '/api/v1/annotation_layer/', }).then(({ json }) => { const layers = json ? json.result.map(layer => ({ diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx index b9fc64a4c26a0..c5fed0b865f61 100644 --- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx +++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx @@ -36,7 +36,7 @@ beforeAll(() => { value => value.value, ); - fetchMock.get('glob:*/annotationlayermodelview/api/read?*', { + fetchMock.get('glob:*/api/v1/annotation_layer/*', { result: [{ label: 'Chart A', value: 'a' }], }); diff --git a/superset/views/annotations.py b/superset/views/annotations.py deleted file mode 100644 index b9ef65be03624..0000000000000 --- a/superset/views/annotations.py +++ /dev/null @@ -1,130 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -from typing import Any, Dict - -from flask_appbuilder import CompactCRUDMixin -from flask_appbuilder.api import expose -from flask_appbuilder.models.sqla.interface import SQLAInterface -from flask_appbuilder.security.decorators import has_access -from flask_babel import lazy_gettext as _ -from wtforms.validators import StopValidation - -from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod -from superset.models.annotations import Annotation, AnnotationLayer -from superset.superset_typing import FlaskResponse -from superset.views.base import SupersetModelView - - -class StartEndDttmValidator: # pylint: disable=too-few-public-methods - """ - Validates dttm fields. - """ - - def __call__(self, form: Dict[str, Any], field: Any) -> None: - if not form["start_dttm"].data and not form["end_dttm"].data: - raise StopValidation(_("annotation start time or end time is required.")) - if ( - form["end_dttm"].data - and form["start_dttm"].data - and form["end_dttm"].data < form["start_dttm"].data - ): - raise StopValidation( - _("Annotation end time must be no earlier than start time.") - ) - - -class AnnotationModelView( # pylint: disable=too-many-ancestors - SupersetModelView, - CompactCRUDMixin, -): - datamodel = SQLAInterface(Annotation) - include_route_methods = RouteMethod.CRUD_SET | {"annotation"} - - class_permission_name = "Annotation" - method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP - - list_title = _("Annotations") - show_title = _("Show Annotation") - add_title = _("Add Annotation") - edit_title = _("Edit Annotation") - - list_columns = ["short_descr", "start_dttm", "end_dttm"] - edit_columns = [ - "layer", - "short_descr", - "long_descr", - "start_dttm", - "end_dttm", - "json_metadata", - ] - - add_columns = edit_columns - - label_columns = { - "layer": _("Layer"), - "short_descr": _("Label"), - "long_descr": _("Description"), - "start_dttm": _("Start"), - "end_dttm": _("End"), - "json_metadata": _("JSON Metadata"), - } - - description_columns = { - "json_metadata": "This JSON represents any additional metadata this \ - annotation needs to add more context." - } - - validators_columns = {"start_dttm": [StartEndDttmValidator()]} - - def pre_add(self, item: "AnnotationModelView") -> None: - if not item.start_dttm: - item.start_dttm = item.end_dttm - elif not item.end_dttm: - item.end_dttm = item.start_dttm - - def pre_update(self, item: "AnnotationModelView") -> None: - self.pre_add(item) - - @expose("//annotation/", methods=["GET"]) - @has_access - def annotation(self, pk: int) -> FlaskResponse: # pylint: disable=unused-argument - return super().render_app_template() - - -class AnnotationLayerModelView(SupersetModelView): - datamodel = SQLAInterface(AnnotationLayer) - include_route_methods = RouteMethod.CRUD_SET | {RouteMethod.API_READ} - related_views = [AnnotationModelView] - - class_permission_name = "Annotation" - method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP - - list_title = _("Annotation Layers") - show_title = _("Show Annotation Layer") - add_title = _("Add Annotation Layer") - edit_title = _("Edit Annotation Layer") - - list_columns = ["id", "name", "descr"] - edit_columns = ["name", "descr"] - add_columns = edit_columns - - label_columns = {"name": _("Name"), "descr": _("Description")} - - @expose("/list/") - @has_access - def list(self) -> FlaskResponse: - return super().render_app_template() From 4695612a9eae4db65dbfba291051499975e6ad89 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 21 Nov 2022 12:11:05 +0000 Subject: [PATCH 2/9] fix init --- superset/views/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/superset/views/__init__.py b/superset/views/__init__.py index c33601f7278d6..5247f215c1870 100644 --- a/superset/views/__init__.py +++ b/superset/views/__init__.py @@ -17,7 +17,6 @@ from . import ( access_requests, alerts, - annotations, api, base, core, From ac408ae878aa0e90403f44dfc95c18ec920bc3eb Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 21 Nov 2022 12:29:42 +0000 Subject: [PATCH 3/9] fix init --- superset/initialization/__init__.py | 15 ------------ tests/integration_tests/core_tests.py | 35 --------------------------- 2 files changed, 50 deletions(-) diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 65aaeef26eba8..5ba35241ad370 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -149,10 +149,6 @@ def init_views(self) -> None: from superset.security.api import SecurityRestApi from superset.views.access_requests import AccessRequestsModelView from superset.views.alerts import AlertView, ReportView - from superset.views.annotations import ( - AnnotationLayerModelView, - AnnotationModelView, - ) from superset.views.api import Api from superset.views.chart.views import SliceAsync, SliceModelView from superset.views.core import Superset @@ -236,16 +232,6 @@ def init_views(self) -> None: category="Data", category_label=__("Data"), ) - - appbuilder.add_view( - AnnotationLayerModelView, - "Annotation Layers", - label=__("Annotation Layers"), - icon="fa-comment", - category="Manage", - category_label=__("Manage"), - category_icon="", - ) appbuilder.add_view( DashboardModelView, "Dashboards", @@ -323,7 +309,6 @@ def init_views(self) -> None: appbuilder.add_view_no_menu(SliceAsync) appbuilder.add_view_no_menu(SqlLab) appbuilder.add_view_no_menu(SqlMetricInlineView) - appbuilder.add_view_no_menu(AnnotationModelView) appbuilder.add_view_no_menu(Superset) appbuilder.add_view_no_menu(TableColumnInlineView) appbuilder.add_view_no_menu(TableModelView) diff --git a/tests/integration_tests/core_tests.py b/tests/integration_tests/core_tests.py index f25d87d08a8fb..6de21a7200e7e 100644 --- a/tests/integration_tests/core_tests.py +++ b/tests/integration_tests/core_tests.py @@ -231,41 +231,6 @@ def test_get_superset_tables_schema_undefined(self): rv = self.client.get(uri) self.assertEqual(rv.status_code, 422) - def test_annotation_json_endpoint(self): - # Set up an annotation layer and annotation - layer = AnnotationLayer(name="foo", descr="bar") - db.session.add(layer) - db.session.commit() - - annotation = Annotation( - layer_id=layer.id, - short_descr="my_annotation", - start_dttm=datetime.datetime(2020, 5, 20, 18, 21, 51), - end_dttm=datetime.datetime(2020, 5, 20, 18, 31, 51), - ) - - db.session.add(annotation) - db.session.commit() - - self.login() - resp_annotations = json.loads( - self.get_resp("annotationlayermodelview/api/read") - ) - # the UI needs id and name to function - self.assertIn("id", resp_annotations["result"][0]) - self.assertIn("name", resp_annotations["result"][0]) - - response = self.get_resp( - f"/superset/annotation_json/{layer.id}?form_data=" - + quote(json.dumps({"time_range": "100 years ago : now"})) - ) - assert "my_annotation" in response - - # Rollback changes - db.session.delete(annotation) - db.session.delete(layer) - db.session.commit() - def test_admin_only_permissions(self): def assert_admin_permission_in(role_name, assert_func): role = security_manager.find_role(role_name) From e168e106952dc743fd4e0d0521b006bca202789a Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 21 Nov 2022 12:55:28 +0000 Subject: [PATCH 4/9] add menu annotations --- superset/initialization/__init__.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 5ba35241ad370..b6163b6b79a66 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -357,6 +357,15 @@ def init_views(self) -> None: category="SQL Lab", category_label=__("SQL"), ) + appbuilder.add_link( + "Annotation Layers", + label=_("Annotation Layers"), + href="/annotationlayermodelview/list/", + icon="fa-comment", + category_icon="", + category="Manage", + category_label=__("Manage"), + ) appbuilder.add_api(LogRestApi) appbuilder.add_view( From 98eecc96013d3584ec4e9539409ba1cfd87a608c Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 21 Nov 2022 15:22:38 +0000 Subject: [PATCH 5/9] fixes routes --- .../AnnotationLayer.jsx | 2 +- .../views/CRUD/annotation/AnnotationList.tsx | 4 +- .../annotationlayers/AnnotationLayersList.tsx | 6 +-- superset-frontend/src/views/routes.tsx | 4 +- superset/initialization/__init__.py | 30 +++++++++----- superset/views/annotations.py | 40 +++++++++++++++++++ 6 files changed, 69 insertions(+), 17 deletions(-) create mode 100644 superset/views/annotations.py diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx index 359e50510616c..42ca4bb3590b8 100644 --- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx +++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx @@ -115,7 +115,7 @@ const NotFoundContent = () => ( {t('Add an annotation layer')}{' '} diff --git a/superset-frontend/src/views/CRUD/annotation/AnnotationList.tsx b/superset-frontend/src/views/CRUD/annotation/AnnotationList.tsx index 6dedb5e0ead69..3e18c3a92f248 100644 --- a/superset-frontend/src/views/CRUD/annotation/AnnotationList.tsx +++ b/superset-frontend/src/views/CRUD/annotation/AnnotationList.tsx @@ -259,9 +259,9 @@ function AnnotationList({ {t('Annotation Layer %s', annotationLayerName)} {hasHistory ? ( - Back to all + Back to all ) : ( - Back to all + Back to all )} diff --git a/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx b/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx index 3914485a6fb74..97fef41cbf55d 100644 --- a/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx +++ b/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx @@ -144,11 +144,11 @@ function AnnotationLayersList({ if (hasHistory) { return ( - {name} + {name} ); } - return {name}; + return {name}; }, }, { @@ -324,7 +324,7 @@ function AnnotationLayersList({ }; const onLayerAdd = (id?: number) => { - window.location.href = `/annotationmodelview/${id}/annotation`; + window.location.href = `/annotationlayer/${id}/annotation`; }; const onModalHide = () => { diff --git a/superset-frontend/src/views/routes.tsx b/superset-frontend/src/views/routes.tsx index d90feec116670..88efa00a64fc9 100644 --- a/superset-frontend/src/views/routes.tsx +++ b/superset-frontend/src/views/routes.tsx @@ -156,11 +156,11 @@ export const routes: Routes = [ Component: CssTemplatesList, }, { - path: '/annotationlayermodelview/list/', + path: '/annotationlayer/list/', Component: AnnotationLayersList, }, { - path: '/annotationmodelview/:annotationLayerId/annotation/', + path: '/annotationlayer/:annotationLayerId/annotation/', Component: AnnotationList, }, { diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index b6163b6b79a66..4a7ff72fda6ce 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -149,6 +149,7 @@ def init_views(self) -> None: from superset.security.api import SecurityRestApi from superset.views.access_requests import AccessRequestsModelView from superset.views.alerts import AlertView, ReportView + from superset.views.annotations import AnnotationLayerView from superset.views.api import Api from superset.views.chart.views import SliceAsync, SliceModelView from superset.views.core import Superset @@ -357,15 +358,15 @@ def init_views(self) -> None: category="SQL Lab", category_label=__("SQL"), ) - appbuilder.add_link( - "Annotation Layers", - label=_("Annotation Layers"), - href="/annotationlayermodelview/list/", - icon="fa-comment", - category_icon="", - category="Manage", - category_label=__("Manage"), - ) + # appbuilder.add_link( + # "Annotation Layers", + # label=_("Annotation Layers"), + # href="/annotationlayermodelview/list/", + # icon="fa-comment", + # category_icon="", + # category="Manage", + # category_label=__("Manage"), + # ) appbuilder.add_api(LogRestApi) appbuilder.add_view( @@ -395,6 +396,17 @@ def init_views(self) -> None: menu_cond=lambda: feature_flag_manager.is_feature_enabled("ALERT_REPORTS"), ) + appbuilder.add_view( + AnnotationLayerView, + "Annotation Layers", + label=_("Annotation Layers"), + href="/annotationlayer/list/", + icon="fa-comment", + category_icon="", + category="Manage", + category_label=__("Manage"), + ) + appbuilder.add_view( AccessRequestsModelView, "Access requests", diff --git a/superset/views/annotations.py b/superset/views/annotations.py new file mode 100644 index 0000000000000..97bca55836f2c --- /dev/null +++ b/superset/views/annotations.py @@ -0,0 +1,40 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from flask_appbuilder import permission_name +from flask_appbuilder.api import expose +from flask_appbuilder.security.decorators import has_access + +from superset.superset_typing import FlaskResponse + +from .base import BaseSupersetView + + +class AnnotationLayerView(BaseSupersetView): + route_base = "/annotationlayer" + class_permission_name = "Annotation" + + @expose("/list/") + @has_access + @permission_name("read") + def list(self) -> FlaskResponse: + return super().render_app_template() + + @expose("//annotation") + @has_access + @permission_name("read") + def get(self, pk: int) -> FlaskResponse: + return super().render_app_template() From 06940593584b73626d4a7d015287b0844d68495f Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 21 Nov 2022 15:25:28 +0000 Subject: [PATCH 6/9] remove comment --- superset/initialization/__init__.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 4a7ff72fda6ce..8c53c4c8e7c32 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -358,15 +358,6 @@ def init_views(self) -> None: category="SQL Lab", category_label=__("SQL"), ) - # appbuilder.add_link( - # "Annotation Layers", - # label=_("Annotation Layers"), - # href="/annotationlayermodelview/list/", - # icon="fa-comment", - # category_icon="", - # category="Manage", - # category_label=__("Manage"), - # ) appbuilder.add_api(LogRestApi) appbuilder.add_view( From 5ff0c4239872ecc5901c8ddbd3bb999e3c6d196c Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 21 Nov 2022 15:46:39 +0000 Subject: [PATCH 7/9] lint --- .../src/views/CRUD/annotationlayers/AnnotationLayersList.tsx | 4 +--- superset/views/annotations.py | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx b/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx index 97fef41cbf55d..ff583e6b9b60e 100644 --- a/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx +++ b/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx @@ -143,9 +143,7 @@ function AnnotationLayersList({ } if (hasHistory) { - return ( - {name} - ); + return {name}; } return {name}; diff --git a/superset/views/annotations.py b/superset/views/annotations.py index 97bca55836f2c..47983b43d8b5f 100644 --- a/superset/views/annotations.py +++ b/superset/views/annotations.py @@ -33,7 +33,7 @@ class AnnotationLayerView(BaseSupersetView): def list(self) -> FlaskResponse: return super().render_app_template() - @expose("//annotation") + @expose("//annotation") # pylint: disable=unused-argument @has_access @permission_name("read") def get(self, pk: int) -> FlaskResponse: From cbcbbb43c8d56a94bd4491a1f79ae833e5862e4b Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 21 Nov 2022 15:59:16 +0000 Subject: [PATCH 8/9] lint --- superset/views/annotations.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset/views/annotations.py b/superset/views/annotations.py index 47983b43d8b5f..cc8c58fe47f1a 100644 --- a/superset/views/annotations.py +++ b/superset/views/annotations.py @@ -33,8 +33,8 @@ class AnnotationLayerView(BaseSupersetView): def list(self) -> FlaskResponse: return super().render_app_template() - @expose("//annotation") # pylint: disable=unused-argument + @expose("//annotation") @has_access @permission_name("read") - def get(self, pk: int) -> FlaskResponse: + def get(self, pk: int) -> FlaskResponse: # pylint: disable=unused-argument return super().render_app_template() From eb3d59a0f55f4923b75d5cef3e96d3b9d28c0186 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 21 Nov 2022 16:56:35 +0000 Subject: [PATCH 9/9] fix command exception AnnotationDatesValidationError --- superset/annotation_layers/annotations/commands/create.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/annotation_layers/annotations/commands/create.py b/superset/annotation_layers/annotations/commands/create.py index dcfa6c8521b17..26cd968c5a1f4 100644 --- a/superset/annotation_layers/annotations/commands/create.py +++ b/superset/annotation_layers/annotations/commands/create.py @@ -70,7 +70,7 @@ def validate(self) -> None: # validate date time sanity if start_dttm and end_dttm and end_dttm < start_dttm: - exceptions.append(AnnotationDatesValidationError) + exceptions.append(AnnotationDatesValidationError()) if exceptions: exception = AnnotationInvalidError()