Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -247,6 +247,60 @@ def test_search_document_reference_happy_path_with_category(
}


@mock_aws
@mock_repository
def test_search_document_reference_happy_path_with_category_and_type(
repository: DocumentPointerRepository,
):
doc_ref = load_document_reference("Y05868-736253002-Valid")
doc_pointer = DocumentPointer.from_document_reference(doc_ref)
repository.create(doc_pointer)

# Second pointer different category
doc_ref2 = load_document_reference("Y05868-736253002-Valid")
doc_ref2.id = "Y05868-736253002-Valid2"
doc_ref2.type.coding[0].code = PointerTypes.NEWS2_CHART.coding_value()
doc_ref2.type.coding[0].display = TYPE_ATTRIBUTES.get(
PointerTypes.NEWS2_CHART.value
).get("display")
doc_ref2.category[0].coding[0].code = Categories.OBSERVATIONS.coding_value()
doc_ref2.category[0].coding[0].display = CATEGORY_ATTRIBUTES.get(
Categories.OBSERVATIONS.value
).get("display")
repository.create(DocumentPointer.from_document_reference(doc_ref2))

event = create_test_api_gateway_event(
headers=create_headers(),
query_string_parameters={
Copy link
Copy Markdown
Contributor

@katebobyn-nhs katebobyn-nhs May 22, 2025

Choose a reason for hiding this comment

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

If I'm reading this right, the test is

  1. Creating a pointer of type: MHCP, cat: Care Plan
  2. Creating a pointer of type: NEWS2, cat: Obs
  3. Searching for pointers of type: MHCP, cat: Care Plan
    It then expects the one result (pointer 1) since pointer 2 would be filtered out by both criteria.

I think to prove both filters are being applied there needs to be a test scenario where, e.g., you search for type: MHCP, cat: Obs
Then the expected result is an empty searchset bundle because 1 is filtered out on the basis of failure to match category, and 2 is filtered out on the basis of failure to match type.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'll add that I'll also add searching with multiple categories and the one type to show that the optimisation for the single type is still used and that it wont get other categories even if they're provided because it isnt for that type

"subject:identifier": "https://fhir.nhs.uk/Id/nhs-number|6700028191",
"category": "http://snomed.info/sct|734163000",
"type": "http://snomed.info/sct|736253002",
},
)

result = handler(event, create_mock_context())
body = result.pop("body")

assert result == {
"statusCode": "200",
"headers": default_response_headers(),
"isBase64Encoded": False,
}
parsed_body = json.loads(body)
assert parsed_body == {
"resourceType": "Bundle",
"type": "searchset",
"link": [
{
"relation": "self",
"url": "https://pytest.api.service.nhs.uk/record-locator/consumer/FHIR/R4/DocumentReference?subject:identifier=https://fhir.nhs.uk/Id/nhs-number|6700028191&type=http://snomed.info/sct|736253002&category=http://snomed.info/sct|734163000",
}
],
"total": 1,
"entry": [{"resource": doc_ref.model_dump(exclude_none=True)}],
}


@mock_aws
@mock_repository
def test_search_document_reference_happy_path_with_multiple_categories(
Expand Down
43 changes: 15 additions & 28 deletions layer/nrlf/core/dynamodb/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,41 +226,16 @@ def search(
nhs_number=nhs_number,
custodian=custodian,
pointer_types=pointer_types,
categories=categories,
)

key_conditions = ["patient_key = :patient_key"]
filter_expressions = []
expression_names = {}
expression_values = {":patient_key": f"P#{nhs_number}"}

if len(pointer_types) == 1:
# Optimisation for single pointer type
Comment thread
axelkrastek1-nhs marked this conversation as resolved.
category_id, type_id = _get_sk_ids_for_type(pointer_types[0])
patient_sort = f"C#{category_id}#T#{type_id}"
key_conditions.append("begins_with(patient_sort, :patient_sort)")
expression_values[":patient_sort"] = patient_sort
else:
# Handle single/multiple categories and pointer types with filter expressions
if len(categories) == 1:
split_category = categories[0].split("|")
category_id = (
SYSTEM_SHORT_IDS[split_category[0]] + "-" + split_category[1]
)
patient_sort = f"C#{category_id}"
key_conditions.append("begins_with(patient_sort, :patient_sort)")
expression_values[":patient_sort"] = patient_sort

if len(categories) > 1:
expression_names["#category"] = "category"
category_filters = [
f"#category = :category_{i}" for i in range(len(categories))
]
category_filter_values = {
f":category_{i}": categories[i] for i in range(len(categories))
}
filter_expressions.append(f"({' OR '.join(category_filters)})")
expression_values.update(category_filter_values)

# Add pointer_types filter if provided
if pointer_types:
expression_names["#pointer_type"] = "type"
types_filters = [
f"#pointer_type = :type_{i}" for i in range(len(pointer_types))
Expand All @@ -271,6 +246,18 @@ def search(
filter_expressions.append(f"({' OR '.join(types_filters)})")
expression_values.update(types_filter_values)

# Add categories filter if provided
if categories:
expression_names["#category"] = "category"
category_filters = [
f"#category = :category_{i}" for i in range(len(categories))
]
category_filter_values = {
f":category_{i}": categories[i] for i in range(len(categories))
}
filter_expressions.append(f"({' OR '.join(category_filters)})")
expression_values.update(category_filter_values)

if custodian:
logger.log(
LogReference.REPOSITORY016,
Expand Down
Loading