Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
50ec48a
[PRMP-588] Implement document review update functionality with valida…
NogaNHS Nov 24, 2025
45a2d9b
[PRMP-588] Add mock feature flag for document upload iteration and up…
NogaNHS Nov 24, 2025
02478fe
[PRMP-588] Add tests for document update functionality with custom ke…
NogaNHS Nov 24, 2025
4595a26
[PRMP-588] Add tests for document_upload_review_service.py
NogaNHS Nov 24, 2025
65d1683
[PRMP-588] Add tests for test_dynamo_utils.py.py
NogaNHS Nov 24, 2025
8228030
[PRMP-588] Add error handling for invalid document review requests an…
NogaNHS Nov 25, 2025
fa12809
[PRMP-588] Add unit tests for document review update functionality an…
NogaNHS Nov 25, 2025
5d8bb88
[PRMP-588] format
NogaNHS Nov 25, 2025
d59fb37
Merge branch 'main' into PRMP-588
NogaNHS Nov 25, 2025
6044acc
Merge branch 'main' into PRMP-588
NogaNHS Nov 25, 2025
47bd422
Merge branch 'main' into PRMP-588
NogaNHS Nov 25, 2025
28a14f5
[PRMP-588] Update document review handler to include document version…
NogaNHS Nov 25, 2025
63e274c
[PRMP-588] Add document version to retrieval in get_document_review_s…
NogaNHS Nov 25, 2025
f36a5bc
[PRMP-588] Addressing PR comments
NogaNHS Nov 26, 2025
f574ddb
Merge branch 'main' into PRMP-588
NogaNHS Nov 26, 2025
94b43ab
[PRMP-588] fix unit tests
NogaNHS Nov 26, 2025
5756e54
Merge branch 'main' into PRMP-588
NogaNHS Nov 28, 2025
46eb012
Merge branch 'main' into PRMP-588
NogaNHS Nov 28, 2025
f9835f7
Remove redundant import of DocumentReviewException in document_upload…
NogaNHS Nov 28, 2025
7929b29
Improve error messages in DocumentReviewException raises
NogaNHS Nov 28, 2025
8151df1
[PRMP-588] Add version field to document response and update related …
NogaNHS Nov 28, 2025
b74c5a6
[PRMP-588] addressing PR comments
NogaNHS Nov 28, 2025
e9a9789
Refactor document review status updates for improved clarity and main…
NogaNHS Nov 28, 2025
f6d794c
format
NogaNHS Nov 28, 2025
e535b42
Merge branch 'main' into PRMP-588
NogaNHS Dec 1, 2025
bb57188
Refactor document review update logic to improve error handling and c…
NogaNHS Dec 3, 2025
fe2a6f4
Fix error handling in document review service for missing reviews
NogaNHS Dec 3, 2025
8dee06c
Refactor document review service to improve status transition validat…
NogaNHS Dec 3, 2025
def9193
Merge branch 'main' into PRMP-588
NogaNHS Dec 4, 2025
6f966aa
Refactor document update logic to replace 'key_pair' with 'update_key…
NogaNHS Dec 4, 2025
796dcbc
Merge branch 'main' into PRMP-588
NogaNHS Dec 9, 2025
9646ccf
Refactor document review service tests to use 'version' consistently …
NogaNHS Dec 9, 2025
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
14 changes: 14 additions & 0 deletions .github/workflows/base-lambdas-reusable-deploy-all.yml
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,20 @@ jobs:
secrets:
AWS_ASSUME_ROLE: ${{ secrets.AWS_ASSUME_ROLE }}

deploy_patch_document_review_lambda:
name: Deploy patch document review lambda
uses: ./.github/workflows/base-lambdas-reusable-deploy.yml
with:
environment: ${{ inputs.environment}}
python_version: ${{ inputs.python_version }}
build_branch: ${{ inputs.build_branch}}
sandbox: ${{ inputs.sandbox }}
lambda_handler_name: patch_document_review_handler
lambda_aws_name: PatchDocumentReview
lambda_layer_names: "core_lambda_layer"
secrets:
AWS_ASSUME_ROLE: ${{ secrets.AWS_ASSUME_ROLE }}

deploy_get_document_review_lambda:
name: Deploy get document review lambda
uses: ./.github/workflows/base-lambdas-reusable-deploy.yml
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,3 @@ class DocumentReviewQuerystringParameters(StrEnum):
NEXT_PAGE_TOKEN = "nextPageToken"
UPLOADER = "uploader"
NHS_NUMBER = "nhsNumber"

5 changes: 5 additions & 0 deletions lambdas/enums/document_review_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,9 @@
class DocumentReviewStatus(StrEnum):
PENDING_REVIEW = "PENDING_REVIEW"
APPROVED = "APPROVED"
APPROVED_PENDING_DOCUMENTS = "APPROVED_PENDING_DOCUMENTS"
REJECTED = "REJECTED"
REJECTED_DUPLICATE = "REJECTED_DUPLICATE"
REASSIGNED = "REASSIGNED"
REASSIGNED_PATIENT_UNKNOWN = "REASSIGNED_PATIENT_UNKNOWN"
NEVER_REVIEWED = "NEVER_REVIEWED"
27 changes: 27 additions & 0 deletions lambdas/enums/lambda_error.py
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,33 @@ def create_error_body(self, params: Optional[dict] = None, **kwargs) -> str:
"message": "User is unauthorised to view record",
"fhir_coding": UKCoreSpineError.ACCESS_DENIED,
}
"""
Errors for document review lambda
"""
DocumentReviewNotFound = {
"err_code": "DRV_4041",
"message": "Document review not found",
"fhir_coding": UKCoreSpineError.RESOURCE_NOT_FOUND,
}
DocumentReviewGeneralError = {
"err_code": "DRV_4002",
"message": "An error occurred while fetching the document review",
"fhir_coding": FhirIssueCoding.EXCEPTION,
}
UpdateDocStatusUnavailable = {
"err_code": "DRV_4003",
"message": "This Document is not available for review update",
"fhir_coding": FhirIssueCoding.FORBIDDEN,
}
DocumentReviewInvalidBody = {
"err_code": "DRV_4004",
"message": "Invalid request body",
}
DocumentReviewInvalidNhsNumber = {
"err_code": "DRV_4005",
"message": "The NHS number provided is invalid",
}

"""
Errors for get ods report lambda
"""
Expand Down
1 change: 1 addition & 0 deletions lambdas/enums/logging_app_interaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ class LoggingAppInteraction(Enum):
UPLOAD_CONFIRMATION = "Upload confirmation"
UPDATE_UPLOAD_STATE = "Update upload state"
GET_REVIEW_DOCUMENTS = "Get review documents"
UPDATE_REVIEW = "Update review"
23 changes: 6 additions & 17 deletions lambdas/handlers/get_document_review_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from utils.decorators.set_audit_arg import set_request_context_for_logging
from utils.decorators.validate_patient_id import validate_patient_id
from utils.lambda_exceptions import GetDocumentReviewException
from utils.lambda_handler_utils import validate_review_path_parameters
from utils.lambda_response import ApiGatewayResponse
from utils.request_context import request_context

Expand All @@ -35,15 +36,10 @@ def lambda_handler(event, context):

logger.info("Get Document Review handler has been triggered")
feature_flag_service = FeatureFlagService()
upload_lambda_enabled_flag_object = feature_flag_service.get_feature_flags_by_flag(
feature_flag_service.validate_feature_flag(
FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED
)

if not upload_lambda_enabled_flag_object[FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED]:
logger.info("Feature flag not enabled, event will not be processed")
raise GetDocumentReviewException(404, LambdaError.FeatureFlagDisabled)

# Extract patient_id from query string parameters
query_params = event.get("queryStringParameters", {})
patient_id = query_params.get("patientId", "")

Expand All @@ -53,26 +49,19 @@ def lambda_handler(event, context):
400, LambdaError.DocumentReferenceMissingParameters
)

# Extract id from path parameters
path_params = event.get("pathParameters", {})
document_id = path_params.get("id")

if not document_id:
logger.error("Missing id in path parameters")
raise GetDocumentReviewException(
400, LambdaError.DocumentReferenceMissingParameters
)
document_id, document_version = validate_review_path_parameters(event)

request_context.patient_nhs_no = patient_id

logger.info(
f"Retrieving document review for patient_id: {patient_id}, document_id: {document_id}"
)

# Get document review service
document_review_service = GetDocumentReviewService()
document_review = document_review_service.get_document_review(
patient_id=patient_id, document_id=document_id
patient_id=patient_id,
document_id=document_id,
document_version=document_version,
)

if document_review:
Expand Down
79 changes: 79 additions & 0 deletions lambdas/handlers/patch_document_review_handler.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
from enums.feature_flags import FeatureFlags
from enums.lambda_error import LambdaError
from enums.logging_app_interaction import LoggingAppInteraction
from models.document_review import PatchDocumentReviewRequest
from pydantic import ValidationError
from services.feature_flags_service import FeatureFlagService
from services.update_document_review_service import UpdateDocumentReviewService
from utils.audit_logging_setup import LoggingService
from utils.decorators.ensure_env_var import ensure_environment_variables
from utils.decorators.handle_lambda_exceptions import handle_lambda_exceptions
from utils.decorators.override_error_check import override_error_check
from utils.decorators.set_audit_arg import set_request_context_for_logging
from utils.decorators.validate_patient_id import validate_patient_id
from utils.lambda_exceptions import UpdateDocumentReviewException
from utils.lambda_handler_utils import validate_review_path_parameters
from utils.lambda_response import ApiGatewayResponse
from utils.request_context import request_context

logger = LoggingService(__name__)


@set_request_context_for_logging
@validate_patient_id
@ensure_environment_variables(
names=[
"DOCUMENT_REVIEW_DYNAMODB_NAME",
]
)
@override_error_check
@handle_lambda_exceptions
def lambda_handler(event, context):
request_context.app_interaction = LoggingAppInteraction.UPDATE_REVIEW.value

logger.info("Patch Document Review handler has been triggered")
feature_flag_service = FeatureFlagService()
feature_flag_service.validate_feature_flag(
FeatureFlags.UPLOAD_DOCUMENT_ITERATION_3_ENABLED
)

query_params = event.get("queryStringParameters", {})
patient_id = query_params.get("patientId")

if not patient_id:
logger.error("Missing patient_id in query string parameters")
raise UpdateDocumentReviewException(400, LambdaError.PatientIdNoKey)

document_id, document_version = validate_review_path_parameters(event)

reviewer_ods_code = request_context.authorization.get(
"selected_organisation", {}
).get("org_ods_code")

if not reviewer_ods_code:
logger.error("Missing ODS code in authorization token")
raise UpdateDocumentReviewException(
401, LambdaError.DocumentReferenceUnauthorised
)

body = event.get("body")
try:
review_request = PatchDocumentReviewRequest.model_validate_json(body)
except ValidationError as e:
logger.error(f"Invalid request body: {str(e)}")
raise UpdateDocumentReviewException(400, LambdaError.DocumentReviewInvalidBody)

document_review_service = UpdateDocumentReviewService()
document_review_service.update_document_review(
patient_id=patient_id,
document_id=document_id,
document_version=document_version,
update_data=review_request,
reviewer_ods_code=reviewer_ods_code,
)

logger.info(
"Document review updated successfully",
{"Result": "Successful document review update"},
)
return ApiGatewayResponse(200, "", "PATCH").create_api_gateway_response()
73 changes: 66 additions & 7 deletions lambdas/models/document_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
from enums.document_review_status import DocumentReviewStatus
from enums.metadata_field_names import DocumentReferenceMetadataFields
from enums.snomed_codes import SnomedCodes
from pydantic import BaseModel, ConfigDict, Field
from pydantic.alias_generators import to_pascal, to_camel
from pydantic import BaseModel, ConfigDict, Field, model_validator
from pydantic.alias_generators import to_camel, to_pascal
from utils.exceptions import InvalidNhsNumberException
from utils.utilities import validate_nhs_number


class DocumentReviewFileDetails(BaseModel):
Expand Down Expand Up @@ -41,10 +43,8 @@ class DocumentUploadReviewReference(BaseModel):
upload_date: int
files: list[DocumentReviewFileDetails] = Field(min_length=1)
nhs_number: str
ttl: int | None = Field(
alias=str(DocumentReferenceMetadataFields.TTL.value), default=None
)
document_reference_id: str | None = Field(default=None)
version: int = Field(default=1)
document_reference_id: str = Field(default=None)
document_snomed_code_type: str = Field(default=SnomedCodes.LLOYD_GEORGE.value.code)

def model_dump_camel_case(self, *args, **kwargs):
Expand All @@ -66,4 +66,63 @@ def camelize(self, model: dict) -> dict:
value = result
camel_case_dict[to_camel(key)] = value

return camel_case_dict
return camel_case_dict

class PatchDocumentReviewRequest(BaseModel):
model_config = ConfigDict(
validate_by_alias=True,
populate_by_name=True,
alias_generator=to_camel,
use_enum_values=True,
)

review_status: DocumentReviewStatus = Field(..., description="Review outcome")
document_reference_id: str | None = Field(
default=None,
description="Document reference ID (required when status is APPROVED)",
)
nhs_number: str | None = Field(
default=None,
description="New NHS number (required when status is REASSIGNED)",
)

@model_validator(mode="after")
def validate_document_reference_id(self):
"""Ensure document_reference_id is provided when review_status is APPROVED."""
if (
self.review_status == DocumentReviewStatus.APPROVED
and not self.document_reference_id
):
raise ValueError(
"document_reference_id is required when review_status is APPROVED"
)
elif (
self.review_status != DocumentReviewStatus.APPROVED
and self.document_reference_id
):
raise ValueError(
"document_reference_id is not required when review_status is not APPROVED"
)
return self

@model_validator(mode="after")
def validate_reassign_nhs_number(self):
"""
Validate the reassignment of the NHS number after the input data model has been populated.

Checks whether the `reassigned_nhs_number` field has been provided and is valid when the
`review_status` reflects a reassigned state. Raises an error if validation fails.
"""
if (
self.review_status == DocumentReviewStatus.REASSIGNED
and not self.nhs_number
):
raise ValueError(
"reassigned_nhs_number is required when review_status is REASSIGNED or REASSIGNED_PATIENT_UNKNOWN"
)
elif self.review_status == DocumentReviewStatus.REASSIGNED and self.nhs_number:
try:
validate_nhs_number(self.nhs_number)
except InvalidNhsNumberException:
raise ValueError("Invalid NHS number")
return self
8 changes: 4 additions & 4 deletions lambdas/services/authoriser_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ def deny_access_policy(self, path, user_role, nhs_number: str = None):

case doc_ref if re.match(doc_ref_pattern, doc_ref):
deny_resource = True
if (is_user_gp_admin or is_user_gp_clinical) and patient_access_is_allowed:
if (
is_user_gp_admin or is_user_gp_clinical
) and patient_access_is_allowed:
deny_resource = False
if patient_access_is_allowed and access_to_deceased_patient:
deny_resource = True
Expand Down Expand Up @@ -125,9 +127,7 @@ def deny_access_policy(self, path, user_role, nhs_number: str = None):
not patient_access_is_allowed or is_user_gp_clinical or is_user_pcse
)
case path if path.startswith("/DocumentReview/"):
deny_resource = (
not patient_access_is_allowed
)
deny_resource = not patient_access_is_allowed

case "/UploadState":
deny_resource = (
Expand Down
Loading
Loading