Skip to content

Commit ac6dcc4

Browse files
VED-854 High/Medium priority Sonar maintainability issues (#1248)
* high/medium priority Sonar maintainability issues * version * field names * post_validator cleanup --------- Co-authored-by: James Wharmby <james.wharmby1@nhs.net> Co-authored-by: James W. <218585646+JamesW1-NHS@users.noreply.github.com>
1 parent f1553ff commit ac6dcc4

5 files changed

Lines changed: 108 additions & 122 deletions

File tree

lambdas/shared/src/common/models/fhir_immunization_post_validators.py

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,19 @@ def __init__(self, imms, vaccine_type):
1919
# Note that the majority of fields require standard validation. Exception not included in the below list is
2020
# reason_code_coding_code, which has its own bespoke validation function.
2121
# Status is mandatory in FHIR, so there is no post-validation for status as it is handled by the FHIR validator.
22-
# NOTE: SOME FIELDS ARE COMMENTED OUT AS THEY ARE REQUIRED ELEMENTS (VALIDATION SHOULD ALWAYS PASS), AND THE
23-
# MEANS TO ACCESS THE VALUE HAS NOT BEEN CONFIRMED. DO NOT DELETE THE FIELDS, THEY MAY NEED REINSTATED LATER.
22+
23+
# Note: some fields are commented out as they are required elements (validation should always pass), and the
24+
# means to access the value has not been confirmed. Do not delete the fields, they may need reinstating later.
25+
26+
# These are the fields that we may require to reinstate in the future:
27+
# vaccination_procedure_display
28+
# vaccine_code_coding_code
29+
# vaccine_code_coding_display
30+
# site_coding_code
31+
# site_coding_display
32+
# route_coding_code
33+
# route_coding_display
34+
2435
self.fields_with_standard_validation = [
2536
FieldNames.patient_identifier_value,
2637
FieldNames.patient_name_given,
@@ -38,17 +49,10 @@ def __init__(self, imms, vaccine_type):
3849
FieldNames.recorded,
3950
FieldNames.primary_source,
4051
FieldNames.vaccination_procedure_code,
41-
# FieldNames.vaccination_procedure_display,
4252
FieldNames.dose_number_positive_int,
43-
# FieldNames.vaccine_code_coding_code,
44-
# FieldNames.vaccine_code_coding_display,
4553
FieldNames.manufacturer_display,
4654
FieldNames.lot_number,
4755
FieldNames.expiration_date,
48-
# FieldNames.site_coding_code,
49-
# FieldNames.site_coding_display,
50-
# FieldNames.route_coding_code,
51-
# FieldNames.route_coding_display,
5256
FieldNames.dose_quantity_value,
5357
FieldNames.dose_quantity_code,
5458
FieldNames.dose_quantity_unit,
@@ -93,8 +97,8 @@ def validate_field(
9397
field_value = obtain_field_value(self.imms, field_name)
9498
self.run_field_validation(mandation_functions, validation_set, field_name, field_location, field_value)
9599

96-
# NOTE: THIS METHOD IS COMMENTED OUT AS IT IS for A REQUIRED ELEMENT (VALIDATION SHOULD ALWAYS PASS),
97-
# AND THE MEANS TO ACCESS THE VALUE HAS NOT BEEN CONFIRMED. DO NOT DELETE THE METHOD, IT MAY NEED REINSTATED LATER.
100+
# Note: this method is commented out as it is for a required element (validation should always pass),
101+
# and the means to access the value has not been confirmed. Do not delete the method, it may need reinstating later.
98102
# def validate_reason_code_coding_code(self, mandation_functions: MandationFunctions, validation_set: dict):
99103
# """
100104
# Runs standard validation for each instance of reason_code_coding_code (note that, because reason_code
@@ -142,8 +146,9 @@ def validate(self):
142146

143147
# Validate reason_code_coding_code fields. Note that there may be multiple of each of these
144148
# - all instances of the field will be validated by the validate_reason_code_coding_field validator
145-
# NOTE: THIS METHOD IS COMMENTED OUT AS IT IS for A REQUIRED ELEMENT (VALIDATION SHOULD ALWAYS PASS), AND THE
146-
# MEANS TO ACCESS THE VALUE HAS NOT BEEN CONFIRMED. DO NOT DELETE THE METHOD, IT MAY NEED REINSTATED LATER.
149+
150+
# Note: this method is commented out as it is for a required element (validation should always pass), and the
151+
# means to access the value has not been confirmed. Do not delete the method, it may need reinstating later.
147152
# self.validate_reason_code_coding_code(mandation_functions, validation_set)
148153

149154
# Raise any errors

lambdas/shared/src/common/models/field_names.py

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,19 @@
77
class FieldNames:
88
"""Stores the field name strings for fields of note within the FHIR Immunization Resource JSON data"""
99

10+
# Note: some field names are commented out as they are required elements (validation should always pass), and the
11+
# means to access the value has not been confirmed. Do not delete the field names, they may need reinstating later.
12+
13+
# These are the field names that we may require to reinstate in the future:
14+
# vaccination_procedure_display
15+
# vaccine_code_coding_code
16+
# vaccine_code_coding_display
17+
# site_coding_code
18+
# site_coding_display
19+
# route_coding_code
20+
# route_coding_display
21+
# reason_code_coding_code
22+
1023
target_disease = "target_disease"
1124
target_disease_codes = "target_disease_codes"
1225
patient_identifier_value = "patient_identifier_value"
@@ -25,20 +38,12 @@ class FieldNames:
2538
recorded = "recorded"
2639
primary_source = "primary_source"
2740
vaccination_procedure_code = "vaccination_procedure_code"
28-
# vaccination_procedure_display = "vaccination_procedure_display"
2941
dose_number_positive_int = "dose_number_positive_int"
30-
vaccine_code_coding_code = "vaccine_code_coding_code"
31-
vaccine_code_coding_display = "vaccine_code_coding_display"
3242
manufacturer_display = "manufacturer_display"
3343
lot_number = "lot_number"
3444
expiration_date = "expiration_date"
35-
site_coding_code = "site_coding_code"
36-
site_coding_display = "site_coding_display"
37-
route_coding_code = "route_coding_code"
38-
route_coding_display = "route_coding_display"
3945
dose_quantity_value = "dose_quantity_value"
4046
dose_quantity_code = "dose_quantity_code"
4147
dose_quantity_unit = "dose_quantity_unit"
42-
reason_code_coding_code = "reason_code_coding_code"
4348
location_identifier_value = "location_identifier_value"
4449
location_identifier_system = "location_identifier_system"

lambdas/shared/src/common/models/obtain_field_value.py

Lines changed: 37 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -136,30 +136,11 @@ def vaccination_procedure_code(imms: dict):
136136
"""Obtains vaccination_procedure_code value"""
137137
return get_generic_extension_value(imms, Urls.VACCINATION_PROCEDURE, Urls.SNOMED, "code")
138138

139-
# @staticmethod
140-
# def vaccination_procedure_display(imms: dict):
141-
# """Obtains vaccination_procedure_display value"""
142-
# return get_generic_extension_value(imms, Urls.vaccination_procedure, Urls.snomed, "display")
143-
144139
@staticmethod
145140
def dose_number_positive_int(imms: dict):
146141
"""Obtains dose_number_positive_int value"""
147142
return imms["protocolApplied"][0]["doseNumberPositiveInt"]
148143

149-
# NOTE: THIS METHOD IS COMMENTED OUT AS IT IS for A REQUIRED ELEMENT (VALIDATION SHOULD ALWAYS PASS),
150-
# AND THE MEANS TO ACCESS THE VALUE HAS NOT BEEN CONFIRMED. DO NOT DELETE THE METHOD, IT MAY NEED REINSTATED LATER.
151-
# @staticmethod
152-
# def vaccine_code_coding_code(imms: dict):
153-
# """Obtains vaccine_code_coding_code value"""
154-
# return [x for x in imms["vaccineCode"]["coding"] if x.get("system") == Urls.snomed][0]["code"]
155-
156-
# NOTE: THIS METHOD IS COMMENTED OUT AS IT IS for A REQUIRED ELEMENT (VALIDATION SHOULD ALWAYS PASS),
157-
# AND THE MEANS TO ACCESS THE VALUE HAS NOT BEEN CONFIRMED. DO NOT DELETE THE METHOD, IT MAY NEED REINSTATED LATER.
158-
# @staticmethod
159-
# def vaccine_code_coding_display(imms: dict):
160-
# """Obtains vaccine_code_coding_display value"""
161-
# return [x for x in imms["vaccineCode"]["coding"] if x.get("system") == Urls.snomed][0]["display"]
162-
163144
@staticmethod
164145
def manufacturer_display(imms: dict):
165146
"""Obtains manufacturer_display value"""
@@ -175,34 +156,6 @@ def expiration_date(imms: dict):
175156
"""Obtains expiration_date value"""
176157
return imms["expirationDate"]
177158

178-
# NOTE: THIS METHOD IS COMMENTED OUT AS IT IS for A REQUIRED ELEMENT (VALIDATION SHOULD ALWAYS PASS),
179-
# AND THE MEANS TO ACCESS THE VALUE HAS NOT BEEN CONFIRMED. DO NOT DELETE THE METHOD, IT MAY NEED REINSTATED LATER.
180-
# @staticmethod
181-
# def site_coding_code(imms: dict):
182-
# """Obtains site_coding_code value"""
183-
# return [x for x in imms["site"]["coding"] if x.get("system") == Urls.snomed][0]["code"]
184-
185-
# NOTE: THIS METHOD IS COMMENTED OUT AS IT IS for A REQUIRED ELEMENT (VALIDATION SHOULD ALWAYS PASS),
186-
# AND THE MEANS TO ACCESS THE VALUE HAS NOT BEEN CONFIRMED. DO NOT DELETE THE METHOD, IT MAY NEED REINSTATED LATER.
187-
# @staticmethod
188-
# def site_coding_display(imms: dict):
189-
# """Obtains site_coding_display value"""
190-
# return [x for x in imms["site"]["coding"] if x.get("system") == Urls.snomed][0]["display"]
191-
192-
# NOTE: THIS METHOD IS COMMENTED OUT AS IT IS for A REQUIRED ELEMENT (VALIDATION SHOULD ALWAYS PASS),
193-
# AND THE MEANS TO ACCESS THE VALUE HAS NOT BEEN CONFIRMED. DO NOT DELETE THE METHOD, IT MAY NEED REINSTATED LATER.
194-
# @staticmethod
195-
# def route_coding_code(imms: dict):
196-
# """Obtains route_coding_code value"""
197-
# return [x for x in imms["route"]["coding"] if x.get("system") == Urls.snomed][0]["code"]
198-
199-
# NOTE: THIS METHOD IS COMMENTED OUT AS IT IS for A REQUIRED ELEMENT (VALIDATION SHOULD ALWAYS PASS),
200-
# AND THE MEANS TO ACCESS THE VALUE HAS NOT BEEN CONFIRMED. DO NOT DELETE THE METHOD, IT MAY NEED REINSTATED LATER.
201-
# @staticmethod
202-
# def route_coding_display(imms: dict):
203-
# """Obtains route_coding_display value"""
204-
# return [x for x in imms["route"]["coding"] if x.get("system") == Urls.snomed][0]["display"]
205-
206159
@staticmethod
207160
def dose_quantity_value(imms: dict):
208161
"""Obtains dose_quantity_value value"""
@@ -233,8 +186,43 @@ def location_identifier_system(imms: dict):
233186
"""Obtains location_identifier_system value"""
234187
return imms["location"]["identifier"]["system"]
235188

236-
# NOTE: THIS METHOD IS COMMENTED OUT AS IT IS for A REQUIRED ELEMENT (VALIDATION SHOULD ALWAYS PASS),
237-
# AND THE MEANS TO ACCESS THE VALUE HAS NOT BEEN CONFIRMED. DO NOT DELETE THE METHOD, IT MAY NEED REINSTATED LATER.
189+
# Note: these methods are commented out as they are for required elements (validation should always pass),
190+
# and the means to access the values have not been confirmed. Do not delete the methods, they may need reinstating later.
191+
# @staticmethod
192+
# def vaccination_procedure_display(imms: dict):
193+
# """Obtains vaccination_procedure_display value"""
194+
# return get_generic_extension_value(imms, Urls.vaccination_procedure, Urls.snomed, "display")
195+
#
196+
# @staticmethod
197+
# def vaccine_code_coding_code(imms: dict):
198+
# """Obtains vaccine_code_coding_code value"""
199+
# return [x for x in imms["vaccineCode"]["coding"] if x.get("system") == Urls.snomed][0]["code"]
200+
#
201+
# @staticmethod
202+
# def vaccine_code_coding_display(imms: dict):
203+
# """Obtains vaccine_code_coding_display value"""
204+
# return [x for x in imms["vaccineCode"]["coding"] if x.get("system") == Urls.snomed][0]["display"]
205+
#
206+
# @staticmethod
207+
# def site_coding_code(imms: dict):
208+
# """Obtains site_coding_code value"""
209+
# return [x for x in imms["site"]["coding"] if x.get("system") == Urls.snomed][0]["code"]
210+
#
211+
# @staticmethod
212+
# def site_coding_display(imms: dict):
213+
# """Obtains site_coding_display value"""
214+
# return [x for x in imms["site"]["coding"] if x.get("system") == Urls.snomed][0]["display"]
215+
#
216+
# @staticmethod
217+
# def route_coding_code(imms: dict):
218+
# """Obtains route_coding_code value"""
219+
# return [x for x in imms["route"]["coding"] if x.get("system") == Urls.snomed][0]["code"]
220+
#
221+
# @staticmethod
222+
# def route_coding_display(imms: dict):
223+
# """Obtains route_coding_display value"""
224+
# return [x for x in imms["route"]["coding"] if x.get("system") == Urls.snomed][0]["display"]
225+
#
238226
# @staticmethod
239227
# def reason_code_coding_code(imms: dict, index: int):
240228
# """Obtains reason_code_coding_code value"""

lambdas/shared/tests/test_common/test_immunization_post_validator.py

Lines changed: 38 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -447,20 +447,6 @@ def test_post_dose_number_positive_int(self):
447447
valid_json_data["protocolApplied"][0]["doseNumberString"] = "Dose sequence not recorded"
448448
MandationTests.test_missing_field_accepted(self, dose_number_positive_int_field_location, valid_json_data)
449449

450-
# NOTE: THIS TEST IS COMMENTED OUT AS IT IS TESTING A REQUIRED ELEMENT (VALIDATION SHOULD ALWAYS PASS),
451-
# AND THE MEANS TO ACCESS THE VALUE HAS NOT BEEN CONFIRMED. DO NOT DELETE THE TEST, IT MAY NEED REINSTATED LATER.
452-
# def test_post_vaccine_code_coding_code(self):
453-
# """Test that the JSON data is rejected when vaccine_code_coding_code is absent"""
454-
# field_location = "vaccineCode.coding[?(@.system=='http://snomed.info/sct')].code"
455-
# MandationTests.test_missing_field_accepted(self, field_location)
456-
457-
# NOTE: THIS TEST IS COMMENTED OUT AS IT IS TESTING A REQUIRED ELEMENT (VALIDATION SHOULD ALWAYS PASS),
458-
# AND THE MEANS TO ACCESS THE VALUE HAS NOT BEEN CONFIRMED. DO NOT DELETE THE TEST, IT MAY NEED REINSTATED LATER.
459-
# def test_post_vaccine_code_coding_display(self):
460-
# """Test that the JSON data is accepted when vaccine_code_coding_display is absent"""
461-
# field_location = "vaccineCode.coding[?(@.system=='http://snomed.info/sct')].display"
462-
# MandationTests.test_missing_field_accepted(self, field_location)
463-
464450
def test_post_manufacturer_display(self):
465451
"""
466452
Test that present or absent manufacturer_display is accepted or rejected
@@ -504,35 +490,6 @@ def test_post_expiration_date(self):
504490
for vaccine_type in self.all_vaccine_types:
505491
MandationTests.test_missing_field_accepted(self, field_location, self.completed_json_data[vaccine_type])
506492

507-
# NOTE: THIS TEST IS COMMENTED OUT AS IT IS TESTING A REQUIRED ELEMENT (VALIDATION SHOULD ALWAYS PASS),
508-
# AND THE MEANS TO ACCESS THE VALUE HAS NOT BEEN CONFIRMED. DO NOT DELETE THE TEST, IT MAY NEED REINSTATED LATER.
509-
# def test_post_site_coding_code(self):
510-
# """Test that the JSON data is accepted when site_coding_code is absent"""
511-
# MandationTests.test_missing_field_accepted(self, "site.coding[?(@.system=='http://snomed.info/sct')].code")
512-
513-
# NOTE: THIS TEST IS COMMENTED OUT AS IT IS TESTING A REQUIRED ELEMENT (VALIDATION SHOULD ALWAYS PASS),
514-
# AND THE MEANS TO ACCESS THE VALUE HAS NOT BEEN CONFIRMED. DO NOT DELETE THE TEST, IT MAY NEED REINSTATED LATER.
515-
# def test_post_site_coding_display(self):
516-
# """Test that the JSON data is accepted when site_coding_display is absent"""
517-
# MandationTests.test_missing_field_accepted(self, "site.coding[?(@.system=='http://snomed.info/sct')].display")
518-
519-
# NOTE: THIS TEST IS COMMENTED OUT AS IT IS TESTING A REQUIRED ELEMENT (VALIDATION SHOULD ALWAYS PASS),
520-
# AND THE MEANS TO ACCESS THE VALUE HAS NOT BEEN CONFIRMED. DO NOT DELETE THE TEST, IT MAY NEED REINSTATED LATER.
521-
# def test_post_route_coding_code(self):
522-
# """
523-
# Test that present or absent route_coding_code is accepted or rejected
524-
# as appropriate dependent on other fields
525-
# """
526-
# field_location = "route.coding[?(@.system=='http://snomed.info/sct')].code"
527-
# for vaccine_type in self.all_vaccine_types:
528-
# MandationTests.test_missing_field_accepted(self, field_location, self.completed_json_data[vaccine_type])
529-
530-
# NOTE: THIS TEST IS COMMENTED OUT AS IT IS TESTING A REQUIRED ELEMENT (VALIDATION SHOULD ALWAYS PASS),
531-
# AND THE MEANS TO ACCESS THE VALUE HAS NOT BEEN CONFIRMED. DO NOT DELETE THE TEST, IT MAY NEED REINSTATED LATER.
532-
# def test_post_route_coding_display(self):
533-
# """Test that the JSON data is accepted when route_coding_display is absent"""
534-
# MandationTests.test_missing_field_accepted(self, "route.coding[?(@.system=='http://snomed.info/sct')].display")
535-
536493
def test_post_dose_quantity_value(self):
537494
"""
538495
Test that present or absent dose_quantity_value is accepted or rejected as appropriate dependent on other fields
@@ -572,13 +529,6 @@ def test_post_dose_quantity_unit(self):
572529
self.mock_redis_getter.return_value = self.mock_redis
573530
MandationTests.test_missing_field_accepted(self, "doseQuantity.unit")
574531

575-
# NOTE: THIS TEST IS COMMENTED OUT AS IT IS TESTING A REQUIRED ELEMENT (VALIDATION SHOULD ALWAYS PASS),
576-
# AND THE MEANS TO ACCESS THE VALUE HAS NOT BEEN CONFIRMED. DO NOT DELETE THE TEST, IT MAY NEED REINSTATED LATER.
577-
# def test_post_reason_code_coding_code(self):
578-
# """Test that the JSON data is accepted when reason_code_coding_code is absent"""
579-
# for index in range(len(self.completed_json_data["COVID"]["reasonCode"])):
580-
# MandationTests.test_missing_field_accepted(self, f"reasonCode[{index}].coding[0].code")
581-
582532
def test_post_organization_identifier_system(self):
583533
"""Test that the JSON data is rejected if it does not contain organization_identifier_system"""
584534
self.mock_redis.hget.side_effect = None
@@ -703,3 +653,41 @@ def test_post_no_snomed_code(self):
703653

704654
actual_error_message = str(cm.exception)
705655
self.assertIn(expected_error_message, actual_error_message)
656+
657+
# Note: these tests are commented out as they are testing required elements (validation should always pass),
658+
# and the means to access the values have not been confirmed. Do not delete the tests, they may need reinstating later.
659+
# def test_post_vaccine_code_coding_code(self):
660+
# """Test that the JSON data is rejected when vaccine_code_coding_code is absent"""
661+
# field_location = "vaccineCode.coding[?(@.system=='http://snomed.info/sct')].code"
662+
# MandationTests.test_missing_field_accepted(self, field_location)
663+
#
664+
# def test_post_vaccine_code_coding_display(self):
665+
# """Test that the JSON data is accepted when vaccine_code_coding_display is absent"""
666+
# field_location = "vaccineCode.coding[?(@.system=='http://snomed.info/sct')].display"
667+
# MandationTests.test_missing_field_accepted(self, field_location)
668+
#
669+
# def test_post_site_coding_code(self):
670+
# """Test that the JSON data is accepted when site_coding_code is absent"""
671+
# MandationTests.test_missing_field_accepted(self, "site.coding[?(@.system=='http://snomed.info/sct')].code")
672+
#
673+
# def test_post_site_coding_display(self):
674+
# """Test that the JSON data is accepted when site_coding_display is absent"""
675+
# MandationTests.test_missing_field_accepted(self, "site.coding[?(@.system=='http://snomed.info/sct')].display")
676+
#
677+
# def test_post_route_coding_code(self):
678+
# """
679+
# Test that present or absent route_coding_code is accepted or rejected
680+
# as appropriate dependent on other fields
681+
# """
682+
# field_location = "route.coding[?(@.system=='http://snomed.info/sct')].code"
683+
# for vaccine_type in self.all_vaccine_types:
684+
# MandationTests.test_missing_field_accepted(self, field_location, self.completed_json_data[vaccine_type])
685+
#
686+
# def test_post_route_coding_display(self):
687+
# """Test that the JSON data is accepted when route_coding_display is absent"""
688+
# MandationTests.test_missing_field_accepted(self, "route.coding[?(@.system=='http://snomed.info/sct')].display")
689+
#
690+
# def test_post_reason_code_coding_code(self):
691+
# """Test that the JSON data is accepted when reason_code_coding_code is absent"""
692+
# for index in range(len(self.completed_json_data["COVID"]["reasonCode"])):
693+
# MandationTests.test_missing_field_accepted(self, f"reasonCode[{index}].coding[0].code")

sandbox/Dockerfile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# First Stage
2-
FROM alpine:latest
2+
FROM alpine:3.23.3
33

44
# Create a non-root user
55
RUN addgroup -S appgroup && adduser -S appuser -G appgroup && apk add --no-cache jq
@@ -17,7 +17,7 @@ COPY ./specification ./specification
1717
RUN jq ".paths += $(cat HealthStatusEndpoint.json)" immunisation-fhir-api.json > updated-spec.json
1818

1919
# Second Stage
20-
FROM stoplight/prism:latest
20+
FROM stoplight/prism:5
2121

2222
# Create a non-root user in the Prism container
2323
USER root

0 commit comments

Comments
 (0)