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

Add age validation in PatientDetailSerializer #1929

Merged
merged 13 commits into from
Apr 22, 2024

Conversation

rash-27
Copy link
Contributor

@rash-27 rash-27 commented Feb 28, 2024

Proposed Changes

  • Add age validation in PatientDetailSerializer

Associated Issue

Merge Checklist

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

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

@coronasafe/care-backend-maintainers @coronasafe/care-backend-admins

care/facility/api/serializers/patient.py Outdated Show resolved Hide resolved
@rash-27 rash-27 requested a review from sainak March 1, 2024 00:34
@rash-27
Copy link
Contributor Author

rash-27 commented Mar 1, 2024

@sainak here in the tests part I have only considered the case where it gonna fail(dob) , review it and mention if an further changes needed

care/facility/api/serializers/patient.py Outdated Show resolved Hide resolved
care/facility/api/serializers/patient.py Outdated Show resolved Hide resolved
care/facility/models/tests/test_patient.py Outdated Show resolved Hide resolved
@rash-27 rash-27 requested a review from sainak March 1, 2024 19:40
Copy link

codecov bot commented Mar 30, 2024

Codecov Report

Attention: Patch coverage is 45.45455% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 62.52%. Comparing base (51ce81e) to head (76a28cd).
Report is 54 commits behind head on develop.

❗ Current head 76a28cd differs from pull request most recent head 070d669. Consider uploading reports for the commit 070d669 to get more accurate results

Files Patch % Lines
care/facility/api/serializers/patient.py 45.45% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1929      +/-   ##
===========================================
+ Coverage    62.20%   62.52%   +0.32%     
===========================================
  Files          221      223       +2     
  Lines        12204    12281      +77     
  Branches      1742     1754      +12     
===========================================
+ Hits          7591     7679      +88     
+ Misses        4305     4280      -25     
- Partials       308      322      +14     

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

@rash-27 rash-27 requested a review from a team as a code owner April 13, 2024 14:06
Comment on lines 222 to 224

is_antenatal = serializers.BooleanField(default=False)

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
is_antenatal = serializers.BooleanField(default=False)

Copy link
Contributor Author

@rash-27 rash-27 Apr 18, 2024

Choose a reason for hiding this comment

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

@sainak if we remove this , it is throwing 500 error if "is_antenatal" is not passed in request body as null value is going into the database

care/facility/api/serializers/patient.py Outdated Show resolved Hide resolved
care/facility/api/serializers/patient.py Outdated Show resolved Hide resolved
care/facility/api/serializers/patient.py Outdated Show resolved Hide resolved
@sainak sainak requested review from rithviknishad and removed request for rithviknishad April 18, 2024 04:52
Copy link
Member

@rithviknishad rithviknishad left a comment

Choose a reason for hiding this comment

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

LGTM, just a correction in the tests

care/facility/models/tests/test_patient.py Outdated Show resolved Hide resolved
@vigneshhari vigneshhari merged commit fe0423e into coronasafe:develop Apr 22, 2024
2 of 3 checks passed
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.

None yet

5 participants