Fields with the enumerated value other permit free text entry #2714

The free text is stored in an associated string field

data-serving/reusable-data-service/data_service/model/age_range.py

@@ -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
 
@@ -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."""
@@ -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
@@ -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 ["", ""]

data-serving/reusable-data-service/data_service/model/case.py

@@ -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:

data-serving/reusable-data-service/data_service/model/field.py

@@ -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):
         # 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

data-serving/reusable-data-service/tests/test_age_range.py

@@ -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
 

data-serving/reusable-data-service/tests/test_case_end_to_end.py

@@ -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",

data-serving/reusable-data-service/tests/test_case_model.py

@@ -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():
@@ -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():
@@ -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():

data-serving/reusable-data-service/tests/test_case_schema_integration.py

@@ -157,3 +157,41 @@ def test_field_enumerating_allowed_values_forbids_other_value(
         },
     )
     assert response.status_code == 422
+
+
+def test_adding_enumerated_field_with_other_value(client_with_patched_mongo):
+    response = client_with_patched_mongo.post(
+        "/api/schema",
+        json={
+            "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 == 201
+    response = client_with_patched_mongo.post(
+        "/api/cases",
+        json={
+            "confirmationDate": "2022-06-01T00:00:00.000Z",
+            "caseReference": {
+                "status": "UNVERIFIED",
+                "sourceId": "24680135792468013579fedc",
+            },
+            "caseStatus": "probable",
+            "customPathogenStatus": "other",
+            "customPathogenStatus_other": "Neopanspermia",
+        },
+    )
+    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"

data-serving/reusable-data-service/tests/test_geojson_model.py

@@ -5,6 +5,7 @@
 
 from tests.util import does_not_raise
 
+
 def test_point_needs_two_coordinates():
     p = Point()
     p.coordinates = []

data-serving/reusable-data-service/tests/util.py

@@ -2,6 +2,7 @@
 
 from contextlib import contextmanager
 
+
 @contextmanager
 def does_not_raise(exception):
     try: