fix: Fix data element merging of data set elements [DHIS2-14429]

dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataelement/DataElement.java

@@ -171,6 +171,11 @@ public boolean removeDataSetElement(DataSetElement element) {
     return element.getDataSet().getDataSetElements().remove(element);
   }
 
+  public void addDataSetElement(DataSetElement element) {
+    dataSetElements.add(element);
+    element.getDataSet().getDataSetElements().add(element);
+  }
+
   /**
    * Returns the resolved category combinations by joining the category combinations of the data set
    * elements of which this data element is part of and the category combination linked directly

dhis-2/dhis-services/dhis-service-administration/src/main/java/org/hisp/dhis/merge/dataelement/handler/MetadataDataElementMergeHandler.java

@@ -175,17 +175,23 @@ public void handleDataElementOperand(List<DataElement> sources, DataElement targ
   }
 
   /**
-   * Method retrieving {@link DataSetElement}s by source {@link DataElement} references. All
-   * retrieved {@link DataSetElement}s will have their {@link DataElement} replaced with the target
-   * {@link DataElement}.
+   * Method retrieving {@link DataSetElement}s by source {@link DataElement} references. Sources
+   * will have all found {@link DataSetElement}s removed. All found {@link DataSetElement}s will
+   * have their {@link DataElement} set as the target {@link DataElement}. All found {@link
+   * DataSetElement}s will be added to the target.
    *
    * @param sources source {@link DataElement}s used to retrieve {@link DataSetElement}s
    * @param target {@link DataElement} which will be set as the {@link DataElement} for an {@link
    *     DataSetElement}
    */
   public void handleDataSetElement(List<DataElement> sources, DataElement target) {
     List<DataSetElement> dataSetElements = dataSetStore.getDataSetElementsByDataElement(sources);
-    dataSetElements.forEach(dse -> dse.setDataElement(target));
+    dataSetElements.forEach(
+        dse -> {
+          sources.forEach(de -> de.removeDataSetElement(dse));
+          dse.setDataElement(target);
+          target.addDataSetElement(dse);
+        });
   }
 
   /**

dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/merge/dataelement/MetadataDataElementMergeHandlerTest.java

@@ -0,0 +1,150 @@
+/*
+ * Copyright (c) 2004-2024, University of Oslo
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * Redistributions of source code must retain the above copyright notice, this
+ * list of conditions and the following disclaimer.
+ *
+ * Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ * Neither the name of the HISP project nor the names of its contributors may
+ * be used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
+ * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.hisp.dhis.merge.dataelement;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.List;
+import org.hisp.dhis.common.IdentifiableObjectManager;
+import org.hisp.dhis.dataelement.DataElement;
+import org.hisp.dhis.dataelement.DataElementGroup;
+import org.hisp.dhis.dataelement.DataElementGroupService;
+import org.hisp.dhis.dataelement.DataElementService;
+import org.hisp.dhis.dataset.DataSet;
+import org.hisp.dhis.dataset.DataSetElement;
+import org.hisp.dhis.dataset.DataSetService;
+import org.hisp.dhis.merge.dataelement.handler.MetadataDataElementMergeHandler;
+import org.hisp.dhis.test.integration.PostgresIntegrationTestBase;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.transaction.annotation.Transactional;
+
+@Transactional
+class MetadataDataElementMergeHandlerTest extends PostgresIntegrationTestBase {
+
+  @Autowired private MetadataDataElementMergeHandler mergeHandler;
+  @Autowired private DataElementService dataElementService;
+  @Autowired private DataSetService dataSetService;
+  @Autowired private IdentifiableObjectManager manager;
+
+  private DataElement deSource;
+  private DataElement deTarget;
+  private DataSet ds1;
+  private DataSet ds2;
+  private DataElementGroup deg1;
+  private DataElementGroup deg2;
+
+  @BeforeEach
+  public void setUp() {
+    deSource = createDataElement('s');
+    deTarget = createDataElement('t');
+
+    dataElementService.addDataElement(deSource);
+    dataElementService.addDataElement(deTarget);
+
+    ds1 = createDataSet('s');
+    ds2 = createDataSet('t');
+
+    dataSetService.addDataSet(ds1);
+    dataSetService.addDataSet(ds2);
+
+    DataSetElement dse1 = new DataSetElement(ds1, deSource);
+    DataSetElement dse2 = new DataSetElement(ds2, deTarget);
+
+    ds1.addDataSetElement(dse1);
+    deSource.getDataSetElements().add(dse1);
+
+    ds2.addDataSetElement(dse2);
+    deTarget.getDataSetElements().add(dse2);
+
+    deg1 = createDataElementGroup('s');
+    deg2 = createDataElementGroup('t');
+
+    deg1.addDataElement(deSource);
+    deg2.addDataElement(deTarget);
+
+    manager.save(List.of(deg1, deg2));
+    //    deSource.addDataElementGroup(deg1);
+    //    deTarget.addDataElementGroup(deg2);
+
+    dataElementService.updateDataElement(deSource);
+    dataElementService.updateDataElement(deTarget);
+
+    dataSetService.updateDataSet(ds1);
+    dataSetService.updateDataSet(ds2);
+  }
+
+  @Test
+  @DisplayName("Ensure source data elements have no refs to data set elements")
+  void sourceDeNoDseRefsTest() {
+    // given state before merging data set elements
+    assertEquals(
+        1, deSource.getDataSetElements().size(), "source dataSetElements size should be 1");
+    assertEquals(
+        1, deTarget.getDataSetElements().size(), "target dataSetElements size should be 1");
+    assertEquals(1, ds1.getDataSetElements().size(), "dataset dataSetElements size should be 1");
+    assertEquals(1, ds2.getDataSetElements().size(), "dataset dataSetElements size should be 1");
+
+    // when
+    mergeHandler.handleDataSetElement(List.of(deSource), deTarget);
+
+    // then check state after merging data set elements
+    assertEquals(0, deSource.getDataSetElements().size(), "source dataSetElement size should be 0");
+    assertEquals(2, deTarget.getDataSetElements().size(), "target dataSetElement size should be 2");
+    assertEquals(1, ds1.getDataSetElements().size(), "dataset dataSetElements size should be 1");
+    assertTrue(
+        ds1.getDataSetElements().stream()
+            .map(DataSetElement::getDataElement)
+            .toList()
+            .contains(deTarget),
+        "dataset dataSetElement had target DE ref");
+    assertEquals(1, ds2.getDataSetElements().size(), "dataset dataSetElements size should be 1");
+  }
+
+  @Test
+  @DisplayName("Ensure source data element groups have no refs to data set elements")
+  void sourceDeNoDegRefsTest() {
+    // given state before merging data element groups
+    assertEquals(1, deSource.getGroups().size(), "source dataElementGroups size should be 1");
+    assertEquals(1, deTarget.getGroups().size(), "target dataElementGroups size should be 1");
+    assertEquals(1, deg1.getMembers().size(), "dataElementGroup member size should be 1");
+    assertEquals(1, deg2.getMembers().size(), "dataElementGroup member size should be 1");
+
+    // when
+    mergeHandler.handleDataElementGroup(List.of(deSource), deTarget);
+
+    // then check state after merging data element groups
+    assertEquals(0, deSource.getGroups().size(), "source dataElementGroups size should be 0");
+    assertEquals(2, deTarget.getGroups().size(), "target dataElementGroups size should be 2");
+    assertEquals(1, deg1.getMembers().size(), "dataElementGroup member size should be 1");
+    assertEquals(1, deg2.getMembers().size(), "dataElementGroup member size should be 1");
+  }
+}