Issues/7350/log update consistency #8278
Conversation
|
@renoseHarsh is attempting to deploy a commit to the Open Healthcare Network Team on Vercel. A member of the Team first needs to authorize it. |
✅ Deploy Preview for care-egov-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| let newName: string | null = null; | ||
| if (props.name === "daily round details Event") { | ||
| newName = props.name.replace("daily round", "log update"); | ||
| } |
There was a problem hiding this comment.
This is not the right way to solve the problem.
It should instead be changed in the props passed to this component.
Reusable components such as these should not contain any implementation specific to a particular usage.
| @@ -13,6 +13,7 @@ const MENU_TAGS: { [key: string]: string } = { | |||
| external_results: "External Results", | |||
| users: "Users", | |||
| notice_board: "Notice Board", | |||
| "daily-rounds": "Log Update", | |||
There was a problem hiding this comment.
Change the URL's path param itself instead
| "daily-rounds": "Log Update", |
| "PATIENT_CONSULTATION_UPDATED", | ||
| "PATIENT_CONSULTATION_UPDATE_CREATED", | ||
| "PATIENT_CONSULTATION_UPDATE_UPDATED", | ||
| "PATIENT_CONSULTATION_CREATED", | ||
| ]; | ||
| const modifiedData = res.data.results.map((notification: any) => { | ||
| if (toChangeId.includes(notification.event)) { | ||
| return { | ||
| ...notification, | ||
| message: notification.message.replace( | ||
| "Consultation", | ||
| "Log Updates", | ||
| ), | ||
| }; | ||
| } else { | ||
| return notification; | ||
| } |
There was a problem hiding this comment.
This doesn't seem to be the right way to do it.
Why is patient consultation created event being renamed to log update? Consultation creation is not relevant to log updates right?
There was a problem hiding this comment.
Can you clarify if the way I am modifying data is wrong or is it just the part that I have 'PATIENT_CONSULTATION_CREATED' in toChangeId wrong?
There was a problem hiding this comment.
Consultation Create notification message need not be replaced with log updates right?
| @@ -113,7 +113,7 @@ let make = ( | |||
| <div> | |||
| <div | |||
| className="bg-white px-2 md:px-6 py-5 border-b border-secondary-200 sm:px-6 max-w-5xl mx-auto border mt-4 shadow rounded-lg"> | |||
| <h1 className="text-4xl sm:text-5xl"> {str("Consultation Update")} </h1> | |||
| <h1 className="text-4xl sm:text-5xl"> {str("Log Update")} </h1> | |||
There was a problem hiding this comment.
Why wrap it in str(...)?
| <h1 className="text-4xl sm:text-5xl"> {str("Log Update")} </h1> | |
| <h1 className="text-4xl sm:text-5xl">Log Update</h1> |
|
@renoseHarsh It'd be great if you could give a more meaningful title for the PR 😄 |
|
Thank you for the feedback. I will make the necessary changes accordingly. |
Proposed Changes
src/Components/Facility/ConsultationDetails/ConsultationUpdatesTab.tsxchanged Daily Rounds to Log Updatessrc/CAREUI/display/Timeline.tsxadded a check to see if the current Timeine is for "daily round" If yes change it to "log update".NOTIFICATION_EVENTSinsrc/Common/constants.tsxsrc/Components/Notifications/NotificationsList.tsxand change messages from 'consultation' to 'log'src/Components/CriticalCareRecording/CriticalCare__Index.ressrc/Components/Common/Breadcrumbs.tsx@coronasafe/care-fe-code-reviewers @coronasafe/code-reviewers
Merge Checklist