fix: Update data dimension items as part of cat opt combo merge [DHIS…

…2-18321] (#19687) * fix: Handle data dimsion items during coc merge [DHIS2-18321] * fix: Minimise PR change * fix: clean up
dhis2 · Jan 16, 2025 · 4e7da2e · 4e7da2e
1 parent 73ccf63
commit 4e7da2e
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 11 deletions.
diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/datadimensionitem/DataDimensionItemStore.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/datadimensionitem/DataDimensionItemStore.java
@@ -27,6 +27,7 @@
  */
 package org.hisp.dhis.datadimensionitem;
 
+import java.util.Collection;
 import java.util.List;
 import org.hisp.dhis.common.DataDimensionItem;
 import org.hisp.dhis.common.GenericStore;
@@ -47,4 +48,14 @@ public interface DataDimensionItemStore extends GenericStore<DataDimensionItem>
   List<DataDimensionItem> getIndicatorDataDimensionItems(List<Indicator> indicators);
 
   List<DataDimensionItem> getDataElementDataDimensionItems(List<DataElement> dataElements);
+
+  /**
+   * Update the entities with refs to the category option combo ids passed in, with the new category
+   * option combo id passed in.
+   *
+   * @param cocIds category option combo ids to be used to update linked data dimension items
+   * @param newCocId new category option combo id to use
+   * @return number of entities updated
+   */
+  int updateDeoCategoryOptionCombo(Collection<Long> cocIds, long newCocId);
 }
diff --git a/...ava/org/hisp/dhis/merge/category/optioncombo/MetadataCategoryOptionComboMergeHandler.java b/...ava/org/hisp/dhis/merge/category/optioncombo/MetadataCategoryOptionComboMergeHandler.java
@@ -28,6 +28,7 @@
 package org.hisp.dhis.merge.category.optioncombo;
 
 import java.util.List;
+import java.util.stream.Collectors;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.hisp.dhis.category.CategoryCombo;
@@ -37,6 +38,7 @@
 import org.hisp.dhis.category.CategoryOptionStore;
 import org.hisp.dhis.common.BaseIdentifiableObject;
 import org.hisp.dhis.common.UID;
+import org.hisp.dhis.datadimensionitem.DataDimensionItemStore;
 import org.hisp.dhis.dataelement.DataElementOperand;
 import org.hisp.dhis.dataelement.DataElementOperandStore;
 import org.hisp.dhis.minmax.MinMaxDataElement;
