You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by vi...@apache.org on 2021/10/27 05:43:57 UTC
[superset] branch master updated: fix(annotataion): handle required
fields properly (#17234)
This is an automated email from the ASF dual-hosted git repository.
villebro pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 4316fe6 fix(annotataion): handle required fields properly (#17234)
4316fe6 is described below
commit 4316fe6ae64522ff5ac31383b378b94be786ad60
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Wed Oct 27 07:42:28 2021 +0200
fix(annotataion): handle required fields properly (#17234)
---
.../src/views/CRUD/annotation/AnnotationModal.tsx | 6 +-
.../CRUD/annotationlayers/AnnotationLayerModal.tsx | 2 +-
superset/annotation_layers/annotations/schemas.py | 22 ++-
superset/annotation_layers/schemas.py | 2 +-
.../annotation_layers/api_tests.py | 11 +-
.../annotation_layers/fixtures.py | 6 +
.../annotation_layers/schema_tests.py | 157 +++++++++++++++++++++
7 files changed, 195 insertions(+), 11 deletions(-)
diff --git a/superset-frontend/src/views/CRUD/annotation/AnnotationModal.tsx b/superset-frontend/src/views/CRUD/annotation/AnnotationModal.tsx
index bab4d2f..4d8fedc 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<AnnotationModalProps> = ({
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 d683448..e3ea229 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<AnnotationLayerModalProps> = ({
};
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 72edcc1..fd03b14 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 4e7c6de..4cd7493 100644
--- a/superset/annotation_layers/schemas.py
+++ b/superset/annotation_layers/schemas.py
@@ -40,7 +40,7 @@ annotation_layer_descr = "Give a description for this annotation layer"
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 292829c..466bf7e 100644
--- a/tests/integration_tests/annotation_layers/api_tests.py
+++ b/tests/integration_tests/annotation_layers/api_tests.py
@@ -31,6 +31,8 @@ from tests.integration_tests.annotation_layers.fixtures import (
create_annotation_layers,
get_end_dttm,
get_start_dttm,
+ START_STR,
+ END_STR,
)
ANNOTATION_LAYERS_COUNT = 10
@@ -201,7 +203,10 @@ class TestAnnotationLayerApi(SupersetTestCase):
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 @@ class TestAnnotationLayerApi(SupersetTestCase):
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 @@ class TestAnnotationLayerApi(SupersetTestCase):
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 cb7647d..2576cf4 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 @@ from tests.integration_tests.test_app import app
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 0000000..ff7fc30
--- /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"})