PER-341 Configure locations

[aastupilloses]

https://pihemr.atlassian.net/browse/PER-341

[aastupilloses]

As far as I can see, the locations configuration is working properly. It was kind of tricky to make it work on initial setup, but now it's fine. Waiting for review in order to merge it into the master branch.

[brandones]

Awesome, great work!

[mogoodrich]

@brandones why are we using both the "Tag|[Tag Name]" and "Tags" columns here? It looks like there is redundancy?

Otherwise looks good!

[brandones]

Oops! Good eye, @mogoodrich . Yeah, @aastupilloses , please don't use the Tags domain. Instead, add Tag|xyz columns for each Location Tag type you want to manage. If the Location Tag is one that isn't already in the EMR, please add them in a LocationTags CSV.

[aastupilloses]

I have removed the Tag|XYZ columns due to the mentioned redundancy and because I was not able to use that format for all the tags declared on Tags column because after trying to do that I was getting an error when running the OpenMRS server that was giving me no clue about what was wrong. I'll request you to review this again. Thanks!

[brandones]

Oh, interesting.

@aastupilloses , could you share the erroring server logs and the CSV that produced it (the one with Tag|XYZ columns)? This might be a bug.

The reason we prefer Tag|XYZ to Tags is that Tags magically creates location tags as needed, meaning that it can't provide any validation of those location tags. I expect that the problem is that at least one of the location tags you're trying to use isn't part of PIH EMR, and so will be useless to you (I think— @mogoodrich ?).

[mogoodrich]

@brandones I think you are correct. Here's the list of tags that installed by default within the PIH EMR:

https://github.com/PIH/openmrs-module-pihcore/blob/master/api/src/main/java/org/openmrs/module/pihcore/metadata/core/LocationTags.java

It looks like the Facility Location tag is not defined by the PIH EMR (and maybe others, but I noticed that one)...

So if you want to add new location tags you will need to create a Location Tags CSV (see link Brandon provided above)... and then the "Tag|XYZ" format should work.

This model/error is actually intentional... as Brandon said, using the "Tags" column magically creates the tags for you, which is nice, but there's no validation, so if for some reason you misspell "Facity" or something in one of the rows it will create a duplicate erroneous tag silently.

We want to explicitly define each tag we want to use so we can validate there are no errors.

That being said, if this is what was happening in your case, if the error wasn't clear then validation isn't particularly helpful! Would be great to see the error you got.

The location tag stuff was recently refactored, so you are getting to be one of the first to test it out, so thanks for helping us out and apologies for any of the difficulties... :)

[brandones]

That being said, if this is what was happening in your case, if the error wasn't clear then validation isn't particularly helpful! Would be great to see the error you got.

Yes please

[aastupilloses]

Hi, @brandones. Actually, the error message was a very generic one. Something related to a not found view, which lead me to think that it was not the CSV file parsing/formatting exception message, but the message of an exception caused due to not being able to build the locations configuration properly before. I also checked the stacktrace but I didn't find something useful at first sight.

But, considering what @mogoodrich says, I will use the provided Java file to check which tags are not bundled with the base OpenMRS configuration and after that I would add them to the Location Tags CSV file and try again.

[mogoodrich]

Thanks @aastupilloses ... yeah, there was likely something buried in the stack trace, unfortunately... @brandones might be worth checking what the code does.

[brandones]

@aastupilloses Yeah, the stack trace shown in the browser is often useless. If there's something valuable in it, it's usually toward one of the bottom few exceptions shown. More likely the useful error would be in the server log. The server log is the thing to keep an eye on. Your team is going to get very good at reading it.

[aastupilloses]

Thank you for your support, @brandones. This settings file is working as expected on my local instance. Waiting for your approval or comments.

[brandones]

@aastupilloses I don't think we use "Facility Location" for anything. You can get rid of that one if you like.

Great work!

[brandones]

I am assuming, based on your testing, that the locations inherit their location tag permissions from the parent location "COR," and that you can therefore do things like view patients while logged in at them.

[mogoodrich]

Strange, as the build went through, I would have thought this should be up on peru-ci.pih-emr.org by now? Am I missing something? If not, let me know, and I could research more tomorrow...