fix: Fix enrollment and event tables [DHIS2-12600]

[enricocolasante]

In the DB enrollment and event tables were allowing null values for organisationunitid column even though the API is validating that organisation unit is not null for Enrollment and Event when importing.

The PR is changing the DB with a flyway migration to force organisationunitid column to be not null.

If the migration fails, it displays a message detailing how to detect and fix the issue.

The migration notes will be published together with the release notes, a PR can be found here.

Changes needed after having an organisation unit for placeholder enrollment

• When creating the placeholder enrollment an organisation unit must be assigned to it. We are now assigned one of the root organisation unit to it.
• In the enrollment exporter we now need to filter out all placeholder enrollments. Before they were filtered out because organisation unit was null.

[ameenhere]

This looks like there could be multiple root org units? I guess it still does not matter that we arbitrarily chose one of the root org unit.

[enricocolasante]

I was surprised as well that the method is potentially returning more that one root.
In our case, it doesn't matter, we need just a value in order to make the column not null, any random value is actually fine in there.

[muilpp]

maybe I'm missing something...
but here we validate the user capture scope if the strategy is not an UPDATE.
since this method is only called when the strategy is not update, do we need to validate the org unit scope again?

[enricocolasante]

This whole validator is quite messy and difficult to follow.
We already have a bug to double check that everything in here is correct.
https://dhis2.atlassian.net/browse/DHIS2-17335

[teleivo]

What is a placeholder enrollment?
We used the term dummy enrollment AFAIK so far for programs without registration. Is that the same or a different enrollment?

1. 1.1 If any of the Event OU inside the enrollment is not null, update the same as Enrollment OU

Can the events have different OU? If so, what consequence can it have to pick one?

1.2 If all Event OUs are null and the enrollment is a placeholder (link to a program WITHOUT_REGISTRATION) then use the root OU.

What is the root OU?

After step 1, for all Event OU nulls, update the Enrollment OU as the event OU

Do you mean update the event OU with the enrollment OU?

Deleting data that is not ours always feels scary 😨 Are we certain we are not deleting anything that might still be of value? Can we move the data somewhere so it could be analyzed/recovered/made valid? If not we should at least add a big notice to the release notes with a sql query to run beforehand to retrieve the data that would be deleted.

[enricocolasante]

What is a placeholder enrollment? We used the term dummy enrollment AFAIK so far for programs without registration. Is that the same or a different enrollment?

Yes, a placeholder enrollment is the dummy enrollment

1. 1.1 If any of the Event OU inside the enrollment is not null, update the same as Enrollment OU

Can the events have different OU? If so, what consequence can it have to pick one?

Here we are guessing a good OU. This will come together with a big warning in the release notes, if they have a lot of enrollment/events with null OU, they should check the data beforehand and understand what they want to do with them. Our solution is a best effort solution in order to make the system consistent and remove the possibility to have inconsistent data in the DB

1.2 If all Event OUs are null and the enrollment is a placeholder (link to a program WITHOUT_REGISTRATION) then use the root OU.

What is the root OU?

It is a OU with no parent, hence a OU with hierarchy level 1

After step 1, for all Event OU nulls, update the Enrollment OU as the event OU

Do you mean update the event OU with the enrollment OU?

Yes :)

Deleting data that is not ours always feels scary 😨 Are we certain we are not deleting anything that might still be of value? Can we move the data somewhere so it could be analyzed/recovered/made valid? If not we should at least add a big notice to the release notes with a sql query to run beforehand to retrieve the data that would be deleted.

We will have a gib warning on the release notes about it.
Anyway, tracker data without an attached tracked entity is just lost and meaningless data that shouldn't ever be in the DB.
I added that part in the migration because we had some dirty data in SL database that needed to be fixed to make the migration work, but "I hope" that wouldn't be the case in most of DBs out there.
We are going to have a warning in the release notes and maybe also a query to make them check the invalid data before upgrading.

[muilpp]

For Enrollment OU nulls
1.1 If any of the Event OU inside the enrollment is not null, update the same as Enrollment OU
1.2 If all Event OUs are null and the enrollment is a placeholder (link to a program WITHOUT_REGISTRATION) then use the root OU.
1.3 For tracker enrollments that still have null OUs, use the TEI OU as Enrollment OU

After step 1, for all Event OU nulls, update the Enrollment OU as the event OU

If an enrollment or event doesn't have an org unit, are we currently showing it to the user? If we are not, maybe it'd be better to not update the org unit at all and keep on not showing it. Maybe move it to another table and let the implementation decide what to do with that data, something similar to what we've done with change logs.
Again, assuming we are currently not showing it, could it happen that by updating the org unit we end up showing the same program stage twice in the same event?

[sonarqubecloud]

Quality Gate failed

Failed conditions
1 New issue
1 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE