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

added referred to facility field in discharge form #5730

Merged

Conversation

sachdevavaibhav
Copy link
Contributor

@sachdevavaibhav sachdevavaibhav commented Jun 20, 2023

WHAT

Added referred to facility field in discharge form

Proposed Changes

@coronasafe/care-fe-code-reviewers @coronasafe/code-reviewers

Merge Checklist

  • Add specs that demonstrate bug / test a new feature.
  • Update product documentation.
  • Ensure that UI text is kept in I18n files.
  • Prep screenshot or demo video for changelog entry, and attach it to issue.
  • Request for Peer Reviews
  • Completion of QA

HOW

  • Fetches list of facilities from backend
  • created facilities local state
  • used autocomplete component

@sachdevavaibhav sachdevavaibhav requested a review from a team June 20, 2023 09:31
@sachdevavaibhav sachdevavaibhav requested a review from a team as a code owner June 20, 2023 09:31
@vercel
Copy link

vercel bot commented Jun 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
care-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 9, 2023 0:11am

@netlify
Copy link

netlify bot commented Jun 20, 2023

Deploy Preview for care-egov-staging ready!

Name Link
🔨 Latest commit 570fb40
🔍 Latest deploy log https://app.netlify.com/sites/care-egov-staging/deploys/64d3824ee7cdbb000893fa46
😎 Deploy Preview https://deploy-preview-5730--care-egov-staging.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sachdevavaibhav
Copy link
Contributor Author

@rithviknishad Kindly review it. The field can search from the options but it is not able to add values which are not present. Please suggest how can I solve it?

@github-actions github-actions bot added the merge conflict pull requests with merge conflict label Jun 23, 2023
@github-actions
Copy link

👋 Hi, @sachdevavaibhav,
Conflicts have been detected against the base branch. Please rebase your branch against the base branch.


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

@sachdevavaibhav
Copy link
Contributor Author

@rithviknishad @nihal467

@rithviknishad rithviknishad added needs testing and removed merge conflict pull requests with merge conflict labels Jun 24, 2023
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.

Use FacilitySelect component instead of manually duplicating the facility fetching code.

@sachdevavaibhav
Copy link
Contributor Author

@rithviknishad Kindly review the changes.

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.

Are you sure this works without any backend changes required?

@sachdevavaibhav
Copy link
Contributor Author

sachdevavaibhav commented Jul 1, 2023

@rithviknishad
Upon my analysis I found the value is not being stored in the database. Its storing a null value for it.
I found this by setting up care backend locally. It is storing other fields like discharge_reason etc correctly but storing null for referred_to_external.

Kindly see the last sql query in the attached screenshot.

image

Update:
@rithviknishad
I found in the backend that the data is being received properly but PatientConsultationDischargeSerializer is not having referred_to_external field defined. Upon adding that field everything works fine.

image

@sachdevavaibhav
Copy link
Contributor Author

@rithviknishad
Should I create issue and link a PR for the backend?

@rithviknishad
Copy link
Member

@sachdevavaibhav yes! Go ahead!

@github-actions github-actions bot added the merge conflict pull requests with merge conflict label Aug 8, 2023
@github-actions
Copy link

github-actions bot commented Aug 8, 2023

👋 Hi, @sachdevavaibhav,
Conflicts have been detected against the base branch. Please rebase your branch against the base branch.


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

@nihal467
Copy link
Member

nihal467 commented Aug 8, 2023

@sachdevavaibhav can you fix the merge conflict and get it ready for testing, we need to get it merge ASAP

@sachdevavaibhav
Copy link
Contributor Author

@sachdevavaibhav can you fix the merge conflict and get it ready for testing, we need to get it merge ASAP

solved merge conflict. Though I have one question why are we using moment.js and day.js both. Don't they serve the same purpose. I have kept the code coming from develop which is using dayjs.

@nihal467 @Ashesh3

@github-actions github-actions bot removed the merge conflict pull requests with merge conflict label Aug 8, 2023
const dischargeDetails = {
...preDischargeForm,
discharge: value,
discharge_date: moment(preDischargeForm.discharge_date).toISOString(true),
Copy link
Member

@rithviknishad rithviknishad Aug 9, 2023

Choose a reason for hiding this comment

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

@sachdevavaibhav

  • Merge the latest develop (because moment has been uninstalled)
  • Discard the package-lock.json diff
  • replace moment with dayjs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rithviknishad Kindly review

@nihal467
Copy link
Member

LGTM

@khavinshankar khavinshankar merged commit 2340337 into coronasafe:develop Aug 10, 2023
26 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Referred to facility- discharge form
5 participants