fix: make dataDimensionType not null for catOptionGroup and groupSet

dhis2 · Dec 11, 2023 · deeef94 · deeef94
1 parent a0a14f9
commit deeef94
Show file tree

Hide file tree

Showing 21 changed files with 130 additions and 32 deletions.
diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/category/CategoryOptionGroup.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/category/CategoryOptionGroup.java
@@ -63,6 +63,11 @@ public CategoryOptionGroup(String name) {
     this.name = name;
   }
 
+  public CategoryOptionGroup(String name, DataDimensionType dataDimensionType) {
+    this(name);
+    this.dataDimensionType = dataDimensionType;
+  }
+
   // -------------------------------------------------------------------------
   // DimensionalItemObject
   // -------------------------------------------------------------------------

diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/category/CategoryOptionGroupSet.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/category/CategoryOptionGroupSet.java
@@ -68,6 +68,11 @@ public CategoryOptionGroupSet(String name) {
     this.shortName = name;
   }
 
+  public CategoryOptionGroupSet(String name, DataDimensionType dataDimensionType) {
+    this(name);
+    this.dataDimensionType = dataDimensionType;
+  }
+
   // -------------------------------------------------------------------------
   // Logic
   // -------------------------------------------------------------------------

diff --git a/...vice-core/src/main/resources/org/hisp/dhis/category/hibernate/CategoryOptionGroup.hbm.xml b/...vice-core/src/main/resources/org/hisp/dhis/category/hibernate/CategoryOptionGroup.hbm.xml
@@ -36,7 +36,7 @@
       <many-to-many class="org.hisp.dhis.category.CategoryOptionGroupSet" column="categoryoptiongroupsetid" />
     </set>
 
-    <property name="dataDimensionType" column="datadimensiontype" type="org.hisp.dhis.common.DataDimensionTypeUserType" />
+    <property name="dataDimensionType" column="datadimensiontype" type="org.hisp.dhis.common.DataDimensionTypeUserType"  not-null="true"/>
 
     <!-- Dynamic attribute values -->
 

diff --git a/...e-core/src/main/resources/org/hisp/dhis/category/hibernate/CategoryOptionGroupSet.hbm.xml b/...e-core/src/main/resources/org/hisp/dhis/category/hibernate/CategoryOptionGroupSet.hbm.xml
@@ -35,7 +35,7 @@
         foreign-key="fk_categoryoptiongroupsetmembers_categoryoptiongroupid" />
     </list>
 
-    <property name="dataDimensionType" column="datadimensiontype" type="org.hisp.dhis.common.DataDimensionTypeUserType" />
+    <property name="dataDimensionType" column="datadimensiontype" type="org.hisp.dhis.common.DataDimensionTypeUserType" not-null="true" />
 
     <property name="translations" type="jblTranslations"/>
 

diff --git a/...vices/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/csv/DefaultCsvImportService.java b/...vices/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/csv/DefaultCsvImportService.java
@@ -430,6 +430,9 @@ private List<CategoryOptionGroup> categoryOptionGroupsFromCsv(CsvReader reader)
         CategoryOptionGroup object = new CategoryOptionGroup();
         setIdentifiableObject(object, values);
         object.setShortName(getSafe(values, 3, object.getName(), 50));
+        object.setDataDimensionType(
+            DataDimensionType.fromValue(
+                getSafe(values, 4, DataDimensionType.DISAGGREGATION.getValue(), 50)));
         list.add(object);
       }
     }

diff --git a/dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/DhisConvenienceTest.java b/dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/DhisConvenienceTest.java
@@ -726,6 +726,7 @@ public static CategoryOptionGroup createCategoryOptionGroup(
         new CategoryOptionGroup("CategoryOptionGroup" + uniqueIdentifier);
     categoryOptionGroup.setShortName("ShortName" + uniqueIdentifier);
     categoryOptionGroup.setAutoFields();
+    categoryOptionGroup.setDataDimensionType(DISAGGREGATION);
 
     categoryOptionGroup.setMembers(new HashSet<>());
 
@@ -747,6 +748,7 @@ public static CategoryOptionGroupSet createCategoryOptionGroupSet(
     CategoryOptionGroupSet categoryOptionGroupSet =
         new CategoryOptionGroupSet("CategoryOptionGroupSet" + categoryGroupSetUniqueIdentifier);
     categoryOptionGroupSet.setAutoFields();
+    categoryOptionGroupSet.setDataDimensionType(DISAGGREGATION);
 
     for (CategoryOptionGroup categoryOptionGroup : categoryOptionGroups) {
       categoryOptionGroupSet.addCategoryOptionGroup(categoryOptionGroup);

diff --git a/...his-test-integration/src/test/java/org/hisp/dhis/analytics/data/AnalyticsServiceTest.java b/...his-test-integration/src/test/java/org/hisp/dhis/analytics/data/AnalyticsServiceTest.java
@@ -451,7 +451,8 @@ private void setUpValidation() {
     categoryService.saveCategoryOptionGroup(optionGroupA);
     categoryService.saveCategoryOptionGroup(optionGroupB);
 
-    CategoryOptionGroupSet optionGroupSetB = new CategoryOptionGroupSet("OptionGroupSetB");
+    CategoryOptionGroupSet optionGroupSetB =
+        new CategoryOptionGroupSet("OptionGroupSetB", DataDimensionType.DISAGGREGATION);
     categoryService.saveCategoryOptionGroupSet(optionGroupSetB);
     optionGroupSetB.addCategoryOptionGroup(optionGroupA);
     optionGroupSetB.addCategoryOptionGroup(optionGroupB);

diff --git a/...s-test-integration/src/test/java/org/hisp/dhis/category/CategoryOptionGroupStoreTest.java b/...s-test-integration/src/test/java/org/hisp/dhis/category/CategoryOptionGroupStoreTest.java
@@ -28,10 +28,13 @@
 package org.hisp.dhis.category;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.List;
+import org.hibernate.PropertyValueException;
 import org.hisp.dhis.test.integration.SingleSetupIntegrationTestBase;
+import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 
@@ -140,4 +143,25 @@ void testGetByGroupSet() {
     assertTrue(groupsB.contains(cogC));
     assertTrue(groupsB.contains(cogD));
   }
+
+  @Test
+  @DisplayName("Should throw error if adding category option group without data dimension type")
+  void testAddCogWithoutDataDimensionType() {
+    CategoryOptionGroup cogA = createCategoryOptionGroup('A', coA, coB);
+    cogA.setDataDimensionType(null);
+    assertThrows(PropertyValueException.class, () -> categoryOptionGroupStore.save(cogA));
+  }
+
+  @Test
+  @DisplayName("Should throw error if adding category option group set without data dimension type")
+  void testAddCogsWithoutDataDimensionType() {
+    CategoryOptionGroup cogA = createCategoryOptionGroup('A', coA, coB);
+    CategoryOptionGroup cogB = createCategoryOptionGroup('B', coC, coD);
+    categoryOptionGroupStore.save(cogA);
+    categoryOptionGroupStore.save(cogB);
+    CategoryOptionGroupSet cogsA = createCategoryOptionGroupSet('A', cogA, cogB);
+    cogsA.setDataDimensionType(null);
+    assertThrows(
+        PropertyValueException.class, () -> categoryService.saveCategoryOptionGroupSet(cogsA));
+  }
 }
diff --git a/...st-integration/src/test/java/org/hisp/dhis/dataapproval/DataApprovalAuditServiceTest.java b/...st-integration/src/test/java/org/hisp/dhis/dataapproval/DataApprovalAuditServiceTest.java
@@ -47,6 +47,7 @@
 import org.hisp.dhis.category.CategoryOptionGroupSet;
 import org.hisp.dhis.category.CategoryService;
 import org.hisp.dhis.common.BaseIdentifiableObject;
+import org.hisp.dhis.common.DataDimensionType;
 import org.hisp.dhis.common.IdentifiableObjectManager;
 import org.hisp.dhis.organisationunit.OrganisationUnit;
 import org.hisp.dhis.organisationunit.OrganisationUnitService;
@@ -246,7 +247,8 @@ public void setUpTest() throws Exception {
     optionGroupB = createCategoryOptionGroup('B', optionB);
     categoryService.saveCategoryOptionGroup(optionGroupA);
     categoryService.saveCategoryOptionGroup(optionGroupB);
-    optionGroupSetB = new CategoryOptionGroupSet("OptionGroupSetB");
+    optionGroupSetB =
+        new CategoryOptionGroupSet("OptionGroupSetB", DataDimensionType.DISAGGREGATION);
     categoryService.saveCategoryOptionGroupSet(optionGroupSetB);
     optionGroupSetB.addCategoryOptionGroup(optionGroupA);
     optionGroupSetB.addCategoryOptionGroup(optionGroupB);

diff --git a/...st-integration/src/test/java/org/hisp/dhis/dataapproval/DataApprovalLevelServiceTest.java b/...st-integration/src/test/java/org/hisp/dhis/dataapproval/DataApprovalLevelServiceTest.java
@@ -40,6 +40,7 @@
 import java.util.Set;
 import org.hisp.dhis.category.CategoryOptionGroupSet;
 import org.hisp.dhis.category.CategoryService;
+import org.hisp.dhis.common.DataDimensionType;
 import org.hisp.dhis.organisationunit.OrganisationUnit;
 import org.hisp.dhis.organisationunit.OrganisationUnitService;
 import org.hisp.dhis.test.integration.TransactionalIntegrationTest;
@@ -144,10 +145,10 @@ public void setUpTest() {
     // ---------------------------------------------------------------------
     // Add supporting data
     // ---------------------------------------------------------------------
-    setA = new CategoryOptionGroupSet("Set A");
-    setB = new CategoryOptionGroupSet("Set B");
-    setC = new CategoryOptionGroupSet("Set C");
-    setD = new CategoryOptionGroupSet("Set D");
+    setA = new CategoryOptionGroupSet("Set A", DataDimensionType.DISAGGREGATION);
+    setB = new CategoryOptionGroupSet("Set B", DataDimensionType.DISAGGREGATION);
+    setC = new CategoryOptionGroupSet("Set C", DataDimensionType.DISAGGREGATION);
+    setD = new CategoryOptionGroupSet("Set D", DataDimensionType.DISAGGREGATION);
     categoryService.saveCategoryOptionGroupSet(setA);
     categoryService.saveCategoryOptionGroupSet(setB);
     categoryService.saveCategoryOptionGroupSet(setC);

diff --git a/.../src/test/java/org/hisp/dhis/dataapproval/DataApprovalServiceCategoryOptionGroupTest.java b/.../src/test/java/org/hisp/dhis/dataapproval/DataApprovalServiceCategoryOptionGroupTest.java
@@ -52,6 +52,7 @@
 import org.hisp.dhis.category.CategoryOptionGroupSet;
 import org.hisp.dhis.category.CategoryService;
 import org.hisp.dhis.common.BaseIdentifiableObject;
+import org.hisp.dhis.common.DataDimensionType;
 import org.hisp.dhis.common.IdentifiableObjectManager;
 import org.hisp.dhis.dataapproval.exceptions.DataApprovalException;
 import org.hisp.dhis.dataset.DataSet;
@@ -537,8 +538,8 @@ public void setUpTest() throws Exception {
     setAccess(partner2, globalUsers, chinaInteragencyUsers, chinaAgencyAUsers, chinaPartner2Users);
 
     // Agencies and partners category option group sets
-    agencies = new CategoryOptionGroupSet("Agencies");
-    partners = new CategoryOptionGroupSet("Partners");
+    agencies = new CategoryOptionGroupSet("Agencies", DataDimensionType.DISAGGREGATION);
+    partners = new CategoryOptionGroupSet("Partners", DataDimensionType.DISAGGREGATION);
     categoryService.saveCategoryOptionGroupSet(partners);
     categoryService.saveCategoryOptionGroupSet(agencies);
     setAccess(

diff --git a/...is-test-integration/src/test/java/org/hisp/dhis/dataapproval/DataApprovalServiceTest.java b/...is-test-integration/src/test/java/org/hisp/dhis/dataapproval/DataApprovalServiceTest.java
@@ -50,6 +50,7 @@
 import org.hisp.dhis.category.CategoryOptionGroup;
 import org.hisp.dhis.category.CategoryOptionGroupSet;
 import org.hisp.dhis.category.CategoryService;
+import org.hisp.dhis.common.DataDimensionType;
 import org.hisp.dhis.dataapproval.exceptions.DataMayNotBeApprovedException;
 import org.hisp.dhis.dataset.DataSet;
 import org.hisp.dhis.dataset.DataSetService;
@@ -559,8 +560,8 @@ private void setUpCategories() {
     categoryService.saveCategoryOptionGroup(groupCD);
     categoryService.saveCategoryOptionGroup(groupEF);
     categoryService.saveCategoryOptionGroup(groupGH);
-    groupSetABCD = new CategoryOptionGroupSet("GroupSetABCD");
-    groupSetEFGH = new CategoryOptionGroupSet("GroupSetEFGH");
+    groupSetABCD = new CategoryOptionGroupSet("GroupSetABCD", DataDimensionType.DISAGGREGATION);
+    groupSetEFGH = new CategoryOptionGroupSet("GroupSetEFGH", DataDimensionType.DISAGGREGATION);
     categoryService.saveCategoryOptionGroupSet(groupSetABCD);
     categoryService.saveCategoryOptionGroupSet(groupSetEFGH);
     groupSetABCD.addCategoryOptionGroup(groupAB);

diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dxf2/csv/CsvImportServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dxf2/csv/CsvImportServiceTest.java
@@ -32,9 +32,11 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.List;
 import org.hisp.dhis.category.Category;
 import org.hisp.dhis.category.CategoryCombo;
 import org.hisp.dhis.category.CategoryOption;
+import org.hisp.dhis.category.CategoryOptionGroup;
 import org.hisp.dhis.common.DataDimensionType;
 import org.hisp.dhis.common.IdentifiableObjectManager;
 import org.hisp.dhis.dxf2.metadata.Metadata;
@@ -222,4 +224,41 @@ void testImportIndicator() throws IOException {
     assertEquals("#{h0xKKjijTdI}", indicatorB.getNumerator());
     assertEquals(indicatorType.getUid(), indicatorB.getIndicatorType().getUid());
   }
+
+  @Test
+  void testImportCategoryOptionGroup() throws IOException {
+    InputStream in = new ClassPathResource("csv/category_option_group.csv").getInputStream();
+    Metadata metadata =
+        csvImportService.fromCsv(
+            in,
+            new CsvImportOptions()
+                .setImportClass(CsvImportClass.CATEGORY_OPTION_GROUP)
+                .setFirstRowIsHeader(true));
+    assertEquals(3, metadata.getCategoryOptionGroups().size());
+    // make sure all groups have data dimension type set
+    for (CategoryOptionGroup group : metadata.getCategoryOptionGroups()) {
+      assertNotNull(group.getDataDimensionType());
+    }
+
+    ImportReport report =
+        importService.importMetadata(
+            new MetadataImportParams(),
+            new MetadataObjects().addMetadata(schemaService.getMetadataSchemas(), metadata));
+
+    assertEquals(Status.OK, report.getStatus());
+    List<CategoryOptionGroup> allGroups = manager.getAll(CategoryOptionGroup.class);
+    assertEquals(3, allGroups.size());
+    CategoryOptionGroup groupA =
+        allGroups.stream().filter(g -> g.getName().equals("GroupA")).findFirst().orElse(null);
+    assertNotNull(groupA);
+    assertEquals(DataDimensionType.DISAGGREGATION, groupA.getDataDimensionType());
+    CategoryOptionGroup groupB =
+        allGroups.stream().filter(g -> g.getName().equals("GroupB")).findFirst().orElse(null);
+    assertNotNull(groupB);
+    assertEquals(DataDimensionType.DISAGGREGATION, groupB.getDataDimensionType());
+    CategoryOptionGroup groupC =
+        allGroups.stream().filter(g -> g.getName().equals("GroupC")).findFirst().orElse(null);
+    assertNotNull(groupC);
+    assertEquals(DataDimensionType.ATTRIBUTE, groupC.getDataDimensionType());
+  }
 }
diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/security/acl/AclServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/security/acl/AclServiceTest.java
@@ -37,6 +37,7 @@
 import org.apache.commons.collections4.MapUtils;
 import org.hisp.dhis.category.CategoryOption;
 import org.hisp.dhis.category.CategoryOptionGroupSet;
+import org.hisp.dhis.common.DataDimensionType;
 import org.hisp.dhis.common.IdentifiableObjectManager;
 import org.hisp.dhis.dashboard.Dashboard;
 import org.hisp.dhis.dataelement.DataElement;
@@ -1075,6 +1076,7 @@ void testCanDataOrMetadataRead() {
     manager.save(user1);
     // non data shareable object //
     CategoryOptionGroupSet categoryOptionGroupSet = new CategoryOptionGroupSet();
+    categoryOptionGroupSet.setDataDimensionType(DataDimensionType.DISAGGREGATION);
     categoryOptionGroupSet.setAutoFields();
     categoryOptionGroupSet.setName("cogA");
     categoryOptionGroupSet.setShortName("cogA");

diff --git a/...is-test-integration/src/test/java/org/hisp/dhis/validation/ValidationResultStoreTest.java b/...is-test-integration/src/test/java/org/hisp/dhis/validation/ValidationResultStoreTest.java
@@ -51,6 +51,7 @@
 import org.hisp.dhis.category.CategoryOptionGroupSet;
 import org.hisp.dhis.category.CategoryService;
 import org.hisp.dhis.common.BaseIdentifiableObject;
+import org.hisp.dhis.common.DataDimensionType;
 import org.hisp.dhis.common.IdentifiableObjectManager;
 import org.hisp.dhis.expression.Expression;
 import org.hisp.dhis.organisationunit.OrganisationUnit;
@@ -229,7 +230,8 @@ public void setUpTest() throws Exception {
     optionGroupB = createCategoryOptionGroup('B', optionB);
     categoryService.saveCategoryOptionGroup(optionGroupA);
     categoryService.saveCategoryOptionGroup(optionGroupB);
-    optionGroupSetB = new CategoryOptionGroupSet("OptionGroupSetB");
+    optionGroupSetB =
+        new CategoryOptionGroupSet("OptionGroupSetB", DataDimensionType.DISAGGREGATION);
     categoryService.saveCategoryOptionGroupSet(optionGroupSetB);
     optionGroupSetB.addCategoryOptionGroup(optionGroupA);
     optionGroupSetB.addCategoryOptionGroup(optionGroupB);

diff --git a/dhis-2/dhis-test-integration/src/test/resources/csv/category_option_group.csv b/dhis-2/dhis-test-integration/src/test/resources/csv/category_option_group.csv
@@ -0,0 +1,4 @@
+name,uid,code,shortname,datadimensiontype
+GroupA,,GroupA,,
+GroupB,,GroupB,,disaggregation
+GroupC,,GroupC,,attribute
diff --git a/...2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/UserControllerTest.java b/...2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/UserControllerTest.java
@@ -50,6 +50,7 @@
 import org.hisp.dhis.category.CategoryOption;
 import org.hisp.dhis.category.CategoryOptionGroupSet;
 import org.hisp.dhis.common.BaseIdentifiableObject;
+import org.hisp.dhis.common.DataDimensionType;
 import org.hisp.dhis.feedback.ErrorCode;
 import org.hisp.dhis.jsontree.JsonArray;
 import org.hisp.dhis.jsontree.JsonBoolean;
@@ -973,6 +974,7 @@ void testRemoveCogCatDimFromUserCredentialsLegacyFormat() {
 
     CategoryOptionGroupSet categoryOptionGroupSet = new CategoryOptionGroupSet();
     categoryOptionGroupSet.setAutoFields();
+    categoryOptionGroupSet.setDataDimensionType(DataDimensionType.DISAGGREGATION);
     categoryOptionGroupSet.setName("cogA");
     categoryOptionGroupSet.setShortName("cogA");
     manager.save(categoryOptionGroupSet);

diff --git a/...dataintegrity/DataIntegrityCategoryOptionGroupSetExcessGroupMembershipControllerTest.java b/...dataintegrity/DataIntegrityCategoryOptionGroupSetExcessGroupMembershipControllerTest.java
@@ -61,7 +61,7 @@ void testCategoryOptionInMultipleGroups() {
             HttpStatus.CREATED,
             POST(
                 "/categoryOptionGroups",
-                "{ 'name': 'Taste', 'shortName': 'Taste' , 'categoryOptions' : [{'id' : '"
+                "{ 'name': 'Taste', 'shortName': 'Taste' , 'dataDimensionType':'DISAGGREGATION', 'categoryOptions' : [{'id' : '"
                     + categoryOptionSweet
                     + "'}, {'id': '"
                     + categoryOptionSour
@@ -73,7 +73,7 @@ void testCategoryOptionInMultipleGroups() {
         HttpStatus.CREATED,
         POST(
             "/categoryOptionGroupSets",
-            "{ 'name': 'Taste and Color', 'shortName': 'Taste and Color', 'categoryOptionGroups' : [{'id': '"
+            "{ 'name': 'Taste and Color', 'shortName': 'Taste and Color', 'dataDimensionType':'DISAGGREGATION', 'categoryOptionGroups' : [{'id': '"
                 + categoryOptionGroupColor
                 + "'}, {'id' : '"
                 + categoryOptionGroupTaste
@@ -92,7 +92,7 @@ void testCategoryOptionsInOneGroup() {
             HttpStatus.CREATED,
             POST(
                 "/categoryOptionGroups",
-                "{ 'name': 'Taste', 'shortName': 'Taste' , 'categoryOptions' : [{'id' : '"
+                "{ 'name': 'Taste', 'dataDimensionType':'DISAGGREGATION', 'shortName': 'Taste' , 'categoryOptions' : [{'id' : '"
                     + categoryOptionSweet
                     + "'}, {'id': '"
                     + categoryOptionSour
@@ -102,7 +102,7 @@ void testCategoryOptionsInOneGroup() {
         HttpStatus.CREATED,
         POST(
             "/categoryOptionGroupSets",
-            "{ 'name': 'Taste and Color', 'shortName': 'Taste and Color', 'categoryOptionGroups' : [{'id': '"
+            "{ 'name': 'Taste and Color', 'shortName': 'Taste and Color', 'dataDimensionType':'DISAGGREGATION', 'categoryOptionGroups' : [{'id': '"
                 + categoryOptionGroupColor
                 + "'}, {'id' : '"
                 + categoryOptionGroupTaste
@@ -142,7 +142,7 @@ void setUpTest() {
             HttpStatus.CREATED,
             POST(
                 "/categoryOptionGroups",
-                "{ 'name': 'Color', 'shortName': 'Color', 'categoryOptions' : [{'id' : '"
+                "{ 'name': 'Color', 'shortName': 'Color', 'dataDimensionType':'DISAGGREGATION', 'categoryOptions' : [{'id' : '"
                     + categoryOptionRed
                     + "'}, {'id': '"
                     + categoryOptionBlue