Skip to content
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-2474 adding a patient to a queue should use active visit #1033

Merged
merged 3 commits into from
Mar 14, 2024

Conversation

mseaton
Copy link
Member

@mseaton mseaton commented Mar 13, 2024

Requirements

  • This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
  • My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
  • My work includes tests or is validated by existing tests.

Summary

When using the service queues app, there is an option to "Add patient to queue", however the form this enables will always attempt to first start a visit before adding a new queue entry associated with that visit. This fails to correctly handle two scenarios:

  1. If the patient selected already has a currently active visit, then the user should not be prompted to complete a start visit form that includes queue fields, but should just be prompted to fill out a form with queue fields, and this should be associated with the already active visit.
  2. If the patient selected is already active on the queue, then there needs to be improved validation letting the user know that the patient cannot be added because they are already on the queue

Screenshots

adding-to-queue-existing-visit

Related Issue

https://openmrs.atlassian.net/browse/O3-2474
https://openmrs.atlassian.net/browse/O3-2624

Copy link
Contributor

github-actions bot commented Mar 13, 2024

Size Change: -55.8 kB (-2%)

Total Size: 2.97 MB

Filename Size Change
packages/esm-service-queues-app/dist/776.js 0 B -56.4 kB (removed) 🏆
ℹ️ View Unchanged
Filename Size Change
packages/esm-active-visits-app/dist/130.js 123 kB 0 B
packages/esm-active-visits-app/dist/255.js 2.21 kB 0 B
packages/esm-active-visits-app/dist/271.js 762 B 0 B
packages/esm-active-visits-app/dist/277.js 13.4 kB 0 B
packages/esm-active-visits-app/dist/316.js 42.9 kB 0 B
packages/esm-active-visits-app/dist/319.js 683 B 0 B
packages/esm-active-visits-app/dist/382.js 1.15 kB 0 B
packages/esm-active-visits-app/dist/448.js 47.1 kB 0 B
packages/esm-active-visits-app/dist/460.js 784 B 0 B
packages/esm-active-visits-app/dist/574.js 588 B 0 B
packages/esm-active-visits-app/dist/588.js 6.66 kB 0 B
packages/esm-active-visits-app/dist/635.js 1.15 kB 0 B
packages/esm-active-visits-app/dist/644.js 762 B 0 B
packages/esm-active-visits-app/dist/729.js 3.1 kB 0 B
packages/esm-active-visits-app/dist/757.js 695 B 0 B
packages/esm-active-visits-app/dist/784.js 2.63 kB 0 B
packages/esm-active-visits-app/dist/788.js 586 B 0 B
packages/esm-active-visits-app/dist/807.js 918 B 0 B
packages/esm-active-visits-app/dist/833.js 732 B 0 B
packages/esm-active-visits-app/dist/879.js 2.94 kB 0 B
packages/esm-active-visits-app/dist/main.js 65 kB 0 B
packages/esm-active-visits-app/dist/openmrs-esm-active-visits-app.js 3.33 kB 0 B
packages/esm-appointments-app/dist/130.js 123 kB 0 B
packages/esm-appointments-app/dist/152.js 257 B 0 B
packages/esm-appointments-app/dist/255.js 2.21 kB 0 B
packages/esm-appointments-app/dist/271.js 2.21 kB 0 B
packages/esm-appointments-app/dist/303.js 258 B 0 B
packages/esm-appointments-app/dist/319.js 2.12 kB 0 B
packages/esm-appointments-app/dist/368.js 44.3 kB 0 B
packages/esm-appointments-app/dist/426.js 271 kB 0 B
packages/esm-appointments-app/dist/460.js 2.31 kB 0 B
packages/esm-appointments-app/dist/500.js 2.5 kB 0 B
packages/esm-appointments-app/dist/574.js 1.81 kB 0 B
packages/esm-appointments-app/dist/588.js 6.65 kB 0 B
packages/esm-appointments-app/dist/591.js 16.9 kB 0 B
packages/esm-appointments-app/dist/644.js 2.21 kB 0 B
packages/esm-appointments-app/dist/729.js 3.1 kB 0 B
packages/esm-appointments-app/dist/757.js 1.87 kB 0 B
packages/esm-appointments-app/dist/784.js 2.63 kB 0 B
packages/esm-appointments-app/dist/788.js 1.87 kB 0 B
packages/esm-appointments-app/dist/807.js 2.55 kB 0 B
packages/esm-appointments-app/dist/833.js 2.18 kB 0 B
packages/esm-appointments-app/dist/main.js 317 kB 0 B
packages/esm-appointments-app/dist/openmrs-esm-appointments-app.js 3.33 kB 0 B
packages/esm-patient-list-management-app/dist/130.js 123 kB 0 B
packages/esm-patient-list-management-app/dist/255.js 2.21 kB 0 B
packages/esm-patient-list-management-app/dist/271.js 1.56 kB 0 B
packages/esm-patient-list-management-app/dist/295.js 99.3 kB 0 B
packages/esm-patient-list-management-app/dist/319.js 1.52 kB 0 B
packages/esm-patient-list-management-app/dist/382.js 1.15 kB 0 B
packages/esm-patient-list-management-app/dist/435.js 22.7 kB 0 B
packages/esm-patient-list-management-app/dist/460.js 1.7 kB 0 B
packages/esm-patient-list-management-app/dist/574.js 1.34 kB 0 B
packages/esm-patient-list-management-app/dist/588.js 6.66 kB 0 B
packages/esm-patient-list-management-app/dist/591.js 16.9 kB 0 B
packages/esm-patient-list-management-app/dist/635.js 1.15 kB 0 B
packages/esm-patient-list-management-app/dist/644.js 1.56 kB 0 B
packages/esm-patient-list-management-app/dist/716.js 4.66 kB 0 B
packages/esm-patient-list-management-app/dist/729.js 3.1 kB 0 B
packages/esm-patient-list-management-app/dist/757.js 1.5 kB 0 B
packages/esm-patient-list-management-app/dist/784.js 2.64 kB 0 B
packages/esm-patient-list-management-app/dist/788.js 1.34 kB 0 B
packages/esm-patient-list-management-app/dist/807.js 1.84 kB 0 B
packages/esm-patient-list-management-app/dist/833.js 1.58 kB 0 B
packages/esm-patient-list-management-app/dist/main.js 126 kB 0 B
packages/esm-patient-list-management-app/dist/openmrs-esm-patient-list-management-app.js 3.3 kB 0 B
packages/esm-patient-registration-app/dist/130.js 123 kB 0 B
packages/esm-patient-registration-app/dist/152.js 262 B 0 B
packages/esm-patient-registration-app/dist/209.js 36.4 kB 0 B
packages/esm-patient-registration-app/dist/255.js 2.21 kB 0 B
packages/esm-patient-registration-app/dist/271.js 2.01 kB 0 B
packages/esm-patient-registration-app/dist/303.js 260 B 0 B
packages/esm-patient-registration-app/dist/319.js 1.99 kB 0 B
packages/esm-patient-registration-app/dist/460.js 2.12 kB 0 B
packages/esm-patient-registration-app/dist/537.js 2.34 kB 0 B
packages/esm-patient-registration-app/dist/574.js 1.7 kB 0 B
packages/esm-patient-registration-app/dist/591.js 16.9 kB 0 B
packages/esm-patient-registration-app/dist/62.js 6.86 kB 0 B
packages/esm-patient-registration-app/dist/644.js 2.01 kB 0 B
packages/esm-patient-registration-app/dist/729.js 3.1 kB 0 B
packages/esm-patient-registration-app/dist/730.js 115 kB 0 B
packages/esm-patient-registration-app/dist/735.js 464 B 0 B
packages/esm-patient-registration-app/dist/757.js 2.07 kB 0 B
packages/esm-patient-registration-app/dist/784.js 2.64 kB 0 B
packages/esm-patient-registration-app/dist/788.js 1.71 kB 0 B
packages/esm-patient-registration-app/dist/807.js 2.43 kB 0 B
packages/esm-patient-registration-app/dist/833.js 1.97 kB 0 B
packages/esm-patient-registration-app/dist/879.js 2.94 kB 0 B
packages/esm-patient-registration-app/dist/884.js 6.1 kB 0 B
packages/esm-patient-registration-app/dist/main.js 153 kB 0 B
packages/esm-patient-registration-app/dist/openmrs-esm-patient-registration-app.js 3.34 kB 0 B
packages/esm-patient-search-app/dist/130.js 123 kB 0 B
packages/esm-patient-search-app/dist/152.js 261 B 0 B
packages/esm-patient-search-app/dist/255.js 2.21 kB 0 B
packages/esm-patient-search-app/dist/271.js 1.12 kB 0 B
packages/esm-patient-search-app/dist/303.js 260 B 0 B
packages/esm-patient-search-app/dist/319.js 1.06 kB 0 B
packages/esm-patient-search-app/dist/382.js 1.15 kB 0 B
packages/esm-patient-search-app/dist/460.js 1.16 kB 0 B
packages/esm-patient-search-app/dist/48.js 26.4 kB 0 B
packages/esm-patient-search-app/dist/574.js 910 B 0 B
packages/esm-patient-search-app/dist/588.js 6.66 kB 0 B
packages/esm-patient-search-app/dist/591.js 16.9 kB 0 B
packages/esm-patient-search-app/dist/635.js 1.15 kB 0 B
packages/esm-patient-search-app/dist/644.js 1.12 kB 0 B
packages/esm-patient-search-app/dist/710.js 22.8 kB 0 B
packages/esm-patient-search-app/dist/729.js 3.1 kB 0 B
packages/esm-patient-search-app/dist/757.js 1.06 kB 0 B
packages/esm-patient-search-app/dist/784.js 2.63 kB 0 B
packages/esm-patient-search-app/dist/788.js 905 B 0 B
packages/esm-patient-search-app/dist/807.js 1.3 kB 0 B
packages/esm-patient-search-app/dist/833.js 1.08 kB 0 B
packages/esm-patient-search-app/dist/main.js 52.3 kB 0 B
packages/esm-patient-search-app/dist/openmrs-esm-patient-search-app.js 3.34 kB 0 B
packages/esm-service-queues-app/dist/130.js 123 kB 0 B
packages/esm-service-queues-app/dist/152.js 262 B 0 B
packages/esm-service-queues-app/dist/203.js 2.64 kB 0 B
packages/esm-service-queues-app/dist/255.js 2.22 kB 0 B
packages/esm-service-queues-app/dist/271.js 3.85 kB 0 B
packages/esm-service-queues-app/dist/303.js 261 B 0 B
packages/esm-service-queues-app/dist/319.js 3.29 kB 0 B
packages/esm-service-queues-app/dist/328.js 3.13 kB 0 B
packages/esm-service-queues-app/dist/389.js 1.89 kB 0 B
packages/esm-service-queues-app/dist/425.js 2.08 kB 0 B
packages/esm-service-queues-app/dist/460.js 4.09 kB 0 B
packages/esm-service-queues-app/dist/523.js 56.9 kB 0 B
packages/esm-service-queues-app/dist/574.js 3.38 kB +10 B (0%)
packages/esm-service-queues-app/dist/588.js 6.66 kB 0 B
packages/esm-service-queues-app/dist/591.js 16.9 kB 0 B
packages/esm-service-queues-app/dist/616.js 2.71 kB 0 B
packages/esm-service-queues-app/dist/644.js 3.85 kB 0 B
packages/esm-service-queues-app/dist/694.js 158 kB 0 B
packages/esm-service-queues-app/dist/696.js 3.69 kB 0 B
packages/esm-service-queues-app/dist/729.js 3.1 kB 0 B
packages/esm-service-queues-app/dist/738.js 3.69 kB 0 B
packages/esm-service-queues-app/dist/757.js 3.29 kB 0 B
packages/esm-service-queues-app/dist/784.js 2.63 kB 0 B
packages/esm-service-queues-app/dist/788.js 3.29 kB 0 B
packages/esm-service-queues-app/dist/807.js 4.55 kB 0 B
packages/esm-service-queues-app/dist/833.js 3.8 kB 0 B
packages/esm-service-queues-app/dist/981.js 2.96 kB 0 B
packages/esm-service-queues-app/dist/main.js 218 kB +535 B (0%)
packages/esm-service-queues-app/dist/openmrs-esm-service-queues-app.js 3.31 kB +2 B (0%)

compressed-size-action

height: 100vh;
z-index: 99999;
height: calc(100vh - 3rem);
z-index: 3;
Copy link
Member Author

Choose a reason for hiding this comment

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

This is an unrelated issue, but has been bothering me for a while, in that the buttons in the Add to queue overlay are barely visible on the screen. This adjustment here I copied from the appointments esm, which presumably had the same issue and was fixed there. Yet another reason not to have multiple overlay implementations all over the place.

return (
<div>
{isTablet && (
<Row className={styles.headerGridRow}>
Copy link
Member Author

Choose a reason for hiding this comment

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

This form is really just a subset of the full visit-form, and uses the same extension slot to add in the queue fields.

const err = error?.responseBody?.error?.message;
if (err && err === '[queue.entry.duplicate.patient]') {
subtitle = t('patientAlreadyInQueue', 'Patient is already in the queue');
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is my admittedly somewhat hacky solution to the problem presented in https://openmrs.atlassian.net/browse/O3-2624 . In future refactors, we would not get to this point, but would have pre-submission validation in place, but this is an incremental improvement to at least inform the user of the error, which is caught in back end validation with a specific error message returned.

showSnackbar({
kind: 'success',
isLowContrast: true,
title: t('startAVisit', 'Start a visit'),
Copy link
Contributor

@ojwanganto ojwanganto Mar 13, 2024

Choose a reason for hiding this comment

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

You can change this label to reflect the queue operation. I just picked it from the video

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @ojwanganto - looks like this PR was merged already by @makombe . I tried to apply this change last night, but am running into build errors at the moment that is preventing me from committing. @makombe maybe you can fix this for me since you merged the PR? Otherwise, I'll address it in a follow-up commit once I have my environment back.

@ojwanganto
Copy link
Contributor

Looks good, @mseaton. I have flagged just a simple label change.

@ojwanganto ojwanganto merged commit 5d67540 into main Mar 14, 2024
6 checks passed
@ojwanganto ojwanganto deleted the O3-2474 branch March 14, 2024 06:38
Copy link
Contributor

@chibongho chibongho left a comment

Choose a reason for hiding this comment

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

LGTM. Just a few minor comments.

return null;
}

const handleSubmit = useCallback(
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't really need useCallback here (see this). It makes it difficult to maintain with the list of dependencies.

closePanel: () => void;
}

const ExistingVisitForm: React.FC<ExistingVisitFormProps> = ({ visit, toggleSearchType, closePanel }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a comment here? Just from the name, it's hard to know that it's meant to be a form to start a queue entry for a patient that already has an existing visit .

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

Successfully merging this pull request may close these issues.

5 participants