diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/InQueryFilter.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/InQueryFilter.java index 0df52d46f28..cc49c8ffd83 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/InQueryFilter.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/InQueryFilter.java @@ -46,19 +46,19 @@ public class InQueryFilter extends QueryFilter { private final String field; - private final boolean isText; + private final boolean shouldQuote; /** * Construct a InQueryFilter using field name and the original {@link QueryFilter} * * @param field the field on which to construct the InQueryFilter * @param encodedFilter The original encodedFilter in {@link QueryFilter} - * @param isText whether this filter contains text or numeric values + * @param shouldQuote whether this filter contains text or numeric values */ - public InQueryFilter(String field, String encodedFilter, boolean isText) { + public InQueryFilter(String field, String encodedFilter, boolean shouldQuote) { super(IN, encodedFilter); this.field = field; - this.isText = isText; + this.shouldQuote = shouldQuote; } /** @@ -100,7 +100,7 @@ public String getSqlFilter() { } private String quoteIfNecessary(String item) { - return isText ? quote(item) : item; + return shouldQuote ? quote(item) : item; } private boolean hasMissingValue(List filterItems) { diff --git a/dhis-2/dhis-api/src/test/java/org/hisp/dhis/common/InQueryFilterTest.java b/dhis-2/dhis-api/src/test/java/org/hisp/dhis/common/InQueryFilterTest.java index b492f985d2f..2794859657c 100644 --- a/dhis-2/dhis-api/src/test/java/org/hisp/dhis/common/InQueryFilterTest.java +++ b/dhis-2/dhis-api/src/test/java/org/hisp/dhis/common/InQueryFilterTest.java @@ -64,11 +64,11 @@ void verifyNestedSqlStmtInFieldWithNullOnly() { executeTest(field, "NV", true, "(" + field + " is null and exists((select * from xy))) "); } - private void executeTest(String filterValue, boolean isText, String expected) { - executeTest("aField", filterValue, isText, expected); + private void executeTest(String filterValue, boolean shouldQuote, String expected) { + executeTest("aField", filterValue, shouldQuote, expected); } - private void executeTest(String field, String filterValue, boolean isText, String expected) { - assertEquals(new InQueryFilter(field, filterValue, isText).getSqlFilter(), expected); + private void executeTest(String field, String filterValue, boolean shouldQuote, String expected) { + assertEquals(new InQueryFilter(field, filterValue, shouldQuote).getSqlFilter(), expected); } } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/DataQueryService.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/DataQueryService.java index b7ce9bdd221..871032a2e62 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/DataQueryService.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/DataQueryService.java @@ -176,4 +176,15 @@ DimensionalObject getDimension( * @return a list of organisation units. */ List getUserOrgUnits(DataQueryParams params, String userOrgUnit); + + /** + * Based on the given parameters, this method will return a list of {@link OrganisationUnit} UIDs + * based on the given items and user organisation units. + * + * @param items the list of items that might be included into the resulting organisation unit and + * its keywords. + * @param userOrgUnits the list of organisation units associated with the current user. + * @return a list of {@link OrganisationUnit} UIDs. + */ + List getOrgUnitDimensionUid(List items, List userOrgUnits); } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultDataQueryService.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultDataQueryService.java index b5511f5ebad..4ae91c48c73 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultDataQueryService.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultDataQueryService.java @@ -363,6 +363,21 @@ private List getDimensionalObjects(DataQueryRequest request) return list; } + /** + * Based on the given parameters, this method will return a list of {@link OrganisationUnit} UIDs + * based on the given items and user organisation units. + * + * @param items the list of items that might be included into the resulting organisation unit and + * its keywords. + * @param userOrgUnits the list of organisation units associated with the current user. + * @return a list of {@link OrganisationUnit} UIDs. + */ + @Override + public List getOrgUnitDimensionUid( + List items, List userOrgUnits) { + return dimensionalObjectProducer.getOrgUnitDimensionUid(items, userOrgUnits); + } + /** * Returns a {@link DimensionalObject}. * diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DimensionalObjectProvider.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DimensionalObjectProvider.java index c247113b657..4dcce1197a3 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DimensionalObjectProvider.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DimensionalObjectProvider.java @@ -423,6 +423,24 @@ public BaseDimensionalObject getOrgUnitDimension( dimensionalKeywords); } + /** + * This method will return a list of {@link OrganisationUnit} UIDs based on the given items and + * user organisation units. + * + * @param items the list of items that might be included into the resulting organisation unit and + * its keywords. + * @param userOrgUnits the list of organisation units associated with the current user. + * @return a list of {@link OrganisationUnit} UIDs. + */ + public List getOrgUnitDimensionUid( + List items, List userOrgUnits) { + return getOrgUnitDimensionItems( + items, userOrgUnits, IdScheme.UID, new ArrayList<>(), new ArrayList<>()) + .stream() + .map(DimensionalItemObject::getUid) + .toList(); + } + /** * Based on the given parameters, this method will return a list of {@link DimensionalItemObject} * of type {@link OrganisationUnit}. It also adds to the given levels and groups if certain diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManager.java index 47952b35a9d..8055af5b691 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManager.java @@ -1244,13 +1244,14 @@ private static class IdentifiableSql { } /** - * Creates a SQL statement for a single filter inside a query item. + * Creates a SQL statement for a single filter inside a query item. Made public for testing + * purposes. * * @param item the {@link QueryItem}. * @param filter the {@link QueryFilter}. * @param params the {@link EventQueryParams}. */ - private String toSql(QueryItem item, QueryFilter filter, EventQueryParams params) { + public String toSql(QueryItem item, QueryFilter filter, EventQueryParams params) { String field = item.hasAggregationType() ? getSelectSql(filter, item, params) @@ -1258,7 +1259,7 @@ private String toSql(QueryItem item, QueryFilter filter, EventQueryParams params if (IN.equals(filter.getOperator())) { InQueryFilter inQueryFilter = - new InQueryFilter(field, sqlBuilder.escape(filter.getFilter()), item.isText()); + new InQueryFilter(field, sqlBuilder.escape(filter.getFilter()), !item.isNumeric()); return inQueryFilter.getSqlFilter(); } else { diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java index 533bd5c3229..cf13563ca64 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java @@ -37,13 +37,16 @@ import static org.hisp.dhis.analytics.util.AnalyticsUtils.illegalQueryExSupplier; import static org.hisp.dhis.analytics.util.AnalyticsUtils.throwIllegalQueryEx; import static org.hisp.dhis.common.DimensionalObject.DIMENSION_NAME_SEP; +import static org.hisp.dhis.common.DimensionalObject.OPTION_SEP; import static org.hisp.dhis.common.DimensionalObject.PERIOD_DIM_ID; import static org.hisp.dhis.common.DimensionalObjectUtils.getDimensionFromParam; import static org.hisp.dhis.common.DimensionalObjectUtils.getDimensionItemsFromParam; import static org.hisp.dhis.common.DimensionalObjectUtils.getDimensionalItemIds; +import static org.hisp.dhis.common.ValueType.ORGANISATION_UNIT; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Locale; @@ -54,6 +57,7 @@ import java.util.stream.Collectors; import lombok.Getter; import lombok.RequiredArgsConstructor; +import lombok.SneakyThrows; import org.apache.commons.lang3.StringUtils; import org.hisp.dhis.analytics.AnalyticsAggregationType; import org.hisp.dhis.analytics.DataQueryParams; @@ -318,7 +322,7 @@ private void addDimensionsToParams( if (groupableItem != null) { params.addDimension((DimensionalObject) groupableItem); } else { - groupableItem = getQueryItem(dim, pr, request.getOutputType()); + groupableItem = getQueryItem(dim, pr, request.getOutputType(), userOrgUnits); params.addItem((QueryItem) groupableItem); } @@ -511,6 +515,14 @@ private QueryItem getQueryItem( @Override public QueryItem getQueryItem(String dimensionString, Program program, EventOutputType type) { + return getQueryItem(dimensionString, program, type, Collections.emptyList()); + } + + private QueryItem getQueryItem( + String dimensionString, + Program program, + EventOutputType type, + List userOrgUnits) { String[] split = dimensionString.split(DIMENSION_NAME_SEP); if (split.length % 2 != 1) { @@ -536,6 +548,7 @@ public QueryItem getQueryItem(String dimensionString, Program program, EventOutp // FE uses HH.MM time format instead of HH:MM. This is not // compatible with db table/cell values modifyFilterWhenTimeQueryItem(queryItem, filter); + resolveOrgUnitDimensionIfNecessary(queryItem, filter, userOrgUnits); queryItem.addFilter(filter); } } @@ -543,6 +556,17 @@ public QueryItem getQueryItem(String dimensionString, Program program, EventOutp return queryItem; } + @SneakyThrows + private void resolveOrgUnitDimensionIfNecessary( + QueryItem queryItem, QueryFilter queryFilter, List userOrgUnits) { + if (queryItem.getValueType().equals(ORGANISATION_UNIT)) { + List filterItem = QueryFilter.getFilterItems(queryFilter.getFilter()); + List orgUnitDimensionUid = + dataQueryService.getOrgUnitDimensionUid(filterItem, userOrgUnits); + queryFilter.setFilter(String.join(OPTION_SEP, orgUnitDimensionUid)); + } + } + private static void modifyFilterWhenTimeQueryItem(QueryItem queryItem, QueryFilter filter) { if (queryItem.getItem() instanceof DataElement && ((DataElement) queryItem.getItem()).getValueType() == ValueType.TIME) { diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManagerTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManagerTest.java index c7834677135..8cdef3e9a47 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManagerTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManagerTest.java @@ -36,6 +36,7 @@ import static org.hisp.dhis.analytics.AnalyticsAggregationType.fromAggregationType; import static org.hisp.dhis.analytics.DataType.NUMERIC; import static org.hisp.dhis.common.QueryOperator.EQ; +import static org.hisp.dhis.common.QueryOperator.IN; import static org.hisp.dhis.common.QueryOperator.NE; import static org.hisp.dhis.common.QueryOperator.NEQ; import static org.hisp.dhis.common.QueryOperator.NIEQ; @@ -852,6 +853,23 @@ void testGetSelectClauseForAggregatedEnrollments() { assertEquals("select enrollment,Yearly ", select); } + @Test + void testItemsInFilterAreQuotedForOrganisationUnit() { + // Given + QueryItem queryItem = mock(QueryItem.class); + QueryFilter queryFilter = new QueryFilter(IN, "A;B;C"); + EventQueryParams params = + new EventQueryParams.Builder().withStartDate(new Date()).withEndDate(new Date()).build(); + when(queryItem.getItemName()).thenReturn("anyItem"); + when(queryItem.getValueType()).thenReturn(ValueType.ORGANISATION_UNIT); + + // When + String sql = eventSubject.toSql(queryItem, queryFilter, params).trim(); + + // Then + assertEquals("ax.\"anyItem\" in ('A','B','C')", sql); + } + @Test void testGetSelectClauseForQueryEnrollments() { // Given diff --git a/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/event/query/EventQueryTest.java b/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/event/query/EventQueryTest.java index 6ad7d7c4b24..cc12bb525f9 100644 --- a/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/event/query/EventQueryTest.java +++ b/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/event/query/EventQueryTest.java @@ -997,4 +997,69 @@ public void queryMetadataInfoForOptionSetAndOptionsWhenTenRows() throws JSONExce // Assert rows. validateRow(response, 0, List.of("Ngelehun CHC", "1")); } + + @Test + public void queryWithOrgUnitDataElement() throws JSONException { + // Given + String dimensionItems = + String.join( + ";", + "DiszpKrYNg8", + "g8upMTyEZGZ", + "LEVEL-H1KlN4QIauv", + "OU_GROUP-nlX2VoouN63", + "USER_ORGUNIT", + "USER_ORGUNIT_CHILDREN", + "USER_ORGUNIT_GRANDCHILDREN"); + + String dimensionOrgUnitDataElement = "Ge7Eo3FNnbl.rypjN8CV02V:IN:" + dimensionItems; + String dimensionOrgUnit = "ou:USER_ORGUNIT"; + + String dimension = dimensionOrgUnitDataElement + "," + dimensionOrgUnit; + + QueryParamsBuilder params = + new QueryParamsBuilder() + .add("dimension=" + dimension) + .add("headers=ouname,Ge7Eo3FNnbl.rypjN8CV02V") + .add("totalPages=false") + .add("displayProperty=NAME") + .add("pageSize=100") + .add("page=1") + .add("includeMetadataDetails=true") + .add("outputType=EVENT"); + + // When + ApiResponse response = analyticsEventActions.query().get("MoUd5BTQ3lY", params); + + // Then + response + .validate() + .statusCode(200) + .body("headers", hasSize(equalTo(2))) + .body("rows", hasSize(equalTo(0))) + .body("height", equalTo(0)) + .body("width", equalTo(0)) + .body("headerWidth", equalTo(2)); + + // Assert metaData. + String expectedMetaData = + "{\"pager\":{\"isLastPage\":true,\"pageSize\":100,\"page\":1},\"items\":{\"ImspTQPwCqd\":{\"uid\":\"ImspTQPwCqd\",\"code\":\"OU_525\",\"valueType\":\"TEXT\",\"name\":\"Sierra Leone\",\"dimensionItemType\":\"ORGANISATION_UNIT\",\"totalAggregationType\":\"SUM\"},\"USER_ORGUNIT\":{\"organisationUnits\":[\"ImspTQPwCqd\"]},\"ou\":{\"uid\":\"ou\",\"dimensionType\":\"ORGANISATION_UNIT\",\"name\":\"Organisation unit\"},\"Ge7Eo3FNnbl\":{\"uid\":\"Ge7Eo3FNnbl\",\"name\":\"XX MAL RDT - Case Registration\"},\"Ge7Eo3FNnbl.rypjN8CV02V\":{\"uid\":\"rypjN8CV02V\",\"aggregationType\":\"SUM\",\"valueType\":\"TEXT\",\"name\":\"XX MAL RDT TRK - Village of Residence\",\"style\":{\"icon\":\"nullapi\\/icons\\/star_medium_positive\\/icon.svg\"},\"dimensionItemType\":\"DATA_ELEMENT\",\"totalAggregationType\":\"SUM\"},\"MoUd5BTQ3lY\":{\"uid\":\"MoUd5BTQ3lY\",\"name\":\"XX MAL RDT - Case Registration\"},\"USER_ORGUNIT_CHILDREN\":{\"organisationUnits\":[\"at6UHUQatSo\",\"TEQlaapDQoK\",\"PMa2VCrupOd\",\"qhqAxPSTUXp\",\"kJq2mPyFEHo\",\"jmIPBj66vD6\",\"Vth0fbpFcsO\",\"jUb8gELQApl\",\"fdc6uOvgoji\",\"eIQbndfxQMb\",\"O6uvpzGd5pu\",\"lc3eMKXaEfw\",\"bL4ooGhyHRQ\"]},\"rypjN8CV02V\":{\"uid\":\"rypjN8CV02V\",\"aggregationType\":\"SUM\",\"valueType\":\"TEXT\",\"name\":\"XX MAL RDT TRK - Village of Residence\",\"style\":{\"icon\":\"nullapi\\/icons\\/star_medium_positive\\/icon.svg\"},\"dimensionItemType\":\"DATA_ELEMENT\",\"totalAggregationType\":\"SUM\"},\"LAST_12_MONTHS\":{\"name\":\"Last 12 months\"}},\"dimensions\":{\"pe\":[],\"ou\":[\"ImspTQPwCqd\"],\"Ge7Eo3FNnbl.rypjN8CV02V\":[]}}"; + String actualMetaData = new JSONObject((Map) response.extract("metaData")).toString(); + assertEquals(expectedMetaData, actualMetaData, false); + + // Assert headers. + validateHeader( + response, 0, "ouname", "Organisation unit name", "TEXT", "java.lang.String", false, true); + validateHeader( + response, + 1, + "Ge7Eo3FNnbl.rypjN8CV02V", + "XX MAL RDT TRK - Village of Residence", + "ORGANISATION_UNIT", + "org.hisp.dhis.organisationunit.OrganisationUnit", + false, + true); + + // no rows to assert + } }