CRM-21643 : Profile confirmation page shows the wrong values for multi-value custom data

[monishdeb]

Overview

Steps to replicate:

1. Create a custom field set that allows multiple records (for an Individual, say)
2. Create a field in this set
3. Create a new contact record with [email protected]
4. On this contact record, populate the custom field with an initial entry. Enter 'first data' or similar.
5. Create a profile with a) an email address field b) this custom field
6. Set the profile settings to 'update the matching contact'
7. Go to the profile list on Civi, click 'more' and go to 'Use - Create mode'
8. Enter '[email protected]' and 'second data', and submit the form
9. The confirmation page shows 'first data'.
   Expected: The custom field should show the last data entered i.e. 'second data'
   Before

---

After

---

• CRM-21643: Profile confirmation page shows the wrong values for multi-value custom data

[bhahumanists]

Tested on our system and working fine.

[eileenmcnaughton]

@lcdservices I know you use this functionality - do you have comment on this?

For background I know this has been discussed somewhat before with the issue being that it's often assumed but not intrinsic that the last value in a multi set is the 'most recent' & should be shown in this circumstance.

I think the code has been tested & the change is fine / limited to the fact that whenever multivalue records are retrieved the most recent rather than earliest will be grabbed. I'm a soft + 1 on that being more logical but think it would be good to get more comments

(@seamuslee001 @systopia @JoeMurray @Stoob @mlutfy @colemanw @guanhuan .....)

[lcdservices]

@eileenmcnaughton @monishdeb I ran through tests and it works fine. I think the assumption is ok. When in the context of a profile edit form, we don't return the user to the profile view -- we return them to the main profile edit form which displays a list of the multi-records. So the only context in which this applies for multi-record fields is when you are using the profile create form. When adding a new record I think we can assume the last record id is the one just created.

The only potential issue I could think of is if there's a race condition -- e.g. if multiple people are adding multi-records at the same time. That's probably a distant edge case (the second insertion would need to happen between when the first insertion is complete and the record returned, which is a minuscule gap of time).

[monishdeb]

What if to show all multi-values rather then the latest one i.e. if custom field support multivalue then use GROUP_CONCAT on its SELECT column of the search query. I think this fix itself doesn't solve the race condition but a close enough solution to show all the value on view page after save also optimise the search query to have less record due to GROUP BY.

[eileenmcnaughton]

@monishdeb @lcdservices I just looked at this again & I am OK with the change in the screen - but UFGroup::getValues is called from lots of places so this is not very contained - much as I hate to suggest it could we add a $isGetHighestID param to the function & handle that way?

[eileenmcnaughton]

@monishdeb just revisiting this - I think it would be safe if instead of the code in place we changed this

    $details = $query->searchQuery(0, 0, NULL, FALSE, FALSE,
      FALSE, FALSE, FALSE, $additionalWhereClause);

to

    $details = $query->searchQuery(0, 0, NULL, FALSE, FALSE,
      FALSE, FALSE, FALSE, $additionalWhereClause, 'id DESC');

It would need a unit test.

I'm going to close this because I think the discussion is useful & should be linked to but on revisiting I think it needs a test before it is review-ready & given it;s age it's probably better closed & re-opened

[eileenmcnaughton]

Hmm - looks like this duplicates anyway #12337