feat: skipAnalytics and ConfidentialFlag only for QUERY Analytics [DH…

…IS2-18259] (#19350) * feat: don't skipAnalytics and Confidential flag only for query Analytics [DHIS2-18259] Signed-off-by: Giuseppe Nespolino <[email protected]> * fix: e2e tests, peer reviews [DHIS2-18259] Signed-off-by: Giuseppe Nespolino <[email protected]> * fix: e2e tests, peer reviews [DHIS2-18259] Signed-off-by: Giuseppe Nespolino <[email protected]> * fix: peer review, merge conflicts [DHIS2-18259] Signed-off-by: Giuseppe Nespolino <[email protected]> * test: unit test [DHIS2-18259] Signed-off-by: Giuseppe Nespolino <[email protected]> --------- Signed-off-by: Giuseppe Nespolino <[email protected]>
dhis2 · Dec 3, 2024 · dc0fa5e · dc0fa5e
1 parent 9b2ce34
commit dc0fa5e
Show file tree

Hide file tree

Showing 15 changed files with 271 additions and 89 deletions.
diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/DimensionalItemObject.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/DimensionalItemObject.java
@@ -87,4 +87,8 @@ public interface DimensionalItemObject extends NameableObject {
   default int getPeriodOffset() {
     return (getQueryMods() != null) ? getQueryMods().getPeriodOffset() : 0;
   }
+
+  default boolean isOfType(DimensionItemType type) {
+    return type == getDimensionItemType();
+  }
 }
diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java
@@ -505,6 +505,9 @@ public enum ErrorCode {
   E7236("Program stage '{0}' is not associated to program '{0}'"),
   E7237("Sorting must have a valid dimension and a direction"),
   E7238("Sorting dimension ‘{0}’ is not a column"),
+  E7239(
+      "Tracked Entity Attributes marked as 'confidential' can only be used in aggregate analytics: `{0}`"),
+  E7240("Data Elements marked as 'skipAnalytics' can only be used in aggregate analytics: `{0}`"),
 
   /* TE analytics */
   E7250("Dimension is not a fully qualified: `{0}`"),

diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/program/Program.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/program/Program.java
@@ -37,11 +37,9 @@
 import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlProperty;
 import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlRootElement;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
 import org.hisp.dhis.category.CategoryCombo;
@@ -262,26 +260,12 @@ public Set<DataElement> getDataElements() {
     return programStages.stream().flatMap(ps -> ps.getDataElements().stream()).collect(toSet());
   }
 
-  /**
-   * Returns all data elements which are part of the stages of this program and is not skipped in
-   * analytics.
-   */
-  public Set<DataElement> getAnalyticsDataElements() {
-    return programStages.stream()
-        .map(ProgramStage::getProgramStageDataElements)
-        .flatMap(Collection::stream)
-        .filter(Objects::nonNull)
-        .filter(psde -> !psde.getSkipAnalytics())
-        .map(ProgramStageDataElement::getDataElement)
-        .collect(toSet());
-  }
-
   /**
    * Returns data elements which are part of the stages of this program which have a legend set and
    * is of numeric value type.
    */
-  public Set<DataElement> getAnalyticsDataElementsWithLegendSet() {
-    return getAnalyticsDataElements().stream()
+  public Set<DataElement> getDataElementsWithLegendSet() {
+    return getDataElements().stream()
         .filter(de -> de.hasLegendSet() && de.isNumericType())
         .collect(toSet());
   }
@@ -296,23 +280,13 @@ public List<TrackedEntityAttribute> getTrackedEntityAttributes() {
         .collect(Collectors.toList());
   }
 
-  /**
-   * Returns non-confidential TrackedEntityAttributes from ProgramTrackedEntityAttributes. Use
-   * getAttributes() to access the persisted attribute list.
-   */
-  public List<TrackedEntityAttribute> getNonConfidentialTrackedEntityAttributes() {
-    return getTrackedEntityAttributes().stream()
-        .filter(a -> !a.isConfidentialBool())
-        .collect(Collectors.toList());
-  }
-
   /**
    * Returns TrackedEntityAttributes from ProgramTrackedEntityAttributes which have a legend set and
    * is of numeric value type.
    */
-  public List<TrackedEntityAttribute> getNonConfidentialTrackedEntityAttributesWithLegendSet() {
+  public List<TrackedEntityAttribute> getTrackedEntityAttributesWithLegendSet() {
     return getTrackedEntityAttributes().stream()
-        .filter(a -> !a.isConfidentialBool() && a.hasLegendSet() && a.isNumericType())
+        .filter(a -> a.hasLegendSet() && a.isNumericType())
         .collect(Collectors.toList());
   }
 

diff --git a/dhis-2/dhis-api/src/test/java/org/hisp/dhis/program/ProgramTest.java b/dhis-2/dhis-api/src/test/java/org/hisp/dhis/program/ProgramTest.java
@@ -85,7 +85,6 @@ void testGetAnalyticsDataElements() {
     assertEquals(2, prA.getDataElements().size());
     assertTrue(prA.getDataElements().contains(deA));
     assertTrue(prA.getDataElements().contains(deB));
-    assertEquals(1, prA.getAnalyticsDataElements().size());
     assertTrue(prA.getDataElements().contains(deA));
   }
 
@@ -106,7 +105,6 @@ void testCopyOfWithPropertyValuesSet() {
     // check equal
     assertEquals(original.getAccess(), copy.getAccess());
     assertEquals(original.getAccessLevel(), copy.getAccessLevel());
-    assertEquals(original.getAnalyticsDataElements(), copy.getAnalyticsDataElements());
     assertEquals(original.getCategoryCombo(), copy.getCategoryCombo());
     assertEquals(original.getCompleteEventsExpiryDays(), copy.getCompleteEventsExpiryDays());
     assertEquals(original.getDataElements(), copy.getDataElements());
@@ -180,10 +178,7 @@ void testCopyOfWithNulls() {
     assertEquals("copynull", copy.getName());
     assertEquals(original.getAccessLevel(), copy.getAccessLevel());
     assertEquals(original.getDescription(), copy.getDescription());
-    assertTrue(copy.getAnalyticsDataElements().isEmpty());
     assertTrue(copy.getDataElements().isEmpty());
-    assertTrue(copy.getNonConfidentialTrackedEntityAttributes().isEmpty());
-    assertTrue(copy.getNonConfidentialTrackedEntityAttributesWithLegendSet().isEmpty());
     assertTrue(copy.getNotificationTemplates().isEmpty());
     assertTrue(copy.getOrganisationUnits().isEmpty());
     assertTrue(copy.getProgramAttributes().isEmpty());

diff --git a/.../dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/EventQueryParams.java b/.../dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/EventQueryParams.java
@@ -37,6 +37,7 @@
 import static org.hisp.dhis.common.DimensionalObject.PERIOD_DIM_ID;
 import static org.hisp.dhis.common.DimensionalObjectUtils.asList;
 import static org.hisp.dhis.common.DimensionalObjectUtils.asTypedList;
+import static org.hisp.dhis.common.RequestTypeAware.EndpointAction.AGGREGATE;
 import static org.hisp.dhis.common.RequestTypeAware.EndpointAction.QUERY;
 
 import com.google.common.base.MoreObjects;
@@ -998,6 +999,10 @@ public boolean isRowContext() {
     return rowContext;
   }
 
+  public boolean includeConfidentialOrSkipAnalyticsItems() {
+    return endpointAction == AGGREGATE;
+  }
+
   // -------------------------------------------------------------------------
   // Builder of immutable instances
   // -------------------------------------------------------------------------

diff --git a/.../java/org/hisp/dhis/analytics/event/data/DefaultEnrollmentAnalyticsDimensionsService.java b/.../java/org/hisp/dhis/analytics/event/data/DefaultEnrollmentAnalyticsDimensionsService.java
@@ -31,22 +31,26 @@
 import static org.hisp.dhis.analytics.common.DimensionsServiceCommon.OperationType.QUERY;
 import static org.hisp.dhis.analytics.common.DimensionsServiceCommon.collectDimensions;
 import static org.hisp.dhis.analytics.common.DimensionsServiceCommon.filterByValueType;
+import static org.hisp.dhis.analytics.event.data.DefaultEventAnalyticsDimensionsService.getTeasIfRegistration;
 import static org.hisp.dhis.common.PrefixedDimensions.ofItemsWithProgram;
 import static org.hisp.dhis.common.PrefixedDimensions.ofProgramStageDataElements;
 
 import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import lombok.RequiredArgsConstructor;
 import org.hisp.dhis.analytics.common.DimensionsServiceCommon;
+import org.hisp.dhis.analytics.common.DimensionsServiceCommon.OperationType;
 import org.hisp.dhis.analytics.event.EnrollmentAnalyticsDimensionsService;
 import org.hisp.dhis.common.PrefixedDimension;
 import org.hisp.dhis.program.Program;
 import org.hisp.dhis.program.ProgramService;
 import org.hisp.dhis.program.ProgramStage;
+import org.hisp.dhis.program.ProgramStageDataElement;
 import org.hisp.dhis.security.acl.AclService;
-import org.hisp.dhis.trackedentity.TrackedEntityAttribute;
 import org.hisp.dhis.user.CurrentUserUtil;
 import org.hisp.dhis.user.UserDetails;
 import org.springframework.stereotype.Service;
@@ -77,24 +81,30 @@ public List<PrefixedDimension> getQueryDimensionsByProgramId(String programId) {
                                 .collect(Collectors.toSet())),
                         getProgramStageDataElements(QUERY, program),
                         filterByValueType(
-                            QUERY,
-                            ofItemsWithProgram(
-                                program, getTeasIfRegistrationAndNotConfidential(program))))))
+                            QUERY, ofItemsWithProgram(program, getTeasIfRegistration(program))))))
         .orElse(List.of());
   }
 
   private Collection<PrefixedDimension> getProgramStageDataElements(
       DimensionsServiceCommon.OperationType operationType, Program program) {
     return program.getProgramStages().stream()
         .map(ProgramStage::getProgramStageDataElements)
-        .map(
-            programStageDataElements ->
-                filterByValueType(
-                    operationType, ofProgramStageDataElements(programStageDataElements)))
+        .map(psdes -> excludeIfSkipAnalytics(operationType, psdes))
+        .map(psdes -> filterByValueType(operationType, ofProgramStageDataElements(psdes)))
         .flatMap(Collection::stream)
         .collect(Collectors.toList());
   }
 
+  private Set<ProgramStageDataElement> excludeIfSkipAnalytics(
+      OperationType operationType, Set<ProgramStageDataElement> programStageDataElements) {
+    if (operationType == QUERY) {
+      return programStageDataElements.stream()
+          .filter(Predicate.not(ProgramStageDataElement::getSkipAnalytics))
+          .collect(Collectors.toSet());
+    }
+    return programStageDataElements;
+  }
+
   @Override
   public List<PrefixedDimension> getAggregateDimensionsByProgramStageId(String programId) {
     return Optional.of(programId)
@@ -109,19 +119,4 @@ public List<PrefixedDimension> getAggregateDimensionsByProgramStageId(String pro
                             ofItemsWithProgram(program, program.getTrackedEntityAttributes())))))
         .orElse(List.of());
   }
-
-  private Collection<TrackedEntityAttribute> getTeasIfRegistrationAndNotConfidential(
-      Program program) {
-    return Optional.of(program)
-        .filter(Program::isRegistration)
-        .map(Program::getTrackedEntityAttributes)
-        .orElse(List.of())
-        .stream()
-        .filter(this::isNotConfidential)
-        .collect(Collectors.toList());
-  }
-
-  private boolean isNotConfidential(TrackedEntityAttribute trackedEntityAttribute) {
-    return !trackedEntityAttribute.isConfidentialBool();
-  }
 }
diff --git a/.../main/java/org/hisp/dhis/analytics/event/data/DefaultEventAnalyticsDimensionsService.java b/.../main/java/org/hisp/dhis/analytics/event/data/DefaultEventAnalyticsDimensionsService.java
@@ -135,9 +135,7 @@ private List<PrefixedDimension> dimensions(ProgramStage programStage) {
                                 .filter(pi -> aclService.canRead(currentUserDetails, pi))
                                 .collect(Collectors.toSet())),
                         filterByValueType(QUERY, ofDataElements(programStage)),
-                        filterByValueType(
-                            QUERY,
-                            ofItemsWithProgram(p, getTeasIfRegistrationAndNotConfidential(p))),
+                        filterByValueType(QUERY, ofItemsWithProgram(p, getTeasIfRegistration(p))),
                         ofItemsWithProgram(p, getCategories(p)),
                         ofItemsWithProgram(p, getAttributeCategoryOptionGroupSetsIfNeeded(p)))))
         .orElse(List.of());
@@ -187,17 +185,10 @@ private List<Category> getCategories(Program program) {
         .orElse(List.of());
   }
 
-  private List<TrackedEntityAttribute> getTeasIfRegistrationAndNotConfidential(Program program) {
+  static List<TrackedEntityAttribute> getTeasIfRegistration(Program program) {
     return Optional.of(program)
         .filter(Program::isRegistration)
         .map(Program::getTrackedEntityAttributes)
-        .orElse(List.of())
-        .stream()
-        .filter(this::isNotConfidential)
-        .collect(Collectors.toList());
-  }
-
-  private boolean isNotConfidential(TrackedEntityAttribute trackedEntityAttribute) {
-    return !trackedEntityAttribute.isConfidentialBool();
+        .orElse(List.of());
   }
 }
diff --git a/...lytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java b/...lytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java
@@ -36,6 +36,8 @@
 import static org.hisp.dhis.analytics.event.data.DefaultEventDataQueryService.SortableItems.translateItemIfNecessary;
 import static org.hisp.dhis.analytics.util.AnalyticsUtils.illegalQueryExSupplier;
 import static org.hisp.dhis.analytics.util.AnalyticsUtils.throwIllegalQueryEx;
+import static org.hisp.dhis.common.DimensionItemType.DATA_ELEMENT;
+import static org.hisp.dhis.common.DimensionItemType.PROGRAM_ATTRIBUTE;
 import static org.hisp.dhis.common.DimensionalObject.DIMENSION_NAME_SEP;
 import static org.hisp.dhis.common.DimensionalObject.PERIOD_DIM_ID;
 import static org.hisp.dhis.common.DimensionalObjectUtils.getDimensionFromParam;
@@ -52,6 +54,7 @@
 import java.util.Set;
 import java.util.UUID;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import lombok.Getter;
 import lombok.RequiredArgsConstructor;
 import org.apache.commons.lang3.StringUtils;
@@ -67,6 +70,7 @@
 import org.hisp.dhis.analytics.table.EnrollmentAnalyticsColumnName;
 import org.hisp.dhis.analytics.table.EventAnalyticsColumnName;
 import org.hisp.dhis.common.BaseDimensionalItemObject;
+import org.hisp.dhis.common.BaseIdentifiableObject;
 import org.hisp.dhis.common.DimensionalItemObject;
 import org.hisp.dhis.common.DimensionalObject;
 import org.hisp.dhis.common.EventAnalyticalObject;
@@ -90,6 +94,7 @@
 import org.hisp.dhis.program.Program;
 import org.hisp.dhis.program.ProgramService;
 import org.hisp.dhis.program.ProgramStage;
+import org.hisp.dhis.program.ProgramStageDataElement;
 import org.hisp.dhis.program.ProgramStageService;
 import org.hisp.dhis.setting.UserSettings;
 import org.hisp.dhis.trackedentity.TrackedEntityAttribute;
@@ -223,9 +228,73 @@ public EventQueryParams getFromRequest(EventDataQueryRequest request, boolean an
       eventQueryParams = builder.build();
     }
 
+    validateQueryParamsForConfidentialAndSkipAnalytics(eventQueryParams);
+
     return eventQueryParams;
   }
 
+  static void validateQueryParamsForConfidentialAndSkipAnalytics(
+      EventQueryParams eventQueryParams) {
+    if (eventQueryParams.includeConfidentialOrSkipAnalyticsItems()) {
+      return;
+    }
+    Set<TrackedEntityAttribute> confidentialAttributes =
+        Stream.concat(
+                eventQueryParams.getItems().stream(), eventQueryParams.getItemFilters().stream())
+            .map(QueryItem::getItem)
+            .filter(Objects::nonNull)
+            .filter(dimObj -> dimObj.isOfType(PROGRAM_ATTRIBUTE))
+            .map(TrackedEntityAttribute.class::cast)
+            .filter(TrackedEntityAttribute::isConfidentialBool)
+            .collect(Collectors.toSet());
+
+    if (!confidentialAttributes.isEmpty()) {
+      throw new IllegalQueryException(
+          new ErrorMessage(
+              ErrorCode.E7239,
+              confidentialAttributes.stream()
+                  .map(TrackedEntityAttribute::getUid)
+                  .collect(Collectors.joining(", "))));
+    }
+
+    Set<DataElement> skipAnalyticsDataElements =
+        Stream.concat(
+                eventQueryParams.getItems().stream(), eventQueryParams.getItemFilters().stream())
+            .filter(item -> item.getItem().isOfType(DATA_ELEMENT))
+            .filter(DefaultEventDataQueryService::isSkipAnalytics)
+            .map(item -> (DataElement) item.getItem())
+            .collect(Collectors.toSet());
+
+    if (!skipAnalyticsDataElements.isEmpty()) {
+      throw new IllegalQueryException(
+          new ErrorMessage(
+              ErrorCode.E7240,
+              skipAnalyticsDataElements.stream()
+                  .map(DataElement::getUid)
+                  .collect(Collectors.joining(", "))));
+    }
+  }
+
+  /**
+   * Checks if the data element is marked as skip analytics by looking at the program stage data
+   * elements associated with the program stage. If any of the program stage data elements has the
+   * skip analytics flag set to true, the data element in it is considered to be skipAnalytics.
+   *
+   * @param item the query item
+   * @return true if the data element is marked as skip analytics, false otherwise
+   */
+  static boolean isSkipAnalytics(QueryItem item) {
+    return Optional.of(item)
+        .map(QueryItem::getProgramStage)
+        .map(ProgramStage::getProgramStageDataElements)
+        .orElse(Set.of())
+        .stream()
+        .filter(ProgramStageDataElement::getSkipAnalytics)
+        .map(ProgramStageDataElement::getDataElement)
+        .map(BaseIdentifiableObject::getUid)
+        .anyMatch(uid -> uid.equals(item.getItem().getUid()));
+  }
+
   private boolean hasPeriodDimension(EventQueryParams eventQueryParams) {
     return Objects.nonNull(getPeriodDimension(eventQueryParams));
   }

diff --git a/...-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java b/...-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java
@@ -217,7 +217,7 @@ protected void populateTableInternal(AnalyticsTablePartition partition, String f
    * Returns a list of columns based on the given attribute.
    *
    * @param attribute the {@link TrackedEntityAttribute}.
-   * @return a list of {@link AnaylyticsTableColumn}.
+   * @return a list of {@link AnalyticsTableColumn}.
    */
   protected List<AnalyticsTableColumn> getColumnForAttribute(TrackedEntityAttribute attribute) {
     List<AnalyticsTableColumn> columns = new ArrayList<>();

diff --git a/...tics/src/main/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManager.java b/...tics/src/main/java/org/hisp/dhis/analytics/table/JdbcEnrollmentAnalyticsTableManager.java
@@ -190,7 +190,7 @@ private List<AnalyticsTableColumn> getColumns(Program program) {
    * @return a list of {@link AnalyticsTableColumn}.
    */
   private List<AnalyticsTableColumn> getTrackedEntityAttributeColumns(Program program) {
-    return program.getNonConfidentialTrackedEntityAttributes().stream()
+    return program.getTrackedEntityAttributes().stream()
         .map(this::getColumnForAttribute)
         .flatMap(Collection::stream)
         .toList();