-
Notifications
You must be signed in to change notification settings - Fork 233
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
(fix) O3-3661: Admission Requests List should include both Admission and Transfer requests #1255
Changes from 1 commit
ebe3d35
027d128
2c6a3fa
66cb2fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -3,15 +3,13 @@ import styles from './admission-requests-workspace.scss'; | |||||
import AdmissionRequestCard from './admission-request-card.component'; | ||||||
import { getCoreTranslation, type DefaultWorkspaceProps } from '@openmrs/esm-framework'; | ||||||
import { useInpatientRequest } from '../hooks/useInpatientRequest'; | ||||||
import useWardLocation from '../hooks/useWardLocation'; | ||||||
import { Loading } from '@carbon/react'; | ||||||
import { InlineNotification } from '@carbon/react'; | ||||||
import { useTranslation } from 'react-i18next'; | ||||||
|
||||||
const AdmissionRequestsWorkspace: React.FC<DefaultWorkspaceProps> = () => { | ||||||
const { t } = useTranslation(); | ||||||
const { inpatientRequests, isLoading, error } = useInpatientRequest(); | ||||||
const admissionRequests = inpatientRequests?.filter((request) => request.dispositionType == 'ADMIT'); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't have to filter here if we are only fetching types we want? |
||||||
const { inpatientRequests, isLoading, error } = useInpatientRequest(['ADMIT', 'TRANSFER']); | ||||||
|
||||||
if (isLoading) { | ||||||
return <Loading />; | ||||||
|
@@ -29,7 +27,7 @@ const AdmissionRequestsWorkspace: React.FC<DefaultWorkspaceProps> = () => { | |||||
|
||||||
return ( | ||||||
<div className={styles.admissionRequestsWorkspace}> | ||||||
{admissionRequests.map((admissionRequest, indx) => ( | ||||||
{inpatientRequests.map((admissionRequest, indx) => ( | ||||||
<AdmissionRequestCard key={indx} patient={admissionRequest.patient} /> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there something in the response we could use as a key here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or just like @vasharma05 sorry we didn't catch this in the original PR with this change, but you can't just use an integer as a React key; it is very easy to wind up with duplicate keys that way. Duplicate keys can cause really bizarre and confusing bugs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
))} | ||||||
</div> | ||||||
|
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.
technically I didn't need to change the default value, but if we are going to have a default value seemed to make sense to have it equal to the only value we are currently passing in.