-
Notifications
You must be signed in to change notification settings - Fork 432
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
Removed Oxygen Details from Update Facility form #7796
Removed Oxygen Details from Update Facility form #7796
Conversation
… issues/7758/clean-up-oxygen-details
@609harsh 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. |
@609harsh modify the existing cypress test related to facility creation as it is currently failing |
👋 Hi, @609harsh, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
Hi @rithviknishad these are the following updates in ui |
…com/609harsh/os_care_fe into issues/7758/clean-up-oxygen-details
Please review the modified cypress test |
@@ -52,7 +52,7 @@ export const FacilityHome = (props: any) => { | |||
const [editCoverImage, setEditCoverImage] = useState(false); | |||
const [imageKey, setImageKey] = useState(Date.now()); | |||
const authUser = useAuthUser(); | |||
|
|||
console.log("hell", props); |
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.
console.log("hell", props); |
}: any) => { | ||
const [oxygenDetailsModalOpen, setOxygenDetailsModalOpen] = useState(false); | ||
|
||
let capacityList: any = null; |
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.
any type is not allowed. define proper types
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.
Will "let capacityList: ReactElement | null = null" be good? Since it will contain React element(reference line 25 or Line 38)
Moreover this file is almost in similar pattern with and FacilityBedCapacity.tsx. Should there also this variable be modified?
handleUpdate={() => { | ||
facilityFetch(); | ||
}} |
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.
handleUpdate={() => { | |
facilityFetch(); | |
}} | |
handleUpdate={() => facilityFetch()} |
handleUpdate={() => { | ||
facilityFetch(); | ||
return; | ||
}} |
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.
handleUpdate={() => { | |
facilityFetch(); | |
return; | |
}} | |
handleUpdate={() => facilityFetch()} |
oxygen_type === 1 | ||
? facilityData?.oxygen_capacity | ||
: oxygen_type === 2 | ||
? facilityData?.type_b_cylinders | ||
: oxygen_type === 3 | ||
? facilityData?.type_c_cylinders | ||
: facilityData?.type_d_cylinders, | ||
expectedBurnRate: | ||
oxygen_type === 1 | ||
? facilityData?.expected_oxygen_requirement | ||
: oxygen_type === 2 | ||
? facilityData?.expected_type_b_cylinders | ||
: oxygen_type === 3 | ||
? facilityData?.expected_type_c_cylinders | ||
: facilityData?.expected_type_d_cylinders, |
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.
use a map instead? where keys are the oxygen_type and values are the capacity?
Note: Always try to check the similar existing behavior functions (staff capacity pop-up or bed capacity pop-up), in the platform and do a basic functionality check to increase the quality of PR |
@@ -24,6 +24,10 @@ describe("Facility Manage Functions", () => { | |||
const currentOccupied = "80"; | |||
const totalUpdatedCapacity = "120"; | |||
const currentUpdatedOccupied = "100"; | |||
const totalOxygenCapacity = "100"; |
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.
delete the existing unused constants or reuse them when you are modifying the existing test
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.
new ones are added for oxygen and earlier ones are for doctor and bed. no constants are unused
👋 Hi, @609harsh, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
hi @nihal467 regarding the last query currently i am sending down props from parent to use oxygen related. In order to prevent popup closing we need a new endpoint to fetch and update information related to oxygen only (similar to what is followed in staff capacity pop-up or bed capacity pop-up). |
@609harsh you could simply pass the refetch callback from the useQuery and call that instead to refresh latest information. |
👋 Hi, @609harsh, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
@609harsh Can you update the PR? |
Closing this PR due to lack of recent progress. |
Proposed Changes
@coronasafe/care-fe-code-reviewers @coronasafe/code-reviewers
Merge Checklist