Skip to content

Commit

Permalink
fix: Event Report Enrollments not returning correct data[2.39-DHIS2-1…
Browse files Browse the repository at this point in the history
…5291-backport] (#18356)

* fix: Event Report Enrollments not returning correct data[2.39-DHIS2-15291-backport]

* fix: Event Report Enrollments not returning correct data[2.39-DHIS2-15291-backport]
  • Loading branch information
d-bernat authored Aug 16, 2024
1 parent 08bee9e commit 8c324d8
Show file tree
Hide file tree
Showing 14 changed files with 346 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,9 @@ public class EnrollmentAnalyticsQueryCriteria extends AnalyticsPagingCriteria {

/** flag to enable row context in grid response */
private boolean rowContext;

/** Returns true when parameters are incoming from analytics enrollments/aggregate endpoint. */
public boolean isAggregatedEnrollments() {
return isAggregateEndpoint() && isEnrollmentEndpointItem();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ public EventDataQueryRequestBuilder fromCriteria(EnrollmentAnalyticsQueryCriteri
}

Set<String> dimensions;
if (criteria.isQueryEndpoint()) {
if (criteria.isQueryEndpoint() || criteria.isAggregatedEnrollments()) {
/*
* for each AnalyticsDateFilter whose enrollment extractor is
* set, concatenates the timeField with the extracted value:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ public boolean isAggregateEndpoint() {
return AGGREGATE == endpointAction;
}

public boolean isEnrollmentEndpointItem() {
return EndpointItem.ENROLLMENT == endpointItem;
}

public enum EndpointAction {
AGGREGATE,
QUERY,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,11 @@ public int getSortOrderAsInt() {
return DESC == sortOrder ? 1 : 0;
}

/** Returns true when parameters are incoming from analytics enrollments/aggregate entry point */
public boolean isAggregatedEnrollments() {
return endpointAction == EndpointAction.AGGREGATE && endpointItem == EndpointItem.ENROLLMENT;
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import static org.hisp.dhis.common.IdentifiableObjectUtils.getLocalPeriodIdentifiers;
import static org.hisp.dhis.common.IdentifiableObjectUtils.getUids;
import static org.hisp.dhis.common.ValueType.COORDINATE;
import static org.hisp.dhis.commons.util.TextUtils.EMPTY;
import static org.hisp.dhis.organisationunit.OrganisationUnit.getParentGraphMap;
import static org.hisp.dhis.organisationunit.OrganisationUnit.getParentNameGraphMap;

Expand Down Expand Up @@ -123,6 +124,7 @@ protected Grid getGrid(EventQueryParams params) {
.flatMap(dk -> dk.getKeywords().stream())
.collect(toList());

List<DimensionalObject> periods = getPeriods(params);
params = new EventQueryParams.Builder(params).withStartEndDatesForPeriods().build();

// ---------------------------------------------------------------------
Expand All @@ -141,6 +143,16 @@ protected Grid getGrid(EventQueryParams params) {
true));
}

for (DimensionalObject dimension : periods) {
grid.addHeader(
new GridHeader(
dimension.getDimension(),
dimension.getDimensionDisplayName(),
ValueType.TEXT,
false,
true));
}

final DisplayProperty displayProperty = params.getDisplayProperty();
Map<String, Long> repeatedNames =
params.getItems().stream()
Expand Down Expand Up @@ -211,7 +223,14 @@ protected Grid getGrid(EventQueryParams params) {
long count = 0;

if (!params.isSkipData() || params.analyzeOnly()) {
count = addEventData(grid, params);
if (!periods.isEmpty()) {
params =
new EventQueryParams.Builder(params)
.withPeriods(
periods.stream().flatMap(p -> p.getItems().stream()).collect(toList()), EMPTY)
.build();
}
count = addData(grid, params);
}

// ---------------------------------------------------------------------
Expand Down Expand Up @@ -319,7 +338,7 @@ private String getItemUid(QueryItem item) {

protected abstract Grid createGridWithHeaders(EventQueryParams params);

protected abstract long addEventData(Grid grid, EventQueryParams params);
protected abstract long addData(Grid grid, EventQueryParams params);

private void maybeApplyHeaders(EventQueryParams params, Grid grid) {
if (params.hasHeaders()) {
Expand Down Expand Up @@ -686,6 +705,16 @@ private List<String> getDimensionItemUidsFrom(
return dimensionUids;
}

/**
* retrieve all periods as list of dimensional objects
*
* @param params
* @return {@link EventQueryParams} object
*/
protected List<DimensionalObject> getPeriods(EventQueryParams params) {
return List.of();
}

/**
* Substitutes metadata in the given grid.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@
import static org.apache.commons.lang3.ObjectUtils.defaultIfNull;
import static org.apache.commons.lang3.StringUtils.EMPTY;
import static org.apache.commons.lang3.StringUtils.SPACE;
import static org.apache.commons.lang3.StringUtils.isBlank;
import static org.hisp.dhis.analytics.AggregationType.CUSTOM;
import static org.hisp.dhis.analytics.AggregationType.NONE;
import static org.hisp.dhis.analytics.DataQueryParams.LEVEL_PREFIX;
import static org.hisp.dhis.analytics.DataQueryParams.NUMERATOR_DENOMINATOR_PROPERTIES_COUNT;
import static org.hisp.dhis.analytics.DataType.NUMERIC;
import static org.hisp.dhis.analytics.QueryKey.NV;
Expand All @@ -56,6 +58,8 @@
import static org.hisp.dhis.analytics.util.AnalyticsUtils.withExceptionHandling;
import static org.hisp.dhis.common.DimensionItemType.DATA_ELEMENT;
import static org.hisp.dhis.common.DimensionItemType.PROGRAM_INDICATOR;
import static org.hisp.dhis.common.DimensionalObject.ORGUNIT_DIM_ID;
import static org.hisp.dhis.common.DimensionalObject.PERIOD_DIM_ID;
import static org.hisp.dhis.common.DimensionalObjectUtils.COMPOSITE_DIM_OBJECT_PLAIN_SEP;
import static org.hisp.dhis.common.QueryOperator.IN;
import static org.hisp.dhis.common.RequestTypeAware.EndpointItem.ENROLLMENT;
Expand Down Expand Up @@ -101,6 +105,7 @@
import org.hisp.dhis.common.GridHeader;
import org.hisp.dhis.common.IdScheme;
import org.hisp.dhis.common.InQueryFilter;
import org.hisp.dhis.common.OrganisationUnitSelectionMode;
import org.hisp.dhis.common.QueryFilter;
import org.hisp.dhis.common.QueryItem;
import org.hisp.dhis.common.Reference;
Expand All @@ -111,6 +116,7 @@
import org.hisp.dhis.feedback.ErrorCode;
import org.hisp.dhis.jdbc.StatementBuilder;
import org.hisp.dhis.option.Option;
import org.hisp.dhis.organisationunit.OrganisationUnit;
import org.hisp.dhis.period.Period;
import org.hisp.dhis.program.AnalyticsType;
import org.hisp.dhis.program.ProgramIndicator;
Expand All @@ -137,6 +143,8 @@ public abstract class AbstractJdbcEventAnalyticsManager {

private static final String COL_VALUE = "value";

private static final String OUTER_SQL_ALIAS = "t1";

private static final String AND = " and ";

private static final String OR = " or ";
Expand Down Expand Up @@ -353,6 +361,17 @@ private void addDimensionSelectColumns(
.getDimensions()
.forEach(
dimension -> {
if (params.isAggregatedEnrollments()
&& dimension.getDimensionType() == DimensionType.PERIOD) {
dimension
.getItems()
.forEach(
it ->
columns.add(
((Period) it).getPeriodType().getPeriodTypeEnum().getName()));
return;
}

if (isGroupByClause
&& dimension.getDimensionType() == DimensionType.PERIOD
&& params.hasNonDefaultBoundaries()) {
Expand Down Expand Up @@ -886,7 +905,7 @@ private String getTableAndColumn(
* @param maxLimit max number of records to return.
* @return a SQL query.
*/
protected String getEventsOrEnrollmentsSql(EventQueryParams params, int maxLimit) {
protected String getAggregatedEnrollmentsSql(EventQueryParams params, int maxLimit) {

String sql = getSelectClause(params);

Expand All @@ -901,6 +920,93 @@ protected String getEventsOrEnrollmentsSql(EventQueryParams params, int maxLimit
return sql;
}

/**
* Template method that generates a SQL query for retrieving aggregated enrollments.
*
* @param params the {@link List<GridHeader>} to drive the query generation.
* @param params the {@link EventQueryParams} to drive the query generation.
* @return a SQL query.
*/
protected String getAggregatedEnrollmentsSql(List<GridHeader> headers, EventQueryParams params) {
String sql = getSelectClause(params);

sql += getFromClause(params);

sql += getWhereClause(params);

final String tempSql = sql;

String headerColumns =
headers.stream()
.filter(
header ->
!header.getName().equalsIgnoreCase(COL_VALUE)
&& !header.getName().equalsIgnoreCase(PERIOD_DIM_ID)
&& !header.getName().equalsIgnoreCase(ORGUNIT_DIM_ID))
.map(
header -> {
String headerName = header.getName();
if (tempSql.contains(headerName)) {
return OUTER_SQL_ALIAS + "." + quote(headerName);
}
if (headerName.contains(".")) {
headerName = headerName.split("\\.")[1];
}

return OUTER_SQL_ALIAS + "." + quote(headerName);
})
.collect(joining(","));

String orgColumns = EMPTY;

if (!params.isOrganisationUnitMode(OrganisationUnitSelectionMode.SELECTED)
&& !params.isOrganisationUnitMode(OrganisationUnitSelectionMode.CHILDREN)) {

orgColumns =
params.getDimensionOrFilterItems(ORGUNIT_DIM_ID).stream()
.map(d -> LEVEL_PREFIX + ((OrganisationUnit) d).getLevel())
.distinct()
.collect(joining(","));
}

String periodColumns =
params.getDimensions().stream()
.filter(d -> d.getDimensionType() == DimensionType.PERIOD)
.flatMap(
d ->
d.getItems().stream()
.map(
it ->
OUTER_SQL_ALIAS
+ "."
+ ((Period) it).getPeriodType().getPeriodTypeEnum().getName())
.collect(Collectors.toList())
.stream()
.distinct())
.collect(joining(","));

String columns =
(!isBlank(orgColumns) ? orgColumns : "," + ORGUNIT_DIM_ID)
+ (!isBlank(periodColumns) ? "," + periodColumns : EMPTY)
+ (!isBlank(headerColumns) ? "," + headerColumns : EMPTY);

sql =
"select count("
+ OUTER_SQL_ALIAS
+ ".pi) as "
+ COL_VALUE
+ ", "
+ columns
+ " from ("
+ sql
+ ") "
+ OUTER_SQL_ALIAS
+ " group by "
+ columns;

return sql;
}

/**
* Adds a value from the given row set to the grid.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,14 @@
package org.hisp.dhis.analytics.event.data;

import static com.google.common.base.Preconditions.checkNotNull;
import static org.hisp.dhis.analytics.DataQueryParams.VALUE_HEADER_NAME;
import static org.hisp.dhis.analytics.DataQueryParams.VALUE_ID;
import static org.hisp.dhis.common.ValueType.DATETIME;
import static org.hisp.dhis.common.ValueType.NUMBER;
import static org.hisp.dhis.common.ValueType.TEXT;

import java.util.List;
import java.util.stream.Collectors;
import org.hisp.dhis.analytics.AnalyticsSecurityManager;
import org.hisp.dhis.analytics.data.handler.SchemaIdResponseMapper;
import org.hisp.dhis.analytics.event.EnrollmentAnalyticsManager;
Expand All @@ -40,8 +44,11 @@
import org.hisp.dhis.analytics.event.EventQueryPlanner;
import org.hisp.dhis.analytics.event.EventQueryValidator;
import org.hisp.dhis.analytics.event.LabelMapper;
import org.hisp.dhis.common.DimensionType;
import org.hisp.dhis.common.DimensionalObject;
import org.hisp.dhis.common.Grid;
import org.hisp.dhis.common.GridHeader;
import org.hisp.dhis.common.RequestTypeAware;
import org.hisp.dhis.system.grid.ListGrid;
import org.hisp.dhis.util.Timer;
import org.springframework.stereotype.Service;
Expand Down Expand Up @@ -111,6 +118,11 @@ public Grid getEnrollments(EventQueryParams params) {

@Override
protected Grid createGridWithHeaders(EventQueryParams params) {
if (params.getEndpointAction() == RequestTypeAware.EndpointAction.AGGREGATE) {
return new ListGrid()
.addHeader(new GridHeader(VALUE_ID, VALUE_HEADER_NAME, NUMBER, false, false));
}

return new ListGrid()
.addHeader(new GridHeader(ITEM_PI, NAME_PI, TEXT, false, true))
.addHeader(new GridHeader(ITEM_TEI, NAME_TEI, TEXT, false, true))
Expand Down Expand Up @@ -148,24 +160,42 @@ protected Grid createGridWithHeaders(EventQueryParams params) {
.addHeader(new GridHeader(ITEM_PROGRAM_STATUS, NAME_PROGRAM_STATUS, TEXT, false, true));
}

@Override
protected long addEventData(Grid grid, EventQueryParams params) {
protected long addData(Grid grid, EventQueryParams params) {
Timer timer = new Timer().start().disablePrint();

params = queryPlanner.planEnrollmentQuery(params);
List<EventQueryParams> paramsList;

timer.getSplitTime("Planned event query, got partitions: " + params.getPartitions());
if (params.getEndpointAction() == RequestTypeAware.EndpointAction.AGGREGATE) {
paramsList = queryPlanner.planAggregateQuery(params);
} else {
paramsList = List.of(queryPlanner.planEnrollmentQuery(params));
}

long count = 0;

if (params.isTotalPages()) {
count += enrollmentAnalyticsManager.getEnrollmentCount(params);
}
for (EventQueryParams queryParams : paramsList) {
timer.getSplitTime("Planned event query, got partitions: " + queryParams.getPartitions());
if (queryParams.isTotalPages() && !params.isAggregatedEnrollments()) {
count += enrollmentAnalyticsManager.getEnrollmentCount(queryParams);
}

enrollmentAnalyticsManager.getEnrollments(params, grid, queryValidator.getMaxLimit());
enrollmentAnalyticsManager.getEnrollments(queryParams, grid, queryValidator.getMaxLimit());

timer.getTime("Got enrollments " + grid.getHeight());
timer.getTime("Got enrollments " + grid.getHeight());
}

return count;
}

@Override
protected List<DimensionalObject> getPeriods(EventQueryParams params) {
// for aggregated enrollments only
if (!params.isAggregatedEnrollments()) {
return List.of();
}

return params.getDimensions().stream()
.filter(d -> d.getDimensionType() == DimensionType.PERIOD)
.collect(Collectors.toList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,7 @@ protected Grid createGridWithHeaders(EventQueryParams params) {
* @return the count of events.
*/
@Override
protected long addEventData(Grid grid, EventQueryParams params) {
protected long addData(Grid grid, EventQueryParams params) {
Timer timer = new Timer().start().disablePrint();

params = queryPlanner.planEventQuery(params);
Expand Down
Loading

0 comments on commit 8c324d8

Please sign in to comment.