-
Notifications
You must be signed in to change notification settings - Fork 251
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
(feat) O3-4430: Optional Indication field in the drug order form #2228
base: main
Are you sure you want to change the base?
Conversation
Size Change: +306 B (0%) Total Size: 16.3 MB ℹ️ View Unchanged
|
) : null} | ||
<span> | ||
<span className={styles.label01}>{t('indication', 'Indication').toUpperCase()}</span>{' '} | ||
{medication?.order?.orderReasonNonCoded ?? t('none', 'None')} |
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.
{medication?.order?.orderReasonNonCoded ?? t('none', 'None')} | |
{medication?.order?.orderReasonNonCoded ?? t('noIndication', 'No indication provided')} |
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.
In general, be careful when placing text in front of users indicating the absence of data.
@@ -36,6 +36,11 @@ export const configSchema = { | |||
'Number of milliseconds to delay the search operation in the drug search input by after the user starts typing. The useDebounce hook delays the search by 300ms by default', | |||
_default: 300, | |||
}, | |||
isIndicationFieldOptional: { |
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.
isIndicationFieldOptional: { | |
requireIndication: { |
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.
+1 for requireIndication: { _default: true }
.
Positive phrasing is easier to grok (implies "we require this by default") as opposed to the double negative isIndicationFieldOptional: { _default: false }
(implies "it's optional by default").
The validation logic reads better as well:
// Current validation logic
if (!isIndicationFieldOptional) { /* require indication */ }
// With requireIndication
if (requireIndication) { /* require indication */ }
isIndicationFieldOptional: { | ||
_type: Type.Boolean, | ||
_description: 'Whether to make the indication field optional in the drug order form', | ||
_default: false, |
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.
_default: false, | |
_default: true, |
<span> | ||
<span className={styles.label01}>{t('indication', 'Indication').toUpperCase()}</span>{' '} | ||
{medication.orderReasonNonCoded ?? t('none', 'None')} | ||
</span> |
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.
I actually think the previous implementation here is better. Can we reverse this please?
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.
Actually the quantity
text right after the Indication text has a &mdash
, hence when we don't have indication, the row looks like: - QUANTITY 5 tablets
, hence I needed to put a fallback value for the missing indication.
@@ -207,8 +207,9 @@ const OrderDetailsTable: React.FC<OrderDetailsProps> = ({ patientUuid, showAddBu | |||
dosage: | |||
order.type === 'drugorder' ? ( | |||
<div className={styles.singleLineText}>{`${t('indication', 'Indication').toUpperCase()} | |||
${order.orderReasonNonCoded} ${'-'} ${t('quantity', 'Quantity').toUpperCase()} ${order.quantity} ${order | |||
?.quantityUnits?.display} `}</div> | |||
${order.orderReasonNonCoded ?? t('none', 'None')} ${'-'} ${t('quantity', 'Quantity').toUpperCase()} ${ |
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.
"No indication provided"
Co-authored-by: Ian <[email protected]>
…ub.com/openmrs/openmrs-esm-patient-chart into feat/configurable-indication-field
@@ -221,18 +231,14 @@ export function DrugOrderForm({ initialOrderBasketItem, onSave, onCancel, prompt | |||
const config = useConfig<ConfigObject>(); | |||
const isTablet = useLayoutType() === 'tablet'; | |||
const { orderConfigObject, error: errorFetchingOrderConfig } = useOrderConfig(); | |||
const { requireOutpatientQuantity } = useRequireOutpatientQuantity(); | |||
|
|||
const defaultStartDate = useMemo(() => { | |||
if (typeof initialOrderBasketItem?.startDate === 'string') parseDate(initialOrderBasketItem?.startDate); |
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.
if (typeof initialOrderBasketItem?.startDate === 'string') parseDate(initialOrderBasketItem?.startDate); | |
if (typeof initialOrderBasketItem?.startDate === 'string') { | |
return parseDate(initialOrderBasketItem?.startDate); | |
} |
Tangential to your changes, but this logic has a bug - the result of the parseDate
call isn't getting stored or returned. Digging into this some more reveals that when modifying a medication, we're incorrectly setting the startDate
to new Date()
instead of the existing order's dateActivated
property.
So IMO, that line should change to:
startDate: medication.dateActivated,
Unless there's a specific reason for doing otherwise, @ibacher.
We can address this in a separate PR.
@@ -36,6 +36,11 @@ export const configSchema = { | |||
'Number of milliseconds to delay the search operation in the drug search input by after the user starts typing. The useDebounce hook delays the search by 300ms by default', | |||
_default: 300, | |||
}, | |||
isIndicationFieldOptional: { |
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.
+1 for requireIndication: { _default: true }
.
Positive phrasing is easier to grok (implies "we require this by default") as opposed to the double negative isIndicationFieldOptional: { _default: false }
(implies "it's optional by default").
The validation logic reads better as well:
// Current validation logic
if (!isIndicationFieldOptional) { /* require indication */ }
// With requireIndication
if (requireIndication) { /* require indication */ }
Requirements
Summary
This PR allows configuring the indication field as optional when saving the drug order form
Screenshots
Related Issue
https://openmrs.atlassian.net/issues/O3-4430
Other