Skip to content

Commit

Permalink
O3-2904 Queue Module - queue should not have Queue Entries as a direc… (
Browse files Browse the repository at this point in the history 
#59)

* O3-2904 Queue Module - queue should not have Queue Entries as a direct property
* Limit log noise when there is a validation exception when the auto close visit queue entry task runs
  • Loading branch information
@mseaton
mseaton authored Feb 27, 2024
1 parent 95c46d5 commit 0218b94
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 128 deletions.
21 changes: 2 additions & 19 deletions api/src/main/java/org/openmrs/module/queue/exception/DuplicateQueueEntryException.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,34 +9,17 @@
*/
package org.openmrs.module.queue.exception;

import org.openmrs.api.APIException;
import org.openmrs.api.ValidationException;

public class DuplicateQueueEntryException extends APIException {
public class DuplicateQueueEntryException extends ValidationException {

private static final long serialVersionUID = 1L;

public DuplicateQueueEntryException() {
}

/**
* @param message
*/
public DuplicateQueueEntryException(String message) {
super(message);
}

/**
* @param message
* @param cause
*/
public DuplicateQueueEntryException(String message, Throwable cause) {
super(message, cause);
}

/**
* @param cause
*/
public DuplicateQueueEntryException(Throwable cause) {
super(cause);
}
}
34 changes: 2 additions & 32 deletions api/src/main/java/org/openmrs/module/queue/model/Queue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,19 @@
import javax.persistence.Table;

import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.stream.Collectors;

import lombok.Data;
import lombok.EqualsAndHashCode;
import lombok.ToString;
import org.apache.commons.lang.BooleanUtils;
import org.openmrs.BaseChangeableOpenmrsMetadata;
import org.openmrs.Concept;
import org.openmrs.Location;

@EqualsAndHashCode(callSuper = true)
@ToString(callSuper = true)
@Data
@Entity
@Table(name = "queue")
Expand Down Expand Up @@ -62,30 +63,9 @@ public class Queue extends BaseChangeableOpenmrsMetadata {
@JoinColumn(name = "status_concept_set", referencedColumnName = "concept_id")
private Concept statusConceptSet;

@OneToMany(mappedBy = "queue", cascade = CascadeType.ALL, fetch = FetchType.LAZY)
private List<QueueEntry> queueEntries;

@OneToMany(mappedBy = "queue", cascade = CascadeType.ALL, fetch = FetchType.LAZY)
private List<QueueRoom> queueRooms;

/**
* @return all non-voided QueueEntries that do not start in the future and are not already ended
*/
public List<QueueEntry> getActiveQueueEntries() {
List<QueueEntry> activeQueueEntries = new ArrayList<>();
Date now = new Date();
if (queueEntries != null) {
for (QueueEntry queueEntry : queueEntries) {
if (BooleanUtils.isNotTrue(queueEntry.getVoided())) {
if (!queueEntry.getStartedAt().after(now) && queueEntry.getEndedAt() == null) {
activeQueueEntries.add(queueEntry);
}
}
}
}
return activeQueueEntries;
}

/**
* @return all non-retired QueueRooms
*/
Expand All @@ -96,16 +76,6 @@ public List<QueueRoom> getActiveQueueRooms() {
return queueRooms.stream().filter(r -> BooleanUtils.isNotTrue(r.getRetired())).collect(Collectors.toList());
}

/**
* @param queueEntry the QueueEntry to add
*/
public void addQueueEntry(QueueEntry queueEntry) {
if (queueEntries == null) {
queueEntries = new ArrayList<>();
}
queueEntries.add(queueEntry);
}

/**
* @param queueRoom the QueueRoom to add
*/
Expand Down
4 changes: 4 additions & 0 deletions api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import lombok.extern.slf4j.Slf4j;
import org.openmrs.Visit;
import org.openmrs.api.ValidationException;
import org.openmrs.api.context.Context;
import org.openmrs.module.queue.api.QueueEntryService;
import org.openmrs.module.queue.api.search.QueueEntrySearchCriteria;
Expand Down Expand Up @@ -52,6 +53,9 @@ public void run() {
log.info("Queue entry auto-closed following close of visit: " + queueEntry.getQueueEntryId());
}
}
catch (ValidationException ve) {
log.warn("Unable to auto-close queue entry " + queueEntry.getQueueEntryId() + ": " + ve.getMessage());
}
catch (Exception e) {
log.warn("Unable to auto-close queue entry " + queueEntry.getQueueEntryId(), e);
}
Expand Down
73 changes: 1 addition & 72 deletions api/src/test/java/org/openmrs/module/queue/model/QueueTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,30 +10,16 @@
package org.openmrs.module.queue.model;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.*;

import java.util.Date;

import org.apache.commons.lang.time.DateUtils;
import org.junit.Test;

public class QueueTest {

Date now = new Date();

@Test
public void addQueueEntry_shouldAddANewQueueEntryEvenIfQueueEntriesIsNull() {
Queue queue = new Queue();
queue.setQueueEntries(null);
QueueEntry queueEntry = new QueueEntry();
queue.addQueueEntry(queueEntry);
assertThat(queue.getQueueEntries().size(), equalTo(1));
}

@Test
public void addQueueRoom_shouldAddANewQueueRoomEvenIfQueueRoomsIsNull() {
Queue queue = new Queue();
Expand All @@ -43,63 +29,6 @@ public void addQueueRoom_shouldAddANewQueueRoomEvenIfQueueRoomsIsNull() {
assertThat(queue.getQueueRooms().size(), equalTo(1));
}

@Test
public void getActiveQueueEntries_shouldReturnAnEmptyListIfNoQueueEntriesExist() {
Queue queue = new Queue();
queue.setQueueEntries(null);
assertThat(queue.getQueueEntries(), nullValue());
assertThat(queue.getActiveQueueEntries(), notNullValue());
assertThat(queue.getActiveQueueEntries().size(), equalTo(0));
}

@Test
public void getActiveQueueEntries_shouldReturnOnlyNonVoidedEntries() {
Queue queue = new Queue();

QueueEntry queueEntry = new QueueEntry();
queueEntry.setStartedAt(DateUtils.addHours(now, -1));
queueEntry.setEndedAt(null);
queue.addQueueEntry(queueEntry);

assertThat(queue.getQueueEntries(), contains(queueEntry));
assertThat(queue.getActiveQueueEntries(), contains(queueEntry));
queueEntry.setVoided(true);
assertThat(queue.getQueueEntries(), contains(queueEntry));
assertThat(queue.getActiveQueueEntries(), not(contains(queueEntry)));
}

@Test
public void getActiveQueueEntries_shouldNotReturnFutureQueueEntries() {
Queue queue = new Queue();

QueueEntry queueEntry = new QueueEntry();
queueEntry.setStartedAt(DateUtils.addHours(now, -1));
queueEntry.setEndedAt(null);
queue.addQueueEntry(queueEntry);

assertThat(queue.getQueueEntries(), contains(queueEntry));
assertThat(queue.getActiveQueueEntries(), contains(queueEntry));
queueEntry.setStartedAt(DateUtils.addHours(now, 1));
assertThat(queue.getQueueEntries(), contains(queueEntry));
assertThat(queue.getActiveQueueEntries(), not(contains(queueEntry)));
}

@Test
public void getActiveQueueEntries_shouldNotReturnEndedQueueEntries() {
Queue queue = new Queue();

QueueEntry queueEntry = new QueueEntry();
queueEntry.setStartedAt(DateUtils.addHours(now, -2));
queueEntry.setEndedAt(null);
queue.addQueueEntry(queueEntry);

assertThat(queue.getQueueEntries(), contains(queueEntry));
assertThat(queue.getActiveQueueEntries(), contains(queueEntry));
queueEntry.setEndedAt(DateUtils.addHours(now, 1));
assertThat(queue.getQueueEntries(), contains(queueEntry));
assertThat(queue.getActiveQueueEntries(), not(contains(queueEntry)));
}

@Test
public void getActiveQueueRooms_shouldNotReturnRetiredQueueRooms() {
Queue queue = new Queue();
Expand Down
10 changes: 6 additions & 4 deletions integration-tests/src/test/java/org/openmrs/module/queue/api/dao/QueueDaoTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ public class QueueDaoTest extends BaseModuleContextSensitiveTest {

private static final String RETIRED_QUEUE_UUID = "4eb8fe43-2813-4kbc-80dc-2e5d30252bb9";

public static final String EMPTY_QUEUE_UUID = "5ob8gj90-9090-4kbc-80dc-2e5d30252bb3";

private static final String NEW_QUEUE_UUID = "45b9fe43-2813-4kbc-80dc-2e5d30290iik";

private static final String NEW_QUEUE_NAME = "Test triage queue";
Expand Down Expand Up @@ -142,15 +144,15 @@ public void shouldFindAllQueuesIncludingRetired() {

@Test
public void shouldDeleteQueueByUuid() {
dao.delete(QUEUE_UUID);
Optional<Queue> result = dao.get(QUEUE_UUID);
dao.delete(EMPTY_QUEUE_UUID);
Optional<Queue> result = dao.get(EMPTY_QUEUE_UUID);
assertThat(result.isPresent(), is(false));
}

@Test
public void shouldDeleteQueueByEntity() {
dao.get(QUEUE_UUID).ifPresent((queue) -> dao.delete(queue));
Optional<Queue> result = dao.get(QUEUE_UUID);
dao.get(EMPTY_QUEUE_UUID).ifPresent((queue) -> dao.delete(queue));
Optional<Queue> result = dao.get(EMPTY_QUEUE_UUID);
assertThat(result.isPresent(), is(false));
}

Expand Down
7 changes: 6 additions & 1 deletion omod/src/main/java/org/openmrs/module/queue/web/resources/QueueEntrySubResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Optional;

import org.openmrs.api.context.Context;
import org.openmrs.module.queue.api.QueueServicesWrapper;
import org.openmrs.module.queue.api.search.QueueEntrySearchCriteria;
import org.openmrs.module.queue.model.Queue;
import org.openmrs.module.queue.model.QueueEntry;
import org.openmrs.module.webservices.rest.web.RequestContext;
Expand Down Expand Up @@ -63,7 +65,10 @@ public void setParent(QueueEntry queueEntry, Queue queue) {

@Override
public PageableResult doGetAll(Queue queue, RequestContext requestContext) throws ResponseException {
Collection<QueueEntry> queueEntries = queue.getActiveQueueEntries();
QueueEntrySearchCriteria criteria = new QueueEntrySearchCriteria();
criteria.setQueues(Collections.singletonList(queue));
criteria.setIsEnded(false);
Collection<QueueEntry> queueEntries = services.getQueueEntryService().getQueueEntries(criteria);
return new NeedsPaging<>(new ArrayList<>(queueEntries), requestContext);
}

Expand Down

0 comments on commit 0218b94

Please sign in to comment.