dev/core#1790 - Contact Card - Email Links

[eileenmcnaughton]

Overview

Alternate to #17517 by @reneolivo

Before

After

Technical Details

The current PR stalled on the issue that passing 'email' to the email task could load up
the wrong contact. This gets around that by doing a separate (but hopefully quick) lookup
to grab the email id. I also made it dependent on the url having is_show_email_task=1 in
it. I went back & forwards on that vs is_backoffice

Comments

There are other things in #17517 regarding tool tip display & search tpls I have not attempted to cover here - this is just intended to address the one item that had become a blocker

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

[reneolivo]

Thanks a lot @eileenmcnaughton. Sorry about the ghosting. I think our timing didn't match since I had to leave on holidays sometime after sending the PR.

Is there anything else we can do to help this PR move along?

[eileenmcnaughton]

@reneolivo if you want to test that this does the part it's aiming to do I think I can get someone to merge it & then you can revisit what is left in your pr that still needs review

[reneolivo]

@eileenmcnaughton the code is not working as expected because it's harcoded to the primary email. And even for the primary email is not working. For some reason the primary email field name for me it's email-1 instead of email-Primary. This could be because we changed the fields in the contact's card profile.

Regardless, this solution doesn't work for all emails, just the primary one. It would be good if it supported all emails.

[reneolivo]

I would suggest to put each condition in its own line like this:

$emailID = Email::get()->setWhere([
  ['contact_id', '=', $this->_id],
  ['is_primary', '=', TRUE],
  ['on_hold', '=', FALSE],
  ['contact.is_deceased', '=', FALSE],
  ['contact.is_deleted', '=', FALSE],
  ['contact.do_not_email', '=', FALSE],
])->execute()->first()['id'];

[reneolivo]

But would remove the conditions about primary email and would search for the specific email address.

[reneolivo]

@eileenmcnaughton thanks for the work. I think if we make it work for all email fields we can cover the requirements.

In terms of the original PR, do we need to do anything else? The only thing we needed was to make the email addresses clickable, which this PR does in a different way. Do let me know the specific points that we need to work on so I can tackle them as soon as possible. Thanks a lot :)

[eileenmcnaughton]

@reneolivo I've updated it as you suggested - regarding the other PR - I'm not totally sure - this seemed to be the blocker so I picked it off - you can close it & open a new one if there are still outstanding bits

[eileenmcnaughton]

test this please

[reneolivo]

Thanks a lot @eileenmcnaughton ! It's working now. The only issue is that the popover element hides as soon as we try to hover over it, but I can update my PR with just the change so the popover sticks when the user hovers it, but still hides if they move out either of the popover, or the trigger element.

What do you think?

[eileenmcnaughton]

test this please

[eileenmcnaughton]

@reneolivo that makes sense - I would be looking to get @colemanw's input on the js change so a separate PR is good

@colemanw I think this is mergeable based on ^^ (the test fail was unrelated but I set it going again anyway)

[seamuslee001]

Merging as per review