Skip to content

Commit

Permalink
fix: order of headers in TE analytics endpoint - Improve Data Visuali…
Browse files Browse the repository at this point in the history
…zer with fixed period "Weekly (Start Sunday)" labels [DHIS2-17974] [2.42-DHIS2-17955] (#18475)

* fix: fixes deprecated field usage [DHIS2-17908]

Signed-off-by: Giuseppe Nespolino <[email protected]>

* Improve Data Visualizer with fixed period "Weekly (Start Sunday)" labels[2.42-DHIS2-17955]

* Improve Data Visualizer with fixed period "Weekly (Start Sunday)" labels[2.42-DHIS2-17955]

* e2e tests

* NoAnalyticsTable e2e tests fix

* NoAnalyticsTable e2e tests fix

* NoAnalyticsTable e2e tests fix

* decoded urls

* fix: fixes deprecated field usage [DHIS2-17908]

Signed-off-by: Giuseppe Nespolino <[email protected]>

* fix: formatting [DHIS2-17974]

Signed-off-by: Giuseppe Nespolino <[email protected]>

---------

Signed-off-by: Giuseppe Nespolino <[email protected]>
Co-authored-by: d-bernat <[email protected]>
  • Loading branch information
gnespolino and d-bernat authored Aug 29, 2024
1 parent 49942a2 commit b63da1d
Show file tree
Hide file tree
Showing 11 changed files with 640 additions and 223 deletions.
15 changes: 14 additions & 1 deletion dhis-2/dhis-api/src/main/java/org/hisp/dhis/i18n/I18nFormat.java
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,17 @@ public String formatDateTime(Date date) {
* @param period the value to format.
*/
public String formatPeriod(Period period) {
return formatPeriod(period, false);
}

/**
* Formats a period. Returns null if value is null. Returns INVALID_DATE if formatting string is
* invalid.
*
* @param period the value to format.
* @param shortVersion the format for a Name or a shortName.
*/
public String formatPeriod(Period period, boolean shortVersion) {
if (period == null) {
return null;
}
Expand Down Expand Up @@ -282,7 +293,9 @@ public String formatPeriod(Period period) {

if (isWeeklyPeriodType(periodType)) {
return String.format(
"Week %s %d-%02d-%02d - %d-%02d-%02d",
shortVersion
? "W%s %d-%02d-%02d - %d-%02d-%02d"
: "Week %s %d-%02d-%02d - %d-%02d-%02d",
week,
startDate.getYear(),
startDate.getMonth().getValue(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ public CommonParsedParams parse(@Nonnull CommonRequestParams request) {
// Adds all program attributes from all applicable programs as dimensions.
request
.getInternal()
.setProgramAttributes(
getProgramAttributes(programs).map(IdentifiableObject::getUid).collect(toSet()));
.getProgramAttributes()
.addAll(getProgramAttributes(programs).map(IdentifiableObject::getUid).toList());

return CommonParsedParams.builder()
.programs(programs)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@
import static org.hisp.dhis.period.PeriodType.getCalendar;
import static org.hisp.dhis.period.PeriodType.getPeriodFromIsoString;
import static org.hisp.dhis.period.RelativePeriods.getRelativePeriodsFromEnum;
import static org.hisp.dhis.period.WeeklyPeriodType.NAME;
import static org.hisp.dhis.setting.SettingKey.ANALYTICS_FINANCIAL_YEAR_START;
import static org.hisp.dhis.user.CurrentUserUtil.getCurrentUserDetails;

Expand Down Expand Up @@ -337,12 +336,10 @@ private void overridePeriodAttributes(List<Period> periods, Calendar calendar) {

for (Period period : periods) {
String name = format != null ? format.formatPeriod(period) : null;

if (!period.getPeriodType().getName().contains(NAME)) {
period.setShortName(name);
}
String shortName = format != null ? format.formatPeriod(period, true) : null;

period.setName(name);
period.setShortName(shortName);

if (!calendar.isIso8601()) {
period.setUid(getLocalPeriodIdentifier(period, calendar));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,9 @@ public TrackedEntityQueryParams map(
// Adding tracked entity type attributes to the list of dimensions.
requestParams
.getInternal()
.setEntityTypeAttributes(
getTrackedEntityAttributes(trackedEntityType)
.map(IdentifiableObject::getUid)
.collect(toSet()));
.getEntityTypeAttributes()
.addAll(
getTrackedEntityAttributes(trackedEntityType).map(IdentifiableObject::getUid).toList());

return TrackedEntityQueryParams.builder().trackedEntityType(trackedEntityType).build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import static org.hisp.dhis.common.ValueType.ORGANISATION_UNIT;
import static org.hisp.dhis.common.ValueType.REFERENCE;

import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.List;
Expand Down Expand Up @@ -76,6 +77,10 @@
*/
@NoArgsConstructor(access = PRIVATE)
public class TrackedEntityFields {

private static final Comparator<TrackedEntityAttribute> CASE_INSENSITIVE_UID_COMPARATOR =
(o1, o2) -> o1.getUid().compareToIgnoreCase(o2.getUid());

/**
* Retrieves all TEAs attributes from the given param encapsulating them into a stream of {@link
* Field}.
Expand All @@ -89,13 +94,9 @@ public static Stream<Field> getDimensionFields(
CommonParsedParams parsedParams = contextParams.getCommonParsed();

return Stream.concat(
parsedParams.getPrograms().stream()
.map(Program::getProgramAttributes)
.flatMap(List::stream)
.map(ProgramTrackedEntityAttribute::getAttribute)
.map(TrackedEntityAttribute::getUid),
trackedEntityQueryParams.getTrackedEntityType().getTrackedEntityAttributes().stream()
.map(TrackedEntityAttribute::getUid))
getProgramAttributes(parsedParams.getPrograms()),
getTrackedEntityAttributes(trackedEntityQueryParams.getTrackedEntityType()))
.map(TrackedEntityAttribute::getUid)
.distinct()
.map(attr -> Field.of(TRACKED_ENTITY_ALIAS, () -> attr, attr));
}
Expand All @@ -111,7 +112,8 @@ public static Stream<TrackedEntityAttribute> getProgramAttributes(List<Program>
return programs.stream()
.map(Program::getProgramAttributes)
.flatMap(List::stream)
.map(ProgramTrackedEntityAttribute::getAttribute);
.map(ProgramTrackedEntityAttribute::getAttribute)
.sorted(CASE_INSENSITIVE_UID_COMPARATOR);
}

/**
Expand All @@ -123,7 +125,8 @@ public static Stream<TrackedEntityAttribute> getProgramAttributes(List<Program>
public static Stream<TrackedEntityAttribute> getTrackedEntityAttributes(
TrackedEntityType trackedEntityType) {
if (trackedEntityType != null) {
return trackedEntityType.getTrackedEntityAttributes().stream();
return trackedEntityType.getTrackedEntityAttributes().stream()
.sorted(CASE_INSENSITIVE_UID_COMPARATOR);
}

return Stream.empty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ void testAggregateAnalyticsWhenAnalyticsTablesAreMissing() {
ApiResponse response = analyticsAggregateActions.get(params);

// Then
assertNoAnalyticsTableResponse(response);
assertNoAnalyticsTableResponse(
response, "ERROR: relation \"analytics\" does not exist\n Position: 68");
}

@Test
Expand All @@ -96,7 +97,9 @@ void testEventsQueryAnalyticsWhenAnalyticsTablesAreMissing() {
ApiResponse response = analyticsEventActions.query().get("eBAyeGv0exc", JSON, JSON, params);

// Then
assertNoAnalyticsTableResponse(response);
assertNoAnalyticsTableResponse(
response,
"ERROR: relation \"analytics_event_ebayegv0exc\" does not exist\n Position: 263");
}

@Test
Expand All @@ -109,7 +112,9 @@ void testEnrollmentsQueryAnalyticsWhenAnalyticsTablesAreMissing() {
analyticsEnrollmentsActions.query().get("IpHINAT79UW", JSON, JSON, params);

// Then
assertNoAnalyticsTableResponse(response);
assertNoAnalyticsTableResponse(
response,
"ERROR: relation \"analytics_enrollment_iphinat79uw\" does not exist\n Position: 241");
}

@Test
Expand All @@ -124,7 +129,8 @@ void testEventsAggregateAnalyticsWhenAnalyticsTablesAreMissing() {
ApiResponse response = analyticsEventActions.aggregate().get("IpHINAT79UW", JSON, JSON, params);

// Then
assertNoAnalyticsTableResponse(response);
assertNoAnalyticsTableResponse(
response, "ERROR: relation \"analytics_event_iphinat79uw\" does not exist\n Position: 68");
}

@Test
Expand All @@ -140,7 +146,8 @@ public void queryWithProgramAndEnrollmentDateAndInvalidEnrollmentOffset() {
analyticsTrackedEntityActions.query().get("nEenWmSyUEp", JSON, JSON, params);

// Then
assertNoAnalyticsTableResponse(response);
assertNoAnalyticsTableResponse(
response, "ERROR: relation \"analytics_te_neenwmsyuep\" does not exist\n Position: 1955");
}

@Test
Expand Down Expand Up @@ -170,7 +177,7 @@ public void testOutliersAnalyticsWhenOutliersDataAreMissing() {
.body("errorCode", equalTo("E7180"));
}

private void assertNoAnalyticsTableResponse(ApiResponse response) {
private void assertNoAnalyticsTableResponse(ApiResponse response, String expectedMessage) {
response
.validate()
.statusCode(409)
Expand All @@ -180,6 +187,6 @@ private void assertNoAnalyticsTableResponse(ApiResponse response) {
equalTo(
"Query failed because a referenced table does not exist. Please ensure analytics job was run (SqlState: 42P01)"))
.body("errorCode", equalTo("E7144"))
.body("devMessage", equalTo("SqlState: 42P01"));
.body("devMessage", equalTo(expectedMessage));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
/*
* Copyright (c) 2004-2024, University of Oslo
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
* Redistributions of source code must retain the above copyright notice, this
* list of conditions and the following disclaimer.
*
* Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
* Neither the name of the HISP project nor the names of its contributors may
* be used to endorse or promote products derived from this software without
* specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
* ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
package org.hisp.dhis.analytics.aggregate;

import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
import static org.hisp.dhis.analytics.ValidationHelper.validateHeader;
import static org.hisp.dhis.analytics.ValidationHelper.validateRow;
import static org.skyscreamer.jsonassert.JSONAssert.assertEquals;

import java.util.List;
import java.util.Map;
import org.hisp.dhis.AnalyticsApiTest;
import org.hisp.dhis.test.e2e.actions.RestApiActions;
import org.hisp.dhis.test.e2e.dto.ApiResponse;
import org.hisp.dhis.test.e2e.helpers.QueryParamsBuilder;
import org.json.JSONException;
import org.json.JSONObject;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

/** Groups e2e tests for "/analytics" aggregate endpoint. */
public class AnalyticsQueryDv13AutoTest extends AnalyticsApiTest {
private RestApiActions actions;

@BeforeAll
public void setup() {
actions = new RestApiActions("analytics");
}

@Test
public void weeklyShortNameLabel() throws JSONException {
// Given
QueryParamsBuilder params =
new QueryParamsBuilder()
.add("filter=ou:USER_ORGUNIT")
.add("skipData=false")
.add("includeNumDen=true")
.add("displayProperty=SHORTNAME")
.add("skipMeta=false")
.add("dimension=dx:fbfJHSPpUQD,pe:2021SunW52");

// When
ApiResponse response = actions.get(params);

// Then
response
.validate()
.statusCode(200)
.body("headers", hasSize(equalTo(8)))
.body("rows", hasSize(equalTo(1)))
.body("height", equalTo(1))
.body("width", equalTo(8))
.body("headerWidth", equalTo(8));

// Assert metaData.
String expectedMetaData =
"{\"items\":{\"ImspTQPwCqd\":{\"name\":\"Sierra Leone\"},\"dx\":{\"name\":\"Data\"},\"pq2XI5kz2BY\":{\"name\":\"Fixed\"},\"pe\":{\"name\":\"Period\"},\"ou\":{\"name\":\"Organisation unit\"},\"PT59n8BQbqM\":{\"name\":\"Outreach\"},\"2021SunW52\":{\"name\":\"W52 2021-12-26 - 2022-01-01\"},\"fbfJHSPpUQD\":{\"name\":\"ANC 1st visit\"}},\"dimensions\":{\"dx\":[\"fbfJHSPpUQD\"],\"pe\":[\"2021SunW52\"],\"ou\":[\"ImspTQPwCqd\"],\"co\":[\"pq2XI5kz2BY\",\"PT59n8BQbqM\"]}}";
String actualMetaData = new JSONObject((Map) response.extract("metaData")).toString();
assertEquals(expectedMetaData, actualMetaData, false);

// Assert headers.
validateHeader(response, 0, "dx", "Data", "TEXT", "java.lang.String", false, true);
validateHeader(response, 1, "pe", "Period", "TEXT", "java.lang.String", false, true);
validateHeader(response, 2, "value", "Value", "NUMBER", "java.lang.Double", false, false);
validateHeader(
response, 3, "numerator", "Numerator", "NUMBER", "java.lang.Double", false, false);
validateHeader(
response, 4, "denominator", "Denominator", "NUMBER", "java.lang.Double", false, false);
validateHeader(response, 5, "factor", "Factor", "NUMBER", "java.lang.Double", false, false);
validateHeader(
response, 6, "multiplier", "Multiplier", "NUMBER", "java.lang.Double", false, false);
validateHeader(response, 7, "divisor", "Divisor", "NUMBER", "java.lang.Double", false, false);

// Assert rows.
validateRow(response, List.of("fbfJHSPpUQD", "2021SunW52", "462", "", "", "", "", ""));
}

@Test
public void weeklyNameLabel() throws JSONException {
// Given
QueryParamsBuilder params =
new QueryParamsBuilder()
.add("filter=ou:USER_ORGUNIT")
.add("skipData=false")
.add("includeNumDen=true")
.add("displayProperty=NAME")
.add("skipMeta=false")
.add("dimension=dx:fbfJHSPpUQD,pe:2021SunW52");

// When
ApiResponse response = actions.get(params);

// Then
response
.validate()
.statusCode(200)
.body("headers", hasSize(equalTo(8)))
.body("rows", hasSize(equalTo(1)))
.body("height", equalTo(1))
.body("width", equalTo(8))
.body("headerWidth", equalTo(8));

// Assert metaData.
String expectedMetaData =
"{\"items\":{\"ImspTQPwCqd\":{\"name\":\"Sierra Leone\"},\"dx\":{\"name\":\"Data\"},\"pq2XI5kz2BY\":{\"name\":\"Fixed\"},\"pe\":{\"name\":\"Period\"},\"ou\":{\"name\":\"Organisation unit\"},\"PT59n8BQbqM\":{\"name\":\"Outreach\"},\"2021SunW52\":{\"name\":\"Week 52 2021-12-26 - 2022-01-01\"},\"fbfJHSPpUQD\":{\"name\":\"ANC 1st visit\"}},\"dimensions\":{\"dx\":[\"fbfJHSPpUQD\"],\"pe\":[\"2021SunW52\"],\"ou\":[\"ImspTQPwCqd\"],\"co\":[\"pq2XI5kz2BY\",\"PT59n8BQbqM\"]}}";
String actualMetaData = new JSONObject((Map) response.extract("metaData")).toString();
assertEquals(expectedMetaData, actualMetaData, false);

// Assert headers.
validateHeader(response, 0, "dx", "Data", "TEXT", "java.lang.String", false, true);
validateHeader(response, 1, "pe", "Period", "TEXT", "java.lang.String", false, true);
validateHeader(response, 2, "value", "Value", "NUMBER", "java.lang.Double", false, false);
validateHeader(
response, 3, "numerator", "Numerator", "NUMBER", "java.lang.Double", false, false);
validateHeader(
response, 4, "denominator", "Denominator", "NUMBER", "java.lang.Double", false, false);
validateHeader(response, 5, "factor", "Factor", "NUMBER", "java.lang.Double", false, false);
validateHeader(
response, 6, "multiplier", "Multiplier", "NUMBER", "java.lang.Double", false, false);
validateHeader(response, 7, "divisor", "Divisor", "NUMBER", "java.lang.Double", false, false);

// Assert rows.
validateRow(response, List.of("fbfJHSPpUQD", "2021SunW52", "462", "", "", "", "", ""));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,20 @@
"version": {
"min": 42
}
},
{
"name": "weeklyShortNameLabel",
"query": "/api/42/analytics?dimension=dx:fbfJHSPpUQD,pe:2021SunW52&filter=ou:USER_ORGUNIT&displayProperty=SHORTNAME&includeNumDen=true&skipMeta=false&skipData=false",
"version": {
"min": 42
}
},
{
"name": "weeklyNameLabel",
"query": "/api/42/analytics?dimension=dx:fbfJHSPpUQD,pe:2021SunW52&filter=ou:USER_ORGUNIT&displayProperty=NAME&includeNumDen=true&skipMeta=false&skipData=false\n",
"version": {
"min": 42
}
}
]
}
Loading

0 comments on commit b63da1d

Please sign in to comment.