Skip to content

Commit

Permalink
chore: remove annotation layer FAB CRUD model view (#22178)
Browse files Browse the repository at this point in the history
  • Loading branch information
dpgaspar authored Nov 22, 2022
1 parent d1567ba commit a77b2d6
Show file tree
Hide file tree
Showing 10 changed files with 33 additions and 164 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ const NotFoundContent = () => (
<span>
{t('Add an annotation layer')}{' '}
<a
href="/annotationlayermodelview/list"
href="/annotationlayer/list"
target="_blank"
rel="noopener noreferrer"
>
Expand Down Expand Up @@ -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 => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' }],
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,9 @@ function AnnotationList({
<span>{t('Annotation Layer %s', annotationLayerName)}</span>
<span>
{hasHistory ? (
<Link to="/annotationlayermodelview/list/">Back to all</Link>
<Link to="/annotationlayer/list/">Back to all</Link>
) : (
<a href="/annotationlayermodelview/list/">Back to all</a>
<a href="/annotationlayer/list/">Back to all</a>
)}
</span>
</StyledHeader>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,10 @@ function AnnotationLayersList({
}

if (hasHistory) {
return (
<Link to={`/annotationmodelview/${id}/annotation`}>{name}</Link>
);
return <Link to={`/annotationlayer/${id}/annotation`}>{name}</Link>;
}

return <a href={`/annotationmodelview/${id}/annotation`}>{name}</a>;
return <a href={`/annotationlayer/${id}/annotation`}>{name}</a>;
},
},
{
Expand Down Expand Up @@ -324,7 +322,7 @@ function AnnotationLayersList({
};

const onLayerAdd = (id?: number) => {
window.location.href = `/annotationmodelview/${id}/annotation`;
window.location.href = `/annotationlayer/${id}/annotation`;
};

const onModalHide = () => {
Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/src/views/routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
{
Expand Down
2 changes: 1 addition & 1 deletion superset/annotation_layers/annotations/commands/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
27 changes: 12 additions & 15 deletions superset/initialization/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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",
Expand Down
1 change: 0 additions & 1 deletion superset/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
from . import (
access_requests,
alerts,
annotations,
api,
base,
core,
Expand Down
110 changes: 10 additions & 100 deletions superset/views/annotations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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("/<pk>/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("/<int:pk>/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()
35 changes: 0 additions & 35 deletions tests/integration_tests/core_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit a77b2d6

Please sign in to comment.