Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swap {$displayName} smarty for {contact.display_name} token #20867

Merged
merged 1 commit into from
Jul 26, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jul 16, 2021

Overview

Swap displayName smarty for {contact.display_name token}

This swaps over the smartyToken for the token token on the template added on new installs, and adds a test.

To test make sure you enable invoicing & then generate an invoice from search results.
use the demo site due to aforementioned lack of upgrade and check the display name is still on
the left hand side. There should be no change

Before

{$displayName} used in invoice template

After

{contact.display_name} used on new installs

Technical Details

Separately I will look at an upgrade script and other templates

Comments

@civibot
Copy link

civibot bot commented Jul 16, 2021

(Standard links)

@civibot civibot bot added the master label Jul 16, 2021
@eileenmcnaughton eileenmcnaughton changed the title Swap displayName smarty for {contact.display_name token} Swap {$displayName} smarty for {contact.display_name} token Jul 16, 2021
@@ -234,9 +234,9 @@
</tr>
<tr>
{if $organization_name}
<td style="padding-left:17px;"><font size="1" align="center">{$display_name} ({$organization_name})</font></td>
<td style="padding-left:17px;"><font size="1" align="center">{contact.display_name} ({$organization_name})</font></td>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't do organization name because in my testing it was only showing up for organizations not individuals which meant it was repeated - but I'll test that further

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tangential, but since you mention it - I use invoicing a lot, and never found why the organization name was repeated. if an invoice is "care of Some Person", then it would be in the address fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm - good point

@seamuslee001
Copy link
Contributor

Jenkins re test this please

eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 16, 2021
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 17, 2021
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 17, 2021
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 17, 2021
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 17, 2021
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 19, 2021
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 21, 2021
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 22, 2021
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 22, 2021
This swaps over the smartyToken for the token token on the template added on new installs, and adds a test.

Separately I will look at an upgrade script and other templates

To test make sure you enable invoicing & then generate an invoice from search results.
use the demo site due to aforementioned lack of upgrade and check the display name is still on
the left hand side
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 22, 2021
@mlutfy
Copy link
Member

mlutfy commented Jul 23, 2021

Tested on the demo & works for me. The extra test is nice.

Just curious: what motivated you to change it?

@mlutfy mlutfy added the merge ready PR will be merged after a few days if there are no objections label Jul 23, 2021
@eileenmcnaughton
Copy link
Contributor Author

@mlutfy it's really a preliminary to removing smarty tokens from other places where contact tokens would work - mostly because the code works pretty hard to get those tokens onto the form. In the case of this form it is one of very few places in the code that call the dreaded 'loadRelatedContributions' function and it is getting those contact fields from that function

Also loaded via that awful function is organization_name - as you mention it might be a bit odd how it is used and first_name & last name in the online contribution receipt function - although I think those were removed already from the template

@mlutfy
Copy link
Member

mlutfy commented Jul 26, 2021

@eileenmcnaughton Makes sense. Thank you!

@mlutfy mlutfy merged commit f688592 into civicrm:master Jul 26, 2021
@eileenmcnaughton eileenmcnaughton deleted the display_name branch July 26, 2021 13:49
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-test master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants