-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: ✨ add functions to check required and blank fields [3/7] #963
base: feat/sprout-checks-2-simple-helper-functions
Are you sure you want to change the base?
Changes from all commits
b6a5a86
b174133
72db9d8
cad7d43
7881d41
f531760
e5cefa8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
from seedcase_sprout.core.checks.check_error import CheckError | ||
from seedcase_sprout.core.checks.required_fields import RequiredFieldType | ||
from seedcase_sprout.core.sprout_checks.get_blank_value_for_type import ( | ||
get_blank_value_for_type, | ||
) | ||
from seedcase_sprout.core.sprout_checks.get_json_path_to_resource_field import ( | ||
get_json_path_to_resource_field, | ||
) | ||
|
||
SPROUT_BLANK_ERROR_MESSAGE = "The '{field_name}' field is blank, please fill it in." | ||
|
||
|
||
def check_fields_not_blank( | ||
properties: dict, fields: dict[str, RequiredFieldType], index: int | None = None | ||
) -> list[CheckError]: | ||
"""Checks that fields in `fields` are not blank if they are present. | ||
|
||
Fields not present in `properties` are not checked. | ||
|
||
For resource properties, an index may be supplied, if the resource properties are | ||
part of a set of package properties. | ||
|
||
Args: | ||
properties: The properties where the fields are. | ||
fields: A set of fields and their types. | ||
index: The index of the resource properties. Defaults to None. | ||
|
||
Returns: | ||
A list of errors. An empty list if no errors were found. | ||
""" | ||
return [ | ||
CheckError( | ||
message=SPROUT_BLANK_ERROR_MESSAGE.format(field_name=field), | ||
json_path=get_json_path_to_resource_field(field, index), | ||
validator="blank", | ||
) | ||
for field, type in fields.items() | ||
if properties.get(field) == get_blank_value_for_type(type) | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
from seedcase_sprout.core.checks.check_error import CheckError | ||
from seedcase_sprout.core.checks.required_fields import RequiredFieldType | ||
from seedcase_sprout.core.sprout_checks.get_json_path_to_resource_field import ( | ||
get_json_path_to_resource_field, | ||
) | ||
|
||
CHECKS_REQUIRED_ERROR_MESSAGE = "'{field_name}' is a required property" | ||
|
||
|
||
def check_fields_present( | ||
properties: dict, | ||
required_fields: dict[str, RequiredFieldType], | ||
index: int | None = None, | ||
) -> list[CheckError]: | ||
"""Checks that all fields in `required_fields` are present. | ||
|
||
For resource properties, an index may be supplied, if the resource properties are | ||
part of a set of package properties. | ||
|
||
Args: | ||
properties: The properties to check. | ||
required_fields: The set of required fields and their types. | ||
index: The index of the resource properties. Defaults to None. | ||
|
||
Returns: | ||
A list of errors. An empty list if no errors were found. | ||
""" | ||
return [ | ||
CheckError( | ||
message=CHECKS_REQUIRED_ERROR_MESSAGE.format(field_name=field), | ||
json_path=get_json_path_to_resource_field(field, index), | ||
validator="required", | ||
) | ||
for field in required_fields.keys() | ||
if properties.get(field) is None | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
from seedcase_sprout.core.checks.check_error import CheckError | ||
from seedcase_sprout.core.checks.required_fields import RequiredFieldType | ||
from seedcase_sprout.core.sprout_checks.check_fields_not_blank import ( | ||
SPROUT_BLANK_ERROR_MESSAGE, | ||
) | ||
from seedcase_sprout.core.sprout_checks.get_blank_value_for_type import ( | ||
get_blank_value_for_type, | ||
) | ||
|
||
|
||
def check_list_item_field_not_blank( | ||
properties: dict, list_name: str, field_name: str, field_type=RequiredFieldType.str | ||
) -> list[CheckError]: | ||
"""Checks that the specified field of items in a list is not blank. | ||
|
||
Args: | ||
properties: The properties object containing the list. | ||
list_name: The name of the list field. | ||
field_name: The name of the item field. | ||
field_type: The type of the item field. Defaults to str. | ||
|
||
Returns: | ||
A list of errors. An empty list if no errors were found. | ||
""" | ||
return [ | ||
CheckError( | ||
message=SPROUT_BLANK_ERROR_MESSAGE.format(field_name=field_name), | ||
json_path=f"$.{list_name}[{index}].{field_name}", | ||
validator="blank", | ||
) | ||
for index, item in enumerate(properties.get(list_name, [])) | ||
if item.get(field_name) == get_blank_value_for_type(field_type) | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
from seedcase_sprout.core.checks.check_error import CheckError | ||
from seedcase_sprout.core.sprout_checks.check_fields_not_blank import ( | ||
check_fields_not_blank, | ||
) | ||
from seedcase_sprout.core.sprout_checks.check_list_item_field_not_blank import ( | ||
check_list_item_field_not_blank, | ||
) | ||
from seedcase_sprout.core.sprout_checks.required_fields import ( | ||
PACKAGE_SPROUT_REQUIRED_FIELDS, | ||
) | ||
|
||
|
||
def check_required_package_properties_not_blank( | ||
properties: dict, | ||
) -> list[CheckError]: | ||
"""Checks that required package properties fields are not blank. | ||
|
||
Both Sprout-specific required fields and fields required by the Data Package | ||
standard are checked. | ||
|
||
Args: | ||
properties: The package properties. | ||
|
||
Returns: | ||
A list of errors. An empty list if no errors were found. | ||
""" | ||
errors = check_fields_not_blank(properties, PACKAGE_SPROUT_REQUIRED_FIELDS) | ||
errors += check_list_item_field_not_blank(properties, "contributors", "title") | ||
errors += check_list_item_field_not_blank(properties, "sources", "title") | ||
errors += check_list_item_field_not_blank(properties, "licenses", "name") | ||
errors += check_list_item_field_not_blank(properties, "licenses", "path") | ||
Comment on lines
+28
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are nested package properties fields (so, technically, fields on contributor properties, source properties, etc.). It would be nice if they were also coming from a constant like the top-level fields from |
||
return errors |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
from pytest import mark | ||
|
||
from seedcase_sprout.core.checks.required_fields import RequiredFieldType | ||
from seedcase_sprout.core.sprout_checks.check_fields_not_blank import ( | ||
check_fields_not_blank, | ||
) | ||
from seedcase_sprout.core.sprout_checks.get_json_path_to_resource_field import ( | ||
get_json_path_to_resource_field, | ||
) | ||
|
||
FIELDS = {"name": RequiredFieldType.str, "tags": RequiredFieldType.list} | ||
|
||
|
||
@mark.parametrize("index", [None, 2]) | ||
def test_no_error_found_in_properties_with_populated_fields(index): | ||
"""Should pass properties with fields populated.""" | ||
properties = {"name": "My name", "tags": ["a", "b"]} | ||
|
||
assert check_fields_not_blank(properties, FIELDS, index) == [] | ||
|
||
|
||
@mark.parametrize("index", [None, 2]) | ||
def test_no_error_found_in_properties_with_fields_missing(index): | ||
"""Should pass properties without the specified fields.""" | ||
assert check_fields_not_blank({}, FIELDS, index) == [] | ||
|
||
|
||
@mark.parametrize("index", [None, 2]) | ||
def test_error_found_if_properties_have_a_blank_field(index): | ||
"""Should find an error if properties contain a blank field.""" | ||
properties = {"name": "My name", "tags": []} | ||
|
||
errors = check_fields_not_blank(properties, FIELDS, index) | ||
|
||
assert len(errors) == 1 | ||
assert "blank" in errors[0].message | ||
assert errors[0].json_path == get_json_path_to_resource_field("tags", index) | ||
assert errors[0].validator == "blank" | ||
|
||
|
||
@mark.parametrize("index", [None, 2]) | ||
def test_error_found_if_properties_have_multiple_blank_fields(index): | ||
"""Should find an error if properties contain multiple blank fields.""" | ||
properties = {"name": "", "tags": []} | ||
|
||
errors = check_fields_not_blank(properties, FIELDS, index) | ||
|
||
assert len(errors) == 2 | ||
assert all(error.validator == "blank" for error in errors) | ||
assert any( | ||
error.json_path == get_json_path_to_resource_field("name", index) | ||
for error in errors | ||
) | ||
assert any( | ||
error.json_path == get_json_path_to_resource_field("tags", index) | ||
for error in errors | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
from pytest import mark | ||
|
||
from seedcase_sprout.core.checks.required_fields import RequiredFieldType | ||
from seedcase_sprout.core.sprout_checks.check_fields_present import ( | ||
check_fields_present, | ||
) | ||
from seedcase_sprout.core.sprout_checks.get_json_path_to_resource_field import ( | ||
get_json_path_to_resource_field, | ||
) | ||
|
||
REQUIRED_FIELDS = {"name": RequiredFieldType.str, "tags": RequiredFieldType.list} | ||
|
||
|
||
@mark.parametrize("index", [None, 2]) | ||
def test_no_error_found_in_properties_with_required_fields(index): | ||
"""Should pass properties with required fields present and populated.""" | ||
properties = {"name": "My name", "tags": ["a", "b"]} | ||
|
||
assert check_fields_present(properties, REQUIRED_FIELDS, index) == [] | ||
|
||
|
||
@mark.parametrize("index", [None, 2]) | ||
def test_no_error_found_in_properties_with_required_fields_blank(index): | ||
"""Should pass properties with required fields present but blank.""" | ||
properties = {"name": "", "tags": []} | ||
|
||
assert check_fields_present(properties, REQUIRED_FIELDS, index) == [] | ||
|
||
|
||
@mark.parametrize("index", [None, 2]) | ||
def test_error_found_if_there_is_a_missing_required_field(index): | ||
"""Should find an error if there is a missing required field.""" | ||
properties = {"name": "My name"} | ||
|
||
errors = check_fields_present(properties, REQUIRED_FIELDS, index) | ||
|
||
assert len(errors) == 1 | ||
assert "required" in errors[0].message | ||
assert errors[0].json_path == get_json_path_to_resource_field("tags", index) | ||
assert errors[0].validator == "required" | ||
|
||
|
||
@mark.parametrize("index", [None, 2]) | ||
def test_error_found_if_there_are_multiple_missing_required_fields(index): | ||
"""Should find an error if there are multiple missing required fields.""" | ||
errors = check_fields_present({}, REQUIRED_FIELDS, index) | ||
|
||
assert len(errors) == 2 | ||
assert all(error.validator == "required" for error in errors) | ||
assert any( | ||
error.json_path == get_json_path_to_resource_field("name", index) | ||
for error in errors | ||
) | ||
assert any( | ||
error.json_path == get_json_path_to_resource_field("tags", index) | ||
for error in errors | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
from pytest import mark | ||
|
||
from seedcase_sprout.core.checks.required_fields import RequiredFieldType | ||
from seedcase_sprout.core.sprout_checks.check_list_item_field_not_blank import ( | ||
check_list_item_field_not_blank, | ||
) | ||
from seedcase_sprout.core.sprout_checks.get_blank_value_for_type import ( | ||
get_blank_value_for_type, | ||
) | ||
|
||
|
||
def test_no_error_found_in_properties_without_list(): | ||
"""Should pass if the properties do not contain the specified list.""" | ||
assert check_list_item_field_not_blank({}, "items", "field") == [] | ||
|
||
|
||
@mark.parametrize("items", [[], [{}, {}], [{"a": 1}, {"a": 2}]]) | ||
def test_no_error_found_when_list_does_not_contain_field(items): | ||
"""Should pass if list items do not contain the field.""" | ||
properties = {"items": items} | ||
|
||
assert check_list_item_field_not_blank(properties, "items", "field") == [] | ||
|
||
|
||
def test_no_error_found_when_fields_populated(): | ||
"""Should pass if all fields are populated.""" | ||
properties = {"items": [{"field": "value"}, {"field": "value"}]} | ||
|
||
assert check_list_item_field_not_blank(properties, "items", "field") == [] | ||
|
||
|
||
def test_no_error_found_when_fields_are_of_wrong_type(): | ||
"""Should pass if the fields are present but of the wrong type.""" | ||
properties = {"items": [{"field": "value"}, {"field": ""}]} | ||
|
||
assert ( | ||
check_list_item_field_not_blank( | ||
properties, "items", "field", RequiredFieldType.list | ||
) | ||
== [] | ||
) | ||
|
||
|
||
@mark.parametrize( | ||
"field_type,value", | ||
[(RequiredFieldType.str, "value"), (RequiredFieldType.list, [1])], | ||
) | ||
def test_error_found_if_an_item_has_a_blank_field(field_type, value): | ||
"""Should find an error if there is an item with a blank field.""" | ||
properties = { | ||
"items": [{"field": value}, {"field": get_blank_value_for_type(field_type)}] | ||
} | ||
|
||
errors = check_list_item_field_not_blank(properties, "items", "field", field_type) | ||
|
||
assert len(errors) == 1 | ||
assert "blank" in errors[0].message | ||
assert errors[0].json_path == "$.items[1].field" | ||
assert errors[0].validator == "blank" | ||
|
||
|
||
@mark.parametrize("field_type", RequiredFieldType) | ||
def test_error_found_if_multiple_items_have_a_blank_field(field_type): | ||
"""Should find an error if there are multiple items with a blank field.""" | ||
properties = {"items": [{"field": get_blank_value_for_type(field_type)}] * 2} | ||
|
||
errors = check_list_item_field_not_blank(properties, "items", "field", field_type) | ||
|
||
assert len(errors) == 2 | ||
assert all( | ||
"blank" in error.message and error.validator == "blank" for error in errors | ||
) | ||
assert any(error.json_path == "$.items[0].field" for error in errors) | ||
assert any(error.json_path == "$.items[1].field" for error in errors) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't contributor
title
just recommended and not required as described here?Same with sources
title
?They are
required
to have one property, but which property it is isn't specified.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but we have checks for recommendations turned on in Sprout. At least, I turned them on :P