-
Notifications
You must be signed in to change notification settings - Fork 284
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
added referred_to_external field PatientConsultationDischargeSerializer #1430
added referred_to_external field PatientConsultationDischargeSerializer #1430
Conversation
@rithviknishad Kindly review it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sachdevavaibhav lint CI is failing, make sure pre-commit is installed and ensure lints passes.
https://care-be-docs.coronasafe.network/local-setup/configuration.html#setting-up-pre-commit
@rithviknishad Done with formatting. |
@vigneshhari this is a good time to migrate and remove |
@sachdevavaibhav can you add some tests to validate whether this field is accepting correct inputs |
ya sure! |
This is more of an ops question @gigincg |
@sainak @vigneshhari And why is reffered_to deprecated? |
it was changed in #715, but I couldn't find the reference of the issue that requested the change |
@sainak I have a couple of questions:
|
Yes
You can include them in this pr |
@sainak I have written a few test and found that its also accepting integers (may be its converting it to string values). Kindly review the test and should I also add some validation to the input? |
|
added referred_to_external field PatientConsultationDischargeSerializer
Associated Issue
Merge Checklist
/docs
@coronasafe/care-backend-maintainers @coronasafe/care-backend-admins @rithviknishad