Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make dataDimensionType mandatory for catOptionGroup and groupSet #15880

Merged
merged 1 commit into from
Dec 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ public CategoryOptionGroup(String name) {
this.name = name;
}

public CategoryOptionGroup(String name, DataDimensionType dataDimensionType) {
this(name);
this.dataDimensionType = dataDimensionType;
}

// -------------------------------------------------------------------------
// DimensionalItemObject
// -------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ public CategoryOptionGroupSet(String name) {
this.shortName = name;
}

public CategoryOptionGroupSet(String name, DataDimensionType dataDimensionType) {
this(name);
this.dataDimensionType = dataDimensionType;
}

// -------------------------------------------------------------------------
// Logic
// -------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 -->

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"/>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<>());

Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
name,uid,code,shortname,datadimensiontype
GroupA,,GroupA,,
GroupB,,GroupB,,disaggregation
GroupC,,GroupC,,attribute
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading