From 5e5e12e4d38316484b4e39740416a08efe1af1df Mon Sep 17 00:00:00 2001 From: Prafful Date: Mon, 13 Jan 2025 14:09:25 +0530 Subject: [PATCH 1/3] added validation for phone number in create spec --- care/emr/resources/patient/spec.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/care/emr/resources/patient/spec.py b/care/emr/resources/patient/spec.py index 6b66cc82a5..c8d785742e 100644 --- a/care/emr/resources/patient/spec.py +++ b/care/emr/resources/patient/spec.py @@ -1,4 +1,5 @@ import datetime +import re import uuid from enum import Enum @@ -66,6 +67,22 @@ def validate_geo_organization(cls, geo_organization): raise ValueError("Geo Organization does not exist") return geo_organization + @field_validator("phone_number", "emergency_phone_number") + @classmethod + def validate_phone_number(cls, value): + indian_mobile_number_regex = r"^(?=^\+91)(^\+91[6-9]\d{9}$)" + international_mobile_number_regex = r"^(?!^\+91)(^\+\d{1,3}\d{8,14}$)" + landline_number_regex = r"^\+91[2-9]\d{7,9}$" + + if ( + re.match(indian_mobile_number_regex, value) + or re.match(international_mobile_number_regex, value) + or re.match(landline_number_regex, value) + ): + return value + error = f"Invalid phone number format: {value}" + raise ValueError(error) + def perform_extra_deserialization(self, is_update, obj): if not is_update: obj.geo_organization = Organization.objects.get( From 071b4e83253600b9611800e56a78f9e344d4c5f4 Mon Sep 17 00:00:00 2001 From: Prafful Date: Mon, 13 Jan 2025 19:36:19 +0530 Subject: [PATCH 2/3] updated to use phonenumber library --- Pipfile | 2 ++ Pipfile.lock | 49 ++++++++++++++++++++---------- care/emr/resources/base.py | 23 +++++++++++++- care/emr/resources/patient/spec.py | 14 +++++++-- care/emr/tests/test_patient_api.py | 33 +++++++++++--------- 5 files changed, 87 insertions(+), 34 deletions(-) diff --git a/Pipfile b/Pipfile index 0683ed3514..a9b34a705a 100644 --- a/Pipfile +++ b/Pipfile @@ -42,6 +42,8 @@ simplejson = "==3.19.3" sentry-sdk = "==2.18.0" whitenoise = "==6.8.2" django-anymail = {extras = ["amazon-ses"], version = "*"} +phonenumbers = "8.13.52" +pydantic-extra-types = "2.10.1" [dev-packages] boto3-stubs = { extras = ["s3", "boto3"], version = "*" } diff --git a/Pipfile.lock b/Pipfile.lock index 257e5ceb75..a10f8ec4dd 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "e85a5fbff16924dfb8bdf00fb71d83c8fc27ab60e0e53c945be186c4a4837456" + "sha256": "2bb050e0179ee6034fe408a1a5330e743a82f52b4f3783ea7c98e3b0fd5ba258" }, "pipfile-spec": 6, "requires": { @@ -210,11 +210,11 @@ }, "botocore": { "hashes": [ - "sha256:2b3309b356541faa4d88bb957dcac1d8004aa44953c0b7d4521a6cc5d3d5d6ba", - "sha256:d784d944865d8279c79d2301fc09ac28b5221d4e7328fb4e23c642c253b9932c" + "sha256:88f2fab29192ffe2f2115d5bafbbd823ff4b6eb2774296e03ec8b5b0fe074f61", + "sha256:fed4f156b1a9b8ece53738f702ba5851b8c6216b4952de326547f349cc494f14" ], "markers": "python_version >= '3.8'", - "version": "==1.35.94" + "version": "==1.35.97" }, "celery": { "hashes": [ @@ -1060,6 +1060,14 @@ "markers": "python_version >= '3.8'", "version": "==24.2" }, + "phonenumbers": { + "hashes": [ + "sha256:e803210038ece9d208b129e3023dc20e656a820d6bf6f1cb0471d4164f54bada", + "sha256:fdc371ea6a4da052beb1225de63963d5a2fddbbff2bb53e3a957f360e0185f80" + ], + "index": "pypi", + "version": "==8.13.52" + }, "pillow": { "hashes": [ "sha256:00177a63030d612148e659b55ba99527803288cea7c75fb05766ab7981a8c1b7", @@ -1382,6 +1390,15 @@ "markers": "python_version >= '3.8'", "version": "==2.23.4" }, + "pydantic-extra-types": { + "hashes": [ + "sha256:db2c86c04a837bbac0d2d79bbd6f5d46c4c9253db11ca3fdd36a2b282575f1e2", + "sha256:e4f937af34a754b8f1fa228a2fac867091a51f56ed0e8a61d5b3a6719b13c923" + ], + "index": "pypi", + "markers": "python_version >= '3.8'", + "version": "==2.10.1" + }, "pyjwt": { "hashes": [ "sha256:3b02fb0f44517787776cf48f2ae25d8e14f300e6d7545a4315cee571a415e850", @@ -1654,11 +1671,11 @@ }, "setuptools": { "hashes": [ - "sha256:84fb203f278ebcf5cd08f97d3fb96d3fbed4b629d500b29ad60d11e00769b183", - "sha256:886ff7b16cd342f1d1defc16fc98c9ce3fde69e087a4e1983d7ab634e5f41f4f" + "sha256:c5afc8f407c626b8313a86e10311dd3f661c6cd9c09d4bf8c15c0e11f9f2b0e6", + "sha256:e3982f444617239225d675215d51f6ba05f845d4eec313da4418fdbb56fb27e3" ], "markers": "python_version >= '3.9'", - "version": "==75.7.0" + "version": "==75.8.0" }, "simplejson": { "hashes": [ @@ -1826,11 +1843,11 @@ }, "types-setuptools": { "hashes": [ - "sha256:7cbfd3bf2944f88bbcdd321b86ddd878232a277be95d44c78a53585d78ebc2f6", - "sha256:d9478a985057ed48a994c707f548e55aababa85fe1c9b212f43ab5a1fffd3211" + "sha256:96f7ec8bbd6e0a54ea180d66ad68ad7a1d7954e7281a710ea2de75e355545271", + "sha256:a9f12980bbf9bcdc23ecd80755789085bad6bfce4060c2275bc2b4ca9f2bc480" ], "markers": "python_version >= '3.8'", - "version": "==75.6.0.20241223" + "version": "==75.8.0.20250110" }, "typing-extensions": { "hashes": [ @@ -2031,19 +2048,19 @@ }, "botocore": { "hashes": [ - "sha256:2b3309b356541faa4d88bb957dcac1d8004aa44953c0b7d4521a6cc5d3d5d6ba", - "sha256:d784d944865d8279c79d2301fc09ac28b5221d4e7328fb4e23c642c253b9932c" + "sha256:88f2fab29192ffe2f2115d5bafbbd823ff4b6eb2774296e03ec8b5b0fe074f61", + "sha256:fed4f156b1a9b8ece53738f702ba5851b8c6216b4952de326547f349cc494f14" ], "markers": "python_version >= '3.8'", - "version": "==1.35.94" + "version": "==1.35.97" }, "botocore-stubs": { "hashes": [ - "sha256:61c8986ab0cde54ad459e1ae598ab49c09082b893a6e09e5b31d937aad7de55a", - "sha256:71e4414aaefb69f79df57b595bad09c0cb08aaa980c72dcf1ae7d426de2ad5a2" + "sha256:5427684c2248ad3db66c83b96e2486ebbacf13d7ba648a5acad7f422a086c419", + "sha256:aae08ea4a2aa3c360cfd783f8e4c713db64351b429baee148820d5b0a6d9221a" ], "markers": "python_version >= '3.8'", - "version": "==1.35.94" + "version": "==1.35.97" }, "certifi": { "hashes": [ diff --git a/care/emr/resources/base.py b/care/emr/resources/base.py index 9f5f4fa46a..55f41d9873 100644 --- a/care/emr/resources/base.py +++ b/care/emr/resources/base.py @@ -2,9 +2,11 @@ import uuid from enum import Enum from types import UnionType -from typing import get_origin +from typing import Annotated, Union, get_origin +import phonenumbers from pydantic import BaseModel +from pydantic_extra_types.phone_numbers import PhoneNumberValidator from care.emr.fhir.schema.base import Coding @@ -140,3 +142,22 @@ def as_questionnaire(cls, parent_classes=None): # noqa PLR0912 def to_json(self): return self.model_dump(mode="json", exclude=["meta"]) + + +IndianPhoneNumber = Annotated[ + Union[str, phonenumbers.PhoneNumber], # noqa: UP007 + PhoneNumberValidator( + default_region="IN", + supported_regions=["IN"], + number_format="E164", + ), +] + +InternationalPhoneNumber = Annotated[ + Union[str, phonenumbers.PhoneNumber()], # noqa: UP007 + PhoneNumberValidator( + default_region=None, + supported_regions=[], + number_format="E164", + ), +] diff --git a/care/emr/resources/patient/spec.py b/care/emr/resources/patient/spec.py index c8d785742e..c8f98d2a93 100644 --- a/care/emr/resources/patient/spec.py +++ b/care/emr/resources/patient/spec.py @@ -2,13 +2,18 @@ import re import uuid from enum import Enum +from typing import Union from django.utils import timezone from pydantic import UUID4, Field, field_validator, model_validator from care.emr.models import Organization from care.emr.models.patient import Patient -from care.emr.resources.base import EMRResource +from care.emr.resources.base import ( + EMRResource, + IndianPhoneNumber, + InternationalPhoneNumber, +) class BloodGroupChoices(str, Enum): @@ -30,6 +35,9 @@ class GenderChoices(str, Enum): transgender = "transgender" +PatientPhoneNumber = Union[IndianPhoneNumber, InternationalPhoneNumber] # noqa: UP007 + + class PatientBaseSpec(EMRResource): __model__ = Patient __exclude__ = ["geo_organization"] @@ -37,8 +45,8 @@ class PatientBaseSpec(EMRResource): id: UUID4 | None = None name: str = Field(max_length=200) gender: GenderChoices - phone_number: str = Field(max_length=14) - emergency_phone_number: str | None = Field(None, max_length=14) + phone_number: PatientPhoneNumber = Field(max_length=14) + emergency_phone_number: PatientPhoneNumber | None = Field(None, max_length=14) address: str permanent_address: str pincode: int diff --git a/care/emr/tests/test_patient_api.py b/care/emr/tests/test_patient_api.py index fbabae7f9a..00d6dfc0ae 100644 --- a/care/emr/tests/test_patient_api.py +++ b/care/emr/tests/test_patient_api.py @@ -1,16 +1,13 @@ +from secrets import choice + from django.urls import reverse -from polyfactory.factories.pydantic_factory import ModelFactory from rest_framework import status -from care.emr.resources.patient.spec import PatientCreateSpec +from care.emr.resources.patient.spec import BloodGroupChoices, GenderChoices from care.security.permissions.patient import PatientPermissions from care.utils.tests.base import CareAPITestBase -class PatientFactory(ModelFactory[PatientCreateSpec]): - __model__ = PatientCreateSpec - - class TestPatientViewSet(CareAPITestBase): """ Test cases for checking Patient CRUD operations @@ -27,10 +24,22 @@ def setUp(self): super().setUp() # Call parent's setUp to ensure proper initialization self.base_url = reverse("patient-list") - def generate_patient_data(self, **kwargs): + def generate_patient_data(self, geo_organization, **kwargs): + data = { + "name": self.fake.name(), + "gender": choice(list(GenderChoices)), + "address": self.fake.address(), + "permanent_address": self.fake.address(), + "pincode": self.fake.random_int(min=100000, max=999999), + "blood_group": choice(list(BloodGroupChoices)), + "phone_number": "+919876543210", + "emergency_phone_number": "+14155552671", + "geo_organization": geo_organization, + } if "age" not in kwargs and "date_of_birth" not in kwargs: kwargs["age"] = self.fake.random_int(min=1, max=100) - return PatientFactory.build(meta={}, gender="male", **kwargs) + data.update(**kwargs) + return data def test_create_patient_unauthenticated(self): """Test that unauthenticated users cannot create patients""" @@ -57,9 +66,7 @@ def test_create_patient_authorization(self): ) self.attach_role_organization_user(organization, user, role) self.client.force_authenticate(user=user) - response = self.client.post( - self.base_url, patient_data.model_dump(mode="json"), format="json" - ) + response = self.client.post(self.base_url, patient_data, format="json") self.assertEqual(response.status_code, status.HTTP_200_OK) def test_create_patient_unauthorization(self): @@ -75,7 +82,5 @@ def test_create_patient_unauthorization(self): ) self.attach_role_organization_user(organization, user, role) self.client.force_authenticate(user=user) - response = self.client.post( - self.base_url, patient_data.model_dump(mode="json"), format="json" - ) + response = self.client.post(self.base_url, patient_data, format="json") self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) From 61d94c3de350877b68b0b701210b483a584ffe6b Mon Sep 17 00:00:00 2001 From: Prafful Date: Mon, 13 Jan 2025 23:15:06 +0530 Subject: [PATCH 3/3] updated phonenumber to phonenumberlite library --- Pipfile | 2 +- Pipfile.lock | 8 ++--- care/emr/resources/base.py | 11 +----- care/emr/resources/patient/spec.py | 31 ++-------------- care/emr/tests/test_patient_api.py | 57 ++++++++++++++++++++++++++++-- 5 files changed, 64 insertions(+), 45 deletions(-) diff --git a/Pipfile b/Pipfile index a9b34a705a..22538c25d5 100644 --- a/Pipfile +++ b/Pipfile @@ -42,8 +42,8 @@ simplejson = "==3.19.3" sentry-sdk = "==2.18.0" whitenoise = "==6.8.2" django-anymail = {extras = ["amazon-ses"], version = "*"} -phonenumbers = "8.13.52" pydantic-extra-types = "2.10.1" +phonenumberslite = "==8.13.52" [dev-packages] boto3-stubs = { extras = ["s3", "boto3"], version = "*" } diff --git a/Pipfile.lock b/Pipfile.lock index a10f8ec4dd..aaf258c5fe 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "2bb050e0179ee6034fe408a1a5330e743a82f52b4f3783ea7c98e3b0fd5ba258" + "sha256": "8297f447574d46431a5c51b4e8f97210813c6f130047983e2e380213628b014c" }, "pipfile-spec": 6, "requires": { @@ -1060,10 +1060,10 @@ "markers": "python_version >= '3.8'", "version": "==24.2" }, - "phonenumbers": { + "phonenumberslite": { "hashes": [ - "sha256:e803210038ece9d208b129e3023dc20e656a820d6bf6f1cb0471d4164f54bada", - "sha256:fdc371ea6a4da052beb1225de63963d5a2fddbbff2bb53e3a957f360e0185f80" + "sha256:02da5e78c67b213bae95afd6289f40486c93e302e518769911dfa5e7287ddeee", + "sha256:dfa44a4bae2e46d737ae5301cb96b14cdcbf45063236c74c6ddb08f5fd471b0d" ], "index": "pypi", "version": "==8.13.52" diff --git a/care/emr/resources/base.py b/care/emr/resources/base.py index 55f41d9873..0d2d6e4c9f 100644 --- a/care/emr/resources/base.py +++ b/care/emr/resources/base.py @@ -144,16 +144,7 @@ def to_json(self): return self.model_dump(mode="json", exclude=["meta"]) -IndianPhoneNumber = Annotated[ - Union[str, phonenumbers.PhoneNumber], # noqa: UP007 - PhoneNumberValidator( - default_region="IN", - supported_regions=["IN"], - number_format="E164", - ), -] - -InternationalPhoneNumber = Annotated[ +PhoneNumber = Annotated[ Union[str, phonenumbers.PhoneNumber()], # noqa: UP007 PhoneNumberValidator( default_region=None, diff --git a/care/emr/resources/patient/spec.py b/care/emr/resources/patient/spec.py index c8f98d2a93..7c0850103c 100644 --- a/care/emr/resources/patient/spec.py +++ b/care/emr/resources/patient/spec.py @@ -1,19 +1,13 @@ import datetime -import re import uuid from enum import Enum -from typing import Union from django.utils import timezone from pydantic import UUID4, Field, field_validator, model_validator from care.emr.models import Organization from care.emr.models.patient import Patient -from care.emr.resources.base import ( - EMRResource, - IndianPhoneNumber, - InternationalPhoneNumber, -) +from care.emr.resources.base import EMRResource, PhoneNumber class BloodGroupChoices(str, Enum): @@ -35,9 +29,6 @@ class GenderChoices(str, Enum): transgender = "transgender" -PatientPhoneNumber = Union[IndianPhoneNumber, InternationalPhoneNumber] # noqa: UP007 - - class PatientBaseSpec(EMRResource): __model__ = Patient __exclude__ = ["geo_organization"] @@ -45,8 +36,8 @@ class PatientBaseSpec(EMRResource): id: UUID4 | None = None name: str = Field(max_length=200) gender: GenderChoices - phone_number: PatientPhoneNumber = Field(max_length=14) - emergency_phone_number: PatientPhoneNumber | None = Field(None, max_length=14) + phone_number: PhoneNumber = Field(max_length=14) + emergency_phone_number: PhoneNumber | None = Field(None, max_length=14) address: str permanent_address: str pincode: int @@ -75,22 +66,6 @@ def validate_geo_organization(cls, geo_organization): raise ValueError("Geo Organization does not exist") return geo_organization - @field_validator("phone_number", "emergency_phone_number") - @classmethod - def validate_phone_number(cls, value): - indian_mobile_number_regex = r"^(?=^\+91)(^\+91[6-9]\d{9}$)" - international_mobile_number_regex = r"^(?!^\+91)(^\+\d{1,3}\d{8,14}$)" - landline_number_regex = r"^\+91[2-9]\d{7,9}$" - - if ( - re.match(indian_mobile_number_regex, value) - or re.match(international_mobile_number_regex, value) - or re.match(landline_number_regex, value) - ): - return value - error = f"Invalid phone number format: {value}" - raise ValueError(error) - def perform_extra_deserialization(self, is_update, obj): if not is_update: obj.geo_organization = Organization.objects.get( diff --git a/care/emr/tests/test_patient_api.py b/care/emr/tests/test_patient_api.py index 00d6dfc0ae..8e24a8c606 100644 --- a/care/emr/tests/test_patient_api.py +++ b/care/emr/tests/test_patient_api.py @@ -1,6 +1,8 @@ from secrets import choice +import phonenumbers from django.urls import reverse +from phonenumbers import PhoneNumberFormat, PhoneNumberType from rest_framework import status from care.emr.resources.patient.spec import BloodGroupChoices, GenderChoices @@ -8,6 +10,17 @@ from care.utils.tests.base import CareAPITestBase +def generate_random_valid_phone_number() -> str: + regions = ["US", "IN", "GB", "DE", "FR", "JP", "AU", "CA"] + random_region = choice(regions) + example_number = phonenumbers.example_number_for_type( + random_region, PhoneNumberType.MOBILE + ) + if example_number: + return phonenumbers.format_number(example_number, PhoneNumberFormat.E164) + raise ValueError("Unable to generate a valid phone number") + + class TestPatientViewSet(CareAPITestBase): """ Test cases for checking Patient CRUD operations @@ -32,8 +45,8 @@ def generate_patient_data(self, geo_organization, **kwargs): "permanent_address": self.fake.address(), "pincode": self.fake.random_int(min=100000, max=999999), "blood_group": choice(list(BloodGroupChoices)), - "phone_number": "+919876543210", - "emergency_phone_number": "+14155552671", + "phone_number": generate_random_valid_phone_number(), + "emergency_phone_number": generate_random_valid_phone_number(), "geo_organization": geo_organization, } if "age" not in kwargs and "date_of_birth" not in kwargs: @@ -84,3 +97,43 @@ def test_create_patient_unauthorization(self): self.client.force_authenticate(user=user) response = self.client.post(self.base_url, patient_data, format="json") self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_create_patient_with_invalid_phone_number(self): + """Test patient creation with invalid phone number""" + user = self.create_user() + geo_organization = self.create_organization(org_type="govt") + invalid_phone_numbers = ["12345", "abcdef", "+1234567890123456", ""] + + organization = self.create_organization(org_type="govt") + role = self.create_role_with_permissions( + permissions=[PatientPermissions.can_create_patient.name] + ) + self.attach_role_organization_user(organization, user, role) + self.client.force_authenticate(user=user) + + for invalid_number in invalid_phone_numbers: + patient_data = self.generate_patient_data( + geo_organization=geo_organization.external_id, + phone_number=invalid_number, + ) + response = self.client.post(self.base_url, patient_data, format="json") + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_create_patient_with_valid_phone_number(self): + user = self.create_user() + geo_organization = self.create_organization(org_type="govt") + valid_phone_numbers = [generate_random_valid_phone_number() for _ in range(5)] + + organization = self.create_organization(org_type="govt") + role = self.create_role_with_permissions( + permissions=[PatientPermissions.can_create_patient.name] + ) + self.attach_role_organization_user(organization, user, role) + self.client.force_authenticate(user=user) + + for valid_number in valid_phone_numbers: + patient_data = self.generate_patient_data( + geo_organization=geo_organization.external_id, phone_number=valid_number + ) + response = self.client.post(self.base_url, patient_data, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK)