O3-2472: Appointment Calender Feature does not work in Ref App #954
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: -48.5 kB (-2%) Total Size: 2.87 MB
ℹ️ View Unchanged
|
mogoodrich
commented
Jan 26, 2024
|
|
||
| const CalendarView: React.FC<{ | ||
| calendarView: CalendarType; | ||
| events: any; | ||
| events: Array<DailyAppointmentsCountByService>; |
There was a problem hiding this comment.
I typed this to assure consistency throughout the code.
| @@ -23,7 +23,7 @@ const DailyCalendarView: React.FC<DailyCalendarViewProps> = ({ | |||
| }) => { | |||
| return ( | |||
| <div className={styles.container}> | |||
| <DailyHeader type={type} currentDate={currentDate} setCurrentDate={setCurrentDate} events={events} /> | |||
| <DailyHeader type={type} currentDate={currentDate} setCurrentDate={setCurrentDate} /> | |||
There was a problem hiding this comment.
there were some places where this prop wasn't actually being used, so I removed it in those cases.
| const currentData = events?.find( | ||
| (event) => dayjs(event.appointmentDate).format('YYYY-MM-DD') === dayjs(dateTime).format('YYYY-MM-DD'), | ||
| ); | ||
| const colorCoding = { HIV: 'red', 'Lab testing': 'purple', Refill: 'blue' }; |
There was a problem hiding this comment.
There were a couple places where there was coloring based on hard-coded service names... I just removed these, we will need to make this configurable somehow if we want this.
| @@ -38,11 +37,11 @@ const DailyWorkloadView: React.FC<WeeklyCellProps> = ({ type, dateTime, currentD | |||
| <> | |||
| <div className={styles.allDayComponent}> | |||
| <small className={styles.allDay}>All Day</small> | |||
| {currentData?.service && ( | |||
There was a problem hiding this comment.
nit, but I changed "service" to "services" since it was an array of services
| }; | ||
| const filterOnClick = () => { | ||
| // filter functionality | ||
| }; |
There was a problem hiding this comment.
I removed the above two buttons since they have not yet been implemented
| @@ -42,7 +42,8 @@ const WeeklyCalendarView: React.FC<WeeklyCalendarViewProps> = ({ | |||
| ); | |||
| })} | |||
| </p> | |||
| <p className={styles['weekly-calendar-all']}> | |||
| {/* displays hours of day but commented out because showing the appts by hour is not currently implemented within this code | |||
| <p className={styles['weekly-calendar-all']}> | |||
| } | ||
|
|
||
| export const useAppointmentsCalendar = (forDate: string, period: string) => { | ||
| const { startDate, endDate } = evaluateAppointmentCalendarDates(forDate, period); | ||
| const url = `/ws/rest/v1/appointment/appointmentCalendar?startDate=${startDate}&endDate=${endDate}`; | ||
| const url = `/ws/rest/v1/appointment/appointmentSummary?startDate=${startDate}&endDate=${endDate}`; |
There was a problem hiding this comment.
This and the "results" code block below is the meat of this code, basically we are switching from using the "appointmentCalender" endpoint, which is only a custom fork of the Bahmni Appointment module, to use the appointmentSummary endpoint that is in the released module.
| } | ||
| }); | ||
| return acc; | ||
| }, []); |
There was a problem hiding this comment.
This is updated so that that maps the return from appointmentSummary endpoint to the expected data structure used previously... I also typed this data structure as "DailyAppointmentCountByService" (see below)
| serviceName: string; | ||
| count: number; | ||
| }>; | ||
| } |
There was a problem hiding this comment.
this basically just types the existing data structure, so though I did change "service" to "services" to make it more clear.
| @@ -0,0 +1,83 @@ | |||
| { | |||
There was a problem hiding this comment.
I thought I skipped all these, but I guess they got added... :)
|
The failing E2E tests appear unrelated to my commit, or am I missing something? Update: I confirmed that the playwright tests are failing for me locally, even prior to my changes. |
mogoodrich
requested review from
donaldkibet,
CynthiaKamau,
denniskigen,
mseaton,
chibongho and
cioan
Yes, unrelated. Should be fixed soonish. |
donaldkibet
approved these changes
Jan 27, 2024
There was a problem hiding this comment.
LGTM Thanks @mogoodrich, do we plan on having support to display Weekly and Daily appointments with AppointmentSummary endpoint.
mseaton
approved these changes
Jan 29, 2024
@donaldkibet it's a good question, it depends on the priority/need I guess. One clarification though, did the new "appointmentCalendar" endpoint in the Palladium fork support this? My goal of this PR was just to make "feature parity" with the existing Palladium fork, and, looking at the existing code (well, the UI code, at least) in the Daily and Weekly views it didn't seem to support "slotting" appointments by time, it just displayed all the appointments at the top in a row that said something like "all day". It's a good question if a daily view like this is particularly useful, but I think is the way the current code works, unless I missed something? |
|
Thanks @donaldkibet @mseaton , merging this in, also see my response to Donald's question above, happy to discuss further. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requirements
Summary
Screenshots
Related Issue
Other