diff --git a/superset-frontend/src/views/CRUD/annotation/AnnotationModal.tsx b/superset-frontend/src/views/CRUD/annotation/AnnotationModal.tsx index bab4d2f7e774c..4d8fedceca788 100644 --- a/superset-frontend/src/views/CRUD/annotation/AnnotationModal.tsx +++ b/superset-frontend/src/views/CRUD/annotation/AnnotationModal.tsx @@ -220,9 +220,9 @@ const AnnotationModal: FunctionComponent = ({ const validate = () => { if ( currentAnnotation && - currentAnnotation.short_descr.length && - currentAnnotation.start_dttm.length && - currentAnnotation.end_dttm.length + currentAnnotation.short_descr?.length && + currentAnnotation.start_dttm?.length && + currentAnnotation.end_dttm?.length ) { setDisableSave(false); } else { diff --git a/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayerModal.tsx b/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayerModal.tsx index d683448ab5619..e3ea229482216 100644 --- a/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayerModal.tsx +++ b/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayerModal.tsx @@ -171,7 +171,7 @@ const AnnotationLayerModal: FunctionComponent = ({ }; const validate = () => { - if (currentLayer && currentLayer.name.length) { + if (currentLayer && currentLayer.name?.length) { setDisableSave(false); } else { setDisableSave(true); diff --git a/superset/annotation_layers/annotations/schemas.py b/superset/annotation_layers/annotations/schemas.py index 72edcc10d00f6..fd03b14f51b4f 100644 --- a/superset/annotation_layers/annotations/schemas.py +++ b/superset/annotation_layers/annotations/schemas.py @@ -57,11 +57,18 @@ def validate_json(value: Union[bytes, bytearray, str]) -> None: class AnnotationPostSchema(Schema): short_descr = fields.String( - description=annotation_short_descr, allow_none=False, validate=[Length(1, 500)] + description=annotation_short_descr, + required=True, + allow_none=False, + validate=[Length(1, 500)], ) long_descr = fields.String(description=annotation_long_descr, allow_none=True) - start_dttm = fields.DateTime(description=annotation_start_dttm, allow_none=False) - end_dttm = fields.DateTime(description=annotation_end_dttm, allow_none=False) + start_dttm = fields.DateTime( + description=annotation_start_dttm, required=True, allow_none=False, + ) + end_dttm = fields.DateTime( + description=annotation_end_dttm, required=True, allow_none=False + ) json_metadata = fields.String( description=annotation_json_metadata, validate=validate_json, allow_none=True, ) @@ -71,9 +78,14 @@ class AnnotationPutSchema(Schema): short_descr = fields.String( description=annotation_short_descr, required=False, validate=[Length(1, 500)] ) - long_descr = fields.String(description=annotation_long_descr, required=False) + long_descr = fields.String( + description=annotation_long_descr, required=False, allow_none=True + ) start_dttm = fields.DateTime(description=annotation_start_dttm, required=False) end_dttm = fields.DateTime(description=annotation_end_dttm, required=False) json_metadata = fields.String( - description=annotation_json_metadata, validate=validate_json, required=False + description=annotation_json_metadata, + validate=validate_json, + required=False, + allow_none=True, ) diff --git a/superset/annotation_layers/schemas.py b/superset/annotation_layers/schemas.py index 4e7c6ded1d19a..4cd7493a32986 100644 --- a/superset/annotation_layers/schemas.py +++ b/superset/annotation_layers/schemas.py @@ -40,7 +40,7 @@ class AnnotationLayerPostSchema(Schema): name = fields.String( - description=annotation_layer_name, allow_none=False, validate=[Length(1, 250)] + description=annotation_layer_name, required=True, validate=[Length(1, 250)] ) descr = fields.String(description=annotation_layer_descr, allow_none=True) diff --git a/tests/integration_tests/annotation_layers/api_tests.py b/tests/integration_tests/annotation_layers/api_tests.py index 292829c8cf6bf..466bf7ec3f9b8 100644 --- a/tests/integration_tests/annotation_layers/api_tests.py +++ b/tests/integration_tests/annotation_layers/api_tests.py @@ -31,6 +31,8 @@ create_annotation_layers, get_end_dttm, get_start_dttm, + START_STR, + END_STR, ) ANNOTATION_LAYERS_COUNT = 10 @@ -201,7 +203,10 @@ def test_create_annotation_layer(self): Annotation Api: Test create annotation layer """ self.login(username="admin") - annotation_layer_data = {"name": "new3", "descr": "description"} + annotation_layer_data = { + "name": "new3", + "descr": "description", + } uri = "api/v1/annotation_layer/" rv = self.client.post(uri, json=annotation_layer_data) assert rv.status_code == 201 @@ -500,6 +505,8 @@ def test_create_annotation(self): annotation_data = { "short_descr": "new", "long_descr": "description", + "start_dttm": START_STR, + "end_dttm": END_STR, } uri = f"api/v1/annotation_layer/{layer.id}/annotation/" rv = self.client.post(uri, json=annotation_data) @@ -525,6 +532,8 @@ def test_create_annotation_uniqueness(self): annotation_data = { "short_descr": "short_descr2", "long_descr": "description", + "start_dttm": START_STR, + "end_dttm": END_STR, } uri = f"api/v1/annotation_layer/{layer.id}/annotation/" rv = self.client.post(uri, json=annotation_data) diff --git a/tests/integration_tests/annotation_layers/fixtures.py b/tests/integration_tests/annotation_layers/fixtures.py index cb7647d30cdb3..2576cf4eda154 100644 --- a/tests/integration_tests/annotation_layers/fixtures.py +++ b/tests/integration_tests/annotation_layers/fixtures.py @@ -16,9 +16,11 @@ # under the License. # isort:skip_file import pytest +import dateutil.parser from datetime import datetime from typing import Optional + from superset import db from superset.models.annotations import Annotation, AnnotationLayer @@ -27,6 +29,10 @@ ANNOTATION_LAYERS_COUNT = 10 ANNOTATIONS_COUNT = 5 +START_STR = "2019-01-02T03:04:05.678900" +END_STR = "2020-01-02T03:04:05.678900" +START_DTTM = dateutil.parser.parse(START_STR) +END_DTTM = dateutil.parser.parse(END_STR) def get_start_dttm(annotation_id: int) -> datetime: diff --git a/tests/integration_tests/annotation_layers/schema_tests.py b/tests/integration_tests/annotation_layers/schema_tests.py new file mode 100644 index 0000000000000..ff7fc309dc9ac --- /dev/null +++ b/tests/integration_tests/annotation_layers/schema_tests.py @@ -0,0 +1,157 @@ +# 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. +import pytest +from marshmallow.exceptions import ValidationError + +from superset.annotation_layers.annotations.schemas import ( + AnnotationPostSchema, + AnnotationPutSchema, +) +from superset.annotation_layers.schemas import ( + AnnotationLayerPostSchema, + AnnotationLayerPutSchema, +) +from tests.integration_tests.annotation_layers.fixtures import ( + END_DTTM, + END_STR, + START_DTTM, + START_STR, +) + + +def test_annotation_layer_post_schema_with_name(): + result = AnnotationLayerPostSchema().load({"name": "foo"}) + assert result["name"] == "foo" + assert "descr" not in result + + +def test_annotation_layer_post_schema_with_name_and_descr(): + result = AnnotationLayerPostSchema().load({"name": "foo", "descr": "bar"}) + assert result["name"] == "foo" + assert result["descr"] == "bar" + + +def test_annotation_layer_post_schema_with_null_name(): + with pytest.raises(ValidationError): + AnnotationLayerPostSchema().load({"name": None}) + + +def test_annotation_layer_post_schema_empty(): + with pytest.raises(ValidationError): + AnnotationLayerPostSchema().load({}) + + +def test_annotation_layer_put_schema_empty(): + result = AnnotationLayerPutSchema().load({}) + assert result == {} + + +def test_annotation_layer_put_schema_with_null_name(): + with pytest.raises(ValidationError): + AnnotationLayerPutSchema().load({"name": None}) + + +def test_annotation_layer_put_schema_with_null_descr(): + with pytest.raises(ValidationError): + AnnotationLayerPutSchema().load({"descr": None}) + + +def test_annotation_post_schema_basic(): + result = AnnotationPostSchema().load( + {"short_descr": "foo", "start_dttm": START_STR, "end_dttm": END_STR} + ) + assert result["short_descr"] == "foo" + assert result["start_dttm"] == START_DTTM + assert result["end_dttm"] == END_DTTM + + +def test_annotation_post_schema_full(): + result = AnnotationPostSchema().load( + { + "short_descr": "foo", + "long_descr": "bar", + "start_dttm": START_STR, + "end_dttm": END_STR, + "json_metadata": '{"abc": 123}', + } + ) + assert result["short_descr"] == "foo" + assert result["long_descr"] == "bar" + assert result["start_dttm"] == START_DTTM + assert result["end_dttm"] == END_DTTM + assert result["json_metadata"] == '{"abc": 123}' + + +def test_annotation_post_schema_short_descr_null(): + with pytest.raises(ValidationError): + AnnotationPostSchema().load( + {"short_descr": None, "start_dttm": START_STR, "end_dttm": END_STR} + ) + + +def test_annotation_post_schema_start_dttm_null(): + with pytest.raises(ValidationError): + result = AnnotationPostSchema().load( + {"short_descr": "foo", "start_dttm": None, "end_dttm": END_STR} + ) + + +def test_annotation_post_schema_end_dttm_null(): + with pytest.raises(ValidationError): + AnnotationPostSchema().load( + {"short_descr": "foo", "start_dttm": START_STR, "end_dttm": None} + ) + + +def test_annotation_put_schema_empty(): + result = AnnotationPutSchema().load({}) + assert result == {} + + +def test_annotation_put_schema_short_descr_null(): + with pytest.raises(ValidationError): + AnnotationPutSchema().load({"short_descr": None}) + + +def test_annotation_put_schema_start_dttm_null(): + with pytest.raises(ValidationError): + AnnotationPutSchema().load({"start_dttm": None}) + + +def test_annotation_put_schema_end_dttm_null(): + with pytest.raises(ValidationError): + AnnotationPutSchema().load({"end_dttm": None}) + + +def test_annotation_put_schema_json_metadata(): + result = AnnotationPutSchema().load({"json_metadata": '{"abc": 123}'}) + assert result["json_metadata"] == '{"abc": 123}' + + +def test_annotation_put_schema_json_metadata_null(): + result = AnnotationPutSchema().load({"json_metadata": None}) + assert result["json_metadata"] is None + + +def test_annotation_put_schema_json_metadata_empty(): + result = AnnotationPutSchema().load({"json_metadata": ""}) + assert result["json_metadata"] == "" + + +def test_annotation_put_schema_json_metadata_invalid(): + with pytest.raises(ValidationError): + AnnotationPutSchema().load({"json_metadata": "foo bar"})