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 0df52d46f28b..cc49c8ffd83a 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 b492f985d2fe..2794859657c7 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 023c22c32043..1009d03b20c0 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 @@ -173,4 +173,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 7750367f270e..9924f9137f92 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 @@ -341,6 +341,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/DimensionalObjectProducer.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DimensionalObjectProducer.java index 26003d6d249b..4a39d31c35f4 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DimensionalObjectProducer.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DimensionalObjectProducer.java @@ -422,6 +422,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 41b94c9f5b70..2b8c1ca80c9d 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 @@ -1294,13 +1294,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) @@ -1308,7 +1309,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 1300bd58f26d..8dc3e5933419 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.DataQueryService; @@ -331,7 +335,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); } @@ -498,6 +502,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) { @@ -523,6 +535,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); } } @@ -530,6 +543,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 a490416b7c96..4700b41827c9 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 @@ -42,6 +42,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; @@ -860,6 +861,23 @@ void testGetSelectClauseForAggregatedEnrollments() { assertEquals("select pi,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 006639f51aab..699719fed1c1 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 + } }