(fix) O3-5387: Remove redundant mathematical operation and improve type safety in appointments#2233
Conversation
| renderIcon={Download} | ||
| onClick={() => { | ||
| const date = appointments[0]?.startDateTime | ||
| ? formatDate(parseDate(String(appointments[0]?.startDateTime)), { |
There was a problem hiding this comment.
Kindly this parseDate(appointment.startDateTime) should work because parseDate likely already handles both formats.
Alternatively convert number timestamps to ISO strings before parsing
const dateString = typeof appointment.startDateTime === 'number'
? new Date(appointment.startDateTime).toISOString()
: appointment.startDateTime;
parseDate(dateString)
cc @denniskigen , @NethmiRodrigo , @ibacher Kindly can I learn from you please!
| return { | ||
| id: appointment.uuid, | ||
| date: formatDatetime(parseDate(appointment.startDateTime), { mode: 'wide' }), | ||
| date: formatDatetime(parseDate(String(appointment.startDateTime)), { mode: 'wide' }), |
There was a problem hiding this comment.
Kindly we should remove String() wrapper entirely because type is updated to string | number in the interface so parseDate() should handle both formats natively.
There was a problem hiding this comment.
Addressed - removed the String() wrappers entirely and rebased on latest [main]. [usePatientAppointmentHistory.ts] (removed the redundant math) and [index.ts] ([startDateTime: string | any] → [startDateTime: string])
| <StructuredListRow key={index} className={styles.structuredList}> | ||
| <StructuredListCell> | ||
| {formatDate(parseDate(appointment.startDateTime), { mode: 'wide' })} | ||
| {formatDate(parseDate(String(appointment.startDateTime)), { mode: 'wide' })} |
There was a problem hiding this comment.
Kindly remove the String() wrapper entirely. The parseDate() already handles both formats.
There was a problem hiding this comment.
Addressed - removed the String() wrappers entirely and rebased on latest [main]. [usePatientAppointmentHistory.ts] (removed the redundant math) and [index.ts] ([startDateTime: string | any] → [startDateTime: string])
sanks011
force-pushed
the
O3-5387-redundant-mathematical-operation-in-use-patient-appointment-history
branch
from
14b9062 to
9087750
Compare
|
@jwnasambu Thanks for the review! The String() wrapper has been removed. Also worth noting - the entire download button section where this code lived was subsequently removed in a later upstream commit (#2263), so this file no longer contains that code at all after the rebase. |
…use-patient-appointment-history
…use-patient-appointment-history
…use-patient-appointment-history
…use-patient-appointment-history
3 participants
Requirements
Summary
This PR addresses code quality and type safety issues in the appointments app:
1. Removed Redundant Mathematical Operation
(appointment.startDateTime / 1000) * 1000inusePatientAppointmentHistory.tsdayjs(new Date(appointment.startDateTime).toISOString())2. Improved Type Safety
startDateTimetype fromstring | anytostring | numberin the Appointment interfaceanytype annotation that defeated TypeScript's type checkingString()conversion forparseDatecalls in 3 component files to handle both formatsBenefits
anyScreenshots
N/A - No UI changes
Related Issue
https://openmrs.atlassian.net/browse/O3-5387
Other
All existing tests pass successfully. The changes maintain backward compatibility while improving code quality.