dev/core#1790 - Contact Card - Email Links

[reneolivo]

Overview

This PR adds a feature where emails on the contact's card and contact's search results can be clicked. When clicked the email form popup is displayed.

Before

Contact's Card

• Emails are not links
• Can't place the mouse inside the tooltip panel before it closes.

Contact's Search Results

After

Contact's Card

Contact's Search Results

Technical Details

We updated CRM/Contact/Form/Task/Email.php so we can pass an email parameter instead of the email_id parameter to prepopulate the email form. This is important since in some scenarios we do know the email address, but not the email id.

We also updated CRM/Profile/Page/Dynamic.php so we can display the email link on the contact's card. We added a getProfileFieldValue private method that checks the name of the profile field, if it contains the email keyword it will replace the raw email address value for a link to the email. In the future, this function can serve as a strategy handler to display different type of values depending on the field type.

We had to update the tooltip function in js/Common.js to add a small delay of 300ms when moving the mouse away from the triggering element. This allows the mouse to move to the contact's card in a reasonable time before it closes. As a note, we changed mouseout for mouseleave because the former is triggered multiple times while the mouse moves, and the later is only triggered once, which is enough to determine if the mouse is completely outside of the tooltip and its link.

Finally, we updated the templates/CRM/Contact/Form/Selector.tpl template so we include a link to the email form for both: normal contact search results, and advanced search results.

[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).

[eileenmcnaughton]

test this please

[eileenmcnaughton]

@colemanw can you check this one?

[yashodha]

test this please

[yashodha]

test this please

[tunbola]

test this please

[eileenmcnaughton]

This doesn't look good - if this change is affecting this test that is a red flag

[eileenmcnaughton]

@reneolivo I don't think you can change the AuthorizeNetTest - it looks like it's showing a real breakage as a result of your change :-(

[reneolivo]

@eileenmcnaughton thanks for the reply and sorry to come back a bit late about this.

The AuthorizeNetIPNTest change only checks the email response, it doesn't make any changes to the actual AuthorizeNetIPN class. The AuthorizeNetIPN class uses the UFGroup class indirectly because the later is used when composing contribution emails. The "stack trace" would be as follows:

1 - The tests runs the following code:

// tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php
$IPN = new CRM_Core_Payment_AuthorizeNetIPN($this->getRecurTransaction());
$IPN->main();

2 - Since these tests are setup to be recurring transactions the following code gets executed:

// CRM/Core/Payment/AuthorizeNetIPN.php
public function main(...) {
  // ...
  return $this->recur($input, $ids, $objects, $first);
  // ...
}

public function recur(...) {
  // ...
  $this->completeTransaction($input, $ids, $objects, $transaction, $recur);
  // ...
}

3 - The complete transaction is a method that is defined in the CRM_Core_Payment_BaseIPN class:

// CRM/Core/Payment/BaseIPN.php
public function completeTransaction(...) {
  // ...

  CRM_Contribute_BAO_Contribution::completeOrder($input, $ids, $objects, $transaction, $contribution);
}

4 - The complete order method in turn sends an email confirmation using the API interface:

// CRM/Contribute/BAO/Contribution.php
public static function completeOrder(...) {
  // ...
  civicrm_api3('Contribution', 'sendconfirmation', [
    'id' => $contribution->id,
    'payment_processor_id' => $paymentProcessorId,
  ]);
  // ...
}

5 - This sends a contribution email:

// api/v3/Contribution.php
function civicrm_api3_contribution_sendconfirmation($params) {
  // ...
  CRM_Contribute_BAO_Contribution::sendMail($input, $ids, $params['id'], $values);
}

6 - The email is compossed by the BAO Contribution class:

// CRM/Contribute/BAO/Contribution.php
public static function sendMail(...) {
  // ...
  $return = $contribution->composeMessageArray($input, $ids, $values, $returnMessageText);
  // ...
}

public function composeMessageArray(...) {
  // ...
  return CRM_Contribute_BAO_ContributionPage::sendMail($ids['contact'], $values, $isTest, $returnMessageText);
  // ...
}

7 - The CRM_Contribute_BAO_ContributionPage class composes the last few bits for the email and does the sending.
It also calls the CRM_Core_BAO_UFGroup::getValues method to use contact information to include in the email:

// CRM/Contribute/BAO/ContributionPage.php
public static function sendMail(...) {
  // ...
  list($values['customPost_grouptitle'], $values['customPost']) = self::getProfileNameAndFields($postID, $userID, $params['custom_post_id']);

  // ...
}

protected static function getProfileNameAndFields(...) {
  // ...
  CRM_Core_BAO_UFGroup::getValues($cid, $fields, $values, FALSE, $params);
  // ...
}

8 - CRM_Core_BAO_UFGroup::getValues is the method where we made the change.

The change we made is not related to payment at all, it's just that is used when composing contribution emails. This method is also used in several other places, it's just that the CRM_Core_Payment_AuthorizeNetIPN test was the only one able to detect the change.

I think it's safe to add this change to CRM_Core_BAO_UFGroup::getValues because some payment transactions also create activities, but changes to the activity creation should not block the payment processing.

Regarding having the change in CRM_Core_BAO_UFGroup::getValues this is the only place we can include it since this function doesn't return raw data, it returns data as it should be displayed in their final location. For example, we can't display phones in any other format other than the one returned by CRM_Core_BAO_UFGroup::getValues. This is very convenient, yes, but doesn't allow us to control how the data is going to be presented to the user.

In our case we want emails to be clickable on the Contact's card. We are not really interested in affecting other areas such as emails. But we can't simply update the contact's card because we need the Email's ID, which is not provided by CRM_Core_BAO_UFGroup::getValues.

[eileenmcnaughton]

@reneolivo the IPN test is checking to ensure the email is correctly assigned to the template - an unrelated test changing is kinda the definition of an unsafe change :-)

It looks like CRM_Core_BAO_UFGroup::getValues is called from 9 places - and CRM_Profile_Page_Dynamic is called in different contexts - we need to ensure that only this very specific context is altered - including ensuring that any front end presentations of this profile do not include the link. I realise this makes it harder - but sometimes things are hard because they are hard....

We can cut off some of the complexity by doing handling in CRM_Profile_Page_Dynamic -where we see

    if (strtolower($name) == 'summary_overlay') {
      $template->assign('overlayProfile', TRUE);
    }

but we still need to consider that profile could be displayed to non-permissioned users. I'm not sure if it could be used in other ways - .... perhaps @kcristiano or @colemanw has thoughts on that....

[reneolivo]

@eileenmcnaughton thank you for your feedback. Can you please check again?

We have changed the solution completely so it only affects the contact's card. We also added the same feature to the contact's search results, although the implementation is a bit different.

[eileenmcnaughton]

@reneolivo this would fail hard if that email exists more than once in the db -

[eileenmcnaughton]

@reneolivo I took a look to see if we could get this back on track - I think #18623 moves use beyond the thing this got blocked on (ie 'email' does not have a 1-1 match on email_id)

[eileenmcnaughton]

@reneolivo I'm going to close this & you can open a new PR for the remaining js issue - bonus - it will be at the top of the list not the bottom then!