Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

feat(AppointmentsList): add an appointments tab to the patient view #1823

Merged
merged 4 commits into from
Feb 14, 2020

Conversation

igeagonz
Copy link
Contributor

@igeagonz igeagonz commented Feb 13, 2020

add an appointments tab to the patient view which shows appointments list for corresponding patient.
Uses same layout as patient list view and also implements fuzzy search

fixes #1769

Changes proposed in this pull request:

  • add new appointments tab
  • allow fuzzy search

@vercel
Copy link

vercel bot commented Feb 13, 2020

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/hospitalrun/hospitalrun-frontend/1qca0p36u
✅ Preview: https://hospitalrun-frontend-git-fork-igeagonz-patient-appointment-list.hospitalrun.now.sh

@igeagonz
Copy link
Contributor Author

Unit tests are pending. But wanted to get this out to start getting it reviewed.

add an appointments tab to the patient view which shows appointments list for corresponding patient.
Uses same layout as patient list view and also implements fuzzy search

HospitalRun#1769
<ListItem action key={a.id} onClick={() => history.push(`/appointments/${a.id}`)}>
{`${t('scheduling.appointment.location')}: ${a.location}
${t('scheduling.appointment.reason')}: ${a.reason}
${t('scheduling.appointment.type')}: ${a.type}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

location, reason and type aren't required fields when creating an appointment. Could conditionally render, but this would get ugly... unless someone has an idea 😄

Copy link
Member

@jackcmeyer jackcmeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One bug that I noticed was that when on the `/patients/:id/appointments route, both appointments and patients are selected. I would expect that only patients was selected.

The problematic lines are:

active={path.pathname.includes('patient')}

active={path.pathname.includes('appointments')}

Rather than using .includes we might want to match the start of the string instead.

Comment on lines 31 to 35
{`${t('scheduling.appointment.location')}: ${a.location}
${t('scheduling.appointment.reason')}: ${a.reason}
${t('scheduling.appointment.type')}: ${a.type}
${t('scheduling.appointment.startDate')}: ${new Date(a.startDateTime).toLocaleString()}
${t('scheduling.appointment.endDate')}: ${new Date(a.endDateTime).toLocaleString()}`}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for now, let's simply display the start date time of appointment.

Something like 2020-01-02 10:00am (except the internationalized version of that string).

Eventually, once the Table component is finished, we can migrate to that.

Copy link
Contributor Author

@igeagonz igeagonz Feb 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I'll simply reduce it to just the Date.toLocaleString() which defaults to the locale it's running on.

Edit: Do note that the current search bar (which was out of scope) searches on the location, reason or type. In my opinion, having just the date would be a little less intuitive if only a date was displayed. Just something to consider.

Comment on lines +54 to +62
<TextInput
size="lg"
value={searchText}
placeholder={t('actions.search')}
onChange={onSearchBoxChange}
/>
<div className="input-group-append">
<Button onClick={onSearchFormSubmit}>{t('actions.search')}</Button>
</div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we have two search boxes, we should consider abstracting that to a component in the /components folder.

@@ -6,6 +6,38 @@ export class AppointmentRepository extends Repository<Appointment> {
constructor() {
super(appointments)
}

// Fuzzy search for patient appointments. Used for patient appointment search bar
async searchPatientAppointments(patientId: string, text: string): Promise<Appointment[]> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Searching this list is not something that I had previously considered. I think that this is a really neat idea!

@fox1t @tehkapa @lauggh @StefanoMiC,

What does everyone thing the requirements should be here? I see a really cool opportunity for having filter buttons for the different appointment types.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can leave it like this for now. We need to open a new feature issue to search about this. We really want a multi-field fuzzy search on almost every entity.

@jackcmeyer
Copy link
Member

@igeagonz thanks for your contribution 🎉

@vercel vercel bot temporarily deployed to Preview February 14, 2020 16:44 Inactive
Copy link
Member

@fox1t fox1t left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -6,6 +6,38 @@ export class AppointmentRepository extends Repository<Appointment> {
constructor() {
super(appointments)
}

// Fuzzy search for patient appointments. Used for patient appointment search bar
async searchPatientAppointments(patientId: string, text: string): Promise<Appointment[]> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can leave it like this for now. We need to open a new feature issue to search about this. We really want a multi-field fuzzy search on almost every entity.

@fox1t fox1t merged commit f15fced into HospitalRun:master Feb 14, 2020
@fox1t
Copy link
Member

fox1t commented Feb 14, 2020

Thanks for your contribution! 🎉🎉🎉🎉🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Appointments list in Patients profile
4 participants