Skip to content

Commit

Permalink
fix: Fix enrollment and event tables [DHIS2-12600] (#19193)
Browse files Browse the repository at this point in the history
* fix: Fix enrollment and event tables [DHIS2-12600]

* fix: Add dummy orgUnit to tracker objects with null orgUnit

* fix: Raise exception with details on failing migration [DHIS2-12600]

* fix: Raise exception with details on failing migration [DHIS2-12600]

* fix: Fix test

* Update dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_28__Make_ou_event_and_enrollment_not_null.sql

Co-authored-by: teleivo <teleivo@users.noreply.github.com>

---------

Co-authored-by: teleivo <teleivo@users.noreply.github.com>
  • Loading branch information
enricocolasante and teleivo authored Dec 4, 2024
1 parent db805c1 commit 8755493
Show file tree
Hide file tree
Showing 11 changed files with 54 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
</list>

<many-to-one name="organisationUnit" class="org.hisp.dhis.organisationunit.OrganisationUnit" column="organisationunitid"
foreign-key="fk_programinstance_organisationunitid" />
foreign-key="fk_programinstance_organisationunitid" not-null="true"/>

</class>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
<property name="occurredDate" column="occurreddate" type="timestamp" index="programstageinstance_executiondate" />

<many-to-one name="organisationUnit" class="org.hisp.dhis.organisationunit.OrganisationUnit" column="organisationunitid"
foreign-key="fk_programstageinstance_organisationunitid" index="programstageinstance_organisationunitid" />
foreign-key="fk_programstageinstance_organisationunitid" index="programstageinstance_organisationunitid" not-null="true"/>

<property name="status" column="status" type="org.hisp.dhis.program.EventStatusUserType" not-null="true" />

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.hisp.dhis.dxf2.metadata.objectbundle.ObjectBundle;
import org.hisp.dhis.feedback.ErrorCode;
import org.hisp.dhis.feedback.ErrorReport;
import org.hisp.dhis.organisationunit.OrganisationUnitService;
import org.hisp.dhis.preheat.PreheatIdentifier;
import org.hisp.dhis.program.Enrollment;
import org.hisp.dhis.program.EnrollmentStatus;
Expand All @@ -58,6 +59,8 @@ public class ProgramObjectBundleHook extends AbstractObjectBundleHook<Program> {

private final ProgramStageService programStageService;

private final OrganisationUnitService organisationUnitService;

private final AclService aclService;

private final IdentifiableObjectManager identifiableObjectManager;
Expand Down Expand Up @@ -133,6 +136,8 @@ private void addProgramInstance(Program program) {
enrollment.setProgram(program);
enrollment.setStatus(EnrollmentStatus.ACTIVE);
enrollment.setStoredBy("system-process");
enrollment.setOrganisationUnit(
organisationUnitService.getRootOrganisationUnits().iterator().next());

identifiableObjectManager.save(enrollment);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
import org.hisp.dhis.common.IdentifiableObjectManager;
import org.hisp.dhis.feedback.ErrorCode;
import org.hisp.dhis.feedback.ErrorReport;
import org.hisp.dhis.organisationunit.OrganisationUnit;
import org.hisp.dhis.organisationunit.OrganisationUnitService;
import org.hisp.dhis.program.Enrollment;
import org.hisp.dhis.program.EnrollmentStatus;
import org.hisp.dhis.program.EventProgramEnrollmentService;
Expand All @@ -71,6 +73,8 @@ class ProgramObjectBundleHookTest {

@Mock private ProgramStageService programStageService;

@Mock private OrganisationUnitService organisationUnitService;

@Mock private AclService aclService;

@Mock private IdentifiableObjectManager identifiableObjectManager;
Expand All @@ -83,6 +87,7 @@ public void setUp() {
new ProgramObjectBundleHook(
eventProgramEnrollmentService,
programStageService,
organisationUnitService,
aclService,
identifiableObjectManager);

Expand All @@ -106,6 +111,8 @@ void verifyMissingBundleIsIgnored() {

@Test
void verifyProgramInstanceIsSavedForEventProgram() {
when(organisationUnitService.getRootOrganisationUnits())
.thenReturn(List.of(new OrganisationUnit()));
ArgumentCaptor<Enrollment> argument = ArgumentCaptor.forClass(Enrollment.class);

programA.setProgramType(ProgramType.WITHOUT_REGISTRATION);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import org.hisp.dhis.organisationunit.OrganisationUnit;
import org.hisp.dhis.program.Enrollment;
import org.hisp.dhis.program.EnrollmentStatus;
import org.hisp.dhis.program.ProgramType;
import org.hisp.dhis.security.acl.AclService;
import org.hisp.dhis.tracker.export.Order;
import org.hisp.dhis.tracker.export.Page;
Expand Down Expand Up @@ -192,6 +193,9 @@ private QueryWithOrderBy buildEnrollmentHql(EnrollmentQueryParams params) {
hql += hlp.whereAnd() + "en.program.uid = '" + params.getProgram().getUid() + "'";
}

// TODO(DHIS2-17961) This will be removed when dummy enrollments will not exist anymore
hql += hlp.whereAnd() + "en.program.programType = '" + ProgramType.WITH_REGISTRATION + "'";

if (params.hasEnrollmentStatus()) {
hql += hlp.whereAnd() + "en." + STATUS + " = '" + params.getEnrollmentStatus() + "'";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,7 @@ public void validate(
// If event is newly created, or going to be deleted, capture scope
// has to be checked
if (program.isWithoutRegistration() || strategy.isCreate() || strategy.isDelete()) {
if (organisationUnit == null) {
log.warn(ORG_UNIT_NO_USER_ASSIGNED, event.getUid());
} else {
checkOrgUnitInCaptureScope(reporter, event, organisationUnit, bundle.getUser());
}
checkOrgUnitInCaptureScope(reporter, event, organisationUnit, bundle.getUser());
}

UID teUid = getTeUidFromEvent(bundle, event, program);
Expand Down Expand Up @@ -304,9 +300,7 @@ private void checkEventOrgUnitWriteAccess(
OrganisationUnit eventOrgUnit,
boolean isCreatableInSearchScope,
UserDetails user) {
if (eventOrgUnit == null) {
log.warn(ORG_UNIT_NO_USER_ASSIGNED, event.getUid());
} else if (isCreatableInSearchScope
if (isCreatableInSearchScope
? !user.isInUserEffectiveSearchOrgUnitHierarchy(eventOrgUnit.getPath())
: !user.isInUserHierarchy(eventOrgUnit.getPath())) {
reporter.addError(event, ValidationCode.E1000, user, eventOrgUnit);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
DO $$
BEGIN
-- Set null organisation unit of dummy enrollments to root organisation unit
update enrollment en
set organisationunitid = (select organisationunitid from organisationunit order by hierarchylevel limit 1)
where en.organisationunitid is null
and en.programid in (select programid from program where type = 'WITHOUT_REGISTRATION');

alter table if exists enrollment alter column organisationunitid set not null;

alter table if exists event alter column organisationunitid set not null;

EXCEPTION
WHEN not_null_violation THEN
RAISE EXCEPTION 'There is inconsistent data in your DB. Please check https://github.com/dhis2/dhis2-releases/blob/master/releases/2.42/migration-notes.md#null-organisation-unit to have more information on the issue and to find ways to fix it. Detailed error message: %', SQLERRM;

END;
$$;
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ void testDeleteSoftDeletedEnrollmentLinkedToAnEventDataValueChangeLog()
eventA.setScheduledDate(enrollmentDate);
eventA.setUid("UID-A");
eventA.setAttributeOptionCombo(coA);
eventA.setOrganisationUnit(organisationUnit);
manager.save(eventA);
eventChangeLogService.addDataValueChangeLog(
eventA, dataElement, "", "value", ChangeLogType.UPDATE, getCurrentUsername());
Expand Down Expand Up @@ -362,6 +363,7 @@ void testDeleteSoftDeletedEventLinkedToARelationshipItem() {
eventA.setScheduledDate(enrollmentDate);
eventA.setUid(UID.generate().getValue());
eventA.setAttributeOptionCombo(coA);
eventA.setOrganisationUnit(organisationUnit);
manager.save(eventA);
long idA = eventA.getId();
Relationship r = new Relationship();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ void setUp() {

enrollmentA = new Enrollment(new Date(), new Date(), trackedEntityB, programA);
enrollmentA.setUid(CodeGenerator.generateUid());
enrollmentA.setOrganisationUnit(orgUnitA);

eventA = createEvent(stageA, enrollmentA, orgUnitA);
eventA.setScheduledDate(new Date());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,26 +171,28 @@ void setUp() {
Date enrollmentDate = testDate2.toDate();
enrollmentA = new Enrollment(enrollmentDate, incidenDate, trackedEntityA, programA);
enrollmentA.setUid("UID-PIA");
enrollmentA.setOrganisationUnit(organisationUnitA);
manager.save(enrollmentA);
enrollmentB = new Enrollment(enrollmentDate, incidenDate, trackedEntityB, programB);
enrollmentB.setOrganisationUnit(organisationUnitB);
manager.save(enrollmentB);
Event eventA = new Event(enrollmentA, stageA);
Event eventA = new Event(enrollmentA, stageA, organisationUnitA);
eventA.setScheduledDate(enrollmentDate);
eventA.setUid("UID-A");
eventA.setAttributeOptionCombo(coA);
Event eventB = new Event(enrollmentA, stageB);
Event eventB = new Event(enrollmentA, stageB, organisationUnitA);
eventB.setScheduledDate(enrollmentDate);
eventB.setUid("UID-B");
eventB.setAttributeOptionCombo(coA);
Event eventC = new Event(enrollmentB, stageC);
Event eventC = new Event(enrollmentB, stageC, organisationUnitA);
eventC.setScheduledDate(enrollmentDate);
eventC.setUid("UID-C");
eventC.setAttributeOptionCombo(coA);
Event eventD1 = new Event(enrollmentB, stageD);
Event eventD1 = new Event(enrollmentB, stageD, organisationUnitA);
eventD1.setScheduledDate(enrollmentDate);
eventD1.setUid("UID-D1");
eventD1.setAttributeOptionCombo(coA);
Event eventD2 = new Event(enrollmentB, stageD);
Event eventD2 = new Event(enrollmentB, stageD, organisationUnitA);
eventD2.setScheduledDate(enrollmentDate);
eventD2.setUid("UID-D2");
eventD2.setAttributeOptionCombo(coA);
Expand Down Expand Up @@ -281,15 +283,15 @@ void testGetEventsWithScheduledNotifications() {
cal.add(Calendar.DATE, -2);
Date yesterday = cal.getTime();
// Events
Event eventA = new Event(enrollmentA, stageA);
Event eventA = new Event(enrollmentA, stageA, organisationUnitA);
eventA.setScheduledDate(tomorrow);
eventA.setAttributeOptionCombo(coA);
manager.save(eventA);
Event eventB = new Event(enrollmentB, stageB);
Event eventB = new Event(enrollmentB, stageB, organisationUnitA);
eventB.setScheduledDate(today);
eventB.setAttributeOptionCombo(coA);
manager.save(eventB);
Event eventC = new Event(enrollmentB, stageC);
Event eventC = new Event(enrollmentB, stageC, organisationUnitA);
eventC.setScheduledDate(yesterday);
eventC.setAttributeOptionCombo(coA);
manager.save(eventC);
Expand Down Expand Up @@ -415,8 +417,10 @@ void testGetEnrollmentsWithScheduledNotifications() {
Date aWeekAgo = cal.getTime();
// Enrollments
Enrollment enrollmentC = new Enrollment(today, tomorrow, trackedEntityX, programA);
enrollmentC.setOrganisationUnit(organisationUnitA);
manager.save(enrollmentC);
Enrollment enrollmentD = new Enrollment(aWeekAgo, yesterday, trackedEntityY, programA);
enrollmentD.setOrganisationUnit(organisationUnitA);
manager.save(enrollmentD);
// Queries
List<Enrollment> results;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ void testEmptyEventFormHasIntegrityIssues() {
"id": "%s",
"name": "Test",
"shortName": "Test",
"programType": "WITHOUT_REGISTRATION",
"programType": "WITH_REGISTRATION",
"categoryCombo": {"id": "%s"},
"programStages": [{"id": "%s"}]
}
Expand Down

0 comments on commit 8755493

Please sign in to comment.