-
Notifications
You must be signed in to change notification settings - Fork 330
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
base: develop
Are you sure you want to change the base?
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
Proposed Changes
@coronasafe/care-fe-code-reviewers @coronasafe/code-reviewers
Merge Checklist