Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add tests and validation for filterset #2588

Merged
merged 8 commits into from
Nov 25, 2024
Merged

add tests and validation for filterset #2588

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
73e6a80
add tests and validation for filterset
DraKen0009 Nov 9, 2024
1ae5aee
Merge branch 'develop' into additing-validaton-to-patient-filtereset
DraKen0009 Nov 9, 2024
99e69c2
improved test case sample data
DraKen0009 Nov 10, 2024
26c4c6a
improved test cases
DraKen0009 Nov 10, 2024
f0f35eb
adding code rabbit sugestions
DraKen0009 Nov 10, 2024
a6a6ce9
remove validation from phone number fields
DraKen0009 Nov 11, 2024
f4c5d31
Merge branch 'develop' into additing-validaton-to-patient-filtereset
DraKen0009 Nov 19, 2024
603643d
Merge branch 'develop' into additing-validaton-to-patient-filtereset
DraKen0009 Nov 23, 2024
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
49 changes: 34 additions & 15 deletions care/facility/api/viewsets/patient.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from django.conf import settings
from django.contrib.postgres.search import TrigramSimilarity
from django.core.validators import MaxValueValidator, MinValueValidator
from django.db import models
from django.db.models import (
Case,
Expand Down Expand Up @@ -78,10 +79,11 @@
NewDischargeReasonEnum,
)
from care.facility.models.patient_consultation import PatientConsultation
from care.users.models import User
from care.users.models import GENDER_CHOICES, User
from care.utils.cache.cache_allowed_facilities import get_accessible_facilities
from care.utils.filters.choicefilter import CareChoiceFilter
from care.utils.filters.multiselect import MultiSelectFilter
from care.utils.models.validators import mobile_validator
from care.utils.notification_handler import NotificationGenerator
from care.utils.queryset.patient import get_patient_notes_queryset
from config.authentication import (
Expand All @@ -105,17 +107,29 @@ class PatientFilterSet(filters.FilterSet):
field_name="facility__facility_type",
choice_dict=REVERSE_FACILITY_TYPES,
)
phone_number = filters.CharFilter(field_name="phone_number")
emergency_phone_number = filters.CharFilter(field_name="emergency_phone_number")
phone_number = filters.CharFilter(
field_name="phone_number", validators=[mobile_validator]
)
emergency_phone_number = filters.CharFilter(
field_name="emergency_phone_number", validators=[mobile_validator]
)
allow_transfer = filters.BooleanFilter(field_name="allow_transfer")
name = filters.CharFilter(field_name="name", lookup_expr="icontains")
name = filters.CharFilter(
field_name="name", lookup_expr="icontains", max_length=200
)
patient_no = filters.CharFilter(
field_name=f"{last_consultation_field}__patient_no", lookup_expr="iexact"
field_name=f"{last_consultation_field}__patient_no",
lookup_expr="iexact",
max_length=100,
)
gender = filters.ChoiceFilter(field_name="gender", choices=GENDER_CHOICES)
age = filters.NumberFilter(field_name="age", validators=[MinValueValidator(0)])
age_min = filters.NumberFilter(
field_name="age", lookup_expr="gte", validators=[MinValueValidator(0)]
)
age_max = filters.NumberFilter(
field_name="age", lookup_expr="lte", validators=[MinValueValidator(0)]
)
gender = filters.NumberFilter(field_name="gender")
age = filters.NumberFilter(field_name="age")
age_min = filters.NumberFilter(field_name="age", lookup_expr="gte")
age_max = filters.NumberFilter(field_name="age", lookup_expr="lte")
deprecated_covid_category = filters.ChoiceFilter(
field_name=f"{last_consultation_field}__deprecated_covid_category",
choices=COVID_CATEGORY_CHOICES,
Expand All @@ -142,7 +156,7 @@ def filter_by_category(self, queryset, name, value):

created_date = filters.DateFromToRangeFilter(field_name="created_date")
modified_date = filters.DateFromToRangeFilter(field_name="modified_date")
srf_id = filters.CharFilter(field_name="srf_id")
srf_id = filters.CharFilter(field_name="srf_id", max_length=200)
is_declared_positive = filters.BooleanFilter(field_name="is_declared_positive")
date_declared_positive = filters.DateFromToRangeFilter(
field_name="date_declared_positive"
Expand All @@ -160,14 +174,16 @@ def filter_by_category(self, queryset, name, value):
# Location Based Filtering
district = filters.NumberFilter(field_name="district__id")
district_name = filters.CharFilter(
field_name="district__name", lookup_expr="icontains"
field_name="district__name", lookup_expr="icontains", max_length=255
)
local_body = filters.NumberFilter(field_name="local_body__id")
local_body_name = filters.CharFilter(
field_name="local_body__name", lookup_expr="icontains"
field_name="local_body__name", lookup_expr="icontains", max_length=255
)
state = filters.NumberFilter(field_name="state__id")
state_name = filters.CharFilter(field_name="state__name", lookup_expr="icontains")
state_name = filters.CharFilter(
field_name="state__name", lookup_expr="icontains", max_length=255
)
# Consultation Fields
is_kasp = filters.BooleanFilter(field_name=f"{last_consultation_field}__is_kasp")
last_consultation_kasp_enabled_date = filters.DateFromToRangeFilter(
Expand Down Expand Up @@ -226,9 +242,12 @@ def filter_by_bed_type(self, queryset, name, value):
)

# Vaccination Filters
covin_id = filters.CharFilter(field_name="covin_id")
covin_id = filters.CharFilter(field_name="covin_id", max_length=15)
is_vaccinated = filters.BooleanFilter(field_name="is_vaccinated")
number_of_doses = filters.NumberFilter(field_name="number_of_doses")
number_of_doses = filters.NumberFilter(
field_name="number_of_doses",
validators=[MinValueValidator(0), MaxValueValidator(3)],
)
# Permission Filters
assigned_to = filters.NumberFilter(field_name="assigned_to")
# Other Filters
Expand Down
168 changes: 168 additions & 0 deletions care/facility/tests/test_patient_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from enum import Enum
from urllib.parse import quote

from django.utils.timezone import now, timedelta
from rest_framework import status
Expand Down Expand Up @@ -728,6 +729,173 @@ def test_filter_by_has_consents(self):
self.assertEqual(res.status_code, status.HTTP_200_OK)
self.assertEqual(res.json()["count"], 3)

def test_filter_by_invalid_params(self):
self.client.force_authenticate(user=self.user)

# name length > 200 words
invalid_name_param = "a" * 201
res = self.client.get(self.get_base_url() + f"?name={invalid_name_param}")
self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn(
"Ensure this value has at most 200 characters (it has 201).",
res.json()["name"],
)

# invalid gender choice
invalid_gender = 4
res = self.client.get(self.get_base_url() + f"?gender={invalid_gender}")
self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn(
"Select a valid choice. 4 is not one of the available choices.",
res.json()["gender"],
)

# invalid value for age, age max , age min filter (i.e <0)
invalid_age = -2
res = self.client.get(self.get_base_url() + f"?age={invalid_age}")
self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn(
"Ensure this value is greater than or equal to 0.", res.json()["age"]
)

invalid_min_age = -2
res = self.client.get(self.get_base_url() + f"?age_min={invalid_min_age}")
self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn(
"Ensure this value is greater than or equal to 0.", res.json()["age_min"]
)

invalid_max_age = -2
res = self.client.get(self.get_base_url() + f"?age_max={invalid_max_age}")
self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn(
"Ensure this value is greater than or equal to 0.", res.json()["age_max"]
)

# invalid number_of_doses param >3 or <0
invalid_number_of_doses = -2
res = self.client.get(
self.get_base_url() + f"?number_of_doses={invalid_number_of_doses}"
)
self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn(
"Ensure this value is greater than or equal to 0.",
res.json()["number_of_doses"],
)

invalid_number_of_doses = 4
res = self.client.get(
self.get_base_url() + f"?number_of_doses={invalid_number_of_doses}"
)
self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn(
"Ensure this value is less than or equal to 3.",
res.json()["number_of_doses"],
)

# invalid srf id length > 200 words
invalid_srf_param = "a" * 201
res = self.client.get(self.get_base_url() + f"?srf_id={invalid_srf_param}")
self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn(
"Ensure this value has at most 200 characters (it has 201).",
res.json()["srf_id"],
)

# invalid district_name length > 255 words
invalid_district_name_param = "a" * 256
res = self.client.get(
self.get_base_url() + f"?district_name={invalid_district_name_param}"
)
self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn(
"Ensure this value has at most 255 characters (it has 256).",
res.json()["district_name"],
)

# invalid local_body_name length > 255 words
invalid_local_body_name_param = "a" * 256
res = self.client.get(
self.get_base_url() + f"?local_body_name={invalid_local_body_name_param}"
)
self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn(
"Ensure this value has at most 255 characters (it has 256).",
res.json()["local_body_name"],
)

# invalid state_name length > 255 words
invalid_state_name_param = "a" * 256
res = self.client.get(
self.get_base_url() + f"?state_name={invalid_state_name_param}"
)
self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn(
"Ensure this value has at most 255 characters (it has 256).",
res.json()["state_name"],
)

# invalid patient no value > 100
invalid_patient_no_param = "A" * 101
res = self.client.get(
self.get_base_url() + f"?patient_no={invalid_patient_no_param}"
)
self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn(
"Ensure this value has at most 100 characters (it has 101).",
res.json()["patient_no"],
)

Comment on lines +731 to +847
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add validation test for covin_id field.

The PR objectives mention validating the covin_id field with a maximum length of 15 characters, but this test is missing.

Add the following test case:

# Test invalid covin_id length > 15 characters
invalid_covin_id = "A" * 16
res = self.client.get(self.get_base_url() + f"?covin_id={invalid_covin_id}")
self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn(
    "Ensure this value has at most 15 characters (it has 16).",
    res.json()["covin_id"],
)

def test_invalid_phone_params(self):
self.client.force_authenticate(user=self.user)

# invalid phone number (non Indian or non International)
invalid_phone_numbers = [
"9876543210",
"+9123456789",
"+915123456789",
"+9191234",
"+91765432abcd",
"00441234567890",
"+12345",
"+911234567890",
"+151234",
"+44-123-4567-890",
"+1-800-555-1212",
"+91 98765 43210",
"+91987654321000",
"+44 1234 567890",
"+123-456-7890",
"1234567890",
"+91-9876543210",
"+123456",
"+000000000000",
"+9876543210123456",
]

for phone_number in invalid_phone_numbers:
encoded_phone_number = quote(phone_number)
res = self.client.get(
self.get_base_url() + f"?phone_number={encoded_phone_number}"
)
self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn(
f"Invalid phone number. Must be one of the following types: mobile. Received: {phone_number}",
res.json()["phone_number"],
)

for emergency_phone_number in invalid_phone_numbers:
encoded_emergency_phone_number = quote(emergency_phone_number)
res = self.client.get(
self.get_base_url()
+ f"?emergency_phone_number={encoded_emergency_phone_number}"
)
self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn(
f"Invalid phone number. Must be one of the following types: mobile. Received: {emergency_phone_number}",
res.json()["emergency_phone_number"],
)

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Reduce code duplication in phone number validation tests.

The same validation logic is repeated for both phone_number and emergency_phone_number. Consider extracting a helper method:

def _test_invalid_phone_field(self, field_name):
    invalid_phone_numbers = [
        "9876543210",
        "+9123456789",
        # ... rest of the numbers
    ]
    
    for phone_number in invalid_phone_numbers:
        encoded_number = quote(phone_number)
        res = self.client.get(
            self.get_base_url() + f"?{field_name}={encoded_number}"
        )
        self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
        self.assertIn(
            f"Invalid phone number. Must be one of the following types: mobile. Received: {phone_number}",
            res.json()[field_name],
        )

But you know, it's your code, do what you want...


class DischargePatientFilterTestCase(TestUtils, APITestCase):
@classmethod
Expand Down