From 7fad3201d480e435644b1dcf6455885f9efed8fb Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Fri, 8 Nov 2024 08:57:25 +0100 Subject: [PATCH 01/33] Prevent import of duplicate COCs --- .../org/hisp/dhis/feedback/ErrorCode.java | 2 + .../CategoryOptionComboObjectBundleHook.java | 81 +++++++++++++++++++ .../CategoryOptionComboControllerTest.java | 32 ++++++++ 3 files changed, 115 insertions(+) create mode 100644 dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java index 83345ed8dbf3..6311a62aeaa0 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java @@ -66,6 +66,8 @@ public enum ErrorCode { E1119("{0} already exists: `{1}`"), E1120("Update cannot be applied as it would make existing data values inaccessible"), + E1121("Category option combo {0} already exists for category combo {1}"), + /* Org unit merge */ E1500("At least two source orgs unit must be specified"), E1501("Target org unit must be specified"), diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java new file mode 100644 index 000000000000..d9dc81fa3ed5 --- /dev/null +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -0,0 +1,81 @@ +/* + * 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.dxf2.metadata.objectbundle.hooks; + +import java.util.List; +import java.util.function.Consumer; +import java.util.stream.Collectors; +import lombok.AllArgsConstructor; +import org.hisp.dhis.category.CategoryCombo; +import org.hisp.dhis.category.CategoryOptionCombo; +import org.hisp.dhis.category.CategoryService; +import org.hisp.dhis.dxf2.metadata.objectbundle.ObjectBundle; +import org.hisp.dhis.feedback.ErrorCode; +import org.hisp.dhis.feedback.ErrorReport; +import org.springframework.stereotype.Component; + +@Component +@AllArgsConstructor +public class CategoryOptionComboObjectBundleHook + extends AbstractObjectBundleHook { + private final CategoryService categoryService; + + static boolean areEqual(CategoryOptionCombo one, CategoryOptionCombo other) { + return one.equals(other); + } + + private void checkDuplicateCategoryOptionCombos( + CategoryOptionCombo categoryOptionCombo, Consumer addReports) { + + CategoryCombo categoryCombo = categoryOptionCombo.getCategoryCombo(); + + List categoryOptionCombos = + categoryService.getAllCategoryOptionCombos().stream() + .filter(coc -> coc.getCategoryCombo().equals(categoryCombo)) + .collect(Collectors.toList()); + + for (CategoryOptionCombo existingCategoryOptionCombo : categoryOptionCombos) { + if (areEqual(categoryOptionCombo, existingCategoryOptionCombo)) { + addReports.accept( + new ErrorReport( + CategoryOptionCombo.class, + ErrorCode.E1121, + categoryOptionCombo.getName(), + existingCategoryOptionCombo.getName())); + } + } + } + + @Override + public void validate( + CategoryOptionCombo categoryOptionCombo, + ObjectBundle bundle, + Consumer addReports) { + checkDuplicateCategoryOptionCombos(categoryOptionCombo, addReports); + } +} diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java index ab13aa6564e4..e1e3b34738b2 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java @@ -27,6 +27,7 @@ */ package org.hisp.dhis.webapi.controller; +import static org.hisp.dhis.http.HttpAssertions.assertStatus; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -38,6 +39,9 @@ import org.hisp.dhis.category.CategoryService; import org.hisp.dhis.http.HttpStatus; import org.hisp.dhis.jsontree.JsonArray; +import org.hisp.dhis.jsontree.JsonList; +import org.hisp.dhis.jsontree.JsonMixed; +import org.hisp.dhis.jsontree.JsonObject; import org.hisp.dhis.test.webapi.H2ControllerIntegrationTestBase; import org.hisp.dhis.test.webapi.json.domain.JsonCategoryOptionCombo; import org.hisp.dhis.test.webapi.json.domain.JsonIdentifiableObject; @@ -115,4 +119,32 @@ void catOptionCombosExcludingDefaultTest() { assertFalse( catOptionComboNames.contains("default"), "default catOptionCombo is not in payload"); } + + @Test + @DisplayName("Duplicate CategoryOptionCombos should not be allowed") + void catOptionCombosDuplicatedTest() { + + JsonObject response = + GET("/categoryOptionCombos?filter=id:eq:CocUid0001&fields=id,categoryCombo[id],categoryOptions[id]") + .content(); + JsonList catOptionCombos = + response.getList("categoryOptionCombos", JsonCategoryOptionCombo.class); + String catOptionComboAOptions = catOptionCombos.get(0).getCategoryOptions().get(0).getId(); + String catOptionComboACatComboId = catOptionCombos.get(0).getCategoryCombo().getId(); + + HttpResponse postResponse = + POST( + "/categoryOptionCombos", + """ + { "name": "A_1", + "categoryOptions" : [{"id" : "%s"}], + "categoryCombo" : {"id" : "%s"} } + """ + .formatted(catOptionComboAOptions, catOptionComboACatComboId)); + assertStatus(HttpStatus.CONFLICT, postResponse); + JsonMixed postResponseContent = postResponse.content(); + assertEquals( + "CategoryOptionCombo with name A_1 already exists", + postResponseContent.getString("message")); + } } From 8320c5d7f9d5e3c80dde4eebea17d032f63e4e9f Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Fri, 8 Nov 2024 10:33:02 +0100 Subject: [PATCH 02/33] Fix code smells --- .../hooks/CategoryOptionComboObjectBundleHook.java | 3 +-- .../webapi/controller/CategoryOptionComboControllerTest.java | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index d9dc81fa3ed5..faf671ac3cbe 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -56,8 +56,7 @@ private void checkDuplicateCategoryOptionCombos( List categoryOptionCombos = categoryService.getAllCategoryOptionCombos().stream() - .filter(coc -> coc.getCategoryCombo().equals(categoryCombo)) - .collect(Collectors.toList()); + .filter(coc -> coc.getCategoryCombo().equals(categoryCombo)).toList(); for (CategoryOptionCombo existingCategoryOptionCombo : categoryOptionCombos) { if (areEqual(categoryOptionCombo, existingCategoryOptionCombo)) { diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java index e1e3b34738b2..ea6c3ff1bbc3 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java @@ -143,8 +143,9 @@ void catOptionCombosDuplicatedTest() { .formatted(catOptionComboAOptions, catOptionComboACatComboId)); assertStatus(HttpStatus.CONFLICT, postResponse); JsonMixed postResponseContent = postResponse.content(); + String message = postResponseContent.getString("message").string(); assertEquals( "CategoryOptionCombo with name A_1 already exists", - postResponseContent.getString("message")); + message); } } From 7b4a904a0f3f7bc37fa46558fa83c6e64eef9e7b Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Fri, 8 Nov 2024 10:33:17 +0100 Subject: [PATCH 03/33] Linting --- .../hooks/CategoryOptionComboObjectBundleHook.java | 4 ++-- .../webapi/controller/CategoryOptionComboControllerTest.java | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index faf671ac3cbe..f91bb6c58a49 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -29,7 +29,6 @@ import java.util.List; import java.util.function.Consumer; -import java.util.stream.Collectors; import lombok.AllArgsConstructor; import org.hisp.dhis.category.CategoryCombo; import org.hisp.dhis.category.CategoryOptionCombo; @@ -56,7 +55,8 @@ private void checkDuplicateCategoryOptionCombos( List categoryOptionCombos = categoryService.getAllCategoryOptionCombos().stream() - .filter(coc -> coc.getCategoryCombo().equals(categoryCombo)).toList(); + .filter(coc -> coc.getCategoryCombo().equals(categoryCombo)) + .toList(); for (CategoryOptionCombo existingCategoryOptionCombo : categoryOptionCombos) { if (areEqual(categoryOptionCombo, existingCategoryOptionCombo)) { diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java index ea6c3ff1bbc3..d4852c30c942 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java @@ -144,8 +144,6 @@ void catOptionCombosDuplicatedTest() { assertStatus(HttpStatus.CONFLICT, postResponse); JsonMixed postResponseContent = postResponse.content(); String message = postResponseContent.getString("message").string(); - assertEquals( - "CategoryOptionCombo with name A_1 already exists", - message); + assertEquals("CategoryOptionCombo with name A_1 already exists", message); } } From f2932787775aeee6c7c494aa831fd846b2890170 Mon Sep 17 00:00:00 2001 From: David Mackessy Date: Fri, 8 Nov 2024 13:50:11 +0000 Subject: [PATCH 04/33] feat: Update error code & remove redundant equals method --- .../src/main/java/org/hisp/dhis/feedback/ErrorCode.java | 2 +- .../hooks/CategoryOptionComboObjectBundleHook.java | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java index 6311a62aeaa0..f844ffe57064 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java @@ -66,7 +66,7 @@ public enum ErrorCode { E1119("{0} already exists: `{1}`"), E1120("Update cannot be applied as it would make existing data values inaccessible"), - E1121("Category option combo {0} already exists for category combo {1}"), + E1122("Category option combo {0} already exists for category combo {1}"), /* Org unit merge */ E1500("At least two source orgs unit must be specified"), diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index f91bb6c58a49..b888b2600a05 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -44,10 +44,6 @@ public class CategoryOptionComboObjectBundleHook extends AbstractObjectBundleHook { private final CategoryService categoryService; - static boolean areEqual(CategoryOptionCombo one, CategoryOptionCombo other) { - return one.equals(other); - } - private void checkDuplicateCategoryOptionCombos( CategoryOptionCombo categoryOptionCombo, Consumer addReports) { @@ -59,11 +55,11 @@ private void checkDuplicateCategoryOptionCombos( .toList(); for (CategoryOptionCombo existingCategoryOptionCombo : categoryOptionCombos) { - if (areEqual(categoryOptionCombo, existingCategoryOptionCombo)) { + if (categoryOptionCombo.equals(existingCategoryOptionCombo)) { addReports.accept( new ErrorReport( CategoryOptionCombo.class, - ErrorCode.E1121, + ErrorCode.E1122, categoryOptionCombo.getName(), existingCategoryOptionCombo.getName())); } From 2ec75687ce1e26dd8c44ca14a5a471f13e63603b Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Sun, 10 Nov 2024 10:34:44 +0100 Subject: [PATCH 05/33] Rework some code --- .../CategoryOptionComboObjectBundleHook.java | 42 +++++++++++++++---- .../CategoryOptionComboControllerTest.java | 31 +++++++------- 2 files changed, 51 insertions(+), 22 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index b888b2600a05..501636552c12 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -28,9 +28,10 @@ package org.hisp.dhis.dxf2.metadata.objectbundle.hooks; import java.util.List; +import java.util.Set; import java.util.function.Consumer; +import java.util.stream.Collectors; import lombok.AllArgsConstructor; -import org.hisp.dhis.category.CategoryCombo; import org.hisp.dhis.category.CategoryOptionCombo; import org.hisp.dhis.category.CategoryService; import org.hisp.dhis.dxf2.metadata.objectbundle.ObjectBundle; @@ -44,18 +45,43 @@ public class CategoryOptionComboObjectBundleHook extends AbstractObjectBundleHook { private final CategoryService categoryService; + static boolean areEqual(CategoryOptionCombo one, CategoryOptionCombo other) { + if (one == null || other == null) { + return false; + } + + if (one.getCategoryCombo() == null || other.getCategoryCombo() == null) { + return false; + } + + if (!one.getCategoryCombo().getUid().equals(other.getCategoryCombo().getUid())) { + return false; + } + + if (one.getCategoryOptions() == null || other.getCategoryOptions() == null) { + return false; + } + + Set oneCategoryOptionUids = + one.getCategoryOptions().stream() + .map(categoryOption -> categoryOption.getUid()) + .collect(Collectors.toSet()); + + Set otherCategoryOptionUids = + other.getCategoryOptions().stream() + .map(categoryOption -> categoryOption.getUid()) + .collect(Collectors.toSet()); + + return oneCategoryOptionUids.equals(otherCategoryOptionUids); + } + private void checkDuplicateCategoryOptionCombos( CategoryOptionCombo categoryOptionCombo, Consumer addReports) { - CategoryCombo categoryCombo = categoryOptionCombo.getCategoryCombo(); - - List categoryOptionCombos = - categoryService.getAllCategoryOptionCombos().stream() - .filter(coc -> coc.getCategoryCombo().equals(categoryCombo)) - .toList(); + List categoryOptionCombos = categoryService.getAllCategoryOptionCombos(); for (CategoryOptionCombo existingCategoryOptionCombo : categoryOptionCombos) { - if (categoryOptionCombo.equals(existingCategoryOptionCombo)) { + if (areEqual(categoryOptionCombo, existingCategoryOptionCombo)) { addReports.accept( new ErrorReport( CategoryOptionCombo.class, diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java index d4852c30c942..83b7e97209d0 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java @@ -27,9 +27,9 @@ */ package org.hisp.dhis.webapi.controller; -import static org.hisp.dhis.http.HttpAssertions.assertStatus; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import java.util.Set; import java.util.stream.Collectors; @@ -37,13 +37,14 @@ import org.hisp.dhis.category.CategoryOption; import org.hisp.dhis.category.CategoryOptionCombo; import org.hisp.dhis.category.CategoryService; +import org.hisp.dhis.feedback.ErrorCode; import org.hisp.dhis.http.HttpStatus; import org.hisp.dhis.jsontree.JsonArray; import org.hisp.dhis.jsontree.JsonList; -import org.hisp.dhis.jsontree.JsonMixed; import org.hisp.dhis.jsontree.JsonObject; import org.hisp.dhis.test.webapi.H2ControllerIntegrationTestBase; import org.hisp.dhis.test.webapi.json.domain.JsonCategoryOptionCombo; +import org.hisp.dhis.test.webapi.json.domain.JsonErrorReport; import org.hisp.dhis.test.webapi.json.domain.JsonIdentifiableObject; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -132,18 +133,20 @@ void catOptionCombosDuplicatedTest() { String catOptionComboAOptions = catOptionCombos.get(0).getCategoryOptions().get(0).getId(); String catOptionComboACatComboId = catOptionCombos.get(0).getCategoryCombo().getId(); - HttpResponse postResponse = + JsonErrorReport error = POST( - "/categoryOptionCombos", - """ - { "name": "A_1", - "categoryOptions" : [{"id" : "%s"}], - "categoryCombo" : {"id" : "%s"} } - """ - .formatted(catOptionComboAOptions, catOptionComboACatComboId)); - assertStatus(HttpStatus.CONFLICT, postResponse); - JsonMixed postResponseContent = postResponse.content(); - String message = postResponseContent.getString("message").string(); - assertEquals("CategoryOptionCombo with name A_1 already exists", message); + "/categoryOptionCombos/", + """ + { "name": "A_1", + "categoryOptions" : [{"id" : "%s"}], + "categoryCombo" : {"id" : "%s"} } + """ + .formatted(catOptionComboAOptions, catOptionComboACatComboId)) + .content(HttpStatus.CONFLICT) + .find(JsonErrorReport.class, report -> report.getErrorCode() == ErrorCode.E1122); + assertNotNull(error); + assertEquals( + "Category option combo A_1 already exists for category combo CatOptCombo A", + error.getMessage()); } } From f4380dd5b86dfc67c46ac4bd5ab15e7b76087a6a Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Mon, 11 Nov 2024 07:09:06 +0100 Subject: [PATCH 06/33] Fix code smells --- .../CategoryOptionComboObjectBundleHook.java | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index 501636552c12..e1bd77a5e25f 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -32,6 +32,7 @@ import java.util.function.Consumer; import java.util.stream.Collectors; import lombok.AllArgsConstructor; +import org.hisp.dhis.category.CategoryOption; import org.hisp.dhis.category.CategoryOptionCombo; import org.hisp.dhis.category.CategoryService; import org.hisp.dhis.dxf2.metadata.objectbundle.ObjectBundle; @@ -54,7 +55,7 @@ static boolean areEqual(CategoryOptionCombo one, CategoryOptionCombo other) { return false; } - if (!one.getCategoryCombo().getUid().equals(other.getCategoryCombo().getUid())) { + if (!(one.getCategoryCombo().getId() == other.getCategoryCombo().getId())) { return false; } @@ -62,17 +63,13 @@ static boolean areEqual(CategoryOptionCombo one, CategoryOptionCombo other) { return false; } - Set oneCategoryOptionUids = - one.getCategoryOptions().stream() - .map(categoryOption -> categoryOption.getUid()) - .collect(Collectors.toSet()); + Set oneCategoryOptionIds = + one.getCategoryOptions().stream().map(CategoryOption::getId).collect(Collectors.toSet()); - Set otherCategoryOptionUids = - other.getCategoryOptions().stream() - .map(categoryOption -> categoryOption.getUid()) - .collect(Collectors.toSet()); + Set otherCategoryOptionIds = + other.getCategoryOptions().stream().map(CategoryOption::getId).collect(Collectors.toSet()); - return oneCategoryOptionUids.equals(otherCategoryOptionUids); + return oneCategoryOptionIds.equals(otherCategoryOptionIds); } private void checkDuplicateCategoryOptionCombos( From 52c4050980f27942ecf67b60427622a07278de2f Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Mon, 11 Nov 2024 07:13:56 +0100 Subject: [PATCH 07/33] Minor --- .../hooks/CategoryOptionComboObjectBundleHook.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index e1bd77a5e25f..50aa885e7e07 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -55,7 +55,7 @@ static boolean areEqual(CategoryOptionCombo one, CategoryOptionCombo other) { return false; } - if (!(one.getCategoryCombo().getId() == other.getCategoryCombo().getId())) { + if (!one.getCategoryCombo().getUid().equals(other.getCategoryCombo().getUid())) { return false; } @@ -63,13 +63,13 @@ static boolean areEqual(CategoryOptionCombo one, CategoryOptionCombo other) { return false; } - Set oneCategoryOptionIds = - one.getCategoryOptions().stream().map(CategoryOption::getId).collect(Collectors.toSet()); + Set oneCategoryOptionUids = + one.getCategoryOptions().stream().map(CategoryOption::getUid).collect(Collectors.toSet()); - Set otherCategoryOptionIds = - other.getCategoryOptions().stream().map(CategoryOption::getId).collect(Collectors.toSet()); + Set otherCategoryOptionUids = + other.getCategoryOptions().stream().map(CategoryOption::getUid).collect(Collectors.toSet()); - return oneCategoryOptionIds.equals(otherCategoryOptionIds); + return oneCategoryOptionUids.equals(otherCategoryOptionUids); } private void checkDuplicateCategoryOptionCombos( From bd139398e4203b07275c24e3adf373fc6e186974 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Mon, 11 Nov 2024 14:11:48 +0100 Subject: [PATCH 08/33] Partially fix test --- .../CategoryOptionComboObjectBundleHook.java | 6 +- ...rityCategoryOptionComboDuplicatedTest.java | 81 +++++++++---------- 2 files changed, 44 insertions(+), 43 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index 50aa885e7e07..9c325947047d 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -46,7 +46,8 @@ public class CategoryOptionComboObjectBundleHook extends AbstractObjectBundleHook { private final CategoryService categoryService; - static boolean areEqual(CategoryOptionCombo one, CategoryOptionCombo other) { + static boolean haveEqualCatComboCatOptionReferenceIds( + CategoryOptionCombo one, CategoryOptionCombo other) { if (one == null || other == null) { return false; } @@ -78,7 +79,8 @@ private void checkDuplicateCategoryOptionCombos( List categoryOptionCombos = categoryService.getAllCategoryOptionCombos(); for (CategoryOptionCombo existingCategoryOptionCombo : categoryOptionCombos) { - if (areEqual(categoryOptionCombo, existingCategoryOptionCombo)) { + if (haveEqualCatComboCatOptionReferenceIds( + categoryOptionCombo, existingCategoryOptionCombo)) { addReports.accept( new ErrorReport( CategoryOptionCombo.class, diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityCategoryOptionComboDuplicatedTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityCategoryOptionComboDuplicatedTest.java index 66016541b3c3..09df877fcb51 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityCategoryOptionComboDuplicatedTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityCategoryOptionComboDuplicatedTest.java @@ -28,8 +28,14 @@ package org.hisp.dhis.webapi.controller.dataintegrity; import static org.hisp.dhis.http.HttpAssertions.assertStatus; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import java.util.HashSet; import java.util.Set; +import org.hisp.dhis.category.CategoryCombo; +import org.hisp.dhis.category.CategoryOption; +import org.hisp.dhis.category.CategoryOptionCombo; import org.hisp.dhis.http.HttpStatus; import org.hisp.dhis.jsontree.JsonList; import org.hisp.dhis.jsontree.JsonObject; @@ -48,8 +54,6 @@ class DataIntegrityCategoryOptionComboDuplicatedTest extends AbstractDataIntegri private final String check = "category_option_combos_have_duplicates"; - private String cocWithOptionsA; - private String categoryOptionRed; @Test @@ -80,44 +84,40 @@ void testCategoryOptionCombosDuplicated() { + categoryColor + "'}]} ")); - cocWithOptionsA = - assertStatus( - HttpStatus.CREATED, - POST( - "/categoryOptionCombos", - """ - { "name": "Reddish", - "categoryOptions" : [{"id" : "%s"}], - "categoryCombo" : {"id" : "%s"} } - """ - .formatted(categoryOptionRed, testCatCombo))); + HttpResponse response = GET("/categoryOptionCombos?fields=id,name&filter=name:eq:Red"); + assertStatus(HttpStatus.OK, response); + JsonObject responseContent = response.content(); - assertStatus( - HttpStatus.CREATED, - POST( - "/categoryOptionCombos", - """ - { "name": "Not Red", - "categoryOptions" : [{"id" : "%s"}]}, - "categoryCombo" : {"id" : "%s"} } - """ - .formatted(categoryOptionRed, testCatCombo))); + JsonList catOptionCombos = + responseContent.getList("categoryOptionCombos", JsonCategoryOptionCombo.class); + assertEquals(1, catOptionCombos.size()); + String redCategoryOptionComboId = catOptionCombos.get(0).getId(); + + /*We must resort to the service layer as the API will not allow us to create a duplicate*/ + CategoryCombo categoryCombo = categoryService.getCategoryCombo(testCatCombo); + CategoryOptionCombo existingCategoryOptionCombo = + categoryService.getCategoryOptionCombo(redCategoryOptionComboId); + CategoryOptionCombo categoryOptionComboDuplicate = new CategoryOptionCombo(); + categoryOptionComboDuplicate.setAutoFields(); + categoryOptionComboDuplicate.setCategoryCombo(categoryCombo); + Set newCategoryOptions = + new HashSet<>(existingCategoryOptionCombo.getCategoryOptions()); + categoryOptionComboDuplicate.setCategoryOptions(newCategoryOptions); + categoryOptionComboDuplicate.setName("Reddish"); + manager.persist(categoryOptionComboDuplicate); + dbmsManager.clearSession(); + String categoryOptionComboDuplicatedID = categoryOptionComboDuplicate.getUid(); + assertNotNull(categoryOptionComboDuplicatedID); assertNamedMetadataObjectExists("categoryOptionCombos", "default"); assertNamedMetadataObjectExists("categoryOptionCombos", "Red"); assertNamedMetadataObjectExists("categoryOptionCombos", "Reddish"); - assertNamedMetadataObjectExists("categoryOptionCombos", "Not Red"); - - /*We need to get the Red category option combo to be able to check the data integrity issues*/ - JsonObject response = GET("/categoryOptionCombos?fields=id,name&filter=name:eq:Red").content(); - JsonList catOptionCombos = - response.getList("categoryOptionCombos", JsonCategoryOptionCombo.class); - String redCategoryOptionComboId = catOptionCombos.get(0).getId(); - /* There are four total category option combos, so we expect 25% */ - checkDataIntegritySummary(check, 1, 25, true); + /* There are three total category option combos, so we expect 33% */ + checkDataIntegritySummary(check, 1, 33, true); - Set expectedCategoryOptCombos = Set.of(cocWithOptionsA, redCategoryOptionComboId); + Set expectedCategoryOptCombos = + Set.of(categoryOptionComboDuplicatedID, redCategoryOptionComboId); Set expectedMessages = Set.of("Red", "Reddish"); checkDataIntegrityDetailsIssues( check, expectedCategoryOptCombos, expectedMessages, Set.of(), "categoryOptionCombos"); @@ -135,17 +135,16 @@ void testCategoryOptionCombosNotDuplicated() { HttpStatus.CREATED, POST("/categoryOptions", "{ 'name': 'Blue', 'shortName': 'Blue' }")); - cocWithOptionsA = - assertStatus( - HttpStatus.CREATED, - POST( - "/categoryOptionCombos", - """ + assertStatus( + HttpStatus.CREATED, + POST( + "/categoryOptionCombos", + """ { "name": "Color", "categoryOptions" : [{"id" : "%s"} ] } """ - .formatted(categoryOptionRed))); - + .formatted(categoryOptionRed))); + /*TODO: This test should not pass*/ assertStatus( HttpStatus.CREATED, POST( From ce0b9028c9c921ab076fc56c9a541ccd87ad9719 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Mon, 11 Nov 2024 15:11:25 +0100 Subject: [PATCH 09/33] Fix/disable more tests --- .../CategoryOptionComboObjectBundleHook.java | 4 ++-- .../preheat/TrackerPreheatIdentifiersTest.java | 4 +++- .../preheat/TrackerPreheatServiceTest.java | 3 +++ .../test/resources/tracker/event_metadata.json | 18 ------------------ .../resources/tracker/identifier_metadata.json | 17 ----------------- 5 files changed, 8 insertions(+), 38 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index 9c325947047d..9b6cd2964583 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -79,8 +79,8 @@ private void checkDuplicateCategoryOptionCombos( List categoryOptionCombos = categoryService.getAllCategoryOptionCombos(); for (CategoryOptionCombo existingCategoryOptionCombo : categoryOptionCombos) { - if (haveEqualCatComboCatOptionReferenceIds( - categoryOptionCombo, existingCategoryOptionCombo)) { + if (haveEqualCatComboCatOptionReferenceIds(categoryOptionCombo, existingCategoryOptionCombo) + && !categoryOptionCombo.getUid().equals(existingCategoryOptionCombo.getUid())) { addReports.accept( new ErrorReport( CategoryOptionCombo.class, diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java index a70af93fafa7..39c1f0dd8fc9 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java @@ -60,6 +60,7 @@ import org.hisp.dhis.tracker.imports.domain.TrackerObjects; import org.hisp.dhis.user.User; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -162,7 +163,8 @@ void testCategoryOptionIdentifiers() { } } - @Test + @Disabled + // Disabling this as we cannot import duplicate category option combos for default void testCategoryOptionComboIdentifiers() { List> data = buildDataSet("XXXvX50cXC0", "COCA", "COCAname"); for (Pair pair : data) { diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java index 1d083bb5f2e8..ab2c81b331b1 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java @@ -149,7 +149,10 @@ void testPreheatEvents() throws IOException { assertFalse(preheat.getAll(OrganisationUnit.class).isEmpty()); assertFalse(preheat.getAll(ProgramStage.class).isEmpty()); assertFalse(preheat.getAll(CategoryOptionCombo.class).isEmpty()); + /* Disabling this as we cannot import duplicate category option combos for default + https://dhis2.atlassian.net/browse/DHIS2-18401 assertNotNull(preheat.get(CategoryOptionCombo.class, "XXXvX50cXC0")); + */ assertNotNull(preheat.get(CategoryOption.class, "XXXrKDKCefk")); } } diff --git a/dhis-2/dhis-test-integration/src/test/resources/tracker/event_metadata.json b/dhis-2/dhis-test-integration/src/test/resources/tracker/event_metadata.json index 08e0c19e4bf9..8e41a3676f9c 100644 --- a/dhis-2/dhis-test-integration/src/test/resources/tracker/event_metadata.json +++ b/dhis-2/dhis-test-integration/src/test/resources/tracker/event_metadata.json @@ -1820,24 +1820,6 @@ "id": "xYerKDKCefk" } ] - }, - { - "lastUpdated": "2019-01-28T11:01:30.775", - "code": "COCA", - "created": "2019-01-28T11:01:30.771", - "name": "COCAname", - "id": "XXXvX50cXC0", - "ignoreApproval": false, - "categoryCombo": { - "id": "bjDvmb4bfuf" - }, - "translations": [], - "attributeValues": [], - "categoryOptions": [ - { - "id": "xYerKDKCefk" - } - ] } ], "categories": [ diff --git a/dhis-2/dhis-test-integration/src/test/resources/tracker/identifier_metadata.json b/dhis-2/dhis-test-integration/src/test/resources/tracker/identifier_metadata.json index f4925a896ee3..eed1a1b69336 100644 --- a/dhis-2/dhis-test-integration/src/test/resources/tracker/identifier_metadata.json +++ b/dhis-2/dhis-test-integration/src/test/resources/tracker/identifier_metadata.json @@ -204,23 +204,6 @@ "id": "xYerKDKCefk" } ] - }, - { - "lastUpdated": "2019-01-28T11:01:30.775", - "code": "COCA", - "created": "2019-01-28T11:01:30.771", - "name": "COCAname", - "id": "XXXvX50cXC0", - "ignoreApproval": false, - "categoryCombo": { - "id": "bjDvmb4bfuf" - }, - "attributeValues": [], - "categoryOptions": [ - { - "id": "xYerKDKCefk" - } - ] } ], "categories": [ From 1ae338e5e3292f3dec329bf81811ea1420fbfcac Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Mon, 11 Nov 2024 19:11:29 +0100 Subject: [PATCH 10/33] Add additional checks for COCs --- .../org/hisp/dhis/feedback/ErrorCode.java | 2 + .../CategoryOptionComboObjectBundleHook.java | 17 ++++++++ ...rityCategoryOptionComboDuplicatedTest.java | 40 ++++++++++--------- 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java index f844ffe57064..6f2e0138cd55 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java @@ -67,6 +67,8 @@ public enum ErrorCode { E1120("Update cannot be applied as it would make existing data values inaccessible"), E1122("Category option combo {0} already exists for category combo {1}"), + E1123("Category option combo {0} must be associated with a category combo"), + E1124("Category option combo {0} cannot be assocated with the default category combo"), /* Org unit merge */ E1500("At least two source orgs unit must be specified"), diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index 9b6cd2964583..a2c28c7814f0 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -32,6 +32,7 @@ import java.util.function.Consumer; import java.util.stream.Collectors; import lombok.AllArgsConstructor; +import org.hisp.dhis.category.CategoryCombo; import org.hisp.dhis.category.CategoryOption; import org.hisp.dhis.category.CategoryOptionCombo; import org.hisp.dhis.category.CategoryService; @@ -46,6 +47,7 @@ public class CategoryOptionComboObjectBundleHook extends AbstractObjectBundleHook { private final CategoryService categoryService; + static boolean haveEqualCatComboCatOptionReferenceIds( CategoryOptionCombo one, CategoryOptionCombo other) { if (one == null || other == null) { @@ -73,6 +75,12 @@ static boolean haveEqualCatComboCatOptionReferenceIds( return oneCategoryOptionUids.equals(otherCategoryOptionUids); } + private void checkNullCategoryComboReference( + CategoryOptionCombo categoryOptionCombo, Consumer addReports) { + if (categoryOptionCombo.getCategoryCombo() == null) { + addReports.accept(new ErrorReport(CategoryOptionCombo.class, ErrorCode.E1123)); + } + } private void checkDuplicateCategoryOptionCombos( CategoryOptionCombo categoryOptionCombo, Consumer addReports) { @@ -91,11 +99,20 @@ private void checkDuplicateCategoryOptionCombos( } } + private void checkAlternativeDefaultCatOptionCombo( + CategoryOptionCombo categoryOptionCombo, Consumer addReports) { + if (categoryOptionCombo.getCategoryCombo().isDefault() ) { + addReports.accept(new ErrorReport(CategoryOptionCombo.class, ErrorCode.E1124)); + } + } + @Override public void validate( CategoryOptionCombo categoryOptionCombo, ObjectBundle bundle, Consumer addReports) { + checkNullCategoryComboReference(categoryOptionCombo, addReports); + checkAlternativeDefaultCatOptionCombo(categoryOptionCombo, addReports); checkDuplicateCategoryOptionCombos(categoryOptionCombo, addReports); } } diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityCategoryOptionComboDuplicatedTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityCategoryOptionComboDuplicatedTest.java index 09df877fcb51..a474555b4f98 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityCategoryOptionComboDuplicatedTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityCategoryOptionComboDuplicatedTest.java @@ -135,26 +135,28 @@ void testCategoryOptionCombosNotDuplicated() { HttpStatus.CREATED, POST("/categoryOptions", "{ 'name': 'Blue', 'shortName': 'Blue' }")); - assertStatus( - HttpStatus.CREATED, - POST( - "/categoryOptionCombos", - """ - { "name": "Color", - "categoryOptions" : [{"id" : "%s"} ] } - """ - .formatted(categoryOptionRed))); - /*TODO: This test should not pass*/ - assertStatus( - HttpStatus.CREATED, - POST( - "/categoryOptionCombos", - """ - { "name": "Colour", - "categoryOptions" : [{"id" : "%s"} ] } - """ - .formatted(categoryOptionBlue))); + String categoryColor = + assertStatus( + HttpStatus.CREATED, + POST( + "/categories", + "{ 'name': 'Color', 'shortName': 'Color', 'dataDimensionType': 'DISAGGREGATION' ," + + "'categoryOptions' : [{'id' : '" + + categoryOptionRed + + "'}, {'id' : '" + + categoryOptionBlue + + "'} ] }")); + String testCatCombo = + assertStatus( + HttpStatus.CREATED, + POST( + "/categoryCombos", + "{ 'name' : 'Color', " + + "'dataDimensionType' : 'DISAGGREGATION', 'categories' : [" + + "{'id' : '" + + categoryColor + + "'}]} ")); assertHasNoDataIntegrityIssues("categoryOptionCombos", check, true); } From aed074ed859f539934327a3cca1f12de0ccf4ae3 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Mon, 11 Nov 2024 19:11:57 +0100 Subject: [PATCH 11/33] Linting --- .../hooks/CategoryOptionComboObjectBundleHook.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index a2c28c7814f0..bd8eac570aa7 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -32,7 +32,6 @@ import java.util.function.Consumer; import java.util.stream.Collectors; import lombok.AllArgsConstructor; -import org.hisp.dhis.category.CategoryCombo; import org.hisp.dhis.category.CategoryOption; import org.hisp.dhis.category.CategoryOptionCombo; import org.hisp.dhis.category.CategoryService; @@ -47,7 +46,6 @@ public class CategoryOptionComboObjectBundleHook extends AbstractObjectBundleHook { private final CategoryService categoryService; - static boolean haveEqualCatComboCatOptionReferenceIds( CategoryOptionCombo one, CategoryOptionCombo other) { if (one == null || other == null) { @@ -81,6 +79,7 @@ private void checkNullCategoryComboReference( addReports.accept(new ErrorReport(CategoryOptionCombo.class, ErrorCode.E1123)); } } + private void checkDuplicateCategoryOptionCombos( CategoryOptionCombo categoryOptionCombo, Consumer addReports) { @@ -101,8 +100,8 @@ private void checkDuplicateCategoryOptionCombos( private void checkAlternativeDefaultCatOptionCombo( CategoryOptionCombo categoryOptionCombo, Consumer addReports) { - if (categoryOptionCombo.getCategoryCombo().isDefault() ) { - addReports.accept(new ErrorReport(CategoryOptionCombo.class, ErrorCode.E1124)); + if (categoryOptionCombo.getCategoryCombo().isDefault()) { + addReports.accept(new ErrorReport(CategoryOptionCombo.class, ErrorCode.E1124)); } } From 1ef9b79be96023a868635ee7d8ee99233e7dce9a Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Tue, 12 Nov 2024 15:30:20 +0100 Subject: [PATCH 12/33] Update tests --- .../org/hisp/dhis/feedback/ErrorCode.java | 2 +- .../CategoryOptionComboObjectBundleHook.java | 22 ++----- .../CategoryOptionComboControllerTest.java | 65 +++++++++++++++++++ 3 files changed, 72 insertions(+), 17 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java index 6f2e0138cd55..875032c6b3e1 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java @@ -68,7 +68,7 @@ public enum ErrorCode { E1122("Category option combo {0} already exists for category combo {1}"), E1123("Category option combo {0} must be associated with a category combo"), - E1124("Category option combo {0} cannot be assocated with the default category combo"), + E1124("Category option combo {0} cannot be associated with the default category combo"), /* Org unit merge */ E1500("At least two source orgs unit must be specified"), diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index bd8eac570aa7..3f2a90d2379f 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -52,10 +52,16 @@ static boolean haveEqualCatComboCatOptionReferenceIds( return false; } + //One default and one not default if (one.getCategoryCombo() == null || other.getCategoryCombo() == null) { return false; } + //Both default + if (one.getCategoryCombo() == null && other.getCategoryCombo() == null){ + return true; + } + if (!one.getCategoryCombo().getUid().equals(other.getCategoryCombo().getUid())) { return false; } @@ -73,13 +79,6 @@ static boolean haveEqualCatComboCatOptionReferenceIds( return oneCategoryOptionUids.equals(otherCategoryOptionUids); } - private void checkNullCategoryComboReference( - CategoryOptionCombo categoryOptionCombo, Consumer addReports) { - if (categoryOptionCombo.getCategoryCombo() == null) { - addReports.accept(new ErrorReport(CategoryOptionCombo.class, ErrorCode.E1123)); - } - } - private void checkDuplicateCategoryOptionCombos( CategoryOptionCombo categoryOptionCombo, Consumer addReports) { @@ -98,20 +97,11 @@ private void checkDuplicateCategoryOptionCombos( } } - private void checkAlternativeDefaultCatOptionCombo( - CategoryOptionCombo categoryOptionCombo, Consumer addReports) { - if (categoryOptionCombo.getCategoryCombo().isDefault()) { - addReports.accept(new ErrorReport(CategoryOptionCombo.class, ErrorCode.E1124)); - } - } - @Override public void validate( CategoryOptionCombo categoryOptionCombo, ObjectBundle bundle, Consumer addReports) { - checkNullCategoryComboReference(categoryOptionCombo, addReports); - checkAlternativeDefaultCatOptionCombo(categoryOptionCombo, addReports); checkDuplicateCategoryOptionCombos(categoryOptionCombo, addReports); } } diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java index 83b7e97209d0..330aaf9d2250 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java @@ -27,6 +27,7 @@ */ package org.hisp.dhis.webapi.controller; +import static org.hisp.dhis.http.HttpAssertions.assertStatus; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -41,12 +42,14 @@ import org.hisp.dhis.http.HttpStatus; import org.hisp.dhis.jsontree.JsonArray; import org.hisp.dhis.jsontree.JsonList; +import org.hisp.dhis.jsontree.JsonMixed; import org.hisp.dhis.jsontree.JsonObject; import org.hisp.dhis.test.webapi.H2ControllerIntegrationTestBase; import org.hisp.dhis.test.webapi.json.domain.JsonCategoryOptionCombo; import org.hisp.dhis.test.webapi.json.domain.JsonErrorReport; import org.hisp.dhis.test.webapi.json.domain.JsonIdentifiableObject; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -149,4 +152,66 @@ void catOptionCombosDuplicatedTest() { "Category option combo A_1 already exists for category combo CatOptCombo A", error.getMessage()); } + + @Test + @DisplayName( "Duplicate default category option combos should not be allowed" ) + void catOptionCombosDuplicatedDefaultTest() + { + JsonObject response = GET( + "/categoryOptionCombos?filter=name:eq:default&fields=id,categoryCombo[id],categoryOptions[id]" ).content(); + JsonList catOptionCombos = response.getList( "categoryOptionCombos", + JsonCategoryOptionCombo.class ); + String defaultCatOptionComboOptions = catOptionCombos.get( 0 ).getCategoryOptions().get( 0 ).getId(); + String defaultCatOptionComboCatComboId = catOptionCombos.get( 0 ).getCategoryCombo().getId(); + response = POST( "/categoryOptionCombos/", """ + { "name": "Not default", + "categoryOptions" : [{"id" : "%s"}], + "categoryCombo" : {"id" : "%s"} } + """.formatted( defaultCatOptionComboOptions, defaultCatOptionComboCatComboId ) ).content( HttpStatus.CONFLICT ); + + + JsonErrorReport error = response.find( JsonErrorReport.class, report -> report.getErrorCode() == ErrorCode.E1122 ); + assertNotNull( error ); + assertEquals( "Category option combo Not default already exists for category combo default", error.getMessage() ); + + } + + @Disabled("This test is not working as expected") + @DisplayName( "Default category option combos with non-default category options should not be allowed" ) + void catOptionComboDefaultWithNonDefaultOptionsNotAllowedTest() + { + JsonObject response = GET( + "/categoryOptionCombos?filter=name:eq:default&fields=id,categoryCombo[id],categoryOptions[id]" ).content(); + JsonList catOptionCombos = response.getList( "categoryOptionCombos", + JsonCategoryOptionCombo.class ); + String defaultCategoryComboId = catOptionCombos.get( 0 ).getCategoryCombo().getId(); + + String categoryOptionRed = assertStatus( + HttpStatus.CREATED, POST("/categoryOptions", "{ 'name': 'Red', 'shortName': 'Red' }")); + + + JsonMixed response2 = POST( "/categoryOptionCombos/", """ + { "name": "Not default", + "categoryOptions" : [{"id" : "%s"}], + "categoryCombo" : {"id" : "%s"} } + """.formatted( categoryOptionRed, defaultCategoryComboId ) ).content(); + + JsonErrorReport error = response.find( JsonErrorReport.class, report -> report.getErrorCode() == ErrorCode.E1124 ); + assertNotNull( error ); + assertEquals( "Category option combo Not default already exists for category combo default", error.getMessage() ); + + } + + @Test + @DisplayName( "Empty catcombo for a category option combo not allowed" ) + void catOptionComboEmptyCatComboTest() + { + JsonMixed response = POST( "/categoryOptionCombos/", """ + { "name": "Empty catcombo", + "categoryOptions" : [{"id" : "CocUid0001"}], + "categoryCombo" : {} } + """ ).content( HttpStatus.CONFLICT ); + assertEquals( "\"Post-condition was null\"", response.get( "message" ).toString() ); + } + } From 3191aba4384f6146c7a80ffdabc18ece2d5ce34b Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Tue, 12 Nov 2024 15:31:23 +0100 Subject: [PATCH 13/33] Linting --- .../CategoryOptionComboObjectBundleHook.java | 6 +- .../CategoryOptionComboControllerTest.java | 97 +++++++++++-------- 2 files changed, 60 insertions(+), 43 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index 3f2a90d2379f..8803891fc861 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -52,13 +52,13 @@ static boolean haveEqualCatComboCatOptionReferenceIds( return false; } - //One default and one not default + // One default and one not default if (one.getCategoryCombo() == null || other.getCategoryCombo() == null) { return false; } - //Both default - if (one.getCategoryCombo() == null && other.getCategoryCombo() == null){ + // Both default + if (one.getCategoryCombo() == null && other.getCategoryCombo() == null) { return true; } diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java index 330aaf9d2250..734a8a432a09 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java @@ -154,64 +154,81 @@ void catOptionCombosDuplicatedTest() { } @Test - @DisplayName( "Duplicate default category option combos should not be allowed" ) - void catOptionCombosDuplicatedDefaultTest() - { - JsonObject response = GET( - "/categoryOptionCombos?filter=name:eq:default&fields=id,categoryCombo[id],categoryOptions[id]" ).content(); - JsonList catOptionCombos = response.getList( "categoryOptionCombos", - JsonCategoryOptionCombo.class ); - String defaultCatOptionComboOptions = catOptionCombos.get( 0 ).getCategoryOptions().get( 0 ).getId(); - String defaultCatOptionComboCatComboId = catOptionCombos.get( 0 ).getCategoryCombo().getId(); - response = POST( "/categoryOptionCombos/", """ + @DisplayName("Duplicate default category option combos should not be allowed") + void catOptionCombosDuplicatedDefaultTest() { + JsonObject response = + GET("/categoryOptionCombos?filter=name:eq:default&fields=id,categoryCombo[id],categoryOptions[id]") + .content(); + JsonList catOptionCombos = + response.getList("categoryOptionCombos", JsonCategoryOptionCombo.class); + String defaultCatOptionComboOptions = + catOptionCombos.get(0).getCategoryOptions().get(0).getId(); + String defaultCatOptionComboCatComboId = catOptionCombos.get(0).getCategoryCombo().getId(); + response = + POST( + "/categoryOptionCombos/", + """ { "name": "Not default", "categoryOptions" : [{"id" : "%s"}], "categoryCombo" : {"id" : "%s"} } - """.formatted( defaultCatOptionComboOptions, defaultCatOptionComboCatComboId ) ).content( HttpStatus.CONFLICT ); - - - JsonErrorReport error = response.find( JsonErrorReport.class, report -> report.getErrorCode() == ErrorCode.E1122 ); - assertNotNull( error ); - assertEquals( "Category option combo Not default already exists for category combo default", error.getMessage() ); + """ + .formatted(defaultCatOptionComboOptions, defaultCatOptionComboCatComboId)) + .content(HttpStatus.CONFLICT); + JsonErrorReport error = + response.find(JsonErrorReport.class, report -> report.getErrorCode() == ErrorCode.E1122); + assertNotNull(error); + assertEquals( + "Category option combo Not default already exists for category combo default", + error.getMessage()); } @Disabled("This test is not working as expected") - @DisplayName( "Default category option combos with non-default category options should not be allowed" ) - void catOptionComboDefaultWithNonDefaultOptionsNotAllowedTest() - { - JsonObject response = GET( - "/categoryOptionCombos?filter=name:eq:default&fields=id,categoryCombo[id],categoryOptions[id]" ).content(); - JsonList catOptionCombos = response.getList( "categoryOptionCombos", - JsonCategoryOptionCombo.class ); - String defaultCategoryComboId = catOptionCombos.get( 0 ).getCategoryCombo().getId(); - - String categoryOptionRed = assertStatus( - HttpStatus.CREATED, POST("/categoryOptions", "{ 'name': 'Red', 'shortName': 'Red' }")); + @DisplayName( + "Default category option combos with non-default category options should not be allowed") + void catOptionComboDefaultWithNonDefaultOptionsNotAllowedTest() { + JsonObject response = + GET("/categoryOptionCombos?filter=name:eq:default&fields=id,categoryCombo[id],categoryOptions[id]") + .content(); + JsonList catOptionCombos = + response.getList("categoryOptionCombos", JsonCategoryOptionCombo.class); + String defaultCategoryComboId = catOptionCombos.get(0).getCategoryCombo().getId(); + String categoryOptionRed = + assertStatus( + HttpStatus.CREATED, POST("/categoryOptions", "{ 'name': 'Red', 'shortName': 'Red' }")); - JsonMixed response2 = POST( "/categoryOptionCombos/", """ + JsonMixed response2 = + POST( + "/categoryOptionCombos/", + """ { "name": "Not default", "categoryOptions" : [{"id" : "%s"}], "categoryCombo" : {"id" : "%s"} } - """.formatted( categoryOptionRed, defaultCategoryComboId ) ).content(); - - JsonErrorReport error = response.find( JsonErrorReport.class, report -> report.getErrorCode() == ErrorCode.E1124 ); - assertNotNull( error ); - assertEquals( "Category option combo Not default already exists for category combo default", error.getMessage() ); + """ + .formatted(categoryOptionRed, defaultCategoryComboId)) + .content(); + JsonErrorReport error = + response.find(JsonErrorReport.class, report -> report.getErrorCode() == ErrorCode.E1124); + assertNotNull(error); + assertEquals( + "Category option combo Not default already exists for category combo default", + error.getMessage()); } @Test - @DisplayName( "Empty catcombo for a category option combo not allowed" ) - void catOptionComboEmptyCatComboTest() - { - JsonMixed response = POST( "/categoryOptionCombos/", """ + @DisplayName("Empty catcombo for a category option combo not allowed") + void catOptionComboEmptyCatComboTest() { + JsonMixed response = + POST( + "/categoryOptionCombos/", + """ { "name": "Empty catcombo", "categoryOptions" : [{"id" : "CocUid0001"}], "categoryCombo" : {} } - """ ).content( HttpStatus.CONFLICT ); - assertEquals( "\"Post-condition was null\"", response.get( "message" ).toString() ); + """) + .content(HttpStatus.CONFLICT); + assertEquals("\"Post-condition was null\"", response.get("message").toString()); } - } From 2f3a45c8b259ce212db8c7b9dcff601c24203de1 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Wed, 13 Nov 2024 08:10:37 +0100 Subject: [PATCH 14/33] Rework tests --- .../hooks/CategoryOptionComboObjectBundleHook.java | 8 +++----- .../src/test/resources/tracker/idSchemesMetadata.json | 1 + .../controller/CategoryOptionComboControllerTest.java | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index 8803891fc861..f81edd828ab3 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -62,10 +62,6 @@ static boolean haveEqualCatComboCatOptionReferenceIds( return true; } - if (!one.getCategoryCombo().getUid().equals(other.getCategoryCombo().getUid())) { - return false; - } - if (one.getCategoryOptions() == null || other.getCategoryOptions() == null) { return false; } @@ -82,7 +78,9 @@ static boolean haveEqualCatComboCatOptionReferenceIds( private void checkDuplicateCategoryOptionCombos( CategoryOptionCombo categoryOptionCombo, Consumer addReports) { - List categoryOptionCombos = categoryService.getAllCategoryOptionCombos(); + List categoryOptionCombos = categoryService.getAllCategoryOptionCombos().stream() + .filter(coc -> coc.getCategoryCombo().getUid().equals(categoryOptionCombo.getCategoryCombo().getUid())) + .collect(Collectors.toList()); for (CategoryOptionCombo existingCategoryOptionCombo : categoryOptionCombos) { if (haveEqualCatComboCatOptionReferenceIds(categoryOptionCombo, existingCategoryOptionCombo) diff --git a/dhis-2/dhis-test-e2e/src/test/resources/tracker/idSchemesMetadata.json b/dhis-2/dhis-test-e2e/src/test/resources/tracker/idSchemesMetadata.json index 195b156af94c..216c33d1dca1 100644 --- a/dhis-2/dhis-test-e2e/src/test/resources/tracker/idSchemesMetadata.json +++ b/dhis-2/dhis-test-e2e/src/test/resources/tracker/idSchemesMetadata.json @@ -742,6 +742,7 @@ ], "categoryCombos": [ { + "code": "TA_CATEGORY_COMBO_ATTRIBUTE", "name": "TA Category combo attribute", "created": "2022-05-30T11:40:03.717", diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java index 734a8a432a09..6935db349d3f 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java @@ -229,6 +229,6 @@ void catOptionComboEmptyCatComboTest() { "categoryCombo" : {} } """) .content(HttpStatus.CONFLICT); - assertEquals("\"Post-condition was null\"", response.get("message").toString()); + assertEquals("\"One or more errors occurred, please see full details in import report.\"", response.get("message").toString()); } } From db6ace730c4f59daae39a3dcd532a10cec16100e Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Wed, 13 Nov 2024 11:06:11 +0100 Subject: [PATCH 15/33] Rework methods a bit --- .../CategoryOptionComboObjectBundleHook.java | 26 +++++++++++++------ .../CategoryOptionComboControllerTest.java | 4 ++- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index f81edd828ab3..2b94bb8fa5cc 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -52,17 +52,15 @@ static boolean haveEqualCatComboCatOptionReferenceIds( return false; } - // One default and one not default if (one.getCategoryCombo() == null || other.getCategoryCombo() == null) { return false; } - // Both default - if (one.getCategoryCombo() == null && other.getCategoryCombo() == null) { - return true; + if (one.getCategoryOptions() == null || other.getCategoryOptions() == null) { + return false; } - if (one.getCategoryOptions() == null || other.getCategoryOptions() == null) { + if (!one.getCategoryCombo().getUid().equals( other.getCategoryCombo().getUid())) { return false; } @@ -78,10 +76,22 @@ static boolean haveEqualCatComboCatOptionReferenceIds( private void checkDuplicateCategoryOptionCombos( CategoryOptionCombo categoryOptionCombo, Consumer addReports) { - List categoryOptionCombos = categoryService.getAllCategoryOptionCombos().stream() - .filter(coc -> coc.getCategoryCombo().getUid().equals(categoryOptionCombo.getCategoryCombo().getUid())) - .collect(Collectors.toList()); + List categoryOptionCombos = + categoryService.getAllCategoryOptionCombos().stream() + .filter( + coc -> + coc.getCategoryCombo() + .getUid() + .equals(categoryOptionCombo.getCategoryCombo().getUid())) + .collect(Collectors.toList()); + // Check if the categoryOptionCombo is already in the list. This could be an update or re-import + // of the same object. + if (categoryOptionCombos.stream().anyMatch(coc -> coc.getUid().equals(categoryOptionCombo.getUid()))) { + return; + } + //Check to see if the COC already exists in the list of COCs + //If it does, then it is a duplicate for (CategoryOptionCombo existingCategoryOptionCombo : categoryOptionCombos) { if (haveEqualCatComboCatOptionReferenceIds(categoryOptionCombo, existingCategoryOptionCombo) && !categoryOptionCombo.getUid().equals(existingCategoryOptionCombo.getUid())) { diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java index 6935db349d3f..5b229771d3cb 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java @@ -229,6 +229,8 @@ void catOptionComboEmptyCatComboTest() { "categoryCombo" : {} } """) .content(HttpStatus.CONFLICT); - assertEquals("\"One or more errors occurred, please see full details in import report.\"", response.get("message").toString()); + assertEquals( + "\"One or more errors occurred, please see full details in import report.\"", + response.get("message").toString()); } } From 74551607c5eee181c013dddb9c6ee71d95c36a27 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Wed, 13 Nov 2024 11:06:36 +0100 Subject: [PATCH 16/33] Linting --- .../hooks/CategoryOptionComboObjectBundleHook.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index 2b94bb8fa5cc..59be74e0ac24 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -60,7 +60,7 @@ static boolean haveEqualCatComboCatOptionReferenceIds( return false; } - if (!one.getCategoryCombo().getUid().equals( other.getCategoryCombo().getUid())) { + if (!one.getCategoryCombo().getUid().equals(other.getCategoryCombo().getUid())) { return false; } @@ -87,11 +87,12 @@ private void checkDuplicateCategoryOptionCombos( // Check if the categoryOptionCombo is already in the list. This could be an update or re-import // of the same object. - if (categoryOptionCombos.stream().anyMatch(coc -> coc.getUid().equals(categoryOptionCombo.getUid()))) { + if (categoryOptionCombos.stream() + .anyMatch(coc -> coc.getUid().equals(categoryOptionCombo.getUid()))) { return; } - //Check to see if the COC already exists in the list of COCs - //If it does, then it is a duplicate + // Check to see if the COC already exists in the list of COCs + // If it does, then it is a duplicate for (CategoryOptionCombo existingCategoryOptionCombo : categoryOptionCombos) { if (haveEqualCatComboCatOptionReferenceIds(categoryOptionCombo, existingCategoryOptionCombo) && !categoryOptionCombo.getUid().equals(existingCategoryOptionCombo.getUid())) { From 2f0e00b0dd22858b50818c6f019c0be8f33037d3 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Mon, 18 Nov 2024 09:10:22 +0100 Subject: [PATCH 17/33] Fix various issues --- .../org/hisp/dhis/feedback/ErrorCode.java | 1 + .../CategoryOptionComboObjectBundleHook.java | 87 ++++++++++++++++--- .../TrackerPreheatIdentifiersTest.java | 3 +- .../preheat/TrackerPreheatServiceTest.java | 6 +- .../CategoryOptionComboControllerTest.java | 26 +----- 5 files changed, 86 insertions(+), 37 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java index 875032c6b3e1..c67008b2b5e9 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java @@ -70,6 +70,7 @@ public enum ErrorCode { E1123("Category option combo {0} must be associated with a category combo"), E1124("Category option combo {0} cannot be associated with the default category combo"), + E1125("Category option combo {0} contains options not associated with category combo {1}"), /* Org unit merge */ E1500("At least two source orgs unit must be specified"), E1501("Target org unit must be specified"), diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index 59be74e0ac24..b00e05121c4a 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -32,6 +32,8 @@ import java.util.function.Consumer; import java.util.stream.Collectors; import lombok.AllArgsConstructor; +import org.hisp.dhis.category.Category; +import org.hisp.dhis.category.CategoryCombo; import org.hisp.dhis.category.CategoryOption; import org.hisp.dhis.category.CategoryOptionCombo; import org.hisp.dhis.category.CategoryService; @@ -46,6 +48,7 @@ public class CategoryOptionComboObjectBundleHook extends AbstractObjectBundleHook { private final CategoryService categoryService; + static boolean haveEqualCatComboCatOptionReferenceIds( CategoryOptionCombo one, CategoryOptionCombo other) { if (one == null || other == null) { @@ -74,16 +77,17 @@ static boolean haveEqualCatComboCatOptionReferenceIds( } private void checkDuplicateCategoryOptionCombos( - CategoryOptionCombo categoryOptionCombo, Consumer addReports) { + CategoryOptionCombo categoryOptionCombo, + ObjectBundle bundle, + Consumer addReports) { + + if (bundle.isPersisted(categoryOptionCombo)) { + return; // Only check for duplicates if the object is not persisted + } - List categoryOptionCombos = - categoryService.getAllCategoryOptionCombos().stream() - .filter( - coc -> - coc.getCategoryCombo() - .getUid() - .equals(categoryOptionCombo.getCategoryCombo().getUid())) - .collect(Collectors.toList()); + List categoryOptionCombos = categoryService.getAllCategoryOptionCombos().stream() + .filter(coc -> coc.getCategoryCombo().getUid().equals(categoryOptionCombo.getCategoryCombo().getUid())) + .toList(); // Check if the categoryOptionCombo is already in the list. This could be an update or re-import // of the same object. @@ -106,11 +110,74 @@ private void checkDuplicateCategoryOptionCombos( } } + private void checkCategoryOptionsExistInCategoryCombo( + CategoryOptionCombo categoryOptionCombo, ObjectBundle bundle, Consumer addReports) { + +// //Exit early if there are no category options in the combo +// if (!bundle.isPersisted(categoryOptionCombo.getCategoryCombo())) { +// return; +// } + + Set categoryOptionUids = + categoryOptionCombo.getCategoryOptions().stream() + .map(CategoryOption::getUid) + .collect(Collectors.toSet()); + + CategoryCombo existingCategoryCombo = + categoryService.getCategoryCombo(categoryOptionCombo.getCategoryCombo().getUid()); + + if (existingCategoryCombo == null) { + return; + } + + Set existingCategoryOptionsInCombo = + existingCategoryCombo + .getCategoryOptions().stream() + .map(CategoryOption::getUid) + .collect(Collectors.toSet()); + + categoryOptionUids.removeAll(existingCategoryOptionsInCombo); + + if (!categoryOptionUids.isEmpty()) { + addReports.accept( + new ErrorReport( + CategoryOptionCombo.class, + ErrorCode.E1125, + categoryOptionCombo.getName(), + existingCategoryCombo.getName())); + } + } + + private void checkNonStandardDefaultCatOptionCombo( + CategoryOptionCombo categoryOptionCombo, Consumer addReports) { + + + CategoryCombo categoryCombo = categoryOptionCombo.getCategoryCombo(); + CategoryCombo defaultCombo = categoryService.getDefaultCategoryCombo(); + if (!categoryCombo.getUid().equals(defaultCombo.getUid())) { + return; + } + + CategoryOptionCombo defaultCatOptionCombo = categoryService.getDefaultCategoryOptionCombo(); + + if (categoryOptionCombo.getUid() != defaultCatOptionCombo.getUid()) { + addReports.accept( + new ErrorReport( + CategoryOptionCombo.class, + ErrorCode.E1124, + categoryOptionCombo.getName())); + } + } + @Override public void validate( CategoryOptionCombo categoryOptionCombo, ObjectBundle bundle, Consumer addReports) { - checkDuplicateCategoryOptionCombos(categoryOptionCombo, addReports); + + checkNonStandardDefaultCatOptionCombo(categoryOptionCombo, addReports); + checkDuplicateCategoryOptionCombos(categoryOptionCombo, bundle, addReports); + checkCategoryOptionsExistInCategoryCombo(categoryOptionCombo, bundle, addReports); + } } diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java index 39c1f0dd8fc9..53ef2da17351 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java @@ -163,8 +163,7 @@ void testCategoryOptionIdentifiers() { } } - @Disabled - // Disabling this as we cannot import duplicate category option combos for default + @Test void testCategoryOptionComboIdentifiers() { List> data = buildDataSet("XXXvX50cXC0", "COCA", "COCAname"); for (Pair pair : data) { diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java index ab2c81b331b1..fcceb11d15e1 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java @@ -149,10 +149,10 @@ void testPreheatEvents() throws IOException { assertFalse(preheat.getAll(OrganisationUnit.class).isEmpty()); assertFalse(preheat.getAll(ProgramStage.class).isEmpty()); assertFalse(preheat.getAll(CategoryOptionCombo.class).isEmpty()); - /* Disabling this as we cannot import duplicate category option combos for default - https://dhis2.atlassian.net/browse/DHIS2-18401 + + //Need to fix this duplicate category option combo assertNotNull(preheat.get(CategoryOptionCombo.class, "XXXvX50cXC0")); - */ + assertNotNull(preheat.get(CategoryOption.class, "XXXrKDKCefk")); } } diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java index 5b229771d3cb..be51a49d4219 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java @@ -49,7 +49,6 @@ import org.hisp.dhis.test.webapi.json.domain.JsonErrorReport; import org.hisp.dhis.test.webapi.json.domain.JsonIdentifiableObject; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -183,7 +182,7 @@ void catOptionCombosDuplicatedDefaultTest() { error.getMessage()); } - @Disabled("This test is not working as expected") + @Test @DisplayName( "Default category option combos with non-default category options should not be allowed") void catOptionComboDefaultWithNonDefaultOptionsNotAllowedTest() { @@ -207,30 +206,13 @@ void catOptionComboDefaultWithNonDefaultOptionsNotAllowedTest() { "categoryCombo" : {"id" : "%s"} } """ .formatted(categoryOptionRed, defaultCategoryComboId)) - .content(); + .content(HttpStatus.CONFLICT); JsonErrorReport error = - response.find(JsonErrorReport.class, report -> report.getErrorCode() == ErrorCode.E1124); + response2.find(JsonErrorReport.class, report -> report.getErrorCode() == ErrorCode.E1125); assertNotNull(error); - assertEquals( - "Category option combo Not default already exists for category combo default", + assertEquals("Category option combo Not default contains options not associated with category combo default", error.getMessage()); } - @Test - @DisplayName("Empty catcombo for a category option combo not allowed") - void catOptionComboEmptyCatComboTest() { - JsonMixed response = - POST( - "/categoryOptionCombos/", - """ - { "name": "Empty catcombo", - "categoryOptions" : [{"id" : "CocUid0001"}], - "categoryCombo" : {} } - """) - .content(HttpStatus.CONFLICT); - assertEquals( - "\"One or more errors occurred, please see full details in import report.\"", - response.get("message").toString()); - } } From ab8615df93f70f1b70bc68692be8c72e95173bb6 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Mon, 18 Nov 2024 09:10:47 +0100 Subject: [PATCH 18/33] Linting --- .../CategoryOptionComboObjectBundleHook.java | 34 +++++++++---------- .../TrackerPreheatIdentifiersTest.java | 1 - .../preheat/TrackerPreheatServiceTest.java | 2 +- .../CategoryOptionComboControllerTest.java | 4 +-- 4 files changed, 20 insertions(+), 21 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index b00e05121c4a..73cd60953e4b 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -32,7 +32,6 @@ import java.util.function.Consumer; import java.util.stream.Collectors; import lombok.AllArgsConstructor; -import org.hisp.dhis.category.Category; import org.hisp.dhis.category.CategoryCombo; import org.hisp.dhis.category.CategoryOption; import org.hisp.dhis.category.CategoryOptionCombo; @@ -48,7 +47,6 @@ public class CategoryOptionComboObjectBundleHook extends AbstractObjectBundleHook { private final CategoryService categoryService; - static boolean haveEqualCatComboCatOptionReferenceIds( CategoryOptionCombo one, CategoryOptionCombo other) { if (one == null || other == null) { @@ -85,9 +83,14 @@ private void checkDuplicateCategoryOptionCombos( return; // Only check for duplicates if the object is not persisted } - List categoryOptionCombos = categoryService.getAllCategoryOptionCombos().stream() - .filter(coc -> coc.getCategoryCombo().getUid().equals(categoryOptionCombo.getCategoryCombo().getUid())) - .toList(); + List categoryOptionCombos = + categoryService.getAllCategoryOptionCombos().stream() + .filter( + coc -> + coc.getCategoryCombo() + .getUid() + .equals(categoryOptionCombo.getCategoryCombo().getUid())) + .toList(); // Check if the categoryOptionCombo is already in the list. This could be an update or re-import // of the same object. @@ -111,12 +114,14 @@ private void checkDuplicateCategoryOptionCombos( } private void checkCategoryOptionsExistInCategoryCombo( - CategoryOptionCombo categoryOptionCombo, ObjectBundle bundle, Consumer addReports) { + CategoryOptionCombo categoryOptionCombo, + ObjectBundle bundle, + Consumer addReports) { -// //Exit early if there are no category options in the combo -// if (!bundle.isPersisted(categoryOptionCombo.getCategoryCombo())) { -// return; -// } + // //Exit early if there are no category options in the combo + // if (!bundle.isPersisted(categoryOptionCombo.getCategoryCombo())) { + // return; + // } Set categoryOptionUids = categoryOptionCombo.getCategoryOptions().stream() @@ -131,8 +136,7 @@ private void checkCategoryOptionsExistInCategoryCombo( } Set existingCategoryOptionsInCombo = - existingCategoryCombo - .getCategoryOptions().stream() + existingCategoryCombo.getCategoryOptions().stream() .map(CategoryOption::getUid) .collect(Collectors.toSet()); @@ -151,7 +155,6 @@ private void checkCategoryOptionsExistInCategoryCombo( private void checkNonStandardDefaultCatOptionCombo( CategoryOptionCombo categoryOptionCombo, Consumer addReports) { - CategoryCombo categoryCombo = categoryOptionCombo.getCategoryCombo(); CategoryCombo defaultCombo = categoryService.getDefaultCategoryCombo(); if (!categoryCombo.getUid().equals(defaultCombo.getUid())) { @@ -163,9 +166,7 @@ private void checkNonStandardDefaultCatOptionCombo( if (categoryOptionCombo.getUid() != defaultCatOptionCombo.getUid()) { addReports.accept( new ErrorReport( - CategoryOptionCombo.class, - ErrorCode.E1124, - categoryOptionCombo.getName())); + CategoryOptionCombo.class, ErrorCode.E1124, categoryOptionCombo.getName())); } } @@ -178,6 +179,5 @@ public void validate( checkNonStandardDefaultCatOptionCombo(categoryOptionCombo, addReports); checkDuplicateCategoryOptionCombos(categoryOptionCombo, bundle, addReports); checkCategoryOptionsExistInCategoryCombo(categoryOptionCombo, bundle, addReports); - } } diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java index 53ef2da17351..a70af93fafa7 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java @@ -60,7 +60,6 @@ import org.hisp.dhis.tracker.imports.domain.TrackerObjects; import org.hisp.dhis.user.User; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java index fcceb11d15e1..20aaf5c1eddf 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java @@ -150,7 +150,7 @@ void testPreheatEvents() throws IOException { assertFalse(preheat.getAll(ProgramStage.class).isEmpty()); assertFalse(preheat.getAll(CategoryOptionCombo.class).isEmpty()); - //Need to fix this duplicate category option combo + // Need to fix this duplicate category option combo assertNotNull(preheat.get(CategoryOptionCombo.class, "XXXvX50cXC0")); assertNotNull(preheat.get(CategoryOption.class, "XXXrKDKCefk")); diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java index be51a49d4219..b18677adddb2 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java @@ -211,8 +211,8 @@ void catOptionComboDefaultWithNonDefaultOptionsNotAllowedTest() { JsonErrorReport error = response2.find(JsonErrorReport.class, report -> report.getErrorCode() == ErrorCode.E1125); assertNotNull(error); - assertEquals("Category option combo Not default contains options not associated with category combo default", + assertEquals( + "Category option combo Not default contains options not associated with category combo default", error.getMessage()); } - } From 5e6aaeda7d08aec953ba4a67b217c528120e13b9 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Mon, 18 Nov 2024 14:59:09 +0100 Subject: [PATCH 19/33] More sonar cube stuff --- .../hooks/CategoryOptionComboObjectBundleHook.java | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index 73cd60953e4b..c5e7a7f5fb56 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -115,14 +115,8 @@ private void checkDuplicateCategoryOptionCombos( private void checkCategoryOptionsExistInCategoryCombo( CategoryOptionCombo categoryOptionCombo, - ObjectBundle bundle, Consumer addReports) { - // //Exit early if there are no category options in the combo - // if (!bundle.isPersisted(categoryOptionCombo.getCategoryCombo())) { - // return; - // } - Set categoryOptionUids = categoryOptionCombo.getCategoryOptions().stream() .map(CategoryOption::getUid) @@ -163,7 +157,7 @@ private void checkNonStandardDefaultCatOptionCombo( CategoryOptionCombo defaultCatOptionCombo = categoryService.getDefaultCategoryOptionCombo(); - if (categoryOptionCombo.getUid() != defaultCatOptionCombo.getUid()) { + if (!categoryOptionCombo.getUid().equals(defaultCatOptionCombo.getUid() )) { addReports.accept( new ErrorReport( CategoryOptionCombo.class, ErrorCode.E1124, categoryOptionCombo.getName())); @@ -178,6 +172,6 @@ public void validate( checkNonStandardDefaultCatOptionCombo(categoryOptionCombo, addReports); checkDuplicateCategoryOptionCombos(categoryOptionCombo, bundle, addReports); - checkCategoryOptionsExistInCategoryCombo(categoryOptionCombo, bundle, addReports); + checkCategoryOptionsExistInCategoryCombo(categoryOptionCombo, addReports); } } From 630a9e367519181c52bcf0c01a4210e33612ce92 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Mon, 18 Nov 2024 14:59:33 +0100 Subject: [PATCH 20/33] Linting --- .../hooks/CategoryOptionComboObjectBundleHook.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index c5e7a7f5fb56..02fe6424da14 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -114,8 +114,7 @@ private void checkDuplicateCategoryOptionCombos( } private void checkCategoryOptionsExistInCategoryCombo( - CategoryOptionCombo categoryOptionCombo, - Consumer addReports) { + CategoryOptionCombo categoryOptionCombo, Consumer addReports) { Set categoryOptionUids = categoryOptionCombo.getCategoryOptions().stream() @@ -157,7 +156,7 @@ private void checkNonStandardDefaultCatOptionCombo( CategoryOptionCombo defaultCatOptionCombo = categoryService.getDefaultCategoryOptionCombo(); - if (!categoryOptionCombo.getUid().equals(defaultCatOptionCombo.getUid() )) { + if (!categoryOptionCombo.getUid().equals(defaultCatOptionCombo.getUid())) { addReports.accept( new ErrorReport( CategoryOptionCombo.class, ErrorCode.E1124, categoryOptionCombo.getName())); From 04d06e755c84bb2ce7f80ee531574f0bac2d9205 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Mon, 18 Nov 2024 15:07:05 +0100 Subject: [PATCH 21/33] More sonar cube --- ...rityCategoryOptionComboDuplicatedTest.java | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityCategoryOptionComboDuplicatedTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityCategoryOptionComboDuplicatedTest.java index a474555b4f98..f7e4ed3a7022 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityCategoryOptionComboDuplicatedTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityCategoryOptionComboDuplicatedTest.java @@ -147,16 +147,15 @@ void testCategoryOptionCombosNotDuplicated() { + categoryOptionBlue + "'} ] }")); - String testCatCombo = - assertStatus( - HttpStatus.CREATED, - POST( - "/categoryCombos", - "{ 'name' : 'Color', " - + "'dataDimensionType' : 'DISAGGREGATION', 'categories' : [" - + "{'id' : '" - + categoryColor - + "'}]} ")); + assertStatus( + HttpStatus.CREATED, + POST( + "/categoryCombos", + "{ 'name' : 'Color', " + + "'dataDimensionType' : 'DISAGGREGATION', 'categories' : [" + + "{'id' : '" + + categoryColor + + "'}]} ")); assertHasNoDataIntegrityIssues("categoryOptionCombos", check, true); } From d704a18fcb15dc46491f5ecb497412f087b8fbac Mon Sep 17 00:00:00 2001 From: Enrico Date: Tue, 19 Nov 2024 09:13:36 +0100 Subject: [PATCH 22/33] fix: Remove custom COC defaults --- .../preheat/TrackerPreheatIdentifiersTest.java | 3 ++- .../preheat/TrackerPreheatServiceTest.java | 2 +- .../test/resources/tracker/event_events.json | 2 +- .../test/resources/tracker/event_metadata.json | 18 ------------------ .../resources/tracker/identifier_metadata.json | 17 ----------------- 5 files changed, 4 insertions(+), 38 deletions(-) diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java index a70af93fafa7..69891c46c990 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java @@ -164,7 +164,8 @@ void testCategoryOptionIdentifiers() { @Test void testCategoryOptionComboIdentifiers() { - List> data = buildDataSet("XXXvX50cXC0", "COCA", "COCAname"); + List> data = + buildDataSet("HllvX50cXC0", "default", "default"); for (Pair pair : data) { String id = pair.getLeft(); TrackerIdSchemeParam param = pair.getRight(); diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java index 1d083bb5f2e8..49defcb12f24 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java @@ -149,7 +149,7 @@ void testPreheatEvents() throws IOException { assertFalse(preheat.getAll(OrganisationUnit.class).isEmpty()); assertFalse(preheat.getAll(ProgramStage.class).isEmpty()); assertFalse(preheat.getAll(CategoryOptionCombo.class).isEmpty()); - assertNotNull(preheat.get(CategoryOptionCombo.class, "XXXvX50cXC0")); + assertNotNull(preheat.get(CategoryOptionCombo.class, "HllvX50cXC0")); assertNotNull(preheat.get(CategoryOption.class, "XXXrKDKCefk")); } } diff --git a/dhis-2/dhis-test-integration/src/test/resources/tracker/event_events.json b/dhis-2/dhis-test-integration/src/test/resources/tracker/event_events.json index 15b5ac2646e9..12fabd955311 100644 --- a/dhis-2/dhis-test-integration/src/test/resources/tracker/event_events.json +++ b/dhis-2/dhis-test-integration/src/test/resources/tracker/event_events.json @@ -104,7 +104,7 @@ "deleted": false, "attributeOptionCombo": { "idScheme": "UID", - "identifier": "XXXvX50cXC0" + "identifier": "HllvX50cXC0" }, "attributeCategoryOptions": [ { diff --git a/dhis-2/dhis-test-integration/src/test/resources/tracker/event_metadata.json b/dhis-2/dhis-test-integration/src/test/resources/tracker/event_metadata.json index 08e0c19e4bf9..8e41a3676f9c 100644 --- a/dhis-2/dhis-test-integration/src/test/resources/tracker/event_metadata.json +++ b/dhis-2/dhis-test-integration/src/test/resources/tracker/event_metadata.json @@ -1820,24 +1820,6 @@ "id": "xYerKDKCefk" } ] - }, - { - "lastUpdated": "2019-01-28T11:01:30.775", - "code": "COCA", - "created": "2019-01-28T11:01:30.771", - "name": "COCAname", - "id": "XXXvX50cXC0", - "ignoreApproval": false, - "categoryCombo": { - "id": "bjDvmb4bfuf" - }, - "translations": [], - "attributeValues": [], - "categoryOptions": [ - { - "id": "xYerKDKCefk" - } - ] } ], "categories": [ diff --git a/dhis-2/dhis-test-integration/src/test/resources/tracker/identifier_metadata.json b/dhis-2/dhis-test-integration/src/test/resources/tracker/identifier_metadata.json index f4925a896ee3..eed1a1b69336 100644 --- a/dhis-2/dhis-test-integration/src/test/resources/tracker/identifier_metadata.json +++ b/dhis-2/dhis-test-integration/src/test/resources/tracker/identifier_metadata.json @@ -204,23 +204,6 @@ "id": "xYerKDKCefk" } ] - }, - { - "lastUpdated": "2019-01-28T11:01:30.775", - "code": "COCA", - "created": "2019-01-28T11:01:30.771", - "name": "COCAname", - "id": "XXXvX50cXC0", - "ignoreApproval": false, - "categoryCombo": { - "id": "bjDvmb4bfuf" - }, - "attributeValues": [], - "categoryOptions": [ - { - "id": "xYerKDKCefk" - } - ] } ], "categories": [ From 40ac6b2fed9c058add721ae7a5c9e6ac4fde053c Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Tue, 19 Nov 2024 09:35:31 +0100 Subject: [PATCH 23/33] Remove custom default --- .../TrackerPreheatIdentifiersTest.java | 2 +- .../preheat/TrackerPreheatServiceTest.java | 2 +- .../tracker/tracker_basic_metadata.json | 19 ------------------- 3 files changed, 2 insertions(+), 21 deletions(-) diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java index a70af93fafa7..74ed2e85c64e 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java @@ -164,7 +164,7 @@ void testCategoryOptionIdentifiers() { @Test void testCategoryOptionComboIdentifiers() { - List> data = buildDataSet("XXXvX50cXC0", "COCA", "COCAname"); + List> data = buildDataSet("HllvX50cXC0", "default","default"); for (Pair pair : data) { String id = pair.getLeft(); TrackerIdSchemeParam param = pair.getRight(); diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java index 20aaf5c1eddf..9c07fea9e155 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatServiceTest.java @@ -151,7 +151,7 @@ void testPreheatEvents() throws IOException { assertFalse(preheat.getAll(CategoryOptionCombo.class).isEmpty()); // Need to fix this duplicate category option combo - assertNotNull(preheat.get(CategoryOptionCombo.class, "XXXvX50cXC0")); + assertNotNull(preheat.get(CategoryOptionCombo.class, "HllvX50cXC0")); assertNotNull(preheat.get(CategoryOption.class, "XXXrKDKCefk")); } diff --git a/dhis-2/dhis-test-integration/src/test/resources/tracker/tracker_basic_metadata.json b/dhis-2/dhis-test-integration/src/test/resources/tracker/tracker_basic_metadata.json index 66db4ea628af..35e7fa898abd 100644 --- a/dhis-2/dhis-test-integration/src/test/resources/tracker/tracker_basic_metadata.json +++ b/dhis-2/dhis-test-integration/src/test/resources/tracker/tracker_basic_metadata.json @@ -2997,25 +2997,6 @@ "attributeValues": [], "created": "2019-03-25T13:40:24.775" }, - { - "id": "KKKKX50cXC0", - "name": "accesstest1", - "code": "accesstest1", - "categoryOptions": [ - { - "id": "XXXXKDKCefk" - } - ], - "categoryCombo": { - "id": "bjDvmb4bfuf" - }, - "translations": [], - "startDate": "2019-01-01T00:00:00.000", - "lastUpdated": "2019-03-25T13:40:24.779", - "ignoreApproval": false, - "attributeValues": [], - "created": "2019-03-25T13:40:24.775" - }, { "id": "OOlvX50cXC0", "code": "nondef1 cat1", From c570fb73bc7ec24fd00624b1029a9d0b5f7f7072 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Tue, 19 Nov 2024 09:37:00 +0100 Subject: [PATCH 24/33] Linting --- .../tracker/imports/preheat/TrackerPreheatIdentifiersTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java index 74ed2e85c64e..69891c46c990 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatIdentifiersTest.java @@ -164,7 +164,8 @@ void testCategoryOptionIdentifiers() { @Test void testCategoryOptionComboIdentifiers() { - List> data = buildDataSet("HllvX50cXC0", "default","default"); + List> data = + buildDataSet("HllvX50cXC0", "default", "default"); for (Pair pair : data) { String id = pair.getLeft(); TrackerIdSchemeParam param = pair.getRight(); From 654ea8c3decfcc394d319ad48022127f941b2ab0 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Tue, 19 Nov 2024 12:18:52 +0100 Subject: [PATCH 25/33] Try and fix tracker test --- .../validation/EventImportValidationTest.java | 6 +-- .../tracker/tracker_basic_metadata.json | 54 ++++++++++++++++++- .../validations/events-cat-write-access.json | 2 +- 3 files changed, 57 insertions(+), 5 deletions(-) diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventImportValidationTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventImportValidationTest.java index 646276e5e0ec..277dce73d67b 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventImportValidationTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventImportValidationTest.java @@ -170,9 +170,9 @@ void testCantWriteAccessCatCombo() throws IOException { assertHasOnlyErrors( importReport, ValidationCode.E1096, - ValidationCode.E1099, - ValidationCode.E1104, - ValidationCode.E1095); + //ValidationCode.E1099 + ValidationCode.E1104); + //ValidationCode.E1095); } @Test diff --git a/dhis-2/dhis-test-integration/src/test/resources/tracker/tracker_basic_metadata.json b/dhis-2/dhis-test-integration/src/test/resources/tracker/tracker_basic_metadata.json index 35e7fa898abd..813a68e3d134 100644 --- a/dhis-2/dhis-test-integration/src/test/resources/tracker/tracker_basic_metadata.json +++ b/dhis-2/dhis-test-integration/src/test/resources/tracker/tracker_basic_metadata.json @@ -186,7 +186,7 @@ "attributeValues": [], "minDaysFromStart": 0, "sharing": { - "public": "rw------", + "public": "rwrw----", "external": false, "owner": "VfaA5WwHLdP", "users": {}, @@ -751,6 +751,10 @@ "jiFUk9o7gH3": { "id": "jiFUk9o7gH3", "access": "rwrw----" + }, + "VfaA5WwHLdP": { + "id": "VfaA5WwHLdP", + "access": "--------" } }, "userGroups": {} @@ -2682,6 +2686,30 @@ "organisationUnits": [], "created": "2019-03-25T13:40:24.722", "translations": [], + "sharing": { + "public": "--------", + "external": false, + "owner": null, + "users": { + "VfaA5WwHLdP": { + "id": "VfaA5WwHLdP", + "access": "--------" + } + }, + "userGroups": {} + } + }, + { + "id": "WnKa7zOVoLK", + "code": "Alt default", + "name": "Alt default", + "shortName": "Alt default", + "startDate": "2019-01-01T00:00:00.000", + "attributeValues": [], + "lastUpdated": "2019-03-25T13:40:24.783", + "organisationUnits": [], + "created": "2019-03-25T13:40:24.722", + "translations": [], "sharing": { "public": "rwrw----", "external": false, @@ -2835,6 +2863,30 @@ "userGroups": {} } }, + { + "id": "LwulPEjPHnJ", + "name": "Alt default", + "shortName": "Alt default", + "code": "Alt default", + "dataDimensionType": "DISAGGREGATION", + "categoryOptions": [ + { + "id": "WnKa7zOVoLK" + } + ], + "dataDimension": false, + "attributeValues": [], + "translations": [], + "created": "2019-03-25T13:40:24.762", + "lastUpdated": "2019-03-29T08:54:32.360", + "sharing": { + "public": "rw------", + "external": false, + "owner": null, + "users": {}, + "userGroups": {} + } + }, { "id": "LC4ZMxxHtBA", "name": "nondef1", diff --git a/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/events-cat-write-access.json b/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/events-cat-write-access.json index 254eda322691..9523595c2cde 100644 --- a/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/events-cat-write-access.json +++ b/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/events-cat-write-access.json @@ -57,7 +57,7 @@ "deleted": false, "attributeOptionCombo": { "idScheme": "UID", - "identifier": "KKKKX50cXC0" + "identifier": "HllvX50cXC0" }, "attributeCategoryOptions": [], "dataValues": [], From 65730b1f21d7a83abde388c1c08c47eb3e5f8774 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Tue, 19 Nov 2024 12:19:08 +0100 Subject: [PATCH 26/33] Linting --- .../tracker/imports/validation/EventImportValidationTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventImportValidationTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventImportValidationTest.java index 277dce73d67b..81293f828a8a 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventImportValidationTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventImportValidationTest.java @@ -170,9 +170,9 @@ void testCantWriteAccessCatCombo() throws IOException { assertHasOnlyErrors( importReport, ValidationCode.E1096, - //ValidationCode.E1099 + // ValidationCode.E1099 ValidationCode.E1104); - //ValidationCode.E1095); + // ValidationCode.E1095); } @Test From f854eae150b2bc2833e08224163b0bbdc9851753 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Tue, 19 Nov 2024 14:25:38 +0100 Subject: [PATCH 27/33] Try and fix tracker tests --- .../validation/EventImportValidationTest.java | 6 +- .../test/resources/tracker/event_events.json | 2 +- .../tracker/tracker_basic_metadata.json | 74 +++++++++++++------ 3 files changed, 55 insertions(+), 27 deletions(-) diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventImportValidationTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventImportValidationTest.java index 81293f828a8a..561385cb9b4b 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventImportValidationTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventImportValidationTest.java @@ -168,11 +168,7 @@ void testCantWriteAccessCatCombo() throws IOException { ImportReport importReport = trackerImportService.importTracker(params, trackerObjects); assertHasOnlyErrors( - importReport, - ValidationCode.E1096, - // ValidationCode.E1099 - ValidationCode.E1104); - // ValidationCode.E1095); + importReport, ValidationCode.E1096, ValidationCode.E1104, ValidationCode.E1095); } @Test diff --git a/dhis-2/dhis-test-integration/src/test/resources/tracker/event_events.json b/dhis-2/dhis-test-integration/src/test/resources/tracker/event_events.json index 12fabd955311..15b5ac2646e9 100644 --- a/dhis-2/dhis-test-integration/src/test/resources/tracker/event_events.json +++ b/dhis-2/dhis-test-integration/src/test/resources/tracker/event_events.json @@ -104,7 +104,7 @@ "deleted": false, "attributeOptionCombo": { "idScheme": "UID", - "identifier": "HllvX50cXC0" + "identifier": "XXXvX50cXC0" }, "attributeCategoryOptions": [ { diff --git a/dhis-2/dhis-test-integration/src/test/resources/tracker/tracker_basic_metadata.json b/dhis-2/dhis-test-integration/src/test/resources/tracker/tracker_basic_metadata.json index 813a68e3d134..04b2819ab833 100644 --- a/dhis-2/dhis-test-integration/src/test/resources/tracker/tracker_basic_metadata.json +++ b/dhis-2/dhis-test-integration/src/test/resources/tracker/tracker_basic_metadata.json @@ -186,7 +186,7 @@ "attributeValues": [], "minDaysFromStart": 0, "sharing": { - "public": "rwrw----", + "public": "rw------", "external": false, "owner": "VfaA5WwHLdP", "users": {}, @@ -751,10 +751,6 @@ "jiFUk9o7gH3": { "id": "jiFUk9o7gH3", "access": "rwrw----" - }, - "VfaA5WwHLdP": { - "id": "VfaA5WwHLdP", - "access": "--------" } }, "userGroups": {} @@ -2687,23 +2683,18 @@ "created": "2019-03-25T13:40:24.722", "translations": [], "sharing": { - "public": "--------", + "public": "rwrw----", "external": false, "owner": null, - "users": { - "VfaA5WwHLdP": { - "id": "VfaA5WwHLdP", - "access": "--------" - } - }, + "users": {}, "userGroups": {} } }, { - "id": "WnKa7zOVoLK", - "code": "Alt default", - "name": "Alt default", - "shortName": "Alt default", + "id": "WAFwwid9TzE", + "code": "default2", + "name": "default2", + "shortName": "default2", "startDate": "2019-01-01T00:00:00.000", "attributeValues": [], "lastUpdated": "2019-03-25T13:40:24.783", @@ -2864,14 +2855,14 @@ } }, { - "id": "LwulPEjPHnJ", - "name": "Alt default", - "shortName": "Alt default", - "code": "Alt default", + "id": "Vk8PHR8HBAe", + "name": "default2", + "shortName": "default2", + "code": "default2", "dataDimensionType": "DISAGGREGATION", "categoryOptions": [ { - "id": "WnKa7zOVoLK" + "id": "WAFwwid9TzE" } ], "dataDimension": false, @@ -2959,6 +2950,28 @@ "userGroups": {} } }, + { + "id": "WG3syvpSE9o", + "code": "default2", + "name": "default2", + "dataDimensionType": "DISAGGREGATION", + "categories": [ + { + "id": "Vk8PHR8HBAe" + } + ], + "created": "2019-03-25T13:40:24.771", + "lastUpdated": "2019-03-25T13:40:24.781", + "skipTotal": false, + "translations": [], + "sharing": { + "public": "rw------", + "external": false, + "owner": null, + "users": {}, + "userGroups": {} + } + }, { "id": "pFpCRzMVAbO", "code": "nondef1", @@ -3049,6 +3062,25 @@ "attributeValues": [], "created": "2019-03-25T13:40:24.775" }, + { + "id": "KKKKX50cXC0", + "name": "accesstest1", + "code": "accesstest1", + "categoryOptions": [ + { + "id": "XXXXKDKCefk" + } + ], + "categoryCombo": { + "id": "WG3syvpSE9o" + }, + "translations": [], + "startDate": "2019-01-01T00:00:00.000", + "lastUpdated": "2019-03-25T13:40:24.779", + "ignoreApproval": false, + "attributeValues": [], + "created": "2019-03-25T13:40:24.775" + }, { "id": "OOlvX50cXC0", "code": "nondef1 cat1", From 9ad91dd1cf7e017dfddf9952da52afe72f4410de Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Wed, 20 Nov 2024 16:24:22 +0100 Subject: [PATCH 28/33] Remove check on bad options in a COC for now --- .../CategoryOptionComboObjectBundleHook.java | 39 +------------------ .../CategoryOptionComboControllerTest.java | 36 ----------------- 2 files changed, 2 insertions(+), 73 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index 02fe6424da14..ed95588fc917 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -92,14 +92,12 @@ private void checkDuplicateCategoryOptionCombos( .equals(categoryOptionCombo.getCategoryCombo().getUid())) .toList(); - // Check if the categoryOptionCombo is already in the list. This could be an update or re-import - // of the same object. + // This could be an update or re-import of the same object. if (categoryOptionCombos.stream() .anyMatch(coc -> coc.getUid().equals(categoryOptionCombo.getUid()))) { return; } - // Check to see if the COC already exists in the list of COCs - // If it does, then it is a duplicate + // Check to see if the COC already exists in the list of COCs by comparing reference ids for (CategoryOptionCombo existingCategoryOptionCombo : categoryOptionCombos) { if (haveEqualCatComboCatOptionReferenceIds(categoryOptionCombo, existingCategoryOptionCombo) && !categoryOptionCombo.getUid().equals(existingCategoryOptionCombo.getUid())) { @@ -113,38 +111,6 @@ private void checkDuplicateCategoryOptionCombos( } } - private void checkCategoryOptionsExistInCategoryCombo( - CategoryOptionCombo categoryOptionCombo, Consumer addReports) { - - Set categoryOptionUids = - categoryOptionCombo.getCategoryOptions().stream() - .map(CategoryOption::getUid) - .collect(Collectors.toSet()); - - CategoryCombo existingCategoryCombo = - categoryService.getCategoryCombo(categoryOptionCombo.getCategoryCombo().getUid()); - - if (existingCategoryCombo == null) { - return; - } - - Set existingCategoryOptionsInCombo = - existingCategoryCombo.getCategoryOptions().stream() - .map(CategoryOption::getUid) - .collect(Collectors.toSet()); - - categoryOptionUids.removeAll(existingCategoryOptionsInCombo); - - if (!categoryOptionUids.isEmpty()) { - addReports.accept( - new ErrorReport( - CategoryOptionCombo.class, - ErrorCode.E1125, - categoryOptionCombo.getName(), - existingCategoryCombo.getName())); - } - } - private void checkNonStandardDefaultCatOptionCombo( CategoryOptionCombo categoryOptionCombo, Consumer addReports) { @@ -171,6 +137,5 @@ public void validate( checkNonStandardDefaultCatOptionCombo(categoryOptionCombo, addReports); checkDuplicateCategoryOptionCombos(categoryOptionCombo, bundle, addReports); - checkCategoryOptionsExistInCategoryCombo(categoryOptionCombo, addReports); } } diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java index b18677adddb2..32c6fd53a882 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java @@ -27,7 +27,6 @@ */ package org.hisp.dhis.webapi.controller; -import static org.hisp.dhis.http.HttpAssertions.assertStatus; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -42,7 +41,6 @@ import org.hisp.dhis.http.HttpStatus; import org.hisp.dhis.jsontree.JsonArray; import org.hisp.dhis.jsontree.JsonList; -import org.hisp.dhis.jsontree.JsonMixed; import org.hisp.dhis.jsontree.JsonObject; import org.hisp.dhis.test.webapi.H2ControllerIntegrationTestBase; import org.hisp.dhis.test.webapi.json.domain.JsonCategoryOptionCombo; @@ -181,38 +179,4 @@ void catOptionCombosDuplicatedDefaultTest() { "Category option combo Not default already exists for category combo default", error.getMessage()); } - - @Test - @DisplayName( - "Default category option combos with non-default category options should not be allowed") - void catOptionComboDefaultWithNonDefaultOptionsNotAllowedTest() { - JsonObject response = - GET("/categoryOptionCombos?filter=name:eq:default&fields=id,categoryCombo[id],categoryOptions[id]") - .content(); - JsonList catOptionCombos = - response.getList("categoryOptionCombos", JsonCategoryOptionCombo.class); - String defaultCategoryComboId = catOptionCombos.get(0).getCategoryCombo().getId(); - - String categoryOptionRed = - assertStatus( - HttpStatus.CREATED, POST("/categoryOptions", "{ 'name': 'Red', 'shortName': 'Red' }")); - - JsonMixed response2 = - POST( - "/categoryOptionCombos/", - """ - { "name": "Not default", - "categoryOptions" : [{"id" : "%s"}], - "categoryCombo" : {"id" : "%s"} } - """ - .formatted(categoryOptionRed, defaultCategoryComboId)) - .content(HttpStatus.CONFLICT); - - JsonErrorReport error = - response2.find(JsonErrorReport.class, report -> report.getErrorCode() == ErrorCode.E1125); - assertNotNull(error); - assertEquals( - "Category option combo Not default contains options not associated with category combo default", - error.getMessage()); - } } From 88e46ff7b2f28b8c309eecd295052277d5ee03dc Mon Sep 17 00:00:00 2001 From: Enrico Date: Thu, 21 Nov 2024 13:12:22 +0100 Subject: [PATCH 29/33] Fix event test --- .../validation/EventImportValidationTest.java | 18 +- .../tracker/tracker_basic_metadata.json | 155 ++++++++---------- .../enrollments-cat-write-access.json | 29 ++++ .../validations/events-cat-write-access.json | 40 +---- 4 files changed, 116 insertions(+), 126 deletions(-) create mode 100644 dhis-2/dhis-test-integration/src/test/resources/tracker/validations/enrollments-cat-write-access.json diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventImportValidationTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventImportValidationTest.java index 561385cb9b4b..5394088d2d09 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventImportValidationTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventImportValidationTest.java @@ -161,14 +161,26 @@ void testTrackerAndProgramEventUpdateSuccess() throws IOException { @Test void testCantWriteAccessCatCombo() throws IOException { - TrackerObjects trackerObjects = fromJson("tracker/validations/events-cat-write-access.json"); + TrackerObjects trackerObjects = + fromJson("tracker/validations/enrollments-cat-write-access.json"); TrackerImportParams params = new TrackerImportParams(); - injectSecurityContextUser(userService.getUser(USER_6)); ImportReport importReport = trackerImportService.importTracker(params, trackerObjects); + assertNoErrors(importReport); + + trackerObjects = fromJson("tracker/validations/events-cat-write-access.json"); + params = new TrackerImportParams(); + injectSecurityContextUser(userService.getUser(USER_6)); + + importReport = trackerImportService.importTracker(params, trackerObjects); + assertHasOnlyErrors( - importReport, ValidationCode.E1096, ValidationCode.E1104, ValidationCode.E1095); + importReport, + ValidationCode.E1096, + ValidationCode.E1099, + ValidationCode.E1104, + ValidationCode.E1095); } @Test diff --git a/dhis-2/dhis-test-integration/src/test/resources/tracker/tracker_basic_metadata.json b/dhis-2/dhis-test-integration/src/test/resources/tracker/tracker_basic_metadata.json index 04b2819ab833..41715dd25d9d 100644 --- a/dhis-2/dhis-test-integration/src/test/resources/tracker/tracker_basic_metadata.json +++ b/dhis-2/dhis-test-integration/src/test/resources/tracker/tracker_basic_metadata.json @@ -559,7 +559,7 @@ "ignoreOverdueEvents": false, "programSections": [], "created": "2019-03-29T12:41:33.001", - "shortName": "Tracker Program", + "shortName": "Tracker Program no default CC", "incidentDateLabel": "IncidentDate", "translations": [], "style": { @@ -571,10 +571,75 @@ }, "programStages": [ { - "id": "Qmqxq907VNz" + "id": "OilWH8Cj6QU" + } + ], + "selectEnrollmentDatesInFuture": false, + "categoryCombo": { + "id": "WG3syvpSE9o" + }, + "notificationTemplates": [], + "displayIncidentDate": true, + "accessLevel": "OPEN", + "featureType": "POLYGON", + "id": "qiHs9hD2Opf", + "lastUpdatedBy": { + "id": "tTgjgobT1oS" + }, + "completeEventsExpiryDays": 10, + "organisationUnits": [ + { + "id": "QfUVllTs6cS" }, { - "id": "OilWH8Cj6QU" + "id": "QfUVllTs6cZ" + }, + { + "id": "QfUVllTs6cW" + } + ], + "onlyEnrollOnce": false, + "lastUpdated": "2019-03-29T12:41:33.039", + "trackedEntityType": { + "id": "bPJ0FMtcnEh" + }, + "name": "Tracker Program no default CC", + "enrollmentDateLabel": "EnrollmentDate", + "maxTeiCountToReturn": 0, + "attributeValues": [], + "programTrackedEntityAttributes": [], + "useFirstStageDuringRegistration": false, + "minAttributesRequiredToSearch": 1, + "skipOffline": false, + "version": 0, + "programType": "WITH_REGISTRATION", + "selectIncidentDatesInFuture": false, + "displayFrontPageList": false, + "sharing": { + "public": "rw------", + "external": false, + "owner": "tTgjgobT1oS", + "users": {}, + "userGroups": {} + } + }, + { + "ignoreOverdueEvents": false, + "programSections": [], + "created": "2019-03-29T12:41:33.001", + "shortName": "Tracker Program", + "incidentDateLabel": "IncidentDate", + "translations": [], + "style": { + "color": "#d32f2f" + }, + "expiryDays": 0, + "user": { + "id": "tTgjgobT1oS" + }, + "programStages": [ + { + "id": "Qmqxq907VNz" }, { "id": "m7HlXaMvgX2" @@ -2671,25 +2736,6 @@ } ], "categoryOptions": [ - { - "id": "xYerKDKCefk", - "code": "default", - "name": "default", - "shortName": "default", - "startDate": "2019-01-01T00:00:00.000", - "attributeValues": [], - "lastUpdated": "2019-03-25T13:40:24.783", - "organisationUnits": [], - "created": "2019-03-25T13:40:24.722", - "translations": [], - "sharing": { - "public": "rwrw----", - "external": false, - "owner": null, - "users": {}, - "userGroups": {} - } - }, { "id": "WAFwwid9TzE", "code": "default2", @@ -2830,30 +2876,6 @@ } ], "categories": [ - { - "id": "GLevLNI9wkl", - "name": "default", - "shortName": "default", - "code": "default", - "dataDimensionType": "DISAGGREGATION", - "categoryOptions": [ - { - "id": "xYerKDKCefk" - } - ], - "dataDimension": false, - "attributeValues": [], - "translations": [], - "created": "2019-03-25T13:40:24.762", - "lastUpdated": "2019-03-29T08:54:32.360", - "sharing": { - "public": "rw------", - "external": false, - "owner": null, - "users": {}, - "userGroups": {} - } - }, { "id": "Vk8PHR8HBAe", "name": "default2", @@ -2928,28 +2950,6 @@ } ], "categoryCombos": [ - { - "id": "bjDvmb4bfuf", - "code": "default", - "name": "default", - "dataDimensionType": "DISAGGREGATION", - "categories": [ - { - "id": "GLevLNI9wkl" - } - ], - "created": "2019-03-25T13:40:24.771", - "lastUpdated": "2019-03-25T13:40:24.781", - "skipTotal": false, - "translations": [], - "sharing": { - "public": "rw------", - "external": false, - "owner": null, - "users": {}, - "userGroups": {} - } - }, { "id": "WG3syvpSE9o", "code": "default2", @@ -3043,25 +3043,6 @@ } ], "categoryOptionCombos": [ - { - "id": "HllvX50cXC0", - "code": "default", - "name": "default", - "categoryOptions": [ - { - "id": "xYerKDKCefk" - } - ], - "categoryCombo": { - "id": "bjDvmb4bfuf" - }, - "translations": [], - "startDate": "2019-01-01T00:00:00.000", - "lastUpdated": "2019-03-25T13:40:24.779", - "ignoreApproval": false, - "attributeValues": [], - "created": "2019-03-25T13:40:24.775" - }, { "id": "KKKKX50cXC0", "name": "accesstest1", diff --git a/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/enrollments-cat-write-access.json b/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/enrollments-cat-write-access.json new file mode 100644 index 000000000000..7a18012a347d --- /dev/null +++ b/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/enrollments-cat-write-access.json @@ -0,0 +1,29 @@ +{ + "trackedEntities": [], + "enrollments": [ + { + "enrollment": "CzXv1fa1kbX", + "trackedEntity": "Kj6vYde4LHh", + "program": { + "idScheme": "UID", + "identifier": "qiHs9hD2Opf" + }, + "status": "COMPLETED", + "orgUnit": { + "idScheme": "UID", + "identifier": "QfUVllTs6cS" + }, + "enrolledAt": "2019-08-19T00:00:00.000", + "occurredAt": "2019-08-19T00:00:00.000", + "followUp": false, + "deleted": false, + "storedBy": "admin", + "events": [], + "relationships": [], + "attributes": [], + "notes": [] + } + ], + "events": [], + "relationships": [] +} \ No newline at end of file diff --git a/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/events-cat-write-access.json b/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/events-cat-write-access.json index 9523595c2cde..d4c1aef6ca99 100644 --- a/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/events-cat-write-access.json +++ b/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/events-cat-write-access.json @@ -1,35 +1,4 @@ { - "importMode": "COMMIT", - "idSchemes": { - "dataElementIdScheme": { - "idScheme": "UID" - }, - "orgUnitIdScheme": { - "idScheme": "UID" - }, - "programIdScheme": { - "idScheme": "UID" - }, - "programStageIdScheme": { - "idScheme": "UID" - }, - "idScheme": { - "idScheme": "UID" - }, - "categoryOptionComboIdScheme": { - "idScheme": "UID" - }, - "categoryOptionIdScheme": { - "idScheme": "UID" - } - }, - "importStrategy": "CREATE", - "atomicMode": "ALL", - "flushMode": "AUTO", - "validationMode": "FULL", - "skipPatternValidation": false, - "skipSideEffects": false, - "skipRuleEngine": false, "trackedEntities": [], "enrollments": [], "events": [ @@ -38,13 +7,13 @@ "status": "ACTIVE", "program": { "idScheme": "UID", - "identifier": "E8o1E9tAppy" + "identifier": "qiHs9hD2Opf" }, "programStage": { "idScheme": "UID", "identifier": "OilWH8Cj6QU" }, - "enrollment": "MNWZ6hnuhSw", + "enrollment": "CzXv1fa1kbX", "orgUnit": { "idScheme": "UID", "identifier": "QfUVllTs6cS" @@ -57,13 +26,12 @@ "deleted": false, "attributeOptionCombo": { "idScheme": "UID", - "identifier": "HllvX50cXC0" + "identifier": "KKKKX50cXC0" }, "attributeCategoryOptions": [], "dataValues": [], "notes": [] } ], - "relationships": [], - "username": "system-process" + "relationships": [] } \ No newline at end of file From ef31be0ead8c92aed2c663a45f2c233a3cc37ac4 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Wed, 27 Nov 2024 09:51:09 +0100 Subject: [PATCH 30/33] Add missing files --- .../org/hisp/dhis/feedback/ErrorCode.java | 5 +- .../CategoryOptionComboObjectBundleHook.java | 71 +------------------ .../CategoryOptionComboControllerTest.java | 31 +------- 3 files changed, 3 insertions(+), 104 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java index 23a0c14c9f10..1c1cf44d0afe 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java @@ -67,10 +67,7 @@ public enum ErrorCode { E1120("Update cannot be applied as it would make existing data values inaccessible"), E1121("Data element `{0}` value type cannot be changed as it has associated data values"), - E1122("Category option combo {0} already exists for category combo {1}"), - E1123("Category option combo {0} must be associated with a category combo"), - E1124("Category option combo {0} cannot be associated with the default category combo"), - + E1122("Category option combo {0} cannot be associated with the default category combo"), E1125("Category option combo {0} contains options not associated with category combo {1}"), /* Org unit merge */ E1500("At least two source orgs unit must be specified"), diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java index ed95588fc917..8347f96b4e88 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/CategoryOptionComboObjectBundleHook.java @@ -27,13 +27,9 @@ */ package org.hisp.dhis.dxf2.metadata.objectbundle.hooks; -import java.util.List; -import java.util.Set; import java.util.function.Consumer; -import java.util.stream.Collectors; import lombok.AllArgsConstructor; import org.hisp.dhis.category.CategoryCombo; -import org.hisp.dhis.category.CategoryOption; import org.hisp.dhis.category.CategoryOptionCombo; import org.hisp.dhis.category.CategoryService; import org.hisp.dhis.dxf2.metadata.objectbundle.ObjectBundle; @@ -47,70 +43,6 @@ public class CategoryOptionComboObjectBundleHook extends AbstractObjectBundleHook { private final CategoryService categoryService; - static boolean haveEqualCatComboCatOptionReferenceIds( - CategoryOptionCombo one, CategoryOptionCombo other) { - if (one == null || other == null) { - return false; - } - - if (one.getCategoryCombo() == null || other.getCategoryCombo() == null) { - return false; - } - - if (one.getCategoryOptions() == null || other.getCategoryOptions() == null) { - return false; - } - - if (!one.getCategoryCombo().getUid().equals(other.getCategoryCombo().getUid())) { - return false; - } - - Set oneCategoryOptionUids = - one.getCategoryOptions().stream().map(CategoryOption::getUid).collect(Collectors.toSet()); - - Set otherCategoryOptionUids = - other.getCategoryOptions().stream().map(CategoryOption::getUid).collect(Collectors.toSet()); - - return oneCategoryOptionUids.equals(otherCategoryOptionUids); - } - - private void checkDuplicateCategoryOptionCombos( - CategoryOptionCombo categoryOptionCombo, - ObjectBundle bundle, - Consumer addReports) { - - if (bundle.isPersisted(categoryOptionCombo)) { - return; // Only check for duplicates if the object is not persisted - } - - List categoryOptionCombos = - categoryService.getAllCategoryOptionCombos().stream() - .filter( - coc -> - coc.getCategoryCombo() - .getUid() - .equals(categoryOptionCombo.getCategoryCombo().getUid())) - .toList(); - - // This could be an update or re-import of the same object. - if (categoryOptionCombos.stream() - .anyMatch(coc -> coc.getUid().equals(categoryOptionCombo.getUid()))) { - return; - } - // Check to see if the COC already exists in the list of COCs by comparing reference ids - for (CategoryOptionCombo existingCategoryOptionCombo : categoryOptionCombos) { - if (haveEqualCatComboCatOptionReferenceIds(categoryOptionCombo, existingCategoryOptionCombo) - && !categoryOptionCombo.getUid().equals(existingCategoryOptionCombo.getUid())) { - addReports.accept( - new ErrorReport( - CategoryOptionCombo.class, - ErrorCode.E1122, - categoryOptionCombo.getName(), - existingCategoryOptionCombo.getName())); - } - } - } - private void checkNonStandardDefaultCatOptionCombo( CategoryOptionCombo categoryOptionCombo, Consumer addReports) { @@ -125,7 +57,7 @@ private void checkNonStandardDefaultCatOptionCombo( if (!categoryOptionCombo.getUid().equals(defaultCatOptionCombo.getUid())) { addReports.accept( new ErrorReport( - CategoryOptionCombo.class, ErrorCode.E1124, categoryOptionCombo.getName())); + CategoryOptionCombo.class, ErrorCode.E1122, categoryOptionCombo.getName())); } } @@ -136,6 +68,5 @@ public void validate( Consumer addReports) { checkNonStandardDefaultCatOptionCombo(categoryOptionCombo, addReports); - checkDuplicateCategoryOptionCombos(categoryOptionCombo, bundle, addReports); } } diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java index 32c6fd53a882..0268fe8f443e 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java @@ -121,35 +121,6 @@ void catOptionCombosExcludingDefaultTest() { catOptionComboNames.contains("default"), "default catOptionCombo is not in payload"); } - @Test - @DisplayName("Duplicate CategoryOptionCombos should not be allowed") - void catOptionCombosDuplicatedTest() { - - JsonObject response = - GET("/categoryOptionCombos?filter=id:eq:CocUid0001&fields=id,categoryCombo[id],categoryOptions[id]") - .content(); - JsonList catOptionCombos = - response.getList("categoryOptionCombos", JsonCategoryOptionCombo.class); - String catOptionComboAOptions = catOptionCombos.get(0).getCategoryOptions().get(0).getId(); - String catOptionComboACatComboId = catOptionCombos.get(0).getCategoryCombo().getId(); - - JsonErrorReport error = - POST( - "/categoryOptionCombos/", - """ - { "name": "A_1", - "categoryOptions" : [{"id" : "%s"}], - "categoryCombo" : {"id" : "%s"} } - """ - .formatted(catOptionComboAOptions, catOptionComboACatComboId)) - .content(HttpStatus.CONFLICT) - .find(JsonErrorReport.class, report -> report.getErrorCode() == ErrorCode.E1122); - assertNotNull(error); - assertEquals( - "Category option combo A_1 already exists for category combo CatOptCombo A", - error.getMessage()); - } - @Test @DisplayName("Duplicate default category option combos should not be allowed") void catOptionCombosDuplicatedDefaultTest() { @@ -176,7 +147,7 @@ void catOptionCombosDuplicatedDefaultTest() { response.find(JsonErrorReport.class, report -> report.getErrorCode() == ErrorCode.E1122); assertNotNull(error); assertEquals( - "Category option combo Not default already exists for category combo default", + "Category option combo Not default cannot be associated with the default category combo", error.getMessage()); } } From 0a32aef33bc7da37a1511ae374ea8fcd0acc8127 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Thu, 28 Nov 2024 06:33:45 +0100 Subject: [PATCH 31/33] Add test to delete duplicated default COC --- .../CategoryOptionComboControllerTest.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java index 0268fe8f443e..2465e58153d1 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java @@ -31,6 +31,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import java.util.HashSet; import java.util.Set; import java.util.stream.Collectors; import org.hisp.dhis.category.CategoryCombo; @@ -150,4 +151,26 @@ void catOptionCombosDuplicatedDefaultTest() { "Category option combo Not default cannot be associated with the default category combo", error.getMessage()); } + + @Test + @DisplayName("Can delete a duplicate default COC") + void canAllowDeleteDuplicatedDefaultCOC() { + // Revert to the service layer as the API should not allow us to create a duplicate default COC + CategoryOptionCombo defaultCOC = categoryService.getDefaultCategoryOptionCombo(); + CategoryCombo categoryCombo = + categoryService.getCategoryCombo(defaultCOC.getCategoryCombo().getUid()); + CategoryOptionCombo existingCategoryOptionCombo = + categoryService.getCategoryOptionCombo(defaultCOC.getUid()); + CategoryOptionCombo categoryOptionComboDuplicate = new CategoryOptionCombo(); + categoryOptionComboDuplicate.setAutoFields(); + categoryOptionComboDuplicate.setCategoryCombo(categoryCombo); + Set newCategoryOptions = + new HashSet<>(existingCategoryOptionCombo.getCategoryOptions()); + categoryOptionComboDuplicate.setCategoryOptions(newCategoryOptions); + categoryOptionComboDuplicate.setName("dupDefault"); + categoryService.addCategoryOptionCombo(categoryOptionComboDuplicate); + + // Can delete the duplicated default COC + DELETE("/categoryOptionCombos/" + categoryOptionComboDuplicate.getUid()).content(HttpStatus.OK); + } } From 6345578f70dfe8d1d5f301c335415bf2456a9d24 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Thu, 28 Nov 2024 06:44:40 +0100 Subject: [PATCH 32/33] Add missing assertion --- .../webapi/controller/CategoryOptionComboControllerTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java index 2465e58153d1..c12663a85b79 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java @@ -27,6 +27,7 @@ */ package org.hisp.dhis.webapi.controller; +import static org.hisp.dhis.http.HttpAssertions.assertStatus; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -171,6 +172,6 @@ void canAllowDeleteDuplicatedDefaultCOC() { categoryService.addCategoryOptionCombo(categoryOptionComboDuplicate); // Can delete the duplicated default COC - DELETE("/categoryOptionCombos/" + categoryOptionComboDuplicate.getUid()).content(HttpStatus.OK); + assertStatus(HttpStatus.OK,DELETE("/categoryOptionCombos/" + categoryOptionComboDuplicate.getUid())); } } From 11932291b4d0789e122e453d9d5e916e9a5a3819 Mon Sep 17 00:00:00 2001 From: Jason Pickering Date: Thu, 28 Nov 2024 06:44:56 +0100 Subject: [PATCH 33/33] Linting --- .../webapi/controller/CategoryOptionComboControllerTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java index c12663a85b79..dcd627964017 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CategoryOptionComboControllerTest.java @@ -172,6 +172,7 @@ void canAllowDeleteDuplicatedDefaultCOC() { categoryService.addCategoryOptionCombo(categoryOptionComboDuplicate); // Can delete the duplicated default COC - assertStatus(HttpStatus.OK,DELETE("/categoryOptionCombos/" + categoryOptionComboDuplicate.getUid())); + assertStatus( + HttpStatus.OK, DELETE("/categoryOptionCombos/" + categoryOptionComboDuplicate.getUid())); } }