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 SpO2 field in community log update for nurses #8907

Conversation

noufalrahim
Copy link
Contributor

@noufalrahim noufalrahim commented Oct 23, 2024

Proposed Changes

@ohcnetwork/care-fe-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

Summary by CodeRabbit

  • New Features

    • Introduced a new ventilator_spo2 field in the Daily Rounds form for capturing and validating patient SpO2 levels.
    • Enhanced form submission logic to include the new SpO2 data.
  • Localization

    • Added new translations for healthcare-related terms, including "high" and "low," improving user interface terminology.

@noufalrahim noufalrahim requested a review from a team as a code owner October 23, 2024 16:02
Copy link

netlify bot commented Oct 23, 2024

Deploy Preview for care-ohc ready!

Name Link
🔨 Latest commit 04b2baa
🔍 Latest deploy log https://app.netlify.com/sites/care-ohc/deploys/673c385da4a91f0008aa1783
😎 Deploy Preview https://deploy-preview-8907--care-ohc.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.

@@ -788,6 +788,30 @@ export const DailyRounds = (props: any) => {
},
]}
/>
<RangeAutocompleteFormField
Copy link
Contributor

@Jacobjeevan Jacobjeevan Oct 24, 2024

Choose a reason for hiding this comment

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

There's already an SpO2 field, line 848 onwards that's used for doctors/telemedicine rounds type, let's move that out to the earlier block, after line 762.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did the same

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea, but now we have SpO2 field specified twice, when we can achieve the same result with one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we dont need SpO2 field for community nurses log update ?

Copy link
Contributor

@Jacobjeevan Jacobjeevan Oct 24, 2024

Choose a reason for hiding this comment

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

We do.

Please check the file itself, the code block that starts at 726 includes fields that should appear for all 4 round types (that should have SpO2 field under new requirements).

So you can move the code starting at line 848 inside that block, and just remove the code you added.

@rithviknishad
Copy link
Member

@noufalrahim make changes in the same branch

@@ -900,7 +875,30 @@ export const DailyRounds = (props: any) => {
/>
</>
)}

<RangeAutocompleteFormField
Copy link
Contributor

Choose a reason for hiding this comment

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

There's still two instances for the field SpO2, there should only be one.

And this shouldn't be placed here, as Sp02 will be visible for all round types, instead of just normal, telemedicine, doctor's log and community nurse's update.

Always make sure to test your changes - on the frontend, you should see Sp02 occur twice under community nurse's log.

On a side note, how are you merging changes from develop into your branch? 🤔 It shouldn't be showing up as individual commits, but rather a merge commit.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

@Jacobjeevan Jacobjeevan Nov 4, 2024

Choose a reason for hiding this comment

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

And this shouldn't be placed here, as Sp02 will be visible for all round types, instead of just normal, telemedicine, doctor's log and community nurse's update.

@noufalrahim This is still an issue.

As mentioned earlier

Please check the file itself, the code block that starts at 726 includes fields that should appear for all 4 round types (that should have SpO2 field under new requirements).

The new code should be there.

Copy link
Contributor

Choose a reason for hiding this comment

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

@noufalrahim ☝️

@rithviknishad rithviknishad added the invalid This doesn't seem right label Oct 24, 2024
@noufalrahim
Copy link
Contributor Author

@rithviknishad
Can I do this once again from a new branch..?

@rithviknishad
Copy link
Member

are you facing any difficulties updating in the same branch?

@github-actions github-actions bot added the merge conflict pull requests with merge conflict label Oct 30, 2024
Copy link

👋 Hi, @noufalrahim,
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.

@Jacobjeevan Jacobjeevan added the question Further information is requested label Oct 30, 2024
@github-actions github-actions bot removed the merge conflict pull requests with merge conflict label Nov 2, 2024
@Jacobjeevan Jacobjeevan removed the question Further information is requested label Nov 4, 2024
@github-actions github-actions bot added the merge conflict pull requests with merge conflict label Nov 6, 2024
@rithviknishad rithviknishad added needs testing needs review and removed merge conflict pull requests with merge conflict labels Nov 13, 2024
@rithviknishad
Copy link
Member

@noufalrahim resolve conflicts

Copy link
Contributor

@Jacobjeevan Jacobjeevan left a comment

Choose a reason for hiding this comment

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

LGTM

@nihal467
Copy link
Member

LGTM @khavinshankar can you resolve the merge conflict and merge it ASAP

@Jacobjeevan Jacobjeevan removed needs review merge conflict pull requests with merge conflict labels Nov 19, 2024
@khavinshankar khavinshankar merged commit b0305a4 into ohcnetwork:develop Nov 19, 2024
22 of 23 checks passed
Copy link

@noufalrahim Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Spo2 on vitals section of Community Nurse log update
9 participants