Upgrade script for dev/core#1046

[demeritcowboy]

Overview

While the upgrade doesn't require this for day-to-day case operations to keep functioning as-is, converting the labels for the case roles listed in the xml to their name equivalents will prevent some future issues, and is required to make it fully functional when people make future configuration changes.

There are some edge cases where it's not possible to determine the correct replacement. These would normally show up in the status checks that are now present, but they might not see those for a while, so this includes pre-upgrade messages to list these warnings before the upgrade. These are hopefully pretty rare. They are:

• Role listed in the xml that matches two different relationship type's labels.
• Role listed in the xml that appears to be bidirectional label-wise but the machine names are different.
• Role listed in the xml that doesn't exist as any relationship type label.

Before

Possible future issues.

After

Possible future issues averted.

Technical Details

The tests I believe include at least one example of all the possible situations, and also tests non-ascii characters to simulate non-English.

Comments

This should be the last thing related to https://lab.civicrm.org/dev/core/issues/1046

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[demeritcowboy]

Rebased to resolve upgrade file conflict from yesterday's merges.

[colemanw]

@agh1 is this something you could test out?

[demeritcowboy]

If it helps the steps for the simplest test would be:

1. Install 5.18 or 5.19.
2. Enable civicase.
3. Create a new case type so that you'll have one that uses database storage and add e.g. Benefits Specialist is as a role.
4. Change the label for the Benefits Specialist relationship type to Changed Benefits Specialist.
5. Go back to edit the case type and since you haven't upgraded yet you'll see a blank row. Remove the row and add Changed Benefits Specialist.
   1. You'll get an error but pretend you ignore popups.
   2. Click cancel and then go back in to edit and it shows it saved anyway so just shake your head and continue on with your day.
   3. You can check in the database or use api explorer to confirm the xml says Changed Benefits Specialist.
6. Do an upgrade to 5.20/master with this patch.
7. Check in the database or use api explorer to see that the <name> in the xml for that relationship type now has the name.

If looking at the unit test, it looks like a lot of code but note that the only parts that are different in the input and expected xml are the <CaseRoles> <name> elements. And then most of the rest of the php code in both the upgrade and unit test are about warning about the edge cases, which they will eventually also see on the regular admin status checks screen.

[eileenmcnaughton]

@agh1 @demeritcowboy as the last piece of a chunk of work you have both collaborated on this needs to be in the same release (aka 5.20).

I'm going to merge this now as it looks sensible, tested & in accordance with lengthy previous discussions & analysis.

However, can you both please commit to doing a final round of testing on the rc on this upgrade & related changes?

[demeritcowboy]

New phone who dis?
Kidding! I can commit to more testing around 5.20 release time.
Thanks!

[eileenmcnaughton]

it's the squirrels....

[alifrumin]

@agh1 and I ran some tests today and everything looked good to us. See specifics below:

1 - Created a site running civi version 5.13.5 (this is the version DaveD uses in his table)
2 - Created the following relationship types:

name_a_b	name_b_a	label_a_b	label_b_a	contact type a	contact type b	Scenario
A1	A2	A2	A1	Organization	Individual	Switched labels and names different contact type
B1	B2	B2	B1	Individual	Individual	Switched same contact type
C1	C2	C1	C1			bi directional labels unidirectional names
D1	D1	D1	D2			names are the same labels are different
E1	E2	E3	E4			normal
F1	F2	E1	E2			labels match names of other type

3 - Created 2 case types:
Case type alpha: created in 5.13, using our new relationship types that test weird label/name scenarios
Roles: A1 B1 C1 D2 E4 E2

AND

Case type 1046 which matches the case type described by Dave in the gitlab issue:
Roles: Spouse of, Case Coordinator is, Benefits Specialist

4 - Used the timeline tab to set up some auto assignees
5 - UPGRADED to 5.18
6 - Created two new case types (now one can create case roles using relationships in either direction, so we test both directions):

Case type beta - created in 5.18
Roles: A2 B2 C1 D1 E3 E1

AND

Case type gamma - created in 5.18
Roles: A1 B1 C1 D2 E4 E2

7 - Went to the upgrade screen to upgrade to 5.20 got warnings about the role C1 having ambiguous configuration

8 - UPGRADED to 5.20 Result:

The upgrade applied cleanly, a warning was thrown for the scenario where the upgrade script could not figure out what to do, reasonable assumptions were made for the rest. No fatal errors, nothing broke.

[demeritcowboy]

Thanks @alifrumin!
P.S. Your "add role" dropdown is huge - what are you hiding in there?

[eileenmcnaughton]

@alifrumin yay - thanks for checking

[demeritcowboy]

I did some more upgrade testing specifically with external xml files. I didn't see any blockers, but noting:

• If you forget to reinstate your xml files after replacing the old civicrm tree you get a PHP warning about caseRoles because of the new status checks. Prior to the status checks, forgetting to copy over your files wouldn't surface until you go to create a case and then you'd get a fatal error. So it's not an error caused by the changes per se, it's just now surfacing earlier as a warning instead of a fatal, and it's because you forgot to copy the files. So I'm not proposing to do anything. I suppose there could be a permanent preupgrade check that says "hey you have case types defined with no definitions and I can't find the files - did you forget to reinstate them?", but if forgetting to copy the files was a big issue it would have come up by now.
• I should probably do a blog post to tell people using xml files about what it means if they want to make future configuration changes for relationship type labels, post-upgrade. And/or a separate extension/status check that analyzes the files and if they have different names and labels it says "hey you might want to change these parts eventually".