feat: CatCombo in Approval Workflow [DHIS2-11518] (#18833)

dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DefaultDataApprovalService.java

@@ -54,7 +54,6 @@
 import org.hisp.dhis.dataapproval.exceptions.DataMayNotBeApprovedException;
 import org.hisp.dhis.dataapproval.exceptions.DataMayNotBeUnacceptedException;
 import org.hisp.dhis.dataapproval.exceptions.DataMayNotBeUnapprovedException;
-import org.hisp.dhis.dataset.DataSet;
 import org.hisp.dhis.organisationunit.OrganisationUnit;
 import org.hisp.dhis.period.Period;
 import org.hisp.dhis.setting.SystemSettingsProvider;
@@ -608,8 +607,8 @@ private DataApprovalLevel nextHigherLevel(
   }
 
   /**
-   * Makes sure that for any approval we enter into the database, the attributeOptionCombo appears
-   * in the set of optionCombos for at least one of the data sets in the workflow.
+   * Makes sure that for any approval we enter into the database, the attributeOptionCombo belongs
+   * to the workflow's categoryCombo.
    *
    * @param dataApprovalList list of data approvals to test.
    */
@@ -627,10 +626,8 @@ private void validateAttributeOptionCombos(List<DataApproval> dataApprovalList)
    */
   private void validAttributeOptionCombo(
       CategoryOptionCombo attributeOptionCombo, DataApprovalWorkflow workflow) {
-    for (DataSet ds : workflow.getDataSets()) {
-      if (ds.getCategoryCombo().getOptionCombos().contains(attributeOptionCombo)) {
-        return;
-      }
+    if (workflow.getCategoryCombo().getOptionCombos().contains(attributeOptionCombo)) {
+      return;
     }
 
     log.info(

dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/dataapproval/hibernate/DataApprovalWorkflow.hbm.xml

@@ -21,7 +21,7 @@
 
     <many-to-one name="periodType" class="org.hisp.dhis.period.PeriodType" column="periodtypeid" not-null="true" foreign-key="fk_dataapprovalworkflow_periodtypeid" />
 
-    <many-to-one name="categoryCombo" class="org.hisp.dhis.category.CategoryCombo" column="categorycomboid" foreign-key="fk_dataapprovalworkflow_categorycomboid" />
+    <many-to-one name="categoryCombo" class="org.hisp.dhis.category.CategoryCombo" column="categorycomboid" not-null="true" foreign-key="fk_dataapprovalworkflow_categorycomboid" />
 
     <set name="levels" table="dataapprovalworkflowlevels">
       <cache usage="read-write" />

dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_22__Add_catCombo_values_to_dataApprovalWorkflows.sql

@@ -0,0 +1,17 @@
+-- DHIS2-11518: Add categoryCombo values to existing data approval workflows
+-- For workflows with one or more datasets assigned, assign the catCombo from one of the datasets.
+-- (All the datasets in a workflow *should* have the same catCombo but if they don't, we pick a random assigned one.)
+-- If no datasets are assigned, assign the default catCombo.
+--
+-- Note that pre-existing workflows may have a null catCombo (if they were created
+-- before the catCombo column was first added several releases ago) or they may have
+-- the defaut catCombo (if they were created since the catCombo column was added).
+update dataapprovalworkflow daw
+set categorycomboid = coalesce(
+        (select categorycomboid from dataset ds where daw.workflowid = ds.workflowid limit 1),
+        (select categorycomboid from categorycombo where name = 'default' limit 1)
+    )
+where categorycomboid is null
+   or categorycomboid = (select categorycomboid from categorycombo where name = 'default' limit 1);
+
+alter table dataapprovalworkflow alter column categorycomboid set not null;

dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dataapproval/DataApprovalAuditServiceTest.java

@@ -265,9 +265,11 @@ void setUp() {
     dataApprovalLevelService.addDataApprovalLevel(level1);
     dataApprovalLevelService.addDataApprovalLevel(level2);
     dataApprovalLevelService.addDataApprovalLevel(level3);
-    workflowA = new DataApprovalWorkflow("workflowA", periodType, newHashSet(level1));
+    workflowA =
+        new DataApprovalWorkflow("workflowA", periodType, categoryComboA, newHashSet(level1));
     workflowB =
-        new DataApprovalWorkflow("workflowB", periodType, newHashSet(level1, level2, level3));
+        new DataApprovalWorkflow(
+            "workflowB", periodType, categoryComboA, newHashSet(level1, level2, level3));
     dataApprovalService.addWorkflow(workflowA);
     dataApprovalService.addWorkflow(workflowB);
     DataApproval approvalAA1 =

dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dataapproval/DataApprovalAuditStoreTest.java

@@ -125,8 +125,12 @@ void setUp() {
     dataApprovalLevelService.addDataApprovalLevel(level1);
     dataApprovalLevelService.addDataApprovalLevel(level2);
     PeriodType periodType = PeriodType.getPeriodTypeByName("Monthly");
-    workflowA = new DataApprovalWorkflow("workflowA", periodType, newHashSet(level1));
-    workflowB = new DataApprovalWorkflow("workflowB", periodType, newHashSet(level1, level2));
+    CategoryCombo defaultCategoryCombo = categoryService.getDefaultCategoryCombo();
+    workflowA =
+        new DataApprovalWorkflow("workflowA", periodType, defaultCategoryCombo, newHashSet(level1));
+    workflowB =
+        new DataApprovalWorkflow(
+            "workflowB", periodType, defaultCategoryCombo, newHashSet(level1, level2));
     dataApprovalService.addWorkflow(workflowA);
     dataApprovalService.addWorkflow(workflowB);
     periodA = createPeriod(new MonthlyPeriodType(), getDate(2017, 1, 1), getDate(2017, 1, 31));

dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dataapproval/DataApprovalServiceCategoryOptionGroupTest.java

@@ -593,11 +593,13 @@ void setUp() {
         new DataApprovalWorkflow(
             "workflow1",
             periodType,
+            mechanismCategoryCombo,
             Sets.newHashSet(globalLevel1, countryLevel3, agencyLevel4, partnerLevel5));
     workflow2 =
         new DataApprovalWorkflow(
             "workflow2",
             periodType,
+            mechanismCategoryCombo,
             Sets.newHashSet(globalLevel1, globalAgencyLevel2, agencyLevel4, partnerLevel5));
     dataApprovalService.addWorkflow(workflow1);
     dataApprovalService.addWorkflow(workflow2);

dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dataapproval/DataApprovalServiceTest.java

@@ -28,7 +28,7 @@
 package org.hisp.dhis.dataapproval;
 
 import static com.google.common.collect.Lists.newArrayList;
-import static com.google.common.collect.Sets.newHashSet;
+import static java.util.Collections.EMPTY_SET;
 import static java.util.Collections.singleton;
 import static java.util.Collections.singletonList;
 import static org.hisp.dhis.security.acl.AccessStringHelper.CATEGORY_OPTION_DEFAULT;
@@ -43,6 +43,7 @@
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import org.hisp.dhis.category.Category;
 import org.hisp.dhis.category.CategoryCombo;
 import org.hisp.dhis.category.CategoryOption;
@@ -102,7 +103,7 @@ class DataApprovalServiceTest extends PostgresIntegrationTestBase {
   // -------------------------------------------------------------------------
   // Supporting data
   // -------------------------------------------------------------------------
-  private CategoryCombo defaultCategoryCombo;
+  private CategoryCombo defCatCombo;
 
   private CategoryOptionCombo defaultOptionCombo;
 
@@ -277,7 +278,7 @@ void setUp() {
     // ---------------------------------------------------------------------
     // Add supporting data
     // ---------------------------------------------------------------------
-    defaultCategoryCombo = categoryService.getDefaultCategoryCombo();
+    defCatCombo = categoryService.getDefaultCategoryCombo();
     defaultOptionCombo = categoryService.getDefaultCategoryOptionCombo();
     periodType = periodService.reloadPeriodType(PeriodType.getPeriodTypeByName("Monthly"));
     // Monthly: Jan
@@ -342,17 +343,17 @@ void setUp() {
     dataApprovalLevelService.addDataApprovalLevel(level2);
     dataApprovalLevelService.addDataApprovalLevel(level3);
     dataApprovalLevelService.addDataApprovalLevel(level4);
-    workflow0 = new DataApprovalWorkflow("workflow0", periodType, newHashSet());
-    workflow1 = new DataApprovalWorkflow("workflow1", periodType, newHashSet(level1));
-    workflow12 = new DataApprovalWorkflow("workflow12", periodType, newHashSet(level1, level2));
-    workflow12A = new DataApprovalWorkflow("workflow12A", periodType, newHashSet(level1, level2));
-    workflow12B = new DataApprovalWorkflow("workflow12B", periodType, newHashSet(level1, level2));
-    workflow3 = new DataApprovalWorkflow("workflow3", periodType, newHashSet(level3));
+    workflow0 = newWorkflow("workflow0", periodType, defCatCombo, EMPTY_SET);
+    workflow1 = newWorkflow("workflow1", periodType, defCatCombo, Set.of(level1));
+    workflow12 = newWorkflow("workflow12", periodType, defCatCombo, Set.of(level1, level2));
+    workflow12A = newWorkflow("workflow12A", periodType, defCatCombo, Set.of(level1, level2));
+    workflow12B = newWorkflow("workflow12B", periodType, defCatCombo, Set.of(level1, level2));
+    workflow3 = newWorkflow("workflow3", periodType, defCatCombo, Set.of(level3));
     workflow1234 =
-        new DataApprovalWorkflow(
-            "workflow1234", periodType, newHashSet(level1, level2, level3, level4));
-    workflow13 = new DataApprovalWorkflow("workflow13", periodType, newHashSet(level1, level3));
-    workflow24 = new DataApprovalWorkflow("workflow24", periodType, newHashSet(level2, level4));
+        newWorkflow(
+            "workflow1234", periodType, defCatCombo, Set.of(level1, level2, level3, level4));
+    workflow13 = newWorkflow("workflow13", periodType, defCatCombo, Set.of(level1, level3));
+    workflow24 = newWorkflow("workflow24", periodType, defCatCombo, Set.of(level2, level4));
     workflow0.setUid("workflow000");
     workflow1.setUid("workflow001");
     workflow12.setUid("workflow012");
@@ -371,15 +372,15 @@ void setUp() {
     dataApprovalService.addWorkflow(workflow1234);
     dataApprovalService.addWorkflow(workflow13);
     dataApprovalService.addWorkflow(workflow24);
-    dataSetA = createDataSet('A', periodType, defaultCategoryCombo);
-    dataSetB = createDataSet('B', periodType, defaultCategoryCombo);
-    dataSetC = createDataSet('C', periodType, defaultCategoryCombo);
-    dataSetD = createDataSet('D', periodType, defaultCategoryCombo);
-    dataSetE = createDataSet('E', periodType, defaultCategoryCombo);
-    dataSetF = createDataSet('F', periodType, defaultCategoryCombo);
-    dataSetG = createDataSet('G', periodType, defaultCategoryCombo);
-    dataSetI = createDataSet('I', periodType, defaultCategoryCombo);
-    dataSetJ = createDataSet('J', periodType, defaultCategoryCombo);
+    dataSetA = createDataSet('A', periodType, defCatCombo);
+    dataSetB = createDataSet('B', periodType, defCatCombo);
+    dataSetC = createDataSet('C', periodType, defCatCombo);
+    dataSetD = createDataSet('D', periodType, defCatCombo);
+    dataSetE = createDataSet('E', periodType, defCatCombo);
+    dataSetF = createDataSet('F', periodType, defCatCombo);
+    dataSetG = createDataSet('G', periodType, defCatCombo);
+    dataSetI = createDataSet('I', periodType, defCatCombo);
+    dataSetJ = createDataSet('J', periodType, defCatCombo);
     dataSetA.assignWorkflow(workflow0);
     dataSetB.assignWorkflow(workflow1);
     dataSetC.assignWorkflow(workflow12);
@@ -577,8 +578,11 @@ private void setUpCategories() {
     dataApprovalLevelService.addDataApprovalLevel(level2EFGH);
     dataApprovalLevelService.addDataApprovalLevel(level2ABCD);
     workflow12A_H =
-        new DataApprovalWorkflow(
-            "workflow12A_H", periodType, newHashSet(level1, level2, level2ABCD, level2EFGH));
+        newWorkflow(
+            "workflow12A_H",
+            periodType,
+            categoryComboA,
+            Set.of(level1, level2, level2ABCD, level2EFGH));
     workflow12A_H.setUid("workflo12AH");
     dataApprovalService.addWorkflow(workflow12A_H);
     dataSetH = createDataSet('H', periodType, categoryComboA);
@@ -4121,6 +4125,14 @@ void testGetApprovedByOfAcceptedHere() {
     assertEquals(date, status.getPermissions().getApprovedAt());
   }
 
+  private DataApprovalWorkflow newWorkflow(
+      String name,
+      PeriodType periodType,
+      CategoryCombo categoryCombo,
+      Set<DataApprovalLevel> levels) {
+    return new DataApprovalWorkflow(name, periodType, categoryCombo, levels);
+  }
+
   /**
    * Returns approval status and permissions information as a string. This allows a test to compare
    * the result against a string and test several things at once. More importantly, it shows in the

dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dataapproval/DataApprovalStoreIntegrationTest.java

@@ -45,7 +45,6 @@
 import org.hisp.dhis.dbms.DbmsManager;
 import org.hisp.dhis.organisationunit.OrganisationUnit;
 import org.hisp.dhis.organisationunit.OrganisationUnitService;
-import org.hisp.dhis.period.MonthlyPeriodType;
 import org.hisp.dhis.period.Period;
 import org.hisp.dhis.period.PeriodService;
 import org.hisp.dhis.period.PeriodStore;
@@ -167,24 +166,12 @@ void setUp() {
 
     userApprovalLevels = List.of(level1);
 
-    PeriodType periodType = PeriodType.getPeriodTypeByName("Monthly");
-
-    workflowA = new DataApprovalWorkflow("workflowA1", periodType, newHashSet(level1));
-
-    dataApprovalService.addWorkflow(workflowA);
-
     sourceA = createOrganisationUnit('A');
 
     sourceA.setHierarchyLevel(1);
 
     organisationUnitService.addOrganisationUnit(sourceA);
 
-    dataSetA = createDataSet('A', new MonthlyPeriodType(), categoryComboA);
-    dataSetA.assignWorkflow(workflowA);
-    dataSetA.addOrganisationUnit(sourceA);
-
-    dataSetService.addDataSet(dataSetA);
-
     periodJan = createPeriod("202001");
     periodFeb = createPeriod("202002");
     periodMay = createPeriod("202005");
@@ -249,6 +236,19 @@ void setUp() {
 
     categoryService.updateCategoryCombo(categoryComboA);
 
+    PeriodType periodType = PeriodType.getPeriodTypeByName("Monthly");
+
+    workflowA =
+        new DataApprovalWorkflow("workflowA1", periodType, categoryComboA, newHashSet(level1));
+
+    dataApprovalService.addWorkflow(workflowA);
+
+    dataSetA = createDataSet('A', periodType, categoryComboA);
+    dataSetA.assignWorkflow(workflowA);
+    dataSetA.addOrganisationUnit(sourceA);
+
+    dataSetService.addDataSet(dataSetA);
+
     dbmsManager.flushSession();
     dbmsManager.clearSession();
   }

dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dataapproval/DataApprovalStoreTest.java

@@ -33,6 +33,7 @@
 import static org.junit.jupiter.api.Assertions.assertNull;
 
 import java.util.Date;
+import org.hisp.dhis.category.CategoryCombo;
 import org.hisp.dhis.category.CategoryOptionCombo;
 import org.hisp.dhis.category.CategoryService;
 import org.hisp.dhis.organisationunit.OrganisationUnit;
@@ -104,14 +105,21 @@ void setUp() {
     // ---------------------------------------------------------------------
     // Add supporting data
     // ---------------------------------------------------------------------
+    CategoryCombo categoryCombo = categoryService.getDefaultCategoryCombo();
+    categoryOptionCombo = categoryService.getDefaultCategoryOptionCombo();
     level1 = new DataApprovalLevel("01", 1, null);
     level2 = new DataApprovalLevel("02", 2, null);
     dataApprovalLevelService.addDataApprovalLevel(level1);
     dataApprovalLevelService.addDataApprovalLevel(level2);
     PeriodType periodType = PeriodType.getPeriodTypeByName("Monthly");
-    workflowA1 = new DataApprovalWorkflow("workflowA1", periodType, newHashSet(level1));
-    workflowA12 = new DataApprovalWorkflow("workflowA12", periodType, newHashSet(level1, level2));
-    workflowB12 = new DataApprovalWorkflow("workflowB12", periodType, newHashSet(level1, level2));
+    workflowA1 =
+        new DataApprovalWorkflow("workflowA1", periodType, categoryCombo, newHashSet(level1));
+    workflowA12 =
+        new DataApprovalWorkflow(
+            "workflowA12", periodType, categoryCombo, newHashSet(level1, level2));
+    workflowB12 =
+        new DataApprovalWorkflow(
+            "workflowB12", periodType, categoryCombo, newHashSet(level1, level2));
     dataApprovalService.addWorkflow(workflowA1);
     dataApprovalService.addWorkflow(workflowA12);
     dataApprovalService.addWorkflow(workflowB12);
@@ -131,7 +139,6 @@ void setUp() {
     userB = makeUser("B");
     userService.addUser(userA);
     userService.addUser(userB);
-    categoryOptionCombo = categoryService.getDefaultCategoryOptionCombo();
   }
 
   // -------------------------------------------------------------------------

dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dataapproval/DataApprovalStoreUserTest.java

@@ -100,8 +100,10 @@ void setUp() {
     dataApprovalLevelService.addDataApprovalLevel(level2);
     dataApprovalLevelService.addDataApprovalLevel(level3);
     PeriodType periodType = PeriodType.getPeriodTypeByName("Monthly");
+    CategoryCombo defaultCategoryCombo = categoryService.getDefaultCategoryCombo();
     workflowA =
-        new DataApprovalWorkflow("workflowA", periodType, newHashSet(level1, level2, level3));
+        new DataApprovalWorkflow(
+            "workflowA", periodType, defaultCategoryCombo, newHashSet(level1, level2, level3));
     dataApprovalService.addWorkflow(workflowA);
     DataSet dataSetA = createDataSet('A');
     dataSetA.assignWorkflow(workflowA);

dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dataapproval/DataApprovalWorkflowServiceTest.java

@@ -35,6 +35,7 @@
 
 import java.util.List;
 import java.util.Set;
+import org.hisp.dhis.category.CategoryCombo;
 import org.hisp.dhis.period.PeriodType;
 import org.hisp.dhis.test.integration.PostgresIntegrationTestBase;
 import org.junit.jupiter.api.BeforeEach;
@@ -64,7 +65,9 @@ class DataApprovalWorkflowServiceTest extends PostgresIntegrationTestBase {
 
   private DataApprovalLevel level3;
 
-  PeriodType periodType;
+  private PeriodType periodType;
+
+  private CategoryCombo categoryCombo;
 
   @BeforeEach
   void setUp() {
@@ -78,9 +81,13 @@ void setUp() {
     dataApprovalLevelService.addDataApprovalLevel(level2);
     dataApprovalLevelService.addDataApprovalLevel(level3);
     periodType = PeriodType.getPeriodTypeByName("Monthly");
-    workflowA = new DataApprovalWorkflow("A", periodType, newHashSet(level1, level2));
-    workflowB = new DataApprovalWorkflow("B", periodType, newHashSet(level2, level3));
-    workflowC = new DataApprovalWorkflow("C", periodType, newHashSet(level1, level3));
+    categoryCombo = categoryService.getDefaultCategoryCombo();
+    workflowA =
+        new DataApprovalWorkflow("A", periodType, categoryCombo, newHashSet(level1, level2));
+    workflowB =
+        new DataApprovalWorkflow("B", periodType, categoryCombo, newHashSet(level2, level3));
+    workflowC =
+        new DataApprovalWorkflow("C", periodType, categoryCombo, newHashSet(level1, level3));
   }
 
   // -------------------------------------------------------------------------
@@ -163,7 +170,7 @@ void testSaveWorkFlowWithAuthority() {
     createUserAndInjectSecurityContext(false, "F_DATA_APPROVAL_WORKFLOW");
     long idA =
         dataApprovalService.addWorkflow(
-            new DataApprovalWorkflow("H", periodType, newHashSet(level1, level2)));
+            new DataApprovalWorkflow("H", periodType, categoryCombo, newHashSet(level1, level2)));
     assertEquals("H", dataApprovalService.getWorkflow(idA).getName());
   }
 
@@ -174,7 +181,8 @@ void testSaveWorkFlowWithoutAuthority() {
         AccessDeniedException.class,
         () ->
             dataApprovalService.addWorkflow(
-                new DataApprovalWorkflow("F", periodType, newHashSet(level1, level2))));
+                new DataApprovalWorkflow(
+                    "F", periodType, categoryCombo, newHashSet(level1, level2))));
   }
 
   @Test