Ensure contacts without a name are updated when primary email changes #20403
Conversation
|
(Standard links)
|
Contacts with no name use their primary email as display_name and sort_name. This ensures that when their primary email is updated, display_name and sort_name will be updated as well. Adds tests for APIv3, APIv4 & Afform. Fixes dev/core#2622
78334e4 to
da5e820
Compare
|
Concept sounds good... |
| if (!$this->contactHasName) { | ||
| foreach ($params['email'] as $email) { | ||
| if ($email['is_primary']) { | ||
| CRM_Core_DAO::setFieldValue('CRM_Contact_DAO_Contact', $this->_contactId, 'display_name', $email['email']); |
There was a problem hiding this comment.
ohh nasty - set from the form!
| @@ -348,7 +348,7 @@ public static function handlePrimary(&$params, $class) { | |||
| $entity = new $class(); | |||
| $entity->id = $params['id']; | |||
| $entity->find(TRUE); | |||
| $contactId = $entity->contact_id; | |||
| $contactId = $params['contact_id'] = $entity->contact_id; | |||
There was a problem hiding this comment.
OK - this line is only hit if $params['contact_id'] is not set
| @@ -71,6 +71,12 @@ public static function create($params) { | |||
|
|
|||
| $email->save(); | |||
|
|
|||
| $contactId = (int) ($email->contact_id ?? CRM_Core_DAO::getFieldValue(__CLASS__, $email->id, 'contact_id')); | |||
| if ($contactId && $email->is_primary) { | |||
There was a problem hiding this comment.
If it is changed from primary to not primary no update will happen but I think that is ok because what is set in place of this will trigger a change
| $cid = (int) ($contact['id'] ?? NULL); | ||
| $contactType = $contact['contact_type'] ?? NULL; | ||
| if (!$contactType && $cid) { | ||
| $contactType = CRM_Core_DAO::getFieldValue(__CLASS__, $cid, 'contact_type'); |
There was a problem hiding this comment.
So we can get this all down to one extra query if we retrieve the potential name fields at this point too. It's a bit messier php-wise but I'm very conscious that this is a high traffic function
There was a problem hiding this comment.
Except that CRM_Core_DAO::getFieldValue uses caching so if we don't use it we don't get the potential benefit of the cache.
I guess I could write a new getFieldValues() (plural) function which re-uses the same cache...
There was a problem hiding this comment.
Yeah - I don't think we use getFieldValue enough to actually have it cached much? I feel like we have been mostly using the api (increasingly) to retrieve stuff like that. However, I guess it's arguable enough now that I won't hold this up on it
|
@colemanw this looks nice & clean and the test cover is good. My biggest reservation has always been that this is a high-volume function. We can have this add only one extra query rather than 2 with a little more mess & I think that is worth doing. (We could really do with some sort of lazy load object but we''ll punt that a couple more years down the track) |
Overview
Contacts with no name use their primary email as
display_nameandsort_name.This ensures that when their primary email is updated,
display_nameandsort_namewill be updated as well.Fixes https://lab.civicrm.org/dev/core/-/issues/2622
Before
Updating primary email would only update a contact's display/sort name if done through the UI.
After
Works in APIv3, APIv4 & Afform, with tests.
Technical Details
In some circumstances this may cause an extra query or two, but generally this stuff is cached so it shouldn't be a big problem.