Skip to content
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

#2714 fields with associated free other value #2786

Merged
merged 3 commits into from
Jul 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@
"data_dictionary_text": "Age of the individual, specified as a range, either open-ended (<n, >n) or as a range delimited by a hyphen following 5-year age increments (m-n)",
"required": false
},
{
"key": "sexAtBirth",
"type": "string",
"data_dictionary_text": "Sex at birth of an individual.",
"required": true,
"values": [
"male",
"female",
"unknown",
"other"
]
},
{
"key": "confirmationDate",
"type": "date",
Expand Down
22 changes: 15 additions & 7 deletions data-serving/reusable-data-service/data_service/model/age_range.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
from data_service.model.document import Document
from data_service.util.errors import ValidationError


@dataclasses.dataclass
class AgeRange(Document):
"""I represent a numerical range within which a person's age lies (inclusive of both limits).
To avoid reidentifying people who have been anonymised by this
application, I will only tell you their age to within five years (unless they are infants)."""

lower: int = None
upper: int = None

Expand All @@ -21,7 +23,7 @@ def fix_my_boundaries(self):
self.lower = (self.lower // 5) * 5 + 1
if self.upper is not None and self.upper != 1 and self.upper % 5 != 0:
self.upper = ((self.upper // 5) + 1) * 5

def validate(self):
"""I must represent the range [0,1], or a range greater than five years, and must
have a positive lower bound and an upper bound below 121."""
Expand All @@ -31,11 +33,17 @@ def validate(self):
if self.upper is None:
raise ValidationError("Age Range must have an upper bound")
if self.lower < 0:
raise ValidationError(f"Lower bound {self.lower} is below the minimum permissible 0")
raise ValidationError(
f"Lower bound {self.lower} is below the minimum permissible 0"
)
if self.upper < 1:
raise ValidationError(f"Upper bound {self.upper} is below the minimum permissible 1")
raise ValidationError(
f"Upper bound {self.upper} is below the minimum permissible 1"
)
if self.upper > 120:
raise ValidationError(f"Upper bound {self.upper} is above the maximum permissible 120")
raise ValidationError(
f"Upper bound {self.upper} is above the maximum permissible 120"
)
# deal with the special case first
if self.lower == 0 and self.upper == 1:
return
Expand All @@ -47,10 +55,10 @@ def validate(self):
def from_dict(cls, dict_description):
ages = cls()
# age ranges can be open-ended according to the data dictionary, which we map onto our absolute limits
ages.lower = dict_description.get('lower', 0)
ages.upper = dict_description.get('upper', 120)
ages.lower = dict_description.get("lower", 0)
ages.upper = dict_description.get("upper", 120)
return ages

@classmethod
def none_field_values(cls):
return ['', '']
return ["", ""]
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ def make_custom_case_class(name: str, field_models=[]) -> type:
field_models is a list of model objects describing the fields for the data dictionary
and for validation."""
global Case
fields = [f.dataclasses_tuple() for f in field_models]
fields = []
for f in field_models:
fields += f.dataclasses_tuples()
try:
new_case_class = dataclasses.make_dataclass(name, fields, bases=(Document,))
except TypeError as e:
Expand Down
19 changes: 13 additions & 6 deletions data-serving/reusable-data-service/data_service/model/field.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,21 @@ def from_dict(cls, dictionary):
def python_type(self) -> type:
return self.model_type(self.type)

def dataclasses_tuple(self) -> (str, type, dataclasses.Field):
def dataclasses_tuples(self):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-> (str, type, dataclasses.Field)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be -> List[(str, type, dataclasses.Field)] but I couldn't get the type checker in Python 3.10 to accept that. Maybe a bug in the type checker? Anyway it wouldn't load the module so I removed the type hint.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

list[tuple[str, type, dataclasses.Field]] should work (from typing import List, Tuple not required anymore)

# Note that the default value here is always None, even if I have a default value!
# That's because the meaning of "required" in a field model is "a user _is required_ to
# supply a value" and the meaning of "default" is "for cases that don't already have this
# key, use the default value"; if I give every Case the default value then there's no sense
# in which a user is required to define it themselves.
return (
self.key,
self.python_type(),
dataclasses.field(init=False, default=None),
)
fields = [
(
self.key,
self.python_type(),
dataclasses.field(init=False, default=None),
)
]
if self.values is not None and "other" in self.values:
fields.append(
(f"{self.key}_other", str, dataclasses.field(init=False, default=None))
)
return fields
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@
"note": "Excluded upon this day, for reasons"
},
"caseStatus": "omit_error",
"pathogenStatus": "endemic"
"pathogenStatus": "endemic",
"sexAtBirth": "female"
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@
"sourceId": "fedc09876543210987654321"
},
"caseStatus": "probable",
"pathogenStatus": "emerging"
"pathogenStatus": "emerging",
"sexAtBirth": "female"
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@
}
},
"caseStatus": "probable",
"pathogenStatus": "unknown"
"pathogenStatus": "unknown",
"sexAtBirth": "female"
}
2 changes: 1 addition & 1 deletion data-serving/reusable-data-service/tests/test_age_range.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@


def test_age_range_massages_input_to_match_buckets():
ages = AgeRange(2,6)
ages = AgeRange(2, 6)
assert ages.lower == 1
assert ages.upper == 10

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,9 @@ def test_post_case_list_cases_geojson_round_trip(client_with_patched_mongo):
def test_post_case_list_cases_with_age_round_trip(client_with_patched_mongo):
with open("./tests/data/case.minimal.json") as case_file:
case_doc = json.load(case_file)
case_doc['age'] = {
'lower': 4,
'upper': 12,
case_doc["age"] = {
"lower": 4,
"upper": 12,
}
post_response = client_with_patched_mongo.post(
"/api/cases",
Expand Down
18 changes: 9 additions & 9 deletions data-serving/reusable-data-service/tests/test_case_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ def test_case_with_geojson_is_valid():

def test_csv_header():
header_line = Case.csv_header()
header_fields = header_line.split(',')
assert 'caseStatus' in header_fields
assert 'location.latitude' in header_fields
header_fields = header_line.split(",")
assert "caseStatus" in header_fields
assert "location.latitude" in header_fields


def test_csv_row_with_no_id():
Expand All @@ -45,9 +45,9 @@ def test_csv_row_with_no_id():
case.caseStatus = "probable"
case.pathogenStatus = "emerging"
csv = case.to_csv()
csv_fields = csv.split(',')
assert 'probable' in csv_fields
assert '2022-06-13' in csv_fields
csv_fields = csv.split(",")
assert "probable" in csv_fields
assert "2022-06-13" in csv_fields


def test_csv_row_with_id():
Expand All @@ -64,9 +64,9 @@ def test_csv_row_with_id():
case.pathogenStatus = "unknown"
csv = case.to_csv()
csv = case.to_csv()
csv_fields = csv.split(',')
assert 'probable' in csv_fields
assert '2022-06-13' in csv_fields
csv_fields = csv.split(",")
assert "probable" in csv_fields
assert "2022-06-13" in csv_fields


def test_apply_update_to_case():
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import csv
import io
import json

from tests.end_to_end_fixture import client_with_patched_mongo


def test_adding_field_then_downloading_case(client_with_patched_mongo):
with open("./tests/data/case.minimal.json") as json_file:
case_doc = json.load(json_file)
response = client_with_patched_mongo.post(
"/api/schema",
json={
Expand All @@ -14,17 +17,12 @@ def test_adding_field_then_downloading_case(client_with_patched_mongo):
},
)
assert response.status_code == 201

case_doc["mySymptoms"] = "coughs, sneezles"

response = client_with_patched_mongo.post(
"/api/cases",
json={
"confirmationDate": "2022-06-01T00:00:00.000Z",
"caseReference": {
"status": "UNVERIFIED",
"sourceId": "24680135792468013579fedc",
},
"caseStatus": "probable",
"mySymptoms": "coughs, sneezles",
},
json=case_doc,
)
assert response.status_code == 201
response = client_with_patched_mongo.get("/api/cases")
Expand All @@ -36,6 +34,8 @@ def test_adding_field_then_downloading_case(client_with_patched_mongo):


def test_adding_field_then_downloading_csv(client_with_patched_mongo):
with open("./tests/data/case.minimal.json") as json_file:
case_doc = json.load(json_file)
response = client_with_patched_mongo.post(
"/api/schema",
json={
Expand All @@ -45,17 +45,11 @@ def test_adding_field_then_downloading_csv(client_with_patched_mongo):
},
)
assert response.status_code == 201

case_doc["someField"] = "well, what have we here"
response = client_with_patched_mongo.post(
"/api/cases",
json={
"confirmationDate": "2022-06-01T00:00:00.000Z",
"caseReference": {
"status": "UNVERIFIED",
"sourceId": "24680135792468013579fedc",
},
"caseStatus": "probable",
"someField": "well, what have we here",
},
json=case_doc,
)
assert response.status_code == 201
response = client_with_patched_mongo.post(
Expand All @@ -73,16 +67,11 @@ def test_adding_field_then_downloading_csv(client_with_patched_mongo):
def test_required_field_default_value_spread_to_existing_cases(
client_with_patched_mongo,
):
with open("./tests/data/case.minimal.json") as json_file:
case_doc = json.load(json_file)
response = client_with_patched_mongo.post(
"/api/cases",
json={
"confirmationDate": "2022-06-01T00:00:00.000Z",
"caseReference": {
"status": "UNVERIFIED",
"sourceId": "24680135792468013579fedc",
},
"caseStatus": "probable",
},
json=case_doc,
)
assert response.status_code == 201
response = client_with_patched_mongo.post(
Expand All @@ -105,6 +94,8 @@ def test_required_field_default_value_spread_to_existing_cases(


def test_required_field_becomes_required_in_validation(client_with_patched_mongo):
with open("./tests/data/case.minimal.json") as json_file:
case_doc = json.load(json_file)
response = client_with_patched_mongo.post(
"/api/schema",
json={
Expand All @@ -118,21 +109,16 @@ def test_required_field_becomes_required_in_validation(client_with_patched_mongo
assert response.status_code == 201
response = client_with_patched_mongo.post(
"/api/cases",
json={
"confirmationDate": "2022-06-01T00:00:00.000Z",
"caseReference": {
"status": "UNVERIFIED",
"sourceId": "24680135792468013579fedc",
},
"caseStatus": "probable",
},
json=case_doc,
)
assert response.status_code == 422


def test_field_enumerating_allowed_values_forbids_other_value(
client_with_patched_mongo,
):
with open("./tests/data/case.minimal.json") as json_file:
case_doc = json.load(json_file)
response = client_with_patched_mongo.post(
"/api/schema",
json={
Expand All @@ -144,16 +130,45 @@ def test_field_enumerating_allowed_values_forbids_other_value(
},
)
assert response.status_code == 201

case_doc["customPathogenStatus"] = "Something Else"

response = client_with_patched_mongo.post(
"/api/cases",
json=case_doc,
)
assert response.status_code == 422


def test_adding_enumerated_field_with_other_value(client_with_patched_mongo):
with open("./tests/data/case.minimal.json") as json_file:
case_doc = json.load(json_file)
response = client_with_patched_mongo.post(
"/api/schema",
json={
"confirmationDate": "2022-06-01T00:00:00.000Z",
"caseReference": {
"status": "UNVERIFIED",
"sourceId": "24680135792468013579fedc",
},
"caseStatus": "probable",
"customPathogenStatus": "Something Else",
"name": "customPathogenStatus",
"type": "string",
"description": "Whether the infection is associated with an endemic or emerging incidence",
"values": ["Endemic", "Emerging", "Unknown", "other"],
"required": False,
},
)
assert response.status_code == 422
assert response.status_code == 201

case_doc["customPathogenStatus"] = "other"
case_doc["customPathogenStatus_other"] = "Neopanspermia"
response = client_with_patched_mongo.post(
"/api/cases",
json=case_doc,
)
assert response.status_code == 201
response = client_with_patched_mongo.post(
"/api/cases/download", json={"format": "csv"}
)
assert response.status_code == 200
csv_file = io.StringIO(response.get_data().decode("utf-8"))
csv_reader = csv.DictReader(csv_file)
cases = [c for c in csv_reader]
assert len(cases) == 1
case = cases[0]
assert case["customPathogenStatus_other"] == "Neopanspermia"
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from tests.util import does_not_raise


def test_point_needs_two_coordinates():
p = Point()
p.coordinates = []
Expand Down
1 change: 1 addition & 0 deletions data-serving/reusable-data-service/tests/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from contextlib import contextmanager


@contextmanager
def does_not_raise(exception):
try:
Expand Down