(chore) O3-4426: Update OpenMRS version and Initializer version

[wikumChamith]

This updates the OpenMRS version to 2.7.2-SNAPSHOT and the Initilizer version to 2.9.0-SNAPSHOT.

[vasharma05]

CC: @Ruhanga, the E2E tests are failing when updating the Openmrs core and initializer versions.

[dkayiwa]

@jayasanka-sack how do i access the server logs for this?

[NethmiRodrigo]

@dkayiwa I've updated this file to capture the server logs when e2e test fails. These changes should get reverted before this PR gets merged

[dkayiwa]

Is it because you hate server logs? 😊

[NethmiRodrigo]

XD haha purely because there'll be a large number of logs

[denniskigen]

@NethmiRodrigo if we do decide to keep the Upload Logs as Artifact steps, can we add a repo-specific prefix to the artifact names for easier disambiguation?

name: ${{ github.repository | replace('/', '-') }}-server-logs # or similar

[NethmiRodrigo]

@denniskigen yeah that makes sense!

[wikumChamith]

I noticed these errors in backend logs when it is loading the location demo data: https://pastebin.com/34TSwAcK

[samuelmale]

I believe this PR mekomsolutions/openmrs-module-initializer#293 from Mike that fixed "locations support on platform 2.7" introduced a minor bug in the way the attributes are processed. The issue is that a filter that was meant to filter out empty column values was removed here.

[mseaton]

I believe this PR mekomsolutions/openmrs-module-initializer#293 from Mike that fixed "locations support on platform 2.7" introduced a minor bug in the way the attributes are processed. The issue is that a filter that was meant to filter out empty column values was removed here.

Looks like you might be right @samuelmale . I'll have a look.

[mseaton]

Looks like you might be right @samuelmale . I'll have a look.

Actually, the code looks right, and I can't reproduce this in a unit test. What exactly is the problem that you are seeing? @samuelmale

This issue with the error that is linked to above by @wikumChamith is the same error @Ruhanga pinged me about yesterday and I already discussed with him. That is - the error isn't due to the empty value, it's due to the uuid that points to a location attribute type that is not in the system. If you want to add an attribute of a given type, then you need to add initializer code to add that attribute type in the appropriate domain.

[wikumChamith]

Looks like you might be right @samuelmale . I'll have a look.

Actually, the code looks right, and I can't reproduce this in a unit test. What exactly is the problem that you are seeing? @samuelmale

This issue with the error that is linked to above by @wikumChamith is the same error he pinged me about yesterday and I already discussed with him. That is - the error isn't due to the empty value, it's due to the uuid that points to a location attribute type that is not in the system. If you want to add an attribute of a given type, then you need to add initializer code to add that attribute type in the appropriate domain.

@mseaton

Looks like you might be right @samuelmale . I'll have a look.

Actually, the code looks right, and I can't reproduce this in a unit test. What exactly is the problem that you are seeing? @samuelmale

This issue with the error that is linked to above by @wikumChamith is the same error he pinged me about yesterday and I already discussed with him. That is - the error isn't due to the empty value, it's due to the uuid that points to a location attribute type that is not in the system. If you want to add an attribute of a given type, then you need to add initializer code to add that attribute type in the appropriate domain.

Thanks for the clarification! However, I believe there might be a misunderstanding—I don’t recall discussing this issue with you yesterday. It’s possible you had that conversation with someone else.

[mseaton]

Thanks for the clarification! However, I believe there might be a misunderstanding—I don’t recall discussing this issue with you yesterday. It’s possible you had that conversation with someone else

Right, sorry. I updated my comment above, it was @Ruhanga who originally raised this with me yesterday.

[samuelmale]

Actually, the code looks right, and I can't reproduce this in a unit test. What exactly is the problem that you are seeing?

@mseaton I think your changes just unmasked a vulnerability/bug in Iniz. The filter I previously mentioned made it possible to bypass ad-hoc style headers ("Attribute|9eca4f4e-707f-4bb8-8289-2f9b6e93803c") referencing non-existing metadata ("9eca4f4e-707f-4bb8-8289-2f9b6e93803c") for as long as its underlying column is empty.

What's happening with the distro?
Here is the failing CSV file: https://github.com/openmrs/openmrs-content-referenceapplication-demo/blob/ba5061d1b3fc7c5cbd7227c8793d312652d951ee/configuration/backend_configuration/locations/locations-core_demo.csv#L3

The problem seems to come from this column "Attribute|9eca4f4e-707f-4bb8-8289-2f9b6e93803c"; if you notice, the entire column is empty. Prior to your commit, the processor wouldn't complain about the fact that the referenced attribute ("9eca4f4e-707f-4bb8-8289-2f9b6e93803c") doesn't exist because of the filter!

To fix this error we should either create an attribute with UUID "9eca4f4e-707f-4bb8-8289-2f9b6e93803c" or remove such column(s).

[NethmiRodrigo]

Okay so I went back to a commit from 2021 and browsed the files and looks like even back then this attribute existed in the locations file but I can't find anything else with that attribute UUID, which means that the attribute type never existed for it in our attributetypes file. It does exist in the initializer example and is apparently the "Location ISO Code".
I can either remove this attribute from the locations file or add the attribute to the attributetypes file in the demo content package. What do you guys suggest, as I don't know if we would need it? @mseaton @samuelmale @dkayiwa @ibacher @jayasanka-sack @denniskigen @wikumChamith
@jayasanka-sack whatever decision we decide to do, after changing the content package, do I need to upgrade the version of it from 1.0.0-SNAPSHOT to something like 1.0.1-SNAPSHOT?

[wikumChamith]

Okay so I went back to a commit from 2021 and browsed the files and looks like even back then this attribute existed in the locations file but I can't find anything else with that attribute UUID, which means that the attribute type never existed for it in our attributetypes file. It does exist in the initializer example and is apparently the "Location ISO Code". I can either remove this attribute from the locations file or add the attribute to the attributetypes file in the demo content package. What do you guys suggest, as I don't know if we would need it? @mseaton @samuelmale @dkayiwa @ibacher @jayasanka-sack @denniskigen @wikumChamith @jayasanka-sack whatever decision we decide to do, after changing the content package, do I need to upgrade the version of it from 1.0.0-SNAPSHOT to something like 1.0.1-SNAPSHOT?

I think we should add that attribute to the content package. If we plan to use the snapshot we don't need to update the version.

[NethmiRodrigo]

@dkayiwa @wikumChamith I've triggered the workflows to re-run since the location attribute update PR got merged - openmrs/openmrs-content-referenceapplication-demo#2. Hopefully, that's one issue resolved

[wikumChamith]

We need to handle the Attribute|Last Audit Date too. What do you guys recommend?

An attribute value is specified ('') for an attribute type that cannot be resolved by the following identifier: 'Last Audit Date'

[NethmiRodrigo]

@dkayiwa @wikumChamith the changes I made to the workflow to save the server logs had gotten undone, so I committed it again

[NethmiRodrigo]

We need to handle the Attribute|Last Audit Date too. What do you guys recommend?
An attribute value is specified ('') for an attribute type that cannot be resolved by the following identifier: 'Last Audit Date'

@dkayiwa @wikumChamith I've opted to just remove this attribute altogether, because I don't know if I'm supposed to assign a random UUID to the attribute. If it fails, I'll just undo it

[NethmiRodrigo]

Adding the context here too, adding the attribute "9eca4f4e-707f-4bb8-8289-2f9b6e93803c" to the attribute type basically gave us a different error:
org.openmrs.api.ValidationException: 'Ubuntu Hospital' failed to validate with reason: activeAttributes: Location ISO Code is required.
And also an error that another attribute (Last Audit Date) didn't have the attribute definition, so we removed both from the locations file in the demo content package in this PR - openmrs/openmrs-content-referenceapplication-demo#3

[NethmiRodrigo]

Thanks @wikumChamith and @dkayiwa!