@@ -60,6 +62,7 @@ public class MetadataCategoryOptionComboMergeHandler {
   private final CategoryOptionStore categoryOptionStore;
   private final CategoryComboStore categoryComboStore;
   private final DataElementOperandStore dataElementOperandStore;
+  private final DataDimensionItemStore dataDimensionItemStore;
   private final MinMaxDataElementStore minMaxDataElementStore;
   private final PredictorStore predictorStore;
   private final SMSCommandStore smsCommandStore;
@@ -116,6 +119,17 @@ public void handleDataElementOperands(
             UID.of(sources.stream().map(BaseIdentifiableObject::getUid).toList()));
 
     dataElementOperands.forEach(deo -> deo.setCategoryOptionCombo(target));
+
+    // A data element operand is also a data dimension item.
+    // The above update does not cascade the reference change though.
+    // The Data dimension item table also needs updating
+    int dataDimensionItemsUpdated =
+        dataDimensionItemStore.updateDeoCategoryOptionCombo(
+            sources.stream().map(BaseIdentifiableObject::getId).collect(Collectors.toSet()),
+            target.getId());
+    log.info(
+        "{} data dimension items updated as part of category option combo merge",
+        dataDimensionItemsUpdated);
   }
 
   /**

diff --git a/.../main/java/org/hisp/dhis/datadimensionitem/hibernate/HibernateDataDimensionItemStore.java b/.../main/java/org/hisp/dhis/datadimensionitem/hibernate/HibernateDataDimensionItemStore.java
@@ -28,7 +28,10 @@
 package org.hisp.dhis.datadimensionitem.hibernate;
 
 import jakarta.persistence.EntityManager;
+import java.util.Collection;
 import java.util.List;
+import java.util.stream.Collectors;
+import javax.annotation.Nonnull;
 import org.hisp.dhis.common.DataDimensionItem;
 import org.hisp.dhis.dataelement.DataElement;
 import org.hisp.dhis.hibernate.HibernateGenericStore;
@@ -69,4 +72,19 @@ public List<DataDimensionItem> getDataElementDataDimensionItems(List<DataElement
         .setParameter("dataElements", dataElements)
         .getResultList();
   }
+
+  @Override
+  public int updateDeoCategoryOptionCombo(@Nonnull Collection<Long> cocIds, long newCocId) {
+    if (cocIds.isEmpty()) return 0;
+
+    String sql =
+        """
+        update datadimensionitem
+        set dataelementoperand_categoryoptioncomboid = %s
+        where dataelementoperand_categoryoptioncomboid in (%s)
+        """
+            .formatted(
+                newCocId, cocIds.stream().map(String::valueOf).collect(Collectors.joining(",")));
+    return jdbcTemplate.update(sql);
+  }
 }
diff --git a/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/merge/CategoryOptionComboMergeTest.java b/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/merge/CategoryOptionComboMergeTest.java
@@ -69,6 +69,7 @@ class CategoryOptionComboMergeTest extends ApiTest {
   private RestApiActions dataElementApiActions;
   private RestApiActions minMaxActions;
   private MetadataActions metadataActions;
+  private RestApiActions visualizationActions;
   private RestApiActions maintenanceApiActions;
   private RestApiActions dataValueSetActions;
   private UserActions userActions;
@@ -90,6 +91,7 @@ public void before() {
     metadataActions = new MetadataActions();
     maintenanceApiActions = new RestApiActions("maintenance");
     dataValueSetActions = new RestApiActions("dataValueSets");
+    visualizationActions = new RestApiActions("visualizations");
     loginActions.loginAsSuperUser();
 
     // add user with required merge auth
@@ -145,6 +147,10 @@ void validCategoryOptionComboMergeTest() {
         .body("categoryOptions", hasItem(hasEntry("id", "CatOptUid4B")))
         .body("categoryOptions", hasItem(hasEntry("id", "CatOptUid3A")));
 
+    String dataElement = setupDataElement("test de 1");
+    // import visualization to persist data dimension item which has ref to source coc
+    visualizationActions.post(getViz(dataElement, sourceUid1)).validateStatus(201);
+
     // login as merge user
     loginActions.loginAsUser("userWithMergeAuth", "Test1234!");
 
@@ -410,6 +416,49 @@ private void updateDataValuesAoc() {
         .body("response.importCount.updated", equalTo(4));
   }
 
+  private String getViz(String dataElement, String coc) {
+    return """
+
+          {
+            "name": "Test viz with data dimension item - DE operand",
+            "displayName": "Test 1",
+            "type": "PIVOT_TABLE",
+            "filters": [
+              {
+                "dimension": "ou",
+                "items": [
+                  {
+                    "id": "USER_ORGUNIT"
+                  }
+                ]
+              }
+            ],
+            "columns": [
+              {
+                "dimension": "dx",
+                "items": [
+                  {
+                    "id": "%s.%s",
+                    "dimensionItemType": "DATA_ELEMENT_OPERAND"
+                  }
+                ]
+              }
+            ],
+            "rows": [
+              {
+                "dimension": "pe",
+                "items": [
+                  {
+                    "id": "LAST_10_YEARS"
+                  }
+                ]
+              }
+            ]
+          }
+          """
+        .formatted(dataElement, coc);
+  }
+
   private QueryParamsBuilder getDataValueQueryParams() {
     return new QueryParamsBuilder()
         .add("async=false")
@@ -494,7 +543,7 @@ void dbConstraintMinMaxTest() {
     sourceUid2 = getCocWithOptions("1B", "2B");
     targetUid = getCocWithOptions("3A", "4B");
 
-    String dataElement = setupDataElement();
+    String dataElement = setupDataElement("DE test 2");
 
     setupMinMaxDataElements(sourceUid1, sourceUid2, targetUid, dataElement);
 
@@ -516,7 +565,7 @@ void dbConstraintMinMaxTest() {
   }
 
   private void setupMetadata() {
-    metadataActions.post(metadata()).validateStatus(200);
+    metadataActions.importMetadata(metadata()).validateStatus(200);
   }
 
   private void setupMinMaxDataElements(
@@ -546,19 +595,19 @@ private String minMaxDataElements(String coc, String de) {
         .formatted(de, coc);
   }
 
-  private String setupDataElement() {
+  private String setupDataElement(String name) {
     return dataElementApiActions
         .post(
             """
             {
                "aggregationType": "DEFAULT",
                "domainType": "AGGREGATE",
-               "name": "source 19",
-               "shortName": "source 19",
-               "displayName": "source 19",
+               "name": "%s",
+               "shortName": "%s",
                "valueType": "TEXT"
              }
-             """)
+             """
+                .formatted(name, name))
         .validateStatus(201)
         .extractUid();
   }

diff --git a/...ation/src/test/java/org/hisp/dhis/merge/category/CategoryOptionComboMergeServiceTest.java b/...ation/src/test/java/org/hisp/dhis/merge/category/CategoryOptionComboMergeServiceTest.java
@@ -377,15 +377,13 @@ void dataElementOperandRefsReplacedSourcesDeletedTest() throws ConflictException
 
     List<CategoryOptionCombo> allCOCsAfter = categoryService.getAllCategoryOptionCombos();
 
-    assertEquals(7, allCOCsAfter.size(), "7 COCs including 1 default");
-
     // then
+    assertEquals(7, allCOCsAfter.size(), "7 COCs including 1 default");
+    assertFalse(report.hasErrorMessages(), "there should be no merge errors");
     List<DataElementOperand> deoSourcesAfter =
         dataElementOperandStore.getByCategoryOptionCombo(UID.of(cocSource1, cocSource2));
     List<DataElementOperand> deoTargetAfter =
         dataElementOperandStore.getByCategoryOptionCombo(Set.of(UID.of(cocTarget.getUid())));
-
-    assertFalse(report.hasErrorMessages());
     assertEquals(
         0, deoSourcesAfter.size(), "Expect 0 entries with source category option combo refs");
     assertEquals(