basically, the ID of the changeset should correspond to the date in the same way you did in the changelog name.
So, as an example, the file name of one of the changelog files:

it's 2024-02-22-1300
and the changesets within it are:

<changeSet author="kchelstowski" id="220220241300-0">
<changeSet author="kchelstowski" id="220220241300-1">

220220241300-0 -> 22-02-2024-1300 and the "0" is a number of the changeset, so the next one has 1, and the next should be 2, and so on.

why DDMMYYYYTTTT and not YYYMMDDTTTT like we have in the file name? Anyways, I'll fix it like you said.

I think there should be just one quote instead of three.

agreed, I originally had this on multiple lines (hence the triple quotes) but then condensed it to one so the triple quote isn't needed anymore

What is the default value at this moment? Doesn't it throw any error?

It defaults to the first item in the list. This is how we do it in party roles (which is where I got the idea from).

See how there's no X button for Party so you always have to select something. It can't be empty.

What will happen when there are already location types with code null? Shouldn't it be also fixed?

yeah I was thinking about whether or not we want to update all rows that have a code null to have some default code. There aren't any locations like this on obnav, obnavstage, or obdev3 so this will run fine there, but it might still be best to provide a default just in case. The reason I didn't was because I didn't want to silently change behaviour somewhere, but since we're considering location types without a code to be a bug, maybe it's fine. It just felt a little arbitrary picking one at random. Thoughts?

Just to update this thread, I changed the script to update any existing location types that don't have a code to use DEPOT. Again, this shouldn't affect any real data (there's no cases of this in prod) but doing it this way makes the script run without errors on any dev environments that happen to have bad data in them

I decided to just refactor this whole test class while I was here since it's really old.

I still need to figure out how to properly stub Grails' PagedResultList objects (it's really annoying they don't give you an easy way to do this) but I'm going to leave that as a problem for another day.

The Groovy DSL is a pain to work with in the IDE (requires package / doesn't recognize package, doesn't auto-complete, etc) so let's convert this to XML.

Remove any attributes where we aren't changing the default value. I saw this with one of Alan's earlier changelogs and was distracted by these attributes. Since the objectQuotingStrategy and onSqlOutput are just set to the default values let's remove them and use the default.

If you can convert using a dbm plugin command (as Dariusz suggested) please add a comment to the following ticket to document the process.

https://pihemr.atlassian.net/browse/OBS-1618

Convention: Also, start the changeset ID from 1 instead of 0 (180720241200-1). I usually reserve the 0 for when a changeset needs to be inserted before the others. However, the ordering of the IDs doesn't really matter as the changesets should be executed in the order they appear, so do whatever feels ok and make sure the IDs are unique. I just wanted to point out the convention.

Here's the script in case anyone wants to use it. I'll add this to the dbm/liquibase documentation ticket as well.

#!/bin/bash
OUTPUT=$(date +%s%N | cut -b1-13)
echo $OUTPUT
echo -n $OUTPUT | xclip -selection c

Copy to $HOME/bin.

[jmiranda@jmiranda-ThinkPad-W540 ~]$ ls -al /home/jmiranda/bin/generateChangeSetId.sh 
-rwxrwxr-x 1 jmiranda jmiranda 96 Apr 11  2013 /home/jmiranda/bin/generateChangeSetId.sh

NOTE: Holy $#!% I wrote this script over a decade ago. Let me know if there's a better (standard) way to do this (i.e. one improvement would probably to create a bash alias).

Execute bash script (make sure HOME/bin is in the PATH)

[jmiranda@jmiranda-ThinkPad-W540 ~]$ generateChangeSetId.sh 
1721751106382

This was the reason we didn't make the location type code required. We were initially using the location type name (?) to distinguish between the different location types (depot, dispensary, supplier). When I realized that was a bad idea, we added location type code (similar to document type code) to map the different location types to functionality within the system (i.e. default supported activities, where to display locations in dropdowns, etc).

At that time, we didn't have a suitable strategy for dealing with location types without a location type code so we left it NULL since it didn't hurt anything in the system (but forgot about the analytics schema). While I suspect these locations were added by people who didn't know what they were doing, deleting would not be a great idea.

I assumed if we ever got back to this, we would create a new location type code for the NULL case (i.e. NONE, DEFAULT, UNCLASSIFIED, UNCATEGORIZED, UNSPECIFIED, UNDESIGNATED). That way they don't impact the system from a functional standpoint (they would still be ignored), but there's a visual indication to the sysadmin to do something about them.

I'd prefer DEFAULT or UNCLASSIFIED, but would be open to something more location-y, if a synonym exists.

Yeah I'm realizing that I was forgetting about not PIH prod environments when I made this (I'm still not used to open source I guess). We don't have any location types with a null code in PIH prod so the default value here was mainly meant as a "just make the migration pass on dev environments" situation but we definitely don't want to muck around with whatever data anyone else has on their prod.

I like the idea of having some default. I'll think about how best to implement that then we can chat about it in the next tech huddle.