diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataintegrity/DataIntegrityCheckType.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataintegrity/DataIntegrityCheckType.java index ceec5e3e8fba..f863bae673a0 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataintegrity/DataIntegrityCheckType.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataintegrity/DataIntegrityCheckType.java @@ -64,7 +64,6 @@ public enum DataIntegrityCheckType { // OrganisationUnits ORG_UNITS_WITH_CYCLIC_REFERENCES, ORG_UNITS_BEING_ORPHANED, - ORG_UNITS_WITHOUT_GROUPS, ORG_UNITS_VIOLATING_EXCLUSIVE_GROUP_SETS, ORG_UNIT_GROUPS_WITHOUT_GROUP_SETS, diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataintegrity/FlattenedDataIntegrityReport.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataintegrity/FlattenedDataIntegrityReport.java index 0b6fa378afff..92c776b4cb5d 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataintegrity/FlattenedDataIntegrityReport.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataintegrity/FlattenedDataIntegrityReport.java @@ -143,9 +143,9 @@ public FlattenedDataIntegrityReport(Map detailsByN this.orphanedOrganisationUnits = listOfDisplayNameOrUid( detailsByName.get(DataIntegrityCheckType.ORG_UNITS_BEING_ORPHANED.getName())); + // Replaced with SQL based equivalent this.organisationUnitsWithoutGroups = - listOfDisplayNameOrUid( - detailsByName.get(DataIntegrityCheckType.ORG_UNITS_WITHOUT_GROUPS.getName())); + listOfDisplayNameWithUid(detailsByName.get("organisation_units_without_groups")); this.organisationUnitGroupsWithoutGroupSets = listOfDisplayNameOrUid( detailsByName.get(DataIntegrityCheckType.ORG_UNIT_GROUPS_WITHOUT_GROUP_SETS.getName())); @@ -255,6 +255,14 @@ private List listOfDisplayNameOrUid(DataIntegrityDetails details) { .collect(toUnmodifiableList()); } + private List listOfDisplayNameWithUid(DataIntegrityDetails details) { + return details == null + ? null + : details.getIssues().stream() + .map(issue -> issue.getName() + ":" + issue.getId()) + .collect(toUnmodifiableList()); + } + private List> groupedListOfDisplayNameOrUid(DataIntegrityDetails details) { return details == null ? null diff --git a/dhis-2/dhis-services/dhis-service-administration/src/main/java/org/hisp/dhis/dataintegrity/DefaultDataIntegrityService.java b/dhis-2/dhis-services/dhis-service-administration/src/main/java/org/hisp/dhis/dataintegrity/DefaultDataIntegrityService.java index bd25dca42688..fd93a2fc7d09 100644 --- a/dhis-2/dhis-services/dhis-service-administration/src/main/java/org/hisp/dhis/dataintegrity/DefaultDataIntegrityService.java +++ b/dhis-2/dhis-services/dhis-service-administration/src/main/java/org/hisp/dhis/dataintegrity/DefaultDataIntegrityService.java @@ -444,10 +444,6 @@ List getOrphanedOrganisationUnits() { return toSimpleIssueList(organisationUnitService.getOrphanedOrganisationUnits().stream()); } - List getOrganisationUnitsWithoutGroups() { - return toSimpleIssueList(organisationUnitService.getOrganisationUnitsWithoutGroups().stream()); - } - List getOrganisationUnitsViolatingExclusiveGroupSets() { return toIssueList( organisationUnitService.getOrganisationUnitsViolatingExclusiveGroupSets().stream(), @@ -609,10 +605,6 @@ public void initIntegrityChecks() { DataIntegrityCheckType.ORG_UNITS_BEING_ORPHANED, OrganisationUnit.class, this::getOrphanedOrganisationUnits); - registerNonDatabaseIntegrityCheck( - DataIntegrityCheckType.ORG_UNITS_WITHOUT_GROUPS, - OrganisationUnit.class, - this::getOrganisationUnitsWithoutGroups); registerNonDatabaseIntegrityCheck( DataIntegrityCheckType.ORG_UNITS_VIOLATING_EXCLUSIVE_GROUP_SETS, OrganisationUnit.class, @@ -621,7 +613,6 @@ public void initIntegrityChecks() { DataIntegrityCheckType.ORG_UNIT_GROUPS_WITHOUT_GROUP_SETS, OrganisationUnitGroup.class, this::getOrganisationUnitGroupsWithoutGroupSets); - registerNonDatabaseIntegrityCheck( DataIntegrityCheckType.VALIDATION_RULES_WITHOUT_GROUPS, ValidationRule.class, @@ -697,7 +688,9 @@ public FlattenedDataIntegrityReport getReport(Set checks, JobProgress pr checks = Arrays.stream(DataIntegrityCheckType.values()) .map(DataIntegrityCheckType::getName) - .collect(toUnmodifiableSet()); + .collect(Collectors.toSet()); + // Add additional SQL based checks here + checks.add("organisation_units_without_groups"); } runDetailsChecks(checks, progress); return new FlattenedDataIntegrityReport(getDetails(checks, -1L)); diff --git a/dhis-2/dhis-services/dhis-service-administration/src/main/resources/data-integrity-checks.yaml b/dhis-2/dhis-services/dhis-service-administration/src/main/resources/data-integrity-checks.yaml index 42908a8c8a71..a11a8189fb4e 100644 --- a/dhis-2/dhis-services/dhis-service-administration/src/main/resources/data-integrity-checks.yaml +++ b/dhis-2/dhis-services/dhis-service-administration/src/main/resources/data-integrity-checks.yaml @@ -27,6 +27,7 @@ checks: - orgunits/orgunits_invalid_geometry.yaml - orgunits/orgunits_no_geometry.yaml - orgunits/orgunits_same_name_and_parent.yaml + - orgunits/orgunits_no_groups.yaml - option_sets/option_sets_empty.yaml - option_sets/unused_option_sets.yaml - option_sets/option_sets_wrong_sort_order.yaml diff --git a/dhis-2/dhis-services/dhis-service-administration/src/main/resources/data-integrity-checks/orgunits/orgunits_no_groups.yaml b/dhis-2/dhis-services/dhis-service-administration/src/main/resources/data-integrity-checks/orgunits/orgunits_no_groups.yaml new file mode 100644 index 000000000000..7e9c535090b2 --- /dev/null +++ b/dhis-2/dhis-services/dhis-service-administration/src/main/resources/data-integrity-checks/orgunits/orgunits_no_groups.yaml @@ -0,0 +1,54 @@ +# Copyright (c) 2004-2022, 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. +# +--- +name: organisation_units_without_groups +description: Organisation units with no groups. +section: Organisation units +section_order: 13 +summary_sql: >- + WITH orgunits_no_groups as ( + SELECT uid,name from organisationunit + where organisationunitid NOT IN ( + SELECT organisationunitid from orgunitgroupmembers)) + SELECT + COUNT(*)as value, + 100.0 * COUNT(*) / NULLIF( (SELECT COUNT(*) FROM organisationunit), 0) as percent + FROM orgunits_no_groups; +details_sql: >- + SELECT uid,name from organisationunit + where organisationunitid NOT IN ( + SELECT organisationunitid from orgunitgroupmembers) + ORDER BY name; +details_id_type: organisationUnits +severity: WARNING +introduction: | + All organisation units should usually belong to at least one organisation unit group. + When organisation units do not belong to any groups, they become more difficult to identify + in analysis apps like the data visualizer. +recommendation: | + Create useful organisation unit groups to help users filter certain classes of organisation + units. These groups may or may not be used in organisation unit group sets \ No newline at end of file diff --git a/dhis-2/dhis-services/dhis-service-administration/src/test/java/org/hisp/dhis/dataintegrity/DataIntegrityServiceTest.java b/dhis-2/dhis-services/dhis-service-administration/src/test/java/org/hisp/dhis/dataintegrity/DataIntegrityServiceTest.java index bf5a54541aee..1dd6b28dd1bf 100644 --- a/dhis-2/dhis-services/dhis-service-administration/src/test/java/org/hisp/dhis/dataintegrity/DataIntegrityServiceTest.java +++ b/dhis-2/dhis-services/dhis-service-administration/src/test/java/org/hisp/dhis/dataintegrity/DataIntegrityServiceTest.java @@ -428,13 +428,6 @@ void testGetOrphanedOrganisationUnits() { verifyNoMoreInteractions(organisationUnitService); } - @Test - void testGetOrganisationUnitsWithoutGroups() { - subject.getOrganisationUnitsWithoutGroups(); - verify(organisationUnitService).getOrganisationUnitsWithoutGroups(); - verifyNoMoreInteractions(organisationUnitService); - } - @Test void testGetProgramRulesWithNoExpression() { programRuleB.setCondition(null); diff --git a/dhis-2/dhis-services/dhis-service-administration/src/test/java/org/hisp/dhis/dataintegrity/DataIntegrityYamlReaderTest.java b/dhis-2/dhis-services/dhis-service-administration/src/test/java/org/hisp/dhis/dataintegrity/DataIntegrityYamlReaderTest.java index b34c775dee60..b65ab04cb87a 100644 --- a/dhis-2/dhis-services/dhis-service-administration/src/test/java/org/hisp/dhis/dataintegrity/DataIntegrityYamlReaderTest.java +++ b/dhis-2/dhis-services/dhis-service-administration/src/test/java/org/hisp/dhis/dataintegrity/DataIntegrityYamlReaderTest.java @@ -56,7 +56,7 @@ void testReadDataIntegrityYaml() { List checks = new ArrayList<>(); readYaml(checks, "data-integrity-checks.yaml", "data-integrity-checks", CLASS_PATH); - assertEquals(63, checks.size()); + assertEquals(64, checks.size()); // Names should be unique List allNames = checks.stream().map(DataIntegrityCheck::getName).toList(); diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityOrganisationUnitsNoGroupsControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityOrganisationUnitsNoGroupsControllerTest.java new file mode 100644 index 000000000000..688e168f31cb --- /dev/null +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityOrganisationUnitsNoGroupsControllerTest.java @@ -0,0 +1,114 @@ +/* + * Copyright (c) 2004-2022, 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.webapi.controller.dataintegrity; + +import static org.hisp.dhis.web.WebClientUtils.assertStatus; + +import org.hisp.dhis.web.HttpStatus; +import org.junit.jupiter.api.Test; + +/** + * Checks for organisation units with no groups. {@see + * dhis-2/dhis-services/dhis-service-administration/src/main/resources/data-integrity-checks/orgunits/orgunits_no_groups.yaml} + * + * @author Jason P. Pickering + */ +class DataIntegrityOrganisationUnitsNoGroupsControllerTest + extends AbstractDataIntegrityIntegrationTest { + private String orgunitA; + + private String orgunitB; + + private static final String check = "organisation_units_without_groups"; + + private static final String detailsIdType = "organisationUnits"; + + @Test + void testOrgunitsNoGroups() { + + orgunitA = + assertStatus( + HttpStatus.CREATED, + POST( + "/organisationUnits", + "{ 'name': 'Fish District', 'shortName': 'Fish District', 'openingDate' : '2022-01-01'}")); + + orgunitB = + assertStatus( + HttpStatus.CREATED, + POST( + "/organisationUnits", + "{ 'name': 'Pizza District', 'shortName': 'Pizza District', 'openingDate' : '2022-01-01'}")); + + // Create an orgunit group + assertStatus( + HttpStatus.CREATED, + POST( + "/organisationUnitGroups", + "{'name': 'Type A', 'shortName': 'Type A', 'organisationUnits' : [{'id' : '" + + orgunitA + + "'}]}")); + assertHasDataIntegrityIssues( + detailsIdType, check, 50, orgunitB, "Pizza District", "Type", true); + } + + @Test + void testOrgunitsInGroups() { + + orgunitA = + assertStatus( + HttpStatus.CREATED, + POST( + "/organisationUnits", + "{ 'name': 'Fish District', 'shortName': 'Fish District', 'openingDate' : '2022-01-01'}")); + + orgunitB = + assertStatus( + HttpStatus.CREATED, + POST( + "/organisationUnits", + "{ 'name': 'Pizza District', 'shortName': 'Pizza District', 'openingDate' : '2022-01-01'}")); + + // Create an orgunit group + assertStatus( + HttpStatus.CREATED, + POST( + "/organisationUnitGroups", + "{'name': 'Type A', 'shortName': 'Type A', 'organisationUnits' : [{'id' : '" + + orgunitA + + "'}, {'id' : '" + + orgunitB + + "'}]}")); + assertHasNoDataIntegrityIssues(detailsIdType, check, true); + } + + @Test + void testOrgUnitsNoGroupsDivideByZero() { + assertHasNoDataIntegrityIssues(detailsIdType, check, false); + } +} diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityReportControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityReportControllerTest.java index 173b20daee91..5da6e59a4b12 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityReportControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataintegrity/DataIntegrityReportControllerTest.java @@ -118,9 +118,9 @@ void testOrganisationUnitsWithoutGroups() { String ouId = addOrganisationUnit("noGroupSet"); // should not match: addOrganisationUnitGroup("group", addOrganisationUnit("hasGroupSet")); - assertEquals( - singletonList("noGroupSet:" + ouId), - getDataIntegrityReport().getOrganisationUnitsWithoutGroups().toList(JsonString::string)); + List results = + getDataIntegrityReport().getOrganisationUnitsWithoutGroups().toList(JsonString::string); + assertEquals(singletonList("noGroupSet:" + ouId), results); } @Test