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

O3-2635: find all queue entries for a given patient #55

Closed
wants to merge 2 commits into from
Closed

Conversation

cioan
Copy link
Member

@cioan cioan commented Feb 26, 2024

@mseaton , last week while working on the MCOE Check-in form, we have noticed that the queue module was not able to detect and prevent creating duplicate entries for past visits. Here is where we documented this problem:
PIH/openmrs-module-pihcore#481

We could add more checks in the AddPatientToQueueAction post submit action to prevent this use case, but I also took a closer look at the queue module code, and I think the following code is incorrect:
https://github.com/openmrs/openmrs-module-queue/blob/main/api/src/main/java/org/openmrs/module/queue/api/impl/QueueEntryServiceImpl.java#L89

            QueueEntrySearchCriteria searchCriteria = new QueueEntrySearchCriteria();
            searchCriteria.setPatient(queueEntry.getPatient());
            searchCriteria.setQueues(Collections.singletonList(queueEntry.getQueue()));
            List<QueueEntry> queueEntries = getQueueEntries(searchCriteria);
            for (QueueEntry qe : queueEntries) {
                    if (!qe.equals(queueEntry)) {
                            if (QueueUtils.datesOverlap(qe.getStartedAt(), qe.getEndedAt(), queueEntry.getStartedAt(),
                                queueEntry.getEndedAt())) {
                                    throw new DuplicateQueueEntryException("queue.entry.duplicate.patient");
                            }
                    }
            }
            return dao.createOrUpdate(queueEntry);

In this PR, I just wrote a unit test to show how the code we use to check for duplicates misses entries. I think we just need to add in the code above searchCriteria.setIsEnded(null); ? I am surprised that by default it does not include all queue entries.
Do you have any other suggestions on how to fix this problem? Thanks!

@mseaton
Copy link
Member

mseaton commented Feb 26, 2024

@cioan see my PR here: #57 and ticket here: https://openmrs.atlassian.net/browse/O3-2899. There have been other similar issues that have cropped up around this recently.

@mseaton
Copy link
Member

mseaton commented Feb 26, 2024

@cioan I'm closing this PR as the issue is being addressed in O3-2899, and because these changes cause many other unit tests defined in the module to fail.

@mseaton mseaton closed this Feb 26, 2024
@cioan
Copy link
Member Author

cioan commented Feb 26, 2024

No problem, @mseaton ! I wrote this unit test just to highlight the problem. I don't think there was anything else important to merge.

@cioan cioan deleted the O3-2635 branch February 26, 2024 19:41
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.

2 participants