Skip to content

Commit

Permalink
fix(annotataion): handle required fields properly (#17234)
Browse files Browse the repository at this point in the history
  • Loading branch information
villebro authored Oct 27, 2021
1 parent 743f4b6 commit 4316fe6
Show file tree
Hide file tree
Showing 7 changed files with 195 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
22 changes: 17 additions & 5 deletions superset/annotation_layers/annotations/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand All @@ -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,
)
2 changes: 1 addition & 1 deletion superset/annotation_layers/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
11 changes: 10 additions & 1 deletion tests/integration_tests/annotation_layers/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
create_annotation_layers,
get_end_dttm,
get_start_dttm,
START_STR,
END_STR,
)

ANNOTATION_LAYERS_COUNT = 10
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions tests/integration_tests/annotation_layers/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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:
Expand Down
157 changes: 157 additions & 0 deletions tests/integration_tests/annotation_layers/schema_tests.py
Original file line number Diff line number Diff line change
@@ -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"})

0 comments on commit 4316fe6

Please sign in to comment.