modified shared address note under contact dashboard

[revati90]

Overview

While creating the share address for any contact , if we select contact who don't have address there is no way to jump on the selected contact and edit the address so that we can use that contact in for sharing the address.

Before

While creating a new contact and using shared address feature if selected contact don't have any address it will show the note like Selected contact does not have an address..... There is no such way to update selected contact address under Address tab.

After

Given patch gives us a link under shared address note itself to jump on selected contact to update contact address.

Comments

https://lab.civicrm.org/dev/core/issues/1351

[civicrm-builder]

Can one of the admins verify this patch?

[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]

add to whitelist

[eileenmcnaughton]

This looks good but I think we still need the word 'that'

[revati90]

This looks good but I think we still need the word 'that'

Please check updated PR.

[eileenmcnaughton]

I hate to say this but I think I've spotted another issue! 'That contact' is not translated

Ideally the link html should be paramterised - see
https://docs.civicrm.org/dev/en/latest/translation/

Also it looks like we usually use cmURL eg

<a href="{crmURL p='civicrm/contact/view' q="reset=1&cid=`$row.created_id`"}">{ts}{$row.created_by}{/ts}</a>

It's probably less at risk of breaking on different CMS

[revati90]

I hate to say this but I think I've spotted another issue! 'That contact' is not translated

Ideally the link html should be paramterised - see
https://docs.civicrm.org/dev/en/latest/translation/

Also it looks like we usually use cmURL eg

<a href="{crmURL p='civicrm/contact/view' q="reset=1&cid=`$row.created_id`"}">{ts}{$row.created_by}{/ts}</a>

It's probably less at risk of breaking on different CMS

Thanks @eileenmcnaughton ! I have updated PR with required changes. Can you please check once.

[eileenmcnaughton]

Looking good - I think you still need to close the first ts earlier & open a new one at the end - ie

{ts escape='js'}Selected contact does not have an address. Please edit{/ts} & then open another one in a bit

[mlutfy]

hmm, the mix of smarty and javascript from the original code makes this way more complicated to fix/improve than it should.

I'm not sure what's the right way to do this, without rewriting more of the original code to clean it up.

For example, it might be best to move this javascript into a real javascript file, and then we could use the ts javascript-based function.

The current PR mixes smarty and javascript to create URLs, and breaks translation. Sentences should not be split in different 'ts' calls, otherwise they will be impossible to translate accurately.

[mlutfy]

Suggestion from @demeritcowboy on the translation channel chat:

             addressHTML = {/literal}"{ts escape='js'}Selected contact does not have an address. Please click the following link to edit that contact to add an address, or select a different contact.{/ts}"{literal} + ' <a target="_blank" href="' + CRM.url('civicrm/contact/add', 'reset=1&action=update&cid=' + sharedContactId) + '">{/literal}{ts}Edit link{/ts}{literal}</a>';

"Edit Contact Details" might be better than "Edit link" (and already exists as a string in CiviCRM)

[eileenmcnaughton]

OH wow this good drawn out huh? @mlutfy is the final version good

[revati90]

OH wow this good drawn out huh? @mlutfy is the final version good

yeah. I agreed. Thanks @mlutfy and @eileenmcnaughton !

[eileenmcnaughton]

test fail not related but I'll run again to get a green tick since I'm hoping to get a last eyeball on this by @mlutfy

[eileenmcnaughton]

test this please

[mlutfy]

I tested this on a demo site and it looks good.

Only nitpick: could you please use "Edit Contact Details" instead of "Edit link"?

Current patch:

Proposed change would look like this:

I feel like "Edit link" would mean "Edit the link", which is misleading.

[seamuslee001]

Test fail is un related i'm going to merge this and then do a follow up PR for @mlutfy's improvement

[eileenmcnaughton]

Phew - thanks @revati90 @mlutfy @seamuslee001