diff --git a/api/src/main/java/org/openmrs/module/pihapps/PihAppsService.java b/api/src/main/java/org/openmrs/module/pihapps/PihAppsService.java index 72bc1e0..76d5bfa 100644 --- a/api/src/main/java/org/openmrs/module/pihapps/PihAppsService.java +++ b/api/src/main/java/org/openmrs/module/pihapps/PihAppsService.java @@ -18,13 +18,17 @@ import org.openmrs.Location; import org.openmrs.Obs; import org.openmrs.Order; +import org.openmrs.annotation.Authorized; import org.openmrs.api.OpenmrsService; +import org.openmrs.module.pihapps.encounter.EncounterSearchCriteria; +import org.openmrs.module.pihapps.encounter.EncounterSearchResult; import org.openmrs.module.pihapps.obs.ObsSearchCriteria; import org.openmrs.module.pihapps.obs.ObsSearchResult; import org.openmrs.module.pihapps.orders.EncounterFulfillingOrders; import org.openmrs.module.pihapps.orders.OrderSearchCriteria; import org.openmrs.module.pihapps.orders.OrderSearchResult; import org.openmrs.module.pihapps.orders.PatientWithOrdersSearchResult; +import org.openmrs.util.PrivilegeConstants; import java.util.List; import java.util.Map; @@ -51,5 +55,47 @@ public interface PihAppsService extends OpenmrsService { void revertOrdersToOrdered(List orders); + /** + * Searches observations by whatever {@link ObsSearchCriteria} names: the patient, the concepts, + * the users in their audit trail, and a date range over either the observation's own datetime or + * the audit action. Every filter narrows, and a criteria naming none of them matches every + * observation, so a caller that means to search rather than to list is responsible for + * narrowing it. + * + *

Voided observations are left out unless the criteria ask for them. An audit does ask: they + * are the whole point of a voidedBy search, and what an auditor looking at what a user created + * most wants to see. + * + *

Ordering is the caller's to set, and paging without one is not deterministic. An audit + * wants the most recent audit action first, which means ordering by the column belonging to the + * action it named rather than by the observation's own datetime, with the obs id breaking ties + * so that paging cannot repeat or skip a row. + * + * @param searchCriteria what to search for, how to page it and how to sort it + * @return the matching observations and how many there are in total + */ + @Authorized(PrivilegeConstants.GET_OBS) ObsSearchResult getObs(ObsSearchCriteria searchCriteria); + + /** + * Searches encounters by whatever {@link EncounterSearchCriteria} names: the users in their + * audit trail, the provider recorded on them, their type, and a date range. Every filter + * narrows, and a criteria naming none of them matches every encounter, so a caller that means + * to search rather than to list is responsible for narrowing it. + * + *

Voided encounters are left out unless the criteria ask for them. An audit does ask: they + * are the whole point of a voidedBy search, and what an auditor looking at what a user entered + * most wants to see. + * + *

Ordering is the caller's to set, as it is on {@link #getObs(ObsSearchCriteria)}, + * and paging without one is not deterministic. An audit wants the most recent audit action + * first, which means ordering by the column belonging to the action it named — or by the + * encounter's own datetime where only a provider was named — with the encounter id breaking + * ties so that paging cannot repeat or skip a row. + * + * @param searchCriteria what to search for, how to page it and how to sort it + * @return the matching encounters and how many there are in total + */ + @Authorized(PrivilegeConstants.GET_ENCOUNTERS) + EncounterSearchResult getEncounters(EncounterSearchCriteria searchCriteria); } diff --git a/api/src/main/java/org/openmrs/module/pihapps/PihAppsServiceImpl.java b/api/src/main/java/org/openmrs/module/pihapps/PihAppsServiceImpl.java index 125d0a2..c5e0bca 100644 --- a/api/src/main/java/org/openmrs/module/pihapps/PihAppsServiceImpl.java +++ b/api/src/main/java/org/openmrs/module/pihapps/PihAppsServiceImpl.java @@ -21,14 +21,20 @@ import org.hibernate.Criteria; import org.hibernate.FlushMode; import org.hibernate.criterion.Criterion; +import org.hibernate.criterion.DetachedCriteria; import org.hibernate.criterion.Projections; +import org.hibernate.criterion.Subqueries; import org.openmrs.Concept; import org.openmrs.Encounter; +import org.openmrs.EncounterProvider; +import org.openmrs.EncounterType; import org.openmrs.Location; import org.openmrs.LocationTag; import org.openmrs.Obs; import org.openmrs.Order; import org.openmrs.Patient; +import org.openmrs.Provider; +import org.openmrs.User; import org.openmrs.annotation.Authorized; import org.openmrs.api.EncounterService; import org.openmrs.api.LocationService; @@ -37,6 +43,8 @@ import org.openmrs.api.db.hibernate.DbSessionFactory; import org.openmrs.api.impl.BaseOpenmrsService; import org.openmrs.module.emrapi.EmrApiConstants; +import org.openmrs.module.pihapps.encounter.EncounterSearchCriteria; +import org.openmrs.module.pihapps.encounter.EncounterSearchResult; import org.openmrs.module.pihapps.obs.ObsSearchCriteria; import org.openmrs.module.pihapps.obs.ObsSearchResult; import org.openmrs.module.pihapps.orders.EncounterFulfillingOrders; @@ -576,9 +584,10 @@ public void revertOrdersToOrdered(List orders) { } } + @Override @Transactional(readOnly = true) - @Authorized(PrivilegeConstants.GET_PATIENTS) + @Authorized(PrivilegeConstants.GET_OBS) @SuppressWarnings({ "unchecked" }) public ObsSearchResult getObs(ObsSearchCriteria searchCriteria) { ObsSearchResult result = new ObsSearchResult(); @@ -605,7 +614,18 @@ public ObsSearchResult getObs(ObsSearchCriteria searchCriteria) { @SuppressWarnings({ "deprecation" }) private Criteria createHibernateObsSearchCriteria(ObsSearchCriteria searchCriteria, boolean applySortCriteria) { Criteria c = sessionFactory.getHibernateSessionFactory().getCurrentSession().createCriteria(Obs.class); - c.add(eq("voided", false)); + if (!searchCriteria.isIncludeVoided()) { + c.add(eq("voided", false)); + } + if (searchCriteria.getCreatedBy() != null) { + c.add(eq("creator", searchCriteria.getCreatedBy())); + } + if (searchCriteria.getVoidedBy() != null) { + c.add(eq("voidedBy", searchCriteria.getVoidedBy())); + } + // Each range names the column it bounds, so none of this depends on which filters are set. + addDateBounds(c, "dateCreated", searchCriteria.getCreatedOnOrAfter(), searchCriteria.getCreatedOnOrBefore()); + addDateBounds(c, "dateVoided", searchCriteria.getVoidedOnOrAfter(), searchCriteria.getVoidedOnOrBefore()); if (searchCriteria.getPatient() != null) { c.add(eq("person", searchCriteria.getPatient())); } @@ -634,4 +654,93 @@ private Criteria createHibernateObsSearchCriteria(ObsSearchCriteria searchCriter } return c; } + + @Override + @Transactional(readOnly = true) + @Authorized(PrivilegeConstants.GET_ENCOUNTERS) + @SuppressWarnings({ "unchecked" }) + public EncounterSearchResult getEncounters(EncounterSearchCriteria searchCriteria) { + EncounterSearchResult result = new EncounterSearchResult(); + // First query to get total count + Criteria c = createHibernateEncounterSearchCriteria(searchCriteria, false); + c.setProjection(Projections.rowCount()); + Long totalCount = (Long) c.list().get(0); + result.setTotalCount(totalCount); + // Then query to get page of results + c = createHibernateEncounterSearchCriteria(searchCriteria, true); + c.setProjection(null); + Integer startIndex = searchCriteria.getStartIndex(); + Integer limit = searchCriteria.getLimit(); + if (limit != null) { + startIndex = startIndex == null ? 0 : startIndex; + c.setFirstResult(startIndex); + c.setMaxResults(limit); + } + result.setEncounters(c.list()); + return result; + } + + /** + * Both ends run inclusively. The upper end is widened to the end of its day when it carries no + * time, so that a range named in days covers the whole of the last one. + */ + private void addDateBounds(Criteria c, String property, Date onOrAfter, Date onOrBefore) { + if (onOrAfter != null) { + // No adjustment: midnight is already the first moment of its day. + c.add(ge(property, onOrAfter)); + } + Date upperBound = PihAppsUtils.getEndOfDayIfTimeExcluded(onOrBefore); + if (upperBound != null) { + c.add(le(property, upperBound)); + } + } + + @SuppressWarnings({ "deprecation" }) + private Criteria createHibernateEncounterSearchCriteria(EncounterSearchCriteria searchCriteria, + boolean applySortCriteria) { + Criteria c = sessionFactory.getHibernateSessionFactory().getCurrentSession().createCriteria(Encounter.class); + if (!searchCriteria.isIncludeVoided()) { + c.add(eq("voided", false)); + } + if (searchCriteria.getEncounterType() != null) { + c.add(eq("encounterType", searchCriteria.getEncounterType())); + } + if (searchCriteria.getCreatedBy() != null) { + c.add(eq("creator", searchCriteria.getCreatedBy())); + } + if (searchCriteria.getChangedBy() != null) { + c.add(eq("changedBy", searchCriteria.getChangedBy())); + } + if (searchCriteria.getVoidedBy() != null) { + c.add(eq("voidedBy", searchCriteria.getVoidedBy())); + } + // Each range names the column it bounds, so none of this depends on which filters are set. + addDateBounds(c, "dateCreated", searchCriteria.getCreatedOnOrAfter(), searchCriteria.getCreatedOnOrBefore()); + addDateBounds(c, "dateChanged", searchCriteria.getChangedOnOrAfter(), searchCriteria.getChangedOnOrBefore()); + addDateBounds(c, "dateVoided", searchCriteria.getVoidedOnOrAfter(), searchCriteria.getVoidedOnOrBefore()); + addDateBounds(c, "encounterDatetime", searchCriteria.getEncounterDatetimeOnOrAfter(), + searchCriteria.getEncounterDatetimeOnOrBefore()); + if (searchCriteria.getProvider() != null) { + // A subquery rather than a join, so that an encounter naming the provider more than + // once is still returned once — a join would need a distinct, and an in-memory distinct + // would be applied after paging had already counted the duplicate rows. + DetachedCriteria encountersNamingProvider = DetachedCriteria.forClass(EncounterProvider.class, "ep") + .createAlias("ep.encounter", "providerEncounter") + .setProjection(Projections.property("providerEncounter.encounterId")) + .add(eq("ep.provider", searchCriteria.getProvider())) + .add(eq("ep.voided", false)); + c.add(Subqueries.propertyIn("encounterId", encountersNamingProvider)); + } + if (applySortCriteria && searchCriteria.getSortCriteria() != null) { + for (SortCriteria sortCriteria : searchCriteria.getSortCriteria()) { + if (sortCriteria.getDirection() == SortCriteria.Direction.DESC) { + c.addOrder(desc(sortCriteria.getField())); + } else { + c.addOrder(asc(sortCriteria.getField())); + } + } + } + return c; + } + } diff --git a/api/src/main/java/org/openmrs/module/pihapps/PihAppsUtils.java b/api/src/main/java/org/openmrs/module/pihapps/PihAppsUtils.java index a9a9285..0e5967d 100644 --- a/api/src/main/java/org/openmrs/module/pihapps/PihAppsUtils.java +++ b/api/src/main/java/org/openmrs/module/pihapps/PihAppsUtils.java @@ -9,6 +9,8 @@ import org.springframework.stereotype.Component; import java.util.ArrayDeque; +import java.util.Calendar; +import java.util.Date; import java.util.Deque; import java.util.HashSet; import java.util.Locale; @@ -101,6 +103,39 @@ else if (isShort && isEnglish) { * @param root * @return a set of Concepts that are recursive set members of root */ + /** + * The last moment of a date's day if that date carries no time of day, and the date itself + * otherwise. Modelled on the reporting module's {@code DateUtil.getEndOfDayIfTimeExcluded}. + * + *

This is for the upper end of an inclusive range. Someone who names a day means the whole + * of it, so a bound of `2026-09-30` has to reach 23:59:59.999 or everything recorded after + * midnight on the 30th falls outside a range that plainly includes the 30th. A lower bound + * needs no such adjustment: midnight is already the first moment of its day. + * + *

A time of exactly midnight is read as no time of day, since a Date cannot say whether the + * caller wrote `2026-09-30` or `2026-09-30T00:00:00`. A caller that means that first instant + * and nothing more should bound the range a moment earlier. + * + * @param date the upper bound as given, or null for no bound + * @return the bound to search on, or null if none was given + */ + public static Date getEndOfDayIfTimeExcluded(Date date) { + if (date == null) { + return null; + } + Calendar calendar = Calendar.getInstance(); + calendar.setTime(date); + if (calendar.get(Calendar.HOUR_OF_DAY) != 0 || calendar.get(Calendar.MINUTE) != 0 + || calendar.get(Calendar.SECOND) != 0 || calendar.get(Calendar.MILLISECOND) != 0) { + return date; + } + calendar.set(Calendar.HOUR_OF_DAY, 23); + calendar.set(Calendar.MINUTE, 59); + calendar.set(Calendar.SECOND, 59); + calendar.set(Calendar.MILLISECOND, 999); + return calendar.getTime(); + } + public static Set getConceptHierarchy(Concept root) { Set result = new HashSet<>(); Set visited = new HashSet<>(); diff --git a/api/src/main/java/org/openmrs/module/pihapps/encounter/EncounterSearchCriteria.java b/api/src/main/java/org/openmrs/module/pihapps/encounter/EncounterSearchCriteria.java new file mode 100644 index 0000000..ef9ed15 --- /dev/null +++ b/api/src/main/java/org/openmrs/module/pihapps/encounter/EncounterSearchCriteria.java @@ -0,0 +1,87 @@ +package org.openmrs.module.pihapps.encounter; + +import lombok.Data; +import org.openmrs.EncounterType; +import org.openmrs.Provider; +import org.openmrs.User; +import org.openmrs.module.pihapps.SortCriteria; + +import java.util.Date; +import java.util.List; + +/** + * Describes an encounter search. Core's own {@link org.openmrs.parameter.EncounterSearchCriteria} + * carries a providers field that no search handler exposes, and has no creator, changedBy or + * voidedBy field at all, which is why this exists rather than the module reusing it. + * + *

Every filter narrows, so naming several asks for the encounters satisfying all of them, and + * naming none matches every encounter. + */ +@Data +public class EncounterSearchCriteria { + + /** + * Whether voided encounters are returned alongside the surviving ones. Off by default, since a + * search is normally asking what a record says now, and callers can tell the two apart by each + * encounter's voided flag. + * + *

An audit search turns this on: voided encounters are the whole point of a + * {@link #voidedBy} search, and what an auditor most wants to see in the others. + */ + private boolean includeVoided = false; + + /** Restrict to encounters this user created. */ + private User createdBy; + + /** Restrict to encounters this user changed. */ + private User changedBy; + + /** Restrict to encounters this user voided. */ + private User voidedBy; + + /** Restrict to encounters this provider is recorded on. */ + private Provider provider; + + /** Restrict to encounters of this type. */ + private EncounterType encounterType; + + /** + * Bound when the encounter was created. Each of the four ranges below names the column it + * applies to, and each is independent of the filters: `createdOnOrAfter` narrows by creation + * date whether or not `createdBy` is also given, and naming several asks for all of them. + * + *

Which range a search wants is the caller's to decide. An audit of what a user entered + * wants the range against that user's action — an encounter backdated to last year but entered + * this morning was entered this morning — while a provider's caseload is asked about by + * {@link #encounterDatetimeOnOrAfter}, when the encounters actually happened. + * + *

Both ends of every range run inclusively and are applied as given, so a caller that means + * a whole day passes that day's last moment. + */ + private Date createdOnOrAfter; + + /** @see #createdOnOrAfter */ + private Date createdOnOrBefore; + + /** Bound when the encounter was last changed. @see #createdOnOrAfter */ + private Date changedOnOrAfter; + + /** @see #createdOnOrAfter */ + private Date changedOnOrBefore; + + /** Bound when the encounter was voided. @see #createdOnOrAfter */ + private Date voidedOnOrAfter; + + /** @see #createdOnOrAfter */ + private Date voidedOnOrBefore; + + /** Bound the encounter's own datetime — when it happened. @see #createdOnOrAfter */ + private Date encounterDatetimeOnOrAfter; + + /** @see #createdOnOrAfter */ + private Date encounterDatetimeOnOrBefore; + + private List sortCriteria; + private Integer startIndex; + private Integer limit; +} diff --git a/api/src/main/java/org/openmrs/module/pihapps/encounter/EncounterSearchResult.java b/api/src/main/java/org/openmrs/module/pihapps/encounter/EncounterSearchResult.java new file mode 100644 index 0000000..eadb1b6 --- /dev/null +++ b/api/src/main/java/org/openmrs/module/pihapps/encounter/EncounterSearchResult.java @@ -0,0 +1,12 @@ +package org.openmrs.module.pihapps.encounter; + +import lombok.Data; +import org.openmrs.Encounter; + +import java.util.List; + +@Data +public class EncounterSearchResult { + Long totalCount; + List encounters; +} diff --git a/api/src/main/java/org/openmrs/module/pihapps/obs/ObsSearchCriteria.java b/api/src/main/java/org/openmrs/module/pihapps/obs/ObsSearchCriteria.java index 12b32a6..1826bc2 100644 --- a/api/src/main/java/org/openmrs/module/pihapps/obs/ObsSearchCriteria.java +++ b/api/src/main/java/org/openmrs/module/pihapps/obs/ObsSearchCriteria.java @@ -3,6 +3,7 @@ import lombok.Data; import org.openmrs.Concept; import org.openmrs.Patient; +import org.openmrs.User; import org.openmrs.module.pihapps.SortCriteria; import java.util.Date; @@ -14,6 +15,47 @@ public class ObsSearchCriteria { private List concepts; private Date onOrBefore; private Date onOrAfter; + + /** + * Whether voided observations are returned alongside the surviving ones. Off by default, since + * a search is normally asking what a record says now, and callers can tell the two apart by + * each observation's voided flag. + * + *

An audit search turns this on: voided observations are the whole point of a + * {@link #voidedBy} search, and what an auditor most wants to see in a {@link #createdBy} one. + */ + private boolean includeVoided = false; + + /** Restrict to observations this user created. */ + private User createdBy; + + /** Restrict to observations this user voided. See {@link #createdBy}. */ + private User voidedBy; + + /** + * Bound when the observation was created. Each of the ranges below names the column it applies + * to, and each is independent of the filters: `createdOnOrAfter` narrows by creation date + * whether or not {@link #createdBy} is also given, and naming several asks for all of them. + * + *

Which range a search wants is the caller's to decide. An audit bounds the action it is + * about rather than the observation's own datetime — an observation backdated to last year but + * entered this morning was entered this morning — while {@link #onOrAfter} and + * {@link #onOrBefore} bound obsDatetime, when the observation says it was taken. + * + *

Both ends of every range run inclusively. An upper end carrying no time of day is read as + * the whole of that day. + */ + private Date createdOnOrAfter; + + /** @see #createdOnOrAfter */ + private Date createdOnOrBefore; + + /** Bound when the observation was voided. @see #createdOnOrAfter */ + private Date voidedOnOrAfter; + + /** @see #createdOnOrAfter */ + private Date voidedOnOrBefore; + private List sortCriteria; private Integer startIndex; private Integer limit; diff --git a/api/src/test/java/org/openmrs/module/pihapps/PihAppsEncounterSearchTest.java b/api/src/test/java/org/openmrs/module/pihapps/PihAppsEncounterSearchTest.java new file mode 100644 index 0000000..1a4097a --- /dev/null +++ b/api/src/test/java/org/openmrs/module/pihapps/PihAppsEncounterSearchTest.java @@ -0,0 +1,366 @@ +package org.openmrs.module.pihapps; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.openmrs.Encounter; +import org.openmrs.EncounterType; +import org.openmrs.Provider; +import org.openmrs.User; +import org.openmrs.api.context.Context; +import org.openmrs.module.pihapps.SortCriteria; +import org.openmrs.module.pihapps.encounter.EncounterSearchCriteria; +import org.openmrs.module.pihapps.encounter.EncounterSearchResult; +import org.openmrs.test.jupiter.BaseModuleContextSensitiveTest; + +import java.util.ArrayList; +import java.util.Calendar; +import java.util.Collections; +import java.util.Date; +import java.util.List; +import java.util.stream.Collectors; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasItems; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.lessThan; +import static org.hamcrest.Matchers.not; + +/** + * Covers searching encounters by the users in their audit trail, by the provider recorded on them, + * and by encounter type and date. The fixture's audit dates run in a different order from its + * encounter datetimes, so an implementation that bounded or ordered by the wrong column would fail + * here rather than look plausible. + */ +public class PihAppsEncounterSearchTest extends BaseModuleContextSensitiveTest { + + private PihAppsService service; + + private User bruno; + + private User butch; + + private Provider provider; + + private EncounterType typeOne; + + @BeforeEach + public void setup() { + executeDataSet("encounterAuditTestDataset.xml"); + service = Context.getService(PihAppsService.class); + bruno = Context.getUserService().getUser(501); + butch = Context.getUserService().getUser(502); + provider = Context.getProviderService().getProvider(1); + typeOne = Context.getEncounterService().getEncounterType(1); + } + + /** + * The search under test. Building the criteria in one place means a test cannot silently set + * the wrong field, and keeps each case reading as the question it is asking. + */ + private EncounterSearchResult searchResult(User createdBy, User changedBy, User voidedBy, Provider byProvider, + EncounterType encounterType, Date fromDate, Date toDate, Integer startIndex, Integer limit) { + EncounterSearchCriteria searchCriteria = new EncounterSearchCriteria(); + searchCriteria.setCreatedBy(createdBy); + searchCriteria.setChangedBy(changedBy); + searchCriteria.setVoidedBy(voidedBy); + searchCriteria.setProvider(byProvider); + searchCriteria.setEncounterType(encounterType); + // The criteria name the column each range bounds, so the test picks the one the case is + // about — the same choice a client makes. Each case names at most one action. + if (voidedBy != null) { + searchCriteria.setVoidedOnOrAfter(fromDate); + searchCriteria.setVoidedOnOrBefore(toDate); + } + else if (changedBy != null) { + searchCriteria.setChangedOnOrAfter(fromDate); + searchCriteria.setChangedOnOrBefore(toDate); + } + else if (createdBy != null) { + searchCriteria.setCreatedOnOrAfter(fromDate); + searchCriteria.setCreatedOnOrBefore(toDate); + } + else { + searchCriteria.setEncounterDatetimeOnOrAfter(fromDate); + searchCriteria.setEncounterDatetimeOnOrBefore(toDate); + } + searchCriteria.setIncludeVoided(true); + searchCriteria.setSortCriteria(auditSortCriteria(createdBy, changedBy, voidedBy)); + searchCriteria.setStartIndex(startIndex); + searchCriteria.setLimit(limit); + return service.getEncounters(searchCriteria); + } + + /** + * The ordering an audit asks for, which the endpoint sets rather than the service: the audit + * action the search named, most recent first, with the encounter id breaking ties. These tests + * assert on order, so they have to ask for the same one the endpoint does. + */ + private List auditSortCriteria(User createdBy, User changedBy, User voidedBy) { + String actionDate = "encounterDatetime"; + if (voidedBy != null) { + actionDate = "dateVoided"; + } + else if (changedBy != null) { + actionDate = "dateChanged"; + } + else if (createdBy != null) { + actionDate = "dateCreated"; + } + List sortCriteria = new ArrayList<>(); + sortCriteria.add(new SortCriteria(actionDate, SortCriteria.Direction.DESC)); + sortCriteria.add(new SortCriteria("encounterId", SortCriteria.Direction.DESC)); + return sortCriteria; + } + + /** The search under test, with the paging arguments left off. */ + private List search(User createdBy, User changedBy, User voidedBy, Provider byProvider, + EncounterType encounterType, Date fromDate, Date toDate) { + return searchResult(createdBy, changedBy, voidedBy, byProvider, encounterType, fromDate, toDate, null, null) + .getEncounters(); + } + + private Long count(User createdBy, User changedBy, User voidedBy, Provider byProvider, + EncounterType encounterType, Date fromDate, Date toDate) { + return searchResult(createdBy, changedBy, voidedBy, byProvider, encounterType, fromDate, toDate, null, null) + .getTotalCount(); + } + + private List encounterIds(List encounters) { + return encounters.stream().map(Encounter::getEncounterId).collect(Collectors.toList()); + } + + /** Only this fixture's encounters, so the standard test dataset's own rows do not interfere. */ + private List auditedIds(List encounters) { + return encounterIds(encounters).stream().filter(id -> id >= 3000).collect(Collectors.toList()); + } + + private Date day(int month, int dayOfMonth, int hourOfDay, int year) { + Calendar calendar = Calendar.getInstance(); + calendar.clear(); + calendar.set(year, month, dayOfMonth, hourOfDay, 0, 0); + return calendar.getTime(); + } + + private Date day(int month, int dayOfMonth, int hourOfDay) { + Calendar calendar = Calendar.getInstance(); + calendar.clear(); + calendar.set(2026, month, dayOfMonth, hourOfDay, 0, 0); + return calendar.getTime(); + } + + private Date september(int dayOfMonth) { + return day(Calendar.SEPTEMBER, dayOfMonth, 0); + } + + @Test + public void shouldFindEncountersCreatedByAUserMostRecentlyCreatedFirst() { + assertThat(auditedIds(search(bruno, null, null, null, null, null, null)), contains(3001, 3002)); + } + + @Test + public void shouldFindEncountersChangedByAUserMostRecentlyChangedFirst() { + // 3004 was changed on 4 Sep by bruno, 3002 on 3 Sep by butch + assertThat(auditedIds(search(null, bruno, null, null, null, null, null)), contains(3004)); + assertThat(auditedIds(search(null, butch, null, null, null, null, null)), contains(3002)); + } + + @Test + public void shouldFindEncountersVoidedByAUser() { + assertThat(auditedIds(search(null, null, butch, null, null, null, null)), contains(3003)); + } + + @Test + public void shouldIncludeVoidedEncountersWhenSearchingByCreator() { + List results = search(butch, null, null, null, null, null, null); + + assertThat(auditedIds(results), contains(3005, 3004, 3003)); + assertThat(results.stream().anyMatch(Encounter::getVoided), is(true)); + } + + @Test + public void shouldNarrowByEveryUserGiven() { + // 3004 was created by butch and changed by bruno + assertThat(auditedIds(search(butch, bruno, null, null, null, null, null)), contains(3004)); + assertThat(search(bruno, bruno, null, null, null, null, null), is(Collections.emptyList())); + } + + @Test + public void shouldFindEncountersByProvider() { + // 3005 names the provider too, but on a voided row, so it does not count + assertThat(auditedIds(search(null, null, null, provider, null, null, null)), contains(3004)); + } + + @Test + public void shouldNarrowByProviderAndUserTogether() { + assertThat(auditedIds(search(butch, null, null, provider, null, null, null)), contains(3004)); + assertThat(search(bruno, null, null, provider, null, null, null), is(Collections.emptyList())); + } + + @Test + public void shouldNarrowByEncounterType() { + // every encounter in the fixture is of type one except 3005 + assertThat(auditedIds(search(butch, null, null, null, typeOne, null, null)), contains(3004, 3003)); + } + + @Test + public void shouldNarrowByEncounterTypeAndProviderTogether() { + assertThat(auditedIds(search(null, null, null, provider, typeOne, null, null)), contains(3004)); + + // 3005 is of the other type, but its only provider row is voided, so it still does not match + // (the standard dataset has encounters of that type on this provider, hence the scoping) + EncounterType otherType = Context.getEncounterService().getEncounterType(2); + assertThat(auditedIds(search(null, null, null, provider, otherType, null, null)), is(Collections.emptyList())); + } + + @Test + public void shouldBoundEachNamedActionByItsOwnDateColumn() { + // bruno created 3001 on 1 Sep and 3002 on 25 Aug + assertThat(auditedIds(search(bruno, null, null, null, null, september(1), null)), contains(3001)); + // bruno changed 3004 on 4 Sep, so nothing of his was changed from 5 Sep onwards + assertThat(search(null, bruno, null, null, null, september(5), null), is(Collections.emptyList())); + } + + /** + * 3003's encounter_datetime is 20 Aug but it was voided on 5 Sep, so a range applied to the + * encounter's own datetime would miss it. + */ + @Test + public void shouldBoundByTheAuditActionRatherThanTheEncounterDatetime() { + assertThat(auditedIds(search(null, null, butch, null, null, september(1), null)), contains(3003)); + } + + /** + * A provider search names no audit action, so its range bounds the encounter's own datetime — + * what a provider's caseload is asked about, and the one thing core cannot filter by provider. + */ + @Test + public void shouldBoundAProviderSearchByTheEncounterDatetime() { + // encounter 3004 happened on 30 Aug and was entered on 2 Sep + assertThat(auditedIds(search(null, null, null, provider, null, day(Calendar.AUGUST, 1, 0), null)), + contains(3004)); + assertThat(search(null, null, null, provider, null, september(1), null), is(Collections.emptyList())); + } + + /** + * A search naming no audit action has no action column to bound, so the range falls back to the + * encounter's own datetime. Only a provider search used to reach that fallback, which meant a + * type-only or unfiltered search silently ignored the range it was given. + */ + @Test + public void shouldBoundASearchThatNamesNoAuditActionByTheEncounterDatetime() { + // every encounter in the fixture happened in 2026, so a 2099 range must exclude them all + Date from = day(Calendar.JANUARY, 1, 0, 2099); + Date to = day(Calendar.JANUARY, 2, 0, 2099); + + assertThat(auditedIds(search(null, null, null, null, typeOne, from, to)), is(Collections.emptyList())); + assertThat(auditedIds(search(null, null, null, null, null, from, to)), is(Collections.emptyList())); + assertThat(auditedIds(search(null, null, null, provider, null, from, to)), is(Collections.emptyList())); + } + + /** + * Each user filter bounds its own action's column, so a range alongside one must not also be + * applied to the encounter datetime — that would demand the encounter itself fall in the window + * as well, which is stricter than what was asked. + */ + @Test + public void shouldNotAlsoBoundTheEncounterDatetimeWhenAnActionIsNamed() { + // 3001 was entered on 1 Sep but happened on 1 Aug, so a September range finds it by creation + assertThat(auditedIds(search(bruno, null, null, null, null, day(Calendar.SEPTEMBER, 1, 0), + day(Calendar.SEPTEMBER, 1, 23))), contains(3001)); + } + + /** + * Each range names its own column, so they combine rather than one displacing another and none + * of them depends on a matching user filter being given. + */ + @Test + public void shouldApplyEachRangeToItsOwnColumnIndependently() { + EncounterSearchCriteria byCreationAndHappening = new EncounterSearchCriteria(); + byCreationAndHappening.setIncludeVoided(true); + // 3001 happened on 1 Aug and was entered on 1 Sep; 3002 happened on 1 Jul, entered 25 Aug + byCreationAndHappening.setCreatedOnOrAfter(day(Calendar.SEPTEMBER, 1, 0)); + byCreationAndHappening.setEncounterDatetimeOnOrBefore(day(Calendar.AUGUST, 15, 0)); + + assertThat(auditedIds(service.getEncounters(byCreationAndHappening).getEncounters()), contains(3001)); + + // a creation range with no createdBy filter still narrows, which the old single range could + // not express + EncounterSearchCriteria creationOnly = new EncounterSearchCriteria(); + creationOnly.setIncludeVoided(true); + creationOnly.setCreatedOnOrAfter(day(Calendar.JANUARY, 1, 0, 2099)); + + assertThat(auditedIds(service.getEncounters(creationOnly).getEncounters()), is(Collections.emptyList())); + } + + @Test + public void shouldTreatBothEndsOfTheRangeAsInclusive() { + assertThat(auditedIds(search(bruno, null, null, null, null, day(Calendar.SEPTEMBER, 1, 8), + day(Calendar.SEPTEMBER, 1, 8))), contains(3001)); + } + + /** + * The standard test dataset has encounters of its own attributed to these users and this + * provider, so the counts are asserted against the unpaged result rather than against a number + * this fixture alone would explain. + */ + @Test + public void shouldCountTheWholeResultSetRatherThanThePage() { + List byCreator = search(butch, null, null, null, null, null, null); + assertThat(count(butch, null, null, null, null, null, null), is((long) byCreator.size())); + + List byProvider = search(null, null, null, provider, null, null, null); + assertThat(count(null, null, null, provider, null, null, null), is((long) byProvider.size())); + } + + @Test + public void shouldCountWithinAFilteredSearch() { + List filtered = search(butch, null, null, null, typeOne, null, null); + + assertThat(count(butch, null, null, null, typeOne, null, null), is((long) filtered.size())); + } + + @Test + public void shouldPageThroughTheResults() { + // this fixture's encounters were entered in 2026, so they sort ahead of the standard ones + assertThat(auditedIds(searchResult(butch, null, null, null, null, null, null, 0, 2).getEncounters()), + contains(3005, 3004)); + assertThat(auditedIds(searchResult(butch, null, null, null, null, null, null, 2, 2).getEncounters()), + contains(3003)); + } + + /** + * The audit endpoint asks for voided encounters, so the helper above does too. Every other + * caller gets them left out, which is what this pins down. + */ + @Test + public void shouldLeaveOutVoidedEncountersUnlessAskedFor() { + // 3003 is this fixture's voided encounter, voided by butch on 5 Sep + EncounterSearchCriteria searchCriteria = new EncounterSearchCriteria(); + searchCriteria.setEncounterType(typeOne); + + assertThat(auditedIds(service.getEncounters(searchCriteria).getEncounters()), not(hasItem(3003))); + + searchCriteria.setIncludeVoided(true); + assertThat(auditedIds(service.getEncounters(searchCriteria).getEncounters()), hasItem(3003)); + } + + @Test + public void shouldSearchWithoutAnyAuditFilter() { + // The service places no audit-specific requirement on the criteria, so a search naming none + // of them is a plain encounter search. The audit endpoint asks for at least one itself. + List unfiltered = search(null, null, null, null, null, null, null); + + assertThat(auditedIds(unfiltered), hasItems(3001, 3002, 3003, 3004, 3005)); + assertThat(count(null, null, null, null, null, null, null), is((long) unfiltered.size())); + } + + @Test + public void shouldNarrowAnUnfilteredSearchByEncounterTypeAlone() { + List byType = search(null, null, null, null, typeOne, null, null); + + assertThat(byType.stream().allMatch(e -> e.getEncounterType().equals(typeOne)), is(true)); + assertThat(byType.size(), lessThan(search(null, null, null, null, null, null, null).size())); + } +} diff --git a/api/src/test/java/org/openmrs/module/pihapps/PihAppsObsSearchTest.java b/api/src/test/java/org/openmrs/module/pihapps/PihAppsObsSearchTest.java new file mode 100644 index 0000000..403115f --- /dev/null +++ b/api/src/test/java/org/openmrs/module/pihapps/PihAppsObsSearchTest.java @@ -0,0 +1,270 @@ +package org.openmrs.module.pihapps; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.openmrs.Obs; +import org.openmrs.User; +import org.openmrs.api.context.Context; +import org.openmrs.module.pihapps.obs.ObsSearchCriteria; +import org.openmrs.module.pihapps.SortCriteria; +import org.openmrs.module.pihapps.obs.ObsSearchResult; +import org.openmrs.test.jupiter.BaseModuleContextSensitiveTest; + +import java.util.ArrayList; +import java.util.Calendar; +import java.util.Date; +import java.util.List; +import java.util.stream.Collectors; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.hasItems; +import static org.hamcrest.Matchers.is; + +/** + * Covers searching observations by the user who created or voided them. The fixture's creation and + * voiding dates run in a different order from the observation datetimes, so an implementation that + * ordered by the wrong column would fail here rather than look plausible. + */ +public class PihAppsObsSearchTest extends BaseModuleContextSensitiveTest { + + private PihAppsService service; + + private User bruno; + + private User butch; + + @BeforeEach + public void setup() { + executeDataSet("obsAuditTestDataset.xml"); + service = Context.getService(PihAppsService.class); + bruno = Context.getUserService().getUser(501); + butch = Context.getUserService().getUser(502); + } + + private List obsIds(List obs) { + return obs.stream().map(Obs::getObsId).collect(Collectors.toList()); + } + + /** + * The search under test. Building the criteria in one place means a test cannot silently set + * the wrong field, and keeps each case reading as the question it is asking. + */ + private ObsSearchResult search(User createdBy, User voidedBy, Date fromDate, Date toDate, Integer startIndex, + Integer limit) { + ObsSearchCriteria searchCriteria = new ObsSearchCriteria(); + searchCriteria.setCreatedBy(createdBy); + searchCriteria.setVoidedBy(voidedBy); + // The criteria name the column each range bounds, so the test picks the one the case is + // about — the same choice a client makes. Each case names at most one action. + if (voidedBy != null) { + searchCriteria.setVoidedOnOrAfter(fromDate); + searchCriteria.setVoidedOnOrBefore(toDate); + } + else { + searchCriteria.setCreatedOnOrAfter(fromDate); + searchCriteria.setCreatedOnOrBefore(toDate); + } + searchCriteria.setIncludeVoided(true); + searchCriteria.setSortCriteria(auditSortCriteria(voidedBy)); + searchCriteria.setStartIndex(startIndex); + searchCriteria.setLimit(limit); + return service.getObs(searchCriteria); + } + + /** + * The ordering an audit asks for, which the endpoint sets rather than the service: the audit + * action the search named, most recent first, with the obs id breaking ties. These tests assert + * on order, so they have to ask for the same one the endpoint does. + */ + private List auditSortCriteria(User voidedBy) { + List sortCriteria = new ArrayList<>(); + String actionDate = voidedBy != null ? "dateVoided" : "dateCreated"; + sortCriteria.add(new SortCriteria(actionDate, SortCriteria.Direction.DESC)); + sortCriteria.add(new SortCriteria("obsId", SortCriteria.Direction.DESC)); + return sortCriteria; + } + + private List obs(User createdBy, User voidedBy, Date fromDate, Date toDate, Integer startIndex, + Integer limit) { + return search(createdBy, voidedBy, fromDate, toDate, startIndex, limit).getObs(); + } + + private Long count(User createdBy, User voidedBy, Date fromDate, Date toDate) { + return search(createdBy, voidedBy, fromDate, toDate, null, null).getTotalCount(); + } + + /** A moment on a September 2026 day, matching the fixture's audit dates. */ + private Date august(int dayOfMonth, int hourOfDay) { + Calendar calendar = Calendar.getInstance(); + calendar.clear(); + calendar.set(2026, Calendar.AUGUST, dayOfMonth, hourOfDay, 0, 0); + return calendar.getTime(); + } + + private Date september(int dayOfMonth, int hourOfDay) { + Calendar calendar = Calendar.getInstance(); + calendar.clear(); + calendar.set(2026, Calendar.SEPTEMBER, dayOfMonth, hourOfDay, 0, 0); + return calendar.getTime(); + } + + @Test + public void shouldFindObsCreatedByAUserMostRecentlyCreatedFirst() { + List results = obs(bruno, null, null, null, null, null); + + assertThat(obsIds(results), contains(2002, 2001, 2004)); + } + + @Test + public void shouldIncludeVoidedObsWhenSearchingByCreator() { + List results = obs(bruno, null, null, null, null, null); + + assertThat(results.stream().anyMatch(Obs::getVoided), is(true)); + } + + @Test + public void shouldFindObsVoidedByAUserMostRecentlyVoidedFirst() { + List results = obs(null, butch, null, null, null, null); + + assertThat(obsIds(results), contains(2005, 2004)); + } + + @Test + public void shouldNotFindObsVoidedByAnotherUser() { + assertThat(obs(null, bruno, null, null, null, null), is(java.util.Collections.emptyList())); + } + + @Test + public void shouldNarrowByBothUsersWhenBothAreGiven() { + List results = obs(bruno, butch, null, null, null, null); + + assertThat(obsIds(results), contains(2004)); + } + + @Test + public void shouldPageResults() { + assertThat(obsIds(obs(bruno, null, null, null, 0, 2)), contains(2002, 2001)); + assertThat(obsIds(obs(bruno, null, null, null, 2, 2)), contains(2004)); + assertThat(obsIds(obs(bruno, null, null, null, 1, 1)), contains(2001)); + } + + @Test + public void shouldCountTheWholeResultSetRatherThanThePage() { + assertThat(count(bruno, null, null, null), is(3L)); + assertThat(count(null, butch, null, null), is(2L)); + assertThat(count(bruno, butch, null, null), is(1L)); + } + + @Test + public void shouldBoundACreatedBySearchByWhenTheObsWasCreated() { + // obs 2001 was created on 1 Sep, 2002 on 3 Sep, 2004 on 28 Aug + List results = obs(bruno, null, september(1, 0), september(2, 0), null, null); + + assertThat(obsIds(results), contains(2001)); + } + + @Test + public void shouldBoundACreatedBySearchWithOnlyOneEndGiven() { + assertThat(obsIds(obs(bruno, null, september(2, 0), null, null, null)), contains(2002)); + assertThat(obsIds(obs(bruno, null, null, september(2, 0), null, null)), + contains(2001, 2004)); + } + + @Test + public void shouldBoundAVoidedBySearchByWhenTheObsWasVoided() { + // obs 2004 was voided on 4 Sep and 2005 on 5 Sep + List results = obs(null, butch, september(5, 0), null, null, null); + + assertThat(obsIds(results), contains(2005)); + } + + /** + * The fixture's creation dates deliberately run in a different order from its obs datetimes, so + * a range applied to the wrong column would pick different rows. + */ + @Test + public void shouldBoundByTheAuditActionRatherThanTheObsDatetime() { + // obs 2005 has an obs_datetime of 25 Aug but was created on 20 Aug and voided on 5 Sep + assertThat(obsIds(obs(butch, null, september(1, 0), null, null, null)), contains(2003)); + assertThat(obsIds(obs(null, butch, september(1, 0), null, null, null)), + contains(2005, 2004)); + } + + @Test + public void shouldTreatBothEndsOfTheRangeAsInclusive() { + // obs 2001 was created at 08:00 on 1 Sep, so a range of exactly that hour includes it + List results = obs(bruno, null, september(1, 8), september(1, 8), null, null); + + assertThat(obsIds(results), contains(2001)); + } + + @Test + public void shouldCountAndPageWithinTheRange() { + Date from = september(1, 0); + assertThat(count(bruno, null, from, null), is(2L)); + assertThat(obsIds(obs(bruno, null, from, null, 0, 1)), contains(2002)); + assertThat(obsIds(obs(bruno, null, from, null, 1, 1)), contains(2001)); + } + + @Test + public void shouldFindNothingWhenTheRangeExcludesEverything() { + List results = obs(bruno, null, september(20, 0), september(21, 0), null, null); + + assertThat(results, is(java.util.Collections.emptyList())); + assertThat(count(bruno, null, september(20, 0), september(21, 0)), is(0L)); + } + + /** + * The audit endpoint asks for voided observations, so the helper above does too. Every other + * caller gets them left out, which is what this pins down. + */ + @Test + public void shouldLeaveOutVoidedObsUnlessAskedFor() { + // 2004 and 2005 are this fixture's voided observations, both voided by butch + ObsSearchCriteria searchCriteria = new ObsSearchCriteria(); + searchCriteria.setVoidedBy(butch); + + assertThat(service.getObs(searchCriteria).getObs(), is(java.util.Collections.emptyList())); + + searchCriteria.setIncludeVoided(true); + assertThat(obsIds(service.getObs(searchCriteria).getObs()), containsInAnyOrder(2004, 2005)); + } + + /** + * Each range names its own column, so they combine rather than one displacing another and none + * of them depends on a matching user filter being given. + */ + @Test + public void shouldApplyEachRangeToItsOwnColumnIndependently() { + // a creation range with no createdBy filter still narrows, which the old single range could + // not express — it was silently ignored + ObsSearchCriteria creationOnly = new ObsSearchCriteria(); + creationOnly.setIncludeVoided(true); + creationOnly.setCreatedOnOrAfter(september(20, 0)); + + assertThat(service.getObs(creationOnly).getObs(), is(java.util.Collections.emptyList())); + + // 2004 was created 28 Aug and voided 4 Sep; 2005 was voided 5 Sep but created back on 20 + // Aug, so only the creation range tells them apart — the two bound different columns + ObsSearchCriteria both = new ObsSearchCriteria(); + both.setIncludeVoided(true); + both.setCreatedOnOrAfter(august(25, 0)); + both.setVoidedOnOrAfter(september(1, 0)); + + assertThat(obsIds(service.getObs(both).getObs()), contains(2004)); + } + + @Test + public void shouldSearchWithoutAnyAuditFilter() { + // The service places no audit-specific requirement on the criteria, so a search naming none + // of them is a plain obs search. The audit endpoint asks for at least one itself. + ObsSearchCriteria searchCriteria = new ObsSearchCriteria(); + searchCriteria.setIncludeVoided(true); + + assertThat(obsIds(service.getObs(searchCriteria).getObs()), + hasItems(2001, 2002, 2003, 2004, 2005)); + } +} + diff --git a/api/src/test/java/org/openmrs/module/pihapps/PihAppsUtilsTest.java b/api/src/test/java/org/openmrs/module/pihapps/PihAppsUtilsTest.java index 948ddf1..1919a58 100644 --- a/api/src/test/java/org/openmrs/module/pihapps/PihAppsUtilsTest.java +++ b/api/src/test/java/org/openmrs/module/pihapps/PihAppsUtilsTest.java @@ -4,12 +4,16 @@ import org.openmrs.Concept; import java.util.Arrays; +import java.util.Calendar; import java.util.Collections; +import java.util.Date; import java.util.HashSet; import java.util.Set; import java.util.stream.Collectors; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -88,6 +92,37 @@ public void getConceptHierarchy_shouldHandleCycles_withoutInfiniteLoop() { assertEquals(new HashSet<>(Arrays.asList("A", "B")), result); } + private static Date moment(int hour, int minute, int second, int millisecond) { + Calendar calendar = Calendar.getInstance(); + calendar.clear(); + calendar.set(2026, Calendar.SEPTEMBER, 30, hour, minute, second); + calendar.set(Calendar.MILLISECOND, millisecond); + return calendar.getTime(); + } + + @Test + public void getEndOfDayIfTimeExcluded_shouldWidenADateWithNoTimeToTheEndOfItsDay() { + assertEquals(moment(23, 59, 59, 999), PihAppsUtils.getEndOfDayIfTimeExcluded(moment(0, 0, 0, 0))); + } + + @Test + public void getEndOfDayIfTimeExcluded_shouldLeaveADateCarryingATimeAlone() { + Date withTime = moment(13, 45, 0, 0); + assertSame(withTime, PihAppsUtils.getEndOfDayIfTimeExcluded(withTime)); + } + + /** A single millisecond past midnight is a time of day, so the bound stands as given. */ + @Test + public void getEndOfDayIfTimeExcluded_shouldLeaveAMomentJustPastMidnightAlone() { + Date justPast = moment(0, 0, 0, 1); + assertSame(justPast, PihAppsUtils.getEndOfDayIfTimeExcluded(justPast)); + } + + @Test + public void getEndOfDayIfTimeExcluded_shouldReturnNullForNoBound() { + assertNull(PihAppsUtils.getEndOfDayIfTimeExcluded(null)); + } + private static Concept conceptWithId(int id, String uuid) { Concept c = mock(Concept.class); when(c.getConceptId()).thenReturn(id); diff --git a/api/src/test/resources/encounterAuditTestDataset.xml b/api/src/test/resources/encounterAuditTestDataset.xml new file mode 100644 index 0000000..6fbc1fd --- /dev/null +++ b/api/src/test/resources/encounterAuditTestDataset.xml @@ -0,0 +1,27 @@ + + + + + + + + + + + + + + + + + + + + diff --git a/api/src/test/resources/obsAuditTestDataset.xml b/api/src/test/resources/obsAuditTestDataset.xml new file mode 100644 index 0000000..05ccd6a --- /dev/null +++ b/api/src/test/resources/obsAuditTestDataset.xml @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + diff --git a/omod/src/main/java/org/openmrs/module/pihapps/rest/PihAppsEncounterRestController.java b/omod/src/main/java/org/openmrs/module/pihapps/rest/PihAppsEncounterRestController.java new file mode 100644 index 0000000..3362aff --- /dev/null +++ b/omod/src/main/java/org/openmrs/module/pihapps/rest/PihAppsEncounterRestController.java @@ -0,0 +1,194 @@ +package org.openmrs.module.pihapps.rest; + +import org.apache.commons.lang.StringUtils; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.openmrs.EncounterType; +import org.openmrs.Provider; +import org.openmrs.User; +import org.openmrs.api.EncounterService; +import org.openmrs.module.pihapps.PihAppsService; +import org.openmrs.module.pihapps.encounter.EncounterSearchCriteria; +import org.openmrs.module.pihapps.encounter.EncounterSearchResult; +import org.openmrs.module.webservices.rest.SimpleObject; +import org.openmrs.module.webservices.rest.web.RequestContext; +import org.openmrs.module.webservices.rest.web.RestUtil; +import org.openmrs.module.webservices.rest.web.resource.impl.AlreadyPaged; +import org.openmrs.module.webservices.rest.web.response.InvalidSearchException; +import org.openmrs.module.webservices.rest.web.response.ResponseException; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.bind.annotation.ResponseStatus; +import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.util.List; + +/** + * Searches encounters by the user who created, changed or voided them, and by the provider recorded + * on them — none of which the core REST API can do. Core's `EncounterSearchCriteria` carries a + * providers field but no search handler exposes it, and it has no creator, changedBy or voidedBy + * field at all, so those columns can be read off an encounter but not searched on. + * + *

Results are paged and ordered as `sortBy` asks, and each encounter is rendered by the standard + * encounter resource, so `v` behaves as it does anywhere else in the REST API and defaults to the + * same thing. An audit wants `auditInfo` — the creating, changing and voiding users with their + * timestamps — which the default representation leaves out, so it asks for it: + * + * 

+ * GET /openmrs/ws/rest/v1/pihapps/encounter?createdBy=<uuid>&limit=20&totalCount=true
+ * GET /openmrs/ws/rest/v1/pihapps/encounter?createdBy=<uuid>&v=custom:(uuid,display,auditInfo)
+ * GET /openmrs/ws/rest/v1/pihapps/encounter?changedBy=<uuid>&changedOnOrAfter=2026-09-01&changedOnOrBefore=2026-09-30
+ * GET /openmrs/ws/rest/v1/pihapps/encounter?provider=<uuid>&encounterDatetimeOnOrAfter=2026-04-01
+ * GET /openmrs/ws/rest/v1/pihapps/encounter?provider=<uuid>&v=custom:(uuid,encounterDatetime,auditInfo)
+ * GET /openmrs/ws/rest/v1/pihapps/encounter?provider=<uuid>&encounterType=<uuid>
+ * GET /openmrs/ws/rest/v1/pihapps/encounter?voidedBy=<uuid>&includeVoided=true
+ * GET /openmrs/ws/rest/v1/pihapps/encounter?provider=<uuid>&sortBy=encounterDatetime-desc&sortBy=encounterId-desc
+ *
+ * + *

`createdBy`, `changedBy`, `voidedBy` and `provider` are bound by core's property editors, so + * each takes a uuid or a primary key. `encounterType` takes a uuid or its name. + * + *

`includeVoided` decides whether voided encounters come back alongside the surviving ones, and + * is off unless asked for. An audit normally wants them on: a `voidedBy` search returns nothing + * without them, and an auditor looking at what a user entered wants to see what has since been + * deleted just as much as what survives. Callers tell the two apart by each encounter's voided + * flag. + * + *

`sortBy` takes `field-direction`, or just `field` for ascending, and may be given several + * times to order by more than one. Nothing is sorted unless asked, and a page without an ordering + * is not deterministic, so a client that pages should name one ending in something unique such as + * `encounterId`. An audit wants the action it searched on first — `createdBy` with + * `sortBy=dateCreated-desc`, a provider search with `sortBy=encounterDatetime-desc` — since + * ordering by anything else would bury an encounter backdated to last year but entered this + * morning. + * + *

Every filter narrows, so naming several asks for the encounters satisfying all of them, and + * naming none matches every encounter. + * + *

There are four date ranges, each naming the column it bounds: `createdOnOrAfter`/`Before`, + * `changedOnOrAfter`/`Before`, `voidedOnOrAfter`/`Before` and + * `encounterDatetimeOnOrAfter`/`Before`. Each is independent of the filters, so `createdOnOrAfter` + * narrows by creation date whether or not `createdBy` is given, and naming several asks for all of + * them. Which one a search wants is the caller's to decide: an audit of what a user entered wants + * the range against that user's action, since an encounter backdated to last year but entered this + * morning was entered this morning, while a provider's caseload is asked about by + * `encounterDatetime`. All ends run inclusively, and a bare date names the whole of that day. + */ +@Controller +public class PihAppsEncounterRestController { + + protected Log log = LogFactory.getLog(getClass()); + + @Autowired + private EncounterService encounterService; + + @Autowired + private PihAppsService pihAppsService; + + @RequestMapping(value = "/rest/v1/pihapps/encounter", method = RequestMethod.GET) + @ResponseBody + public Object searchEncounters(HttpServletRequest request, HttpServletResponse response, + @RequestParam(value = "createdBy", required = false) User createdBy, + @RequestParam(value = "changedBy", required = false) User changedBy, + @RequestParam(value = "voidedBy", required = false) User voidedBy, + @RequestParam(value = "provider", required = false) Provider provider, + @RequestParam(value = "encounterType", required = false) String encounterType, + @RequestParam(value = "createdOnOrAfter", required = false) String createdOnOrAfter, + @RequestParam(value = "createdOnOrBefore", required = false) String createdOnOrBefore, + @RequestParam(value = "changedOnOrAfter", required = false) String changedOnOrAfter, + @RequestParam(value = "changedOnOrBefore", required = false) String changedOnOrBefore, + @RequestParam(value = "voidedOnOrAfter", required = false) String voidedOnOrAfter, + @RequestParam(value = "voidedOnOrBefore", required = false) String voidedOnOrBefore, + @RequestParam(value = "encounterDatetimeOnOrAfter", + required = false) String encounterDatetimeOnOrAfter, + @RequestParam(value = "encounterDatetimeOnOrBefore", + required = false) String encounterDatetimeOnOrBefore, + @RequestParam(value = "includeVoided", required = false, + defaultValue = "false") boolean includeVoided, + @RequestParam(value = "sortBy", required = false) List sortBy) + throws ResponseException { + + try { + EncounterType type = null; + if (StringUtils.isNotBlank(encounterType)) { + type = getEncounterType(encounterType); + if (type == null) { + throw new InvalidSearchException("No encounter type found with id: " + encounterType); + } + } + + + RequestContext context = RestUtil.getRequestContext(request, response); + + EncounterSearchCriteria searchCriteria = new EncounterSearchCriteria(); + searchCriteria.setCreatedBy(createdBy); + searchCriteria.setChangedBy(changedBy); + searchCriteria.setVoidedBy(voidedBy); + searchCriteria.setProvider(provider); + searchCriteria.setEncounterType(type); + searchCriteria.setIncludeVoided(includeVoided); + try { + searchCriteria.setCreatedOnOrAfter(PihAppsRestSupport.parseDate(createdOnOrAfter)); + searchCriteria.setCreatedOnOrBefore(PihAppsRestSupport.parseDate(createdOnOrBefore)); + searchCriteria.setChangedOnOrAfter(PihAppsRestSupport.parseDate(changedOnOrAfter)); + searchCriteria.setChangedOnOrBefore(PihAppsRestSupport.parseDate(changedOnOrBefore)); + searchCriteria.setVoidedOnOrAfter(PihAppsRestSupport.parseDate(voidedOnOrAfter)); + searchCriteria.setVoidedOnOrBefore(PihAppsRestSupport.parseDate(voidedOnOrBefore)); + searchCriteria.setEncounterDatetimeOnOrAfter(PihAppsRestSupport.parseDate(encounterDatetimeOnOrAfter)); + searchCriteria.setEncounterDatetimeOnOrBefore(PihAppsRestSupport.parseDate(encounterDatetimeOnOrBefore)); + } + catch (Exception e) { + throw new InvalidSearchException(PihAppsRestSupport.dateFormatMessage(), e); + } + searchCriteria.setSortCriteria(PihAppsRestSupport.parseSortCriteria(sortBy)); + searchCriteria.setStartIndex(context.getStartIndex()); + searchCriteria.setLimit(context.getLimit()); + + EncounterSearchResult result = pihAppsService.getEncounters(searchCriteria); + Long totalCount = result.getTotalCount(); + boolean hasMore = totalCount > context.getStartIndex() + context.getLimit(); + + return new AlreadyPaged<>(context, result.getEncounters(), hasMore, totalCount).toSimpleObject(null); + } + catch (InvalidSearchException e) { + response.setStatus(HttpServletResponse.SC_BAD_REQUEST); + return RestUtil.wrapErrorResponse(e, PihAppsRestSupport.INVALID_SEARCH_REASON); + } + catch (Exception e) { + log.error("Failed to search encounters by audit user", e); + response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + return RestUtil.wrapErrorResponse(e, "Failed to search encounters by audit user"); + } + } + + /** + * A parameter core's editors could not resolve is still a bad request, so it is answered in the + * same shape as every other one this endpoint rejects rather than in Spring's own error format. + */ + @ExceptionHandler(MethodArgumentTypeMismatchException.class) + @ResponseStatus(HttpStatus.BAD_REQUEST) + @ResponseBody + public SimpleObject handleUnresolvedParameter(MethodArgumentTypeMismatchException e) { + return PihAppsRestSupport.unresolvedParameterResponse(e); + } + + /** + * There is no EncounterType Property Editor registered with core, so convert from a String + * uuid or name to an EncounterType. + */ + EncounterType getEncounterType(String uuidOrName) { + EncounterType encounterType = encounterService.getEncounterTypeByUuid(uuidOrName); + if (encounterType == null) { + encounterType = encounterService.getEncounterType(uuidOrName); + } + return encounterType; + } +} diff --git a/omod/src/main/java/org/openmrs/module/pihapps/rest/PihAppsObsRestController.java b/omod/src/main/java/org/openmrs/module/pihapps/rest/PihAppsObsRestController.java new file mode 100644 index 0000000..13c3a15 --- /dev/null +++ b/omod/src/main/java/org/openmrs/module/pihapps/rest/PihAppsObsRestController.java @@ -0,0 +1,147 @@ +package org.openmrs.module.pihapps.rest; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.openmrs.User; +import org.openmrs.module.pihapps.PihAppsService; +import org.openmrs.module.pihapps.obs.ObsSearchCriteria; +import org.openmrs.module.pihapps.obs.ObsSearchResult; +import org.openmrs.module.webservices.rest.SimpleObject; +import org.openmrs.module.webservices.rest.web.RequestContext; +import org.openmrs.module.webservices.rest.web.RestUtil; +import org.openmrs.module.webservices.rest.web.resource.impl.AlreadyPaged; +import org.openmrs.module.webservices.rest.web.response.InvalidSearchException; +import org.openmrs.module.webservices.rest.web.response.ResponseException; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.bind.annotation.ResponseStatus; +import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.util.List; + +/** + * Searches observations by the user who created them or the user who voided them, which the core + * REST API cannot do: neither the obs resource, the observation search handler nor core's own + * {@link org.openmrs.parameter.ObsSearchCriteria} has a creator or voidedBy field, so those columns + * can be read off an observation but not searched on. + * + *

Results are paged and ordered as `sortBy` asks, and each observation is rendered by the + * standard obs resource, so `v` behaves as it does anywhere else in the REST API and defaults to + * the same thing. An audit wants `auditInfo` — the creator and the voiding user with their + * timestamps — which the default representation leaves out, so it asks for it: + * + * 

+ * GET /openmrs/ws/rest/v1/pihapps/obs?createdBy=<uuid>&limit=20&totalCount=true
+ * GET /openmrs/ws/rest/v1/pihapps/obs?createdBy=<uuid>&v=custom:(uuid,display,auditInfo)
+ * GET /openmrs/ws/rest/v1/pihapps/obs?voidedBy=cd8a4b8e-...&v=custom:(uuid,concept:(display),auditInfo)
+ * GET /openmrs/ws/rest/v1/pihapps/obs?createdBy=<uuid>&createdOnOrAfter=2026-09-01&createdOnOrBefore=2026-09-30
+ * GET /openmrs/ws/rest/v1/pihapps/obs?voidedBy=<uuid>&voidedOnOrAfter=2026-09-01
+ * GET /openmrs/ws/rest/v1/pihapps/obs?voidedBy=<uuid>&includeVoided=true
+ * GET /openmrs/ws/rest/v1/pihapps/obs?createdBy=<uuid>&sortBy=dateCreated-desc&sortBy=obsId-desc
+ *
+ * + *

`createdBy` and `voidedBy` are bound by core's property editors, so each takes a uuid or a + * primary key. + * + *

`includeVoided` decides whether voided observations come back alongside the surviving ones, + * and is off unless asked for. An audit normally wants them on: a `voidedBy` search returns nothing + * without them, and an auditor looking at what a user created wants to see what has since been + * deleted just as much as what survives. Callers tell the two apart by each observation's voided + * flag. + * + *

`sortBy` takes `field-direction`, or just `field` for ascending, and may be given several + * times to order by more than one. Nothing is sorted unless asked, and a page without an ordering + * is not deterministic, so a client that pages should name one ending in something unique such as + * `obsId`. An audit wants the action it searched on first — `createdBy` with + * `sortBy=dateCreated-desc`, `voidedBy` with `sortBy=dateVoided-desc` — since ordering by the + * observation's own datetime would bury an obs backdated to last year but entered this morning. + * + *

Every filter narrows, and naming none matches every observation. + * + *

There are two date ranges, each naming the column it bounds: `createdOnOrAfter`/`Before` and + * `voidedOnOrAfter`/`Before`. Each is independent of the filters, so `createdOnOrAfter` narrows by + * creation date whether or not `createdBy` is given, and naming both asks for both. They bound when + * the audit action happened rather than the observation's own datetime, since an observation + * backdated to last year but entered this morning was entered this morning. Both ends run + * inclusively, and an upper end with no time of day names the whole of that day. + */ +@Controller +public class PihAppsObsRestController { + + protected Log log = LogFactory.getLog(getClass()); + + @Autowired + private PihAppsService pihAppsService; + + @RequestMapping(value = "/rest/v1/pihapps/obs", method = RequestMethod.GET) + @ResponseBody + public Object searchObs(HttpServletRequest request, HttpServletResponse response, + @RequestParam(value = "createdBy", required = false) User createdBy, + @RequestParam(value = "voidedBy", required = false) User voidedBy, + @RequestParam(value = "createdOnOrAfter", required = false) String createdOnOrAfter, + @RequestParam(value = "createdOnOrBefore", required = false) String createdOnOrBefore, + @RequestParam(value = "voidedOnOrAfter", required = false) String voidedOnOrAfter, + @RequestParam(value = "voidedOnOrBefore", required = false) String voidedOnOrBefore, + @RequestParam(value = "includeVoided", required = false, + defaultValue = "false") boolean includeVoided, + @RequestParam(value = "sortBy", required = false) List sortBy) + throws ResponseException { + + try { + RequestContext context = RestUtil.getRequestContext(request, response); + + ObsSearchCriteria searchCriteria = new ObsSearchCriteria(); + searchCriteria.setCreatedBy(createdBy); + searchCriteria.setVoidedBy(voidedBy); + try { + searchCriteria.setCreatedOnOrAfter(PihAppsRestSupport.parseDate(createdOnOrAfter)); + searchCriteria.setCreatedOnOrBefore(PihAppsRestSupport.parseDate(createdOnOrBefore)); + searchCriteria.setVoidedOnOrAfter(PihAppsRestSupport.parseDate(voidedOnOrAfter)); + searchCriteria.setVoidedOnOrBefore(PihAppsRestSupport.parseDate(voidedOnOrBefore)); + } + catch (Exception e) { + throw new InvalidSearchException(PihAppsRestSupport.dateFormatMessage(), e); + } + searchCriteria.setIncludeVoided(includeVoided); + searchCriteria.setSortCriteria(PihAppsRestSupport.parseSortCriteria(sortBy)); + searchCriteria.setStartIndex(context.getStartIndex()); + searchCriteria.setLimit(context.getLimit()); + + ObsSearchResult result = pihAppsService.getObs(searchCriteria); + Long totalCount = result.getTotalCount(); + boolean hasMore = totalCount > context.getStartIndex() + context.getLimit(); + + // AlreadyPaged renders the envelope every other OpenMRS list endpoint returns: results in + // the requested representation, next and previous links, and totalCount when asked for. + return new AlreadyPaged<>(context, result.getObs(), hasMore, totalCount).toSimpleObject(null); + } + catch (InvalidSearchException e) { + response.setStatus(HttpServletResponse.SC_BAD_REQUEST); + return RestUtil.wrapErrorResponse(e, PihAppsRestSupport.INVALID_SEARCH_REASON); + } + catch (Exception e) { + log.error("Failed to search observations by audit user", e); + response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + return RestUtil.wrapErrorResponse(e, "Failed to search observations by audit user"); + } + } + + /** + * A parameter core's editors could not resolve is still a bad request, so it is answered in the + * same shape as every other one this endpoint rejects rather than in Spring's own error format. + */ + @ExceptionHandler(MethodArgumentTypeMismatchException.class) + @ResponseStatus(HttpStatus.BAD_REQUEST) + @ResponseBody + public SimpleObject handleUnresolvedParameter(MethodArgumentTypeMismatchException e) { + return PihAppsRestSupport.unresolvedParameterResponse(e); + } +} diff --git a/omod/src/main/java/org/openmrs/module/pihapps/rest/PihAppsRestSupport.java b/omod/src/main/java/org/openmrs/module/pihapps/rest/PihAppsRestSupport.java new file mode 100644 index 0000000..b9fb728 --- /dev/null +++ b/omod/src/main/java/org/openmrs/module/pihapps/rest/PihAppsRestSupport.java @@ -0,0 +1,100 @@ +package org.openmrs.module.pihapps.rest; + +import org.apache.commons.lang.StringUtils; +import org.openmrs.Provider; +import org.openmrs.module.pihapps.SortCriteria; +import org.openmrs.module.webservices.rest.SimpleObject; +import org.openmrs.module.webservices.rest.web.ConversionUtil; +import org.openmrs.module.webservices.rest.web.RestUtil; +import org.openmrs.module.webservices.rest.web.response.InvalidSearchException; +import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; + +import java.util.ArrayList; +import java.util.Date; +import java.util.List; + +/** + * The parts the search endpoints in this package have in common: reading a date bound off a request + * parameter, and wording a rejected search the same way. Kept together so that they cannot drift + * apart on what a date-only upper bound means, or on how a bad parameter reads to a client. + */ +final class PihAppsRestSupport { + + /** + * The context a rejected search is reported under. `RestUtil.wrapErrorResponse` prints it ahead + * of the exception's own message, so it says what kind of failure this is and leaves the + * particulars to the message. + */ + static final String INVALID_SEARCH_REASON = "Invalid search parameters"; + + private PihAppsRestSupport() { + } + + /** + * Reads a date parameter with no bound semantics, for an endpoint that decides what an end of a + * range means further down. Blank means the parameter was not given. + * + * @param value the parameter as it arrived, or null or blank for no date + * @return the date, or null if none was given + */ + static Date parseDate(String value) { + if (StringUtils.isBlank(value)) { + return null; + } + return (Date) ConversionUtil.convert(value.trim(), Date.class); + } + + /** The format message for an endpoint with more date parameters than are worth listing. */ + static String dateFormatMessage() { + return "Date parameters must be ISO 8601, e.g. 2026-09-01 or 2026-09-01T13:45:00.000+0000"; + } + + /** + * Reads the `sortBy` parameter, in the `field-direction` form this module's other endpoints + * accept: `encounterDatetime-desc`, or just `encounterDatetime` for ascending. Several may be + * given, and they order the results outermost first. + * + *

Ordering is the caller's to choose, and a page without one is not deterministic — two + * requests for the same page can repeat or skip a row — so a client that pages should always + * name one, ending in something unique such as the primary key. + * + * @param sortBy the parameter as it arrived, or null or empty for no ordering + * @return the criteria in the order given, empty if none were named + * @throws InvalidSearchException if a direction is neither asc nor desc + */ + static List parseSortCriteria(List sortBy) { + List sortCriteria = new ArrayList<>(); + if (sortBy == null) { + return sortCriteria; + } + for (String value : sortBy) { + if (StringUtils.isBlank(value)) { + continue; + } + String[] components = value.trim().split("-", 2); + SortCriteria.Direction direction = SortCriteria.Direction.ASC; + if (components.length > 1) { + try { + direction = SortCriteria.Direction.valueOf(components[1].toUpperCase()); + } + catch (IllegalArgumentException e) { + throw new InvalidSearchException( + "sortBy direction must be asc or desc, e.g. encounterDatetime-desc: " + value, e); + } + } + sortCriteria.add(new SortCriteria(components[0], direction)); + } + return sortCriteria; + } + + /** + * Answers a parameter core's property editors could not resolve into the object it names. Kept + * here so that both endpoints word the same failure the same way, and so that a bad `createdBy` + * reads the same to a client whichever one it asked. + */ + static SimpleObject unresolvedParameterResponse(MethodArgumentTypeMismatchException e) { + String noun = Provider.class.equals(e.getRequiredType()) ? "provider" : "user"; + return RestUtil.wrapErrorResponse(new InvalidSearchException("No " + noun + " found with id: " + e.getValue()), + INVALID_SEARCH_REASON); + } +}