-
Notifications
You must be signed in to change notification settings - Fork 23
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-2448 - Visit queue number customization #44
Changes from all commits
b268578
e796bdb
6707946
0aa2b3a
4bd7ec3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,13 +30,16 @@ | |
import org.openmrs.module.queue.api.dao.VisitQueueEntryDao; | ||
import org.openmrs.module.queue.model.QueueEntry; | ||
import org.openmrs.module.queue.model.VisitQueueEntry; | ||
import org.openmrs.module.queue.processor.VisitQueueEntryProcessor; | ||
import org.springframework.transaction.annotation.Transactional; | ||
|
||
@Slf4j | ||
@Transactional | ||
@Setter(AccessLevel.MODULE) | ||
@Setter(AccessLevel.PUBLIC) | ||
public class VisitQueueEntryServiceImpl extends BaseOpenmrsService implements VisitQueueEntryService { | ||
|
||
private VisitQueueEntryProcessor visitQueueEntryProcessor; | ||
|
||
private VisitQueueEntryDao<VisitQueueEntry> dao; | ||
|
||
public void setDao(VisitQueueEntryDao<VisitQueueEntry> dao) { | ||
|
@@ -49,23 +52,23 @@ public Optional<VisitQueueEntry> getVisitQueueEntryByUuid(@NotNull String uuid) | |
} | ||
|
||
@Override | ||
public VisitQueueEntry createVisitQueueEntry(@NotNull VisitQueueEntry visitQueueEntry) { | ||
//verify visit -patient & queue entry patient | ||
//Todo more refactor | ||
Visit visit = Context.getVisitService().getVisitByUuid(visitQueueEntry.getVisit().getUuid()); | ||
Patient visitPatient = visit.getPatient(); | ||
Patient queueEntryPatient = visitQueueEntry.getQueueEntry().getPatient(); | ||
if (visitPatient != null & queueEntryPatient != null) { | ||
boolean isPatientSame = visitPatient.getUuid().equals(queueEntryPatient.getUuid()); | ||
if (!isPatientSame) { | ||
throw new IllegalArgumentException("Patient mismatch - visit.patient does not match queueEntry.patient"); | ||
} | ||
QueueEntry newlyCreatedQueueEntry = Context.getService(QueueEntryService.class) | ||
.createQueueEntry(visitQueueEntry.getQueueEntry()); | ||
visitQueueEntry.setQueueEntry(newlyCreatedQueueEntry); | ||
return this.dao.createOrUpdate(visitQueueEntry); | ||
public synchronized VisitQueueEntry createVisitQueueEntry(@NotNull VisitQueueEntry visitQueueEntry) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made this method synchronized, as there tend to be situations where the queue number to generate is based on the queue entries and/or existing queue numbers assigned that exist in the database, and may need to protect against several threads generating the same queue number at the same time in different threads. See some of the comments in O3-2448 |
||
Visit visit = visitQueueEntry.getVisit(); | ||
Patient visitPatient = Context.getVisitService().getVisitByUuid(visit.getUuid()).getPatient(); | ||
QueueEntry queueEntry = visitQueueEntry.getQueueEntry(); | ||
if (queueEntry.getPatient() == null) { | ||
queueEntry.setPatient(visitPatient); | ||
} else if (!queueEntry.getPatient().getUuid().equals(visitPatient.getUuid())) { | ||
throw new IllegalArgumentException("Patient mismatch - visit.patient does not match queueEntry.patient"); | ||
} | ||
queueEntry = Context.getService(QueueEntryService.class).createQueueEntry(queueEntry); | ||
visitQueueEntry.setQueueEntry(queueEntry); | ||
visitQueueEntry = dao.createOrUpdate(visitQueueEntry); | ||
|
||
if (visitQueueEntryProcessor != null) { | ||
visitQueueEntryProcessor.beforeSaveVisitQueueEntry(visitQueueEntry); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the main area that addresses the issue and allows customization. If a visitQueueEntryProcessor is set on the service, then it will be called to generate and/or populate any properties of the visitQueueEntry prior to final saving and completion of the transaction. An initial call to save happens before this is invoked in case this want to use generated data from the first save (for example, the primary key id). |
||
} | ||
return null; | ||
return dao.createOrUpdate(visitQueueEntry); | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
/* | ||
* This Source Code Form is subject to the terms of the Mozilla Public License, | ||
* v. 2.0. If a copy of the MPL was not distributed with this file, You can | ||
* obtain one at http://mozilla.org/MPL/2.0/. OpenMRS is also distributed under | ||
* the terms of the Healthcare Disclaimer located at http://openmrs.org/license. | ||
* | ||
* Copyright (C) OpenMRS Inc. OpenMRS is a registered trademark and the OpenMRS | ||
* graphic logo is a trademark of OpenMRS Inc. | ||
*/ | ||
package org.openmrs.module.queue.processor; | ||
|
||
import lombok.extern.slf4j.Slf4j; | ||
import org.apache.commons.lang.StringUtils; | ||
import org.openmrs.module.queue.model.QueueEntry; | ||
import org.openmrs.module.queue.model.VisitQueueEntry; | ||
import org.springframework.stereotype.Component; | ||
|
||
/** | ||
* Basic implementation of a VisitQueueEntryProcessor which simply returns the primary kye of the | ||
* VisitQueueEntry as a string. | ||
*/ | ||
@Slf4j | ||
@Component | ||
public class BasicPatientQueueNumberGenerator implements VisitQueueEntryProcessor { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I originally had a different implementation here that was a bit more complex, but in the end I decided to make this as basic as possible. This may mean this is never appropriate for use in any real implementation, but it does serve to provide a default implementation and an example of how one would create a custom generator. |
||
|
||
/** | ||
* This populates the given VisitQueueEntry with an appropriate patient queue number | ||
*/ | ||
public void beforeSaveVisitQueueEntry(VisitQueueEntry visitQueueEntry) { | ||
QueueEntry queueEntry = visitQueueEntry.getQueueEntry(); | ||
if (StringUtils.isBlank(queueEntry.getPatientQueueNumber())) { | ||
queueEntry.setPatientQueueNumber(Integer.toString(visitQueueEntry.getId())); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic here (combined with the logic in the service below) has essentially been moved to the VisitAttributeQueueNumberProcessor. Implementations that want to retain this behavior can enable that processor by setting it on the service from a downstream module. That said, if we don't feel this is something we need/want to keep in the main codebase, I'd be supportive of moving the entire VisitAttributeQueueNumberProcessor over to a Palladium module, and not having to support that within the module.
Note, the code relocated to the processor is not exactly the same. It does not hit the DAO, but rather just iterates over the entries in the queue to achieve the same result. This may need to be optimized for speed, but didn't want to do that pre-emptively if this ultimately gets moved out of the module.
It was never really clear to me what purpose the location had here, given that queue has a location on it. If there needs to be different location than that of the queue or the visit, this will need to be looked at. That said, there are some potential issues in this method as documented in O3-2448, so it likely needs a review overally