Skip to content

[PRMP-1465] Post User Restriction#1131

Draft
SWhyteAnswer wants to merge 23 commits intomainfrom
PRMP-1465
Draft

[PRMP-1465] Post User Restriction#1131
SWhyteAnswer wants to merge 23 commits intomainfrom
PRMP-1465

Conversation

@SWhyteAnswer
Copy link
Contributor

Overview

Jira ticket: TBC

Description

Context

Checklist

Tasks for all changes:

Additional tasks for UI changes (delete if not applicable):

  • 1. I have added evidence (to this PR) e.g. screenshots/gifs of all visual changes.

@override_error_check
@ensure_environment_variables(
names=[
"USER_RESTRICTIONS_TABLE_NAME",
Copy link
Contributor

Choose a reason for hiding this comment

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

double check this is the right var from the terraform

Comment on lines 30 to 52
body = event.get("body")
if not body:
logger.error("Missing request body")
return ApiGatewayResponse(
400, "Missing request body", "POST"
).create_api_gateway_response()

try:
payload = json.loads(body)
except json.JSONDecodeError as exc:
logger.error(exc)
return ApiGatewayResponse(
400, "Invalid request body", "POST"
).create_api_gateway_response()

# Required fields for creating a restriction.
smart_card_id = payload.get("smart_card_id")
nhs_number = payload.get("nhs_number")
if not smart_card_id or not nhs_number:
logger.error("Missing required fields")
return ApiGatewayResponse(
400, "Missing required fields", "POST"
).create_api_gateway_response()
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be extracted into a separate function, parse_body?
think the body should always be valid JSON so might not need to handle this error
do we want a model to validate the request body against?
body will be coming from front end so probably camel case

Comment on lines 55 to 62
authorization = request_context.authorization or {}
creator = authorization.get("nhs_user_id")
ods_code = authorization.get("selected_organisation", {}).get("org_ods_code")
if not creator or not ods_code:
logger.error("Missing user context")
return ApiGatewayResponse(
400, "Missing user context", "POST"
).create_api_gateway_response()
Copy link
Contributor

Choose a reason for hiding this comment

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

we could use the ods_util here, or create a new one that does both the creator and ods to be used with the other lambdas too?

creator: str,
) -> UserRestriction:
if smart_card_id == creator:
raise ValueError("You cannot create a restriction for yourself")
Copy link
Contributor

Choose a reason for hiding this comment

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

create custom exception

nhs_number=nhs_number,
custodian=ods_code,
creator=creator,
restricted_user_name=restricted_user_name,
Copy link
Contributor

Choose a reason for hiding this comment

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

not on the model, no need to write to db

restriction = service.create_restriction(
smart_card_id=smart_card_id,
nhs_number=nhs_number,
ods_code=ods_code,
Copy link
Contributor

Choose a reason for hiding this comment

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

custodian

raise ValueError("You cannot create a restriction for yourself")

practitioner = self.healthcare_service.get_practitioner(smart_card_id)
restricted_user_name = f"{practitioner.first_name} {practitioner.last_name}"
Copy link
Contributor

Choose a reason for hiding this comment

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

believe we are returning this the the frontend, may need to change what is returned from this function

Comment on lines 26 to 31
@pytest.fixture
def set_env(monkeypatch, set_env):
monkeypatch.setenv("USER_RESTRICTIONS_TABLE_NAME", MOCK_RESTRICTIONS_TABLE)
monkeypatch.setenv("HEALTHCARE_WORKER_API_URL", HEALTHCARE_WORKER_API_URL)
yield

Copy link
Contributor

Choose a reason for hiding this comment

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

use conftest

Comment on lines +16 to +18
MOCK_SMART_CARD_ID = "smartcard-uuid-1234"
MOCK_CREATOR_ID = "creator-uuid-5678"
MOCK_RESTRICTIONS_TABLE = "test-user-restrictions-table"
Copy link
Contributor

Choose a reason for hiding this comment

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

conftest

@github-actions
Copy link

Code security issues found

View full details here.

@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants