diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx index 64d09364f6134..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')}{' '} @@ -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-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..ff583e6b9b60e 100644 --- a/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx +++ b/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx @@ -143,12 +143,10 @@ function AnnotationLayersList({ } if (hasHistory) { - return ( - {name} - ); + return {name}; } - return {name}; + return {name}; }, }, { @@ -324,7 +322,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/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() diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 65aaeef26eba8..8c53c4c8e7c32 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -149,10 +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 ( - AnnotationLayerModelView, - AnnotationModelView, - ) + 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 @@ -236,16 +233,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 +310,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) @@ -401,6 +387,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/__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, diff --git a/superset/views/annotations.py b/superset/views/annotations.py index b9ef65be03624..cc8c58fe47f1a 100644 --- a/superset/views/annotations.py +++ b/superset/views/annotations.py @@ -14,117 +14,27 @@ # 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 import permission_name 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.") - ) +from .base import BaseSupersetView -class AnnotationModelView( # pylint: disable=too-many-ancestors - SupersetModelView, - CompactCRUDMixin, -): - datamodel = SQLAInterface(Annotation) - include_route_methods = RouteMethod.CRUD_SET | {"annotation"} - +class AnnotationLayerView(BaseSupersetView): + route_base = "/annotationlayer" 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"]) + @expose("/list/") @has_access - def annotation(self, pk: int) -> FlaskResponse: # pylint: disable=unused-argument + @permission_name("read") + def list(self) -> FlaskResponse: 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/") + @expose("//annotation") @has_access - def list(self) -> FlaskResponse: + @permission_name("read") + def get(self, pk: int) -> FlaskResponse: # pylint: disable=unused-argument return super().render_app_template() 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)