feat: program param is optional in TEI controller [DHIS2-16051] (#15517)

* feat: program param is optional in TEI controller [DHIS2-16051] * Apply suggestions from code review Co-authored-by: Maikel Arabori <[email protected]> --------- Co-authored-by: Maikel Arabori <[email protected]>
dhis2 · Oct 27, 2023 · e1cdd70 · e1cdd70
1 parent 6f97d60
commit e1cdd70
Show file tree

Hide file tree

Showing 11 changed files with 143 additions and 49 deletions.
diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/analytics/common/CommonQueryRequest.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/analytics/common/CommonQueryRequest.java
@@ -187,10 +187,10 @@ public class CommonQueryRequest {
 
   private String lastUpdated;
 
-  /** weather the query should consider only items with lat/long coordinates */
+  /** whether the query should consider only items with lat/long coordinates */
   private boolean coordinatesOnly;
 
-  /** weather the query should consider only items with geometry */
+  /** whether the query should consider only items with geometry */
   private boolean geometryOnly;
 
   /**

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
@@ -414,7 +414,6 @@ public enum ErrorCode {
   E7133("Query cannot be executed, possibly because of invalid types or invalid operation"),
   E7134("Cannot retrieve total value for data elements with skip total category combination"),
   E7135("Date time is not parsable: `{0}`"),
-  E7136("Program is not specified"),
   E7137("Expression is not parsable: `{0}`"),
   E7138("Invalid offset: `{0}`"),
   E7139("Parameters programStatus and enrollmentStatus cannot be used together"),

diff --git a/...s-service-analytics/src/main/java/org/hisp/dhis/analytics/common/params/CommonParams.java b/...s-service-analytics/src/main/java/org/hisp/dhis/analytics/common/params/CommonParams.java
@@ -42,6 +42,7 @@
 import lombok.Builder;
 import lombok.Getter;
 import lombok.Setter;
+import lombok.With;
 import org.hisp.dhis.analytics.common.params.dimension.DimensionIdentifier;
 import org.hisp.dhis.analytics.common.params.dimension.DimensionParam;
 import org.hisp.dhis.common.DimensionalItemObject;
@@ -148,12 +149,15 @@ public class CommonParams {
   /** Indicates if additional ou hierarchy data should be provided. */
   private final boolean showHierarchy;
 
-  /** weather the query should consider only items with lat/long coordinates */
+  /** whether the query should consider only items with lat/long coordinates */
   private boolean coordinatesOnly;
 
-  /** weather the query should consider only items with geometry */
+  /** whether the query should consider only items with geometry */
   private boolean geometryOnly;
 
+  /** whether the programs comes from the request or not */
+  @With private final boolean programsFromRequest;
+
   /**
    * Indicates whether this query defines a master identifier scheme different from the default
    * (UID).

diff --git a/...alytics/src/main/java/org/hisp/dhis/analytics/common/params/dimension/DimensionParam.java b/...alytics/src/main/java/org/hisp/dhis/analytics/common/params/dimension/DimensionParam.java
@@ -84,7 +84,7 @@ public class DimensionParam implements UidObject {
    * @param dimensionalObjectOrQueryItem either a {@link DimensionalObject} or {@link QueryItem}, or
    *     a static dimension.
    * @param dimensionParamType the {@link DimensionParamType} for the {@link DimensionParam}
-   *     returned (weather it's a filter or a dimension).
+   *     returned (whether it's a filter or a dimension).
    * @param items the list of items parameters for this DimensionParam.
    * @return a new instance of {@link DimensionParam}.
    */

diff --git a/.../src/main/java/org/hisp/dhis/analytics/common/processing/CommonQueryRequestValidator.java b/.../src/main/java/org/hisp/dhis/analytics/common/processing/CommonQueryRequestValidator.java
@@ -31,7 +31,6 @@
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
 import static org.hisp.dhis.common.CodeGenerator.isValidUid;
 import static org.hisp.dhis.feedback.ErrorCode.E4014;
-import static org.hisp.dhis.feedback.ErrorCode.E7136;
 import static org.hisp.dhis.feedback.ErrorCode.E7139;
 
 import org.hisp.dhis.analytics.common.CommonQueryRequest;
@@ -51,9 +50,6 @@ public class CommonQueryRequestValidator implements Validator<CommonQueryRequest
    */
   @Override
   public void validate(CommonQueryRequest commonQueryRequest) {
-    if (!commonQueryRequest.hasPrograms()) {
-      throw new IllegalQueryException(new ErrorMessage(E7136));
-    }
 
     for (String programUid : commonQueryRequest.getProgram()) {
       if (!isValidUid(programUid)) {

diff --git a/...is-service-analytics/src/main/java/org/hisp/dhis/analytics/tei/TeiQueryRequestMapper.java b/...is-service-analytics/src/main/java/org/hisp/dhis/analytics/tei/TeiQueryRequestMapper.java
@@ -27,12 +27,12 @@
  */
 package org.hisp.dhis.analytics.tei;
 
-import static java.util.stream.Collectors.toList;
 import static org.apache.commons.collections4.CollectionUtils.isNotEmpty;
 import static org.hisp.dhis.analytics.tei.query.TeiFields.getTrackedEntityAttributes;
 import static org.hisp.dhis.feedback.ErrorCode.E7125;
 import static org.hisp.dhis.feedback.ErrorCode.E7142;
 
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -77,24 +77,41 @@ public TeiQueryParams map(QueryRequest<TeiQueryRequest> queryRequest) {
     TrackedEntityType trackedEntityType =
         getTrackedEntityType(queryRequest.getRequest().getTrackedEntityType());
 
-    checkTetForPrograms(queryRequest, trackedEntityType);
+    boolean programsFromRequest = true;
+
+    if (!queryRequest.getCommonQueryRequest().hasPrograms()) {
+      queryRequest
+          .getCommonQueryRequest()
+          .setProgram(getProgramUidsFromTrackedEntityType(trackedEntityType));
+      programsFromRequest = false;
+    } else {
+      checkTetForPrograms(queryRequest, trackedEntityType);
+    }
 
     // Adding tracked entity type attributes to the list of dimensions.
     queryRequest
         .getCommonQueryRequest()
         .getDimension()
         .addAll(
-            getTrackedEntityAttributes(trackedEntityType)
-                .map(IdentifiableObject::getUid)
-                .collect(toList()));
+            getTrackedEntityAttributes(trackedEntityType).map(IdentifiableObject::getUid).toList());
 
     return teiQueryParamPostProcessor.process(
         TeiQueryParams.builder()
             .trackedEntityType(trackedEntityType)
-            .commonParams(commonQueryRequestMapper.map(queryRequest.getCommonQueryRequest()))
+            .commonParams(
+                commonQueryRequestMapper
+                    .map(queryRequest.getCommonQueryRequest())
+                    .withProgramsFromRequest(programsFromRequest))
             .build());
   }
 
+  private Set<String> getProgramUidsFromTrackedEntityType(TrackedEntityType trackedEntityType) {
+    return programService.getAllPrograms().stream()
+        .filter(program -> matchesTet(program, trackedEntityType))
+        .map(Program::getUid)
+        .collect(Collectors.toSet());
+  }
+
   /**
    * Checks if the given programs are valid for the given tracked entity type.
    *
@@ -127,7 +144,8 @@ private void checkTetForPrograms(
    * @return true if the program matches the tracked entity type.
    */
   private boolean matchesTet(Program program, TrackedEntityType trackedEntityType) {
-    return StringUtils.equals(program.getTrackedEntityType().getUid(), trackedEntityType.getUid());
+    return Objects.nonNull(program.getTrackedEntityType())
+        && StringUtils.equals(program.getTrackedEntityType().getUid(), trackedEntityType.getUid());
   }
 
   /**

diff --git a/...ybuilder/ProgramEnrolledQueryBuilder.java → ...uilder/EnrolledInProgramQueryBuilder.java b/...ybuilder/ProgramEnrolledQueryBuilder.java → ...uilder/EnrolledInProgramQueryBuilder.java
@@ -28,6 +28,7 @@
 package org.hisp.dhis.analytics.tei.query.context.querybuilder;
 
 import java.util.List;
+import java.util.function.Function;
 import java.util.stream.Stream;
 import org.hisp.dhis.analytics.common.params.dimension.DimensionIdentifier;
 import org.hisp.dhis.analytics.common.params.dimension.DimensionParam;
@@ -43,13 +44,31 @@
  * generated conditions are "ungrouped", since each one needs to be a separate "AND" condition.
  */
 @Service
-public class ProgramEnrolledQueryBuilder extends SqlQueryBuilderAdaptor {
+public class EnrolledInProgramQueryBuilder extends SqlQueryBuilderAdaptor {
+
+  private static final String PROGRAM_ENROLLED_GROUP = "PROGRAM_ENROLLED_GROUP";
+
   @Override
   protected Stream<GroupableCondition> getWhereClauses(
       QueryContext queryContext, List<DimensionIdentifier<DimensionParam>> unused) {
+
+    boolean programsFromRequest =
+        queryContext.getTeiQueryParams().getCommonParams().isProgramsFromRequest();
+
+    Function<String, GroupableCondition> conditionMapper =
+        EnrolledInProgramQueryBuilder::asGroupedEnrolledInProgramCondition;
+
+    if (programsFromRequest) {
+      conditionMapper = EnrolledInProgramQueryBuilder::asUngroupedEnrolledInProgramCondition;
+    }
+
     return queryContext.getTeiQueryParams().getCommonParams().getPrograms().stream()
         .map(IdentifiableObject::getUid)
-        .map(ProgramEnrolledQueryBuilder::asUngroupedEnrolledInProgramCondition);
+        .map(conditionMapper);
+  }
+
+  private static GroupableCondition asGroupedEnrolledInProgramCondition(String programUid) {
+    return GroupableCondition.of(PROGRAM_ENROLLED_GROUP, EnrolledInProgramCondition.of(programUid));
   }
 
   private static GroupableCondition asUngroupedEnrolledInProgramCondition(String programUid) {

diff --git a/...ervice-analytics/src/test/java/org/hisp/dhis/analytics/tei/TeiQueryRequestMapperTest.java b/...ervice-analytics/src/test/java/org/hisp/dhis/analytics/tei/TeiQueryRequestMapperTest.java
@@ -29,12 +29,14 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import java.util.Set;
 import org.hisp.dhis.analytics.common.CommonQueryRequest;
 import org.hisp.dhis.analytics.common.QueryRequest;
+import org.hisp.dhis.analytics.common.params.CommonParams;
 import org.hisp.dhis.analytics.common.processing.CommonQueryRequestMapper;
 import org.hisp.dhis.analytics.common.processing.Processor;
 import org.hisp.dhis.common.IllegalQueryException;
@@ -53,11 +55,14 @@ class TeiQueryRequestMapperTest {
 
   private TeiQueryRequestMapper teiQueryRequestMapper;
 
+  private CommonQueryRequestMapper commonQueryRequestMapper;
+
   @BeforeEach
   public void setUp() {
+    commonQueryRequestMapper = mock(CommonQueryRequestMapper.class);
     teiQueryRequestMapper =
         new TeiQueryRequestMapper(
-            mock(CommonQueryRequestMapper.class),
+            commonQueryRequestMapper,
             trackedEntityTypeService,
             mock(Processor.class),
             programService);
@@ -119,6 +124,30 @@ void testOK() {
 
     when(programService.getPrograms(Set.of("A", "B"))).thenReturn(Set.of(programA, programB));
 
+    when(commonQueryRequestMapper.map(any())).thenReturn(CommonParams.builder().build());
+
+    TeiQueryParams mapped = teiQueryRequestMapper.map(queryRequest);
+  }
+
+  @Test
+  void testOKWhenNoPrograms() {
+    String trackedEntityTypeUid = "T1";
+
+    TrackedEntityType trackedEntityType = stubTrackedEntityType(trackedEntityTypeUid);
+
+    QueryRequest<TeiQueryRequest> queryRequest =
+        QueryRequest.<TeiQueryRequest>builder()
+            .request(new TeiQueryRequest(trackedEntityTypeUid))
+            .commonQueryRequest(new CommonQueryRequest())
+            .build();
+
+    when(trackedEntityTypeService.getTrackedEntityType(trackedEntityTypeUid))
+        .thenReturn(trackedEntityType);
+
+    when(programService.getPrograms(Set.of("A", "B"))).thenReturn(Set.of());
+
+    when(commonQueryRequestMapper.map(any())).thenReturn(CommonParams.builder().build());
+
     TeiQueryParams mapped = teiQueryRequestMapper.map(queryRequest);
   }
 

diff --git a/...ice-analytics/src/test/java/org/hisp/dhis/analytics/tei/TeiQueryRequestValidatorTest.java b/...ice-analytics/src/test/java/org/hisp/dhis/analytics/tei/TeiQueryRequestValidatorTest.java
@@ -67,32 +67,6 @@ void testValidateWithSuccess() {
     assertDoesNotThrow(() -> teiQueryRequestValidator.validate(queryRequest));
   }
 
-  @Test
-  void testValidateWhenProgramIsNotDefined() {
-    // Given
-    TeiQueryRequest teiQueryRequest = new TeiQueryRequest("uidabcdef11");
-    CommonQueryRequest commonQueryRequest = new CommonQueryRequest();
-    commonQueryRequest.setDimension(Set.of("ou:jmIPBj66vD6"));
-
-    QueryRequest<TeiQueryRequest> queryRequest =
-        QueryRequest.<TeiQueryRequest>builder()
-            .commonQueryRequest(commonQueryRequest)
-            .request(teiQueryRequest)
-            .build();
-
-    CommonQueryRequestValidator commonQueryRequestValidator = new CommonQueryRequestValidator();
-    TeiQueryRequestValidator teiQueryRequestValidator =
-        new TeiQueryRequestValidator(commonQueryRequestValidator);
-
-    // When
-    IllegalQueryException exception =
-        assertThrows(
-            IllegalQueryException.class, () -> teiQueryRequestValidator.validate(queryRequest));
-
-    // Then
-    assertEquals("Program is not specified", exception.getMessage());
-  }
-
   @Test
   void testValidateWhenPeIsDefined() {
     // Given

diff --git a/...is-service-analytics/src/test/java/org/hisp/dhis/analytics/tei/query/TeiSqlQueryTest.java b/...is-service-analytics/src/test/java/org/hisp/dhis/analytics/tei/query/TeiSqlQueryTest.java
@@ -30,6 +30,7 @@
 import static org.hisp.dhis.utils.Assertions.assertContains;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import java.util.List;
 import org.apache.commons.lang3.StringUtils;
@@ -43,11 +44,11 @@
 import org.hisp.dhis.analytics.common.params.dimension.ElementWithOffset;
 import org.hisp.dhis.analytics.tei.TeiQueryParams;
 import org.hisp.dhis.analytics.tei.query.context.querybuilder.DataElementQueryBuilder;
+import org.hisp.dhis.analytics.tei.query.context.querybuilder.EnrolledInProgramQueryBuilder;
 import org.hisp.dhis.analytics.tei.query.context.querybuilder.LimitOffsetQueryBuilder;
 import org.hisp.dhis.analytics.tei.query.context.querybuilder.MainTableQueryBuilder;
 import org.hisp.dhis.analytics.tei.query.context.querybuilder.OrgUnitQueryBuilder;
 import org.hisp.dhis.analytics.tei.query.context.querybuilder.PeriodQueryBuilder;
-import org.hisp.dhis.analytics.tei.query.context.querybuilder.ProgramEnrolledQueryBuilder;
 import org.hisp.dhis.analytics.tei.query.context.querybuilder.ProgramIndicatorQueryBuilder;
 import org.hisp.dhis.analytics.tei.query.context.querybuilder.TeiQueryBuilder;
 import org.hisp.dhis.analytics.tei.query.context.sql.SqlQueryBuilder;
@@ -79,7 +80,7 @@ void setUp() {
             new MainTableQueryBuilder(),
             new OrgUnitQueryBuilder(),
             new PeriodQueryBuilder(),
-            new ProgramEnrolledQueryBuilder(),
+            new EnrolledInProgramQueryBuilder(),
             new TeiQueryBuilder(),
             new ProgramIndicatorQueryBuilder(programIndicatorService));
     sqlQueryCreatorService = new SqlQueryCreatorService(queryBuilders);
@@ -125,6 +126,60 @@ void testSqlQueryRenderingWithCommonDimensionalObject() {
             "(\"prabcdefghA[1].pgabcdefghS[1]\".\"eventdatavalues\" -> 'abc' ->> 'value')::TEXT as \"prabcdefghA[1].pgabcdefghS[1].abc\""));
   }
 
+  @Test
+  void testEnrolledInProgramWhenSpecifiedInRequest() {
+    // given
+    CommonParams commonParams =
+        CommonParams.builder()
+            .programs(List.of(mockProgram("program1"), mockProgram("program2")))
+            .build()
+            .withProgramsFromRequest(true);
+
+    TeiQueryParams teiQueryParams =
+        TeiQueryParams.builder()
+            .trackedEntityType(createTrackedEntityType('A'))
+            .commonParams(commonParams)
+            .build();
+
+    // when
+    String sql =
+        sqlQueryCreatorService.getSqlQueryCreator(teiQueryParams).createForSelect().getStatement();
+
+    // then
+    assertTrue(sql.contains("ouname"));
+    assertContains("t_1.\"program1\" and t_1.\"program2\"", sql);
+  }
+
+  @Test
+  void testEnrolledInProgramWhenNotSpecifiedInRequest() {
+    // given
+    CommonParams commonParams =
+        CommonParams.builder()
+            .programs(List.of(mockProgram("program1"), mockProgram("program2")))
+            .build()
+            .withProgramsFromRequest(false);
+
+    TeiQueryParams teiQueryParams =
+        TeiQueryParams.builder()
+            .trackedEntityType(createTrackedEntityType('A'))
+            .commonParams(commonParams)
+            .build();
+
+    // when
+    String sql =
+        sqlQueryCreatorService.getSqlQueryCreator(teiQueryParams).createForSelect().getStatement();
+
+    // then
+    assertTrue(sql.contains("ouname"));
+    assertContains("(t_1.\"program1\" or t_1.\"program2\")", sql);
+  }
+
+  private Program mockProgram(String uid) {
+    Program program = mock(Program.class);
+    when(program.getUid()).thenReturn(uid);
+    return program;
+  }
+
   private CommonParams stubSortingCommonParams(
       Program program, int offset, Object dimensionalObject) {
     ElementWithOffset<Program> prg =

diff --git a/...ce-dxf2/src/test/java/org/hisp/dhis/dxf2/metadata/objectbundle/ObjectBundleHooksTest.java b/...ce-dxf2/src/test/java/org/hisp/dhis/dxf2/metadata/objectbundle/ObjectBundleHooksTest.java
@@ -48,7 +48,7 @@
 import org.junit.jupiter.api.Test;
 
 /**
- * Tests weather or not the {@link ObjectBundleHooks#getObjectHooks(Object)} and {@link
+ * Tests whether or not the {@link ObjectBundleHooks#getObjectHooks(Object)} and {@link
  * ObjectBundleHooks#getTypeImportHooks(Class)} methods assemble the expected lists of {@link
  * ObjectBundleHook}s.
  *