Allow multiple values with same location without swapping primary keys

[beto-aveiga]

Overview

Webform CiviCRM does not support multiple fields with same location type, for example: 3 emails of location type personal.

This lack of support for multiple fields also creates related issues as the swapping of primary keys on save, and even data corruption (only if webform is configured with multiple fields with same location)

The objective of this task is to enhance webform_civicrm capabilities to allow multiple fields with any location types (repeated or not) work as expected in the same webform, without swapping primary keys unexpectedly between the configured fields and saving all values in a predictable way.

Before

When saving multiple fields with the same location type the last value that changes will be saved as the primary field.

All fields are same location type. Notice the order of values before and after submit.

After

When saving multiple fields with the same location type the first value will remain as the primary field, no matter if it changes or not.

All fields are same location type. Notice the order of values before and after submit.

[mattwire]

@powdevel #147 has now been merged.

To reproduce the error saving updates:

1. Enable phone type custom fields (via api explorer or other).

$result = civicrm_api3('OptionValue', 'create', [
  'option_group_id' => "cg_extend_objects",
  'name' => "Phone",
  'label' => "Phone",
  'value' => "civicrm_phone",
]);

2. Add a custom field of type phone.
3. Add that custom field to a webform.
4. On first submit of webform (ie. phone is created, all values are saved).
5. On second submit of webform (with same phone), only the "phone" parameter is updated.

With the patch in #147 all submitted values are updated.

[beto-aveiga]

@mattwire thanks for your explanation. I'm going to test that outside of my work schedule. Right now I have to document the PRs I did and create patches for 7.x-4.20 because that is for today soon :)

[beto-aveiga]

@mattwire I already documented the PR, sorry for the late. Hope it is clear now.

[colemanw]

@mattwire it would be great if you could test this out.

[beto-aveiga]

@colemanw I will, once I finish a few tasks I will test this PR. Thanks.

[beto-aveiga]

@KarinG @colemanw I merged locally #146 and #148 into the master of this repo. I made several tests and everything worked just fine.
I tested with multiple emails, multiple addresses, and multiple phones. Also tested with multiple fields with location types selectable by the user, and the results were the the expected results.
If there is something else to do please tell me, I need the both mentioned branches get merged as soon as possible to incorporate those changes in a patched version of webform_civicrm, for CiviHR and probably CiviCRM.
Thanks!

[beto-aveiga]

@KarinG @colemanw is there anything else from my side so this PR could be merge? I'm just trying to be sure if you are waiting something from me. Thanks.

[jamienovick]

@colemanw - whilst we are fixing things could we try and close this one off? If it need modificaiton let me know and I'll ask @vinuvarshith to look at it.

[colemanw]

This could be written more concisely as

Suggested change

	$is_primary_location_type = isset($contact[$location][1]['location_type_id']) ? $contact[$location][1]['location_type_id'] : null;
	$is_primary_location_type = $contact[$location][1]['location_type_id'] ?? NULL;

[vinuvarshith]

@colemanw I made this change and it works fine.

But just to note, the is only available on PHP 7.x and I guess its ok for 7.x-5.x branch.

[colemanw]

@jamienovick if you could ask @vinuvarshith to grab this patch and make a new PR against the 7.x-5.x branch & then test it out that would be great. I've made 1 code-level suggestion but haven't tested it myself.

[jamienovick]

@colemanw will do!