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.

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

feat: Added validation for max dosage and corresponding test cases #2383 #2531

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from

Conversation

himanshu-sharmav
Copy link

Fixes: #2383

  • Added validation to ensure max dosage is greater than or equal to base dosage in PrescriptionSerializer.
  • Added test cases to validate the new dosage constraints.
  • Updated test_prescriptions_api.py to include tests for valid, equal, and invalid dosage scenarios.

Resolves issue with incorrect dosage validation.

Merge Checklist

  • [ X] Tests added/fixed
  • [ X] Update docs in /docs
  • [ X] Linting Complete
  • [ X] Any other necessary step

Only PR's with test cases included and passing lint and test pipelines will be reviewed

@ohcnetwork/care-backend-maintainers @ohcnetwork/care-backend-admins

- Added validation to ensure max dosage is greater than or equal to base dosage in PrescriptionSerializer.
- Added test cases to validate the new dosage constraints.
- Updated test_prescriptions_api.py to include tests for valid, equal, and invalid dosage scenarios.

Resolves issue with incorrect dosage validation.
@himanshu-sharmav himanshu-sharmav requested a review from a team as a code owner October 12, 2024 19:39
Comment on lines 102 to 108
def extract_numeric_value(dosage):

match = re.match(r"(\d+(\.\d+)?)", dosage) # Matches digits and optional decimal part
if match:
return float(match.group(1))
raise serializers.ValidationError({"dosage": "Invalid dosage format."})

Copy link
Member

Choose a reason for hiding this comment

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

This validation is not required as it's already handled by DenominationValidator which is used by dosage-like fields.

Copy link
Author

Choose a reason for hiding this comment

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

So should I only remove the validation error part?

Copy link
Member

Choose a reason for hiding this comment

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

Keep only validations mentioned in the issue.

Copy link
Author

Choose a reason for hiding this comment

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

Fine will do that.

Copy link
Author

Choose a reason for hiding this comment

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

Done that, please review it again.Thanks

Copy link

codecov bot commented Oct 14, 2024

Codecov Report

Attention: Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 65.62%. Comparing base (08f0c72) to head (55d23bf).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
care/facility/api/serializers/prescription.py 91.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2531      +/-   ##
===========================================
+ Coverage    65.59%   65.62%   +0.02%     
===========================================
  Files          223      223              
  Lines        13383    13395      +12     
  Branches      1855     1858       +3     
===========================================
+ Hits          8779     8790      +11     
  Misses        4218     4218              
- Partials       386      387       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@himanshu-sharmav
Copy link
Author

What is the issue here? @rithviknishad @sainak please help

Copy link
Member

@sainak sainak left a comment

Choose a reason for hiding this comment

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

regex is not required here as the value is already validated, so a simple split does the job
avoid writing comments for obvious code

Comment on lines +143 to +150
base_dosage_value = extract_numeric_value(base_dosage)
max_dosage_value = extract_numeric_value(max_dosage)

# Raise error if max_dosage is less than base_dosage
if max_dosage_value < base_dosage_value:
raise serializers.ValidationError(
{"max_dosage": "Max dosage in 24 hours should be greater than or equal to base dosage."}
)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
base_dosage_value = extract_numeric_value(base_dosage)
max_dosage_value = extract_numeric_value(max_dosage)
# Raise error if max_dosage is less than base_dosage
if max_dosage_value < base_dosage_value:
raise serializers.ValidationError(
{"max_dosage": "Max dosage in 24 hours should be greater than or equal to base dosage."}
)
with contextlib.suppress(ValueError):
base_dosage_value = float(base_dosage.split(" ", maxsplit=1)[0])
max_dosage_value = float(max_dosage.split(" ", maxsplit=1)[0])
if max_dosage_value < base_dosage_value:
raise serializers.ValidationError(
{"max_dosage": "Max dosage in 24 hours should be greater than or equal to base dosage."}
)

max_dosage = attrs.get("max_dosage")

if base_dosage and max_dosage:
# Extract numeric values from dosage strings
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Extract numeric values from dosage strings

@@ -127,6 +134,21 @@ def validate(self, attrs):
{"base_dosage": "Base dosage is required."}
)

# Validate max_dosage is greater than or equal to base_dosage
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Validate max_dosage is greater than or equal to base_dosage

@@ -9,6 +9,7 @@
PrescriptionDosageType,
)
from care.users.api.serializers.user import UserBaseMinimumSerializer
import re
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import re
import contextlib

Comment on lines +102 to +107
def extract_numeric_value(dosage):

match = re.match(r"(\d+(\.\d+)?)", dosage) # Matches digits and optional decimal part
if match:
return float(match.group(1))

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def extract_numeric_value(dosage):
match = re.match(r"(\d+(\.\d+)?)", dosage) # Matches digits and optional decimal part
if match:
return float(match.group(1))

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.

Add validation for max dosage in PRN prescription
3 participants