dev/core#1344 Fix logic for displaying BillingName and Credit Card De… #15651

seamuslee001 · 2019-10-29T19:25:36Z

…tails in email Receipts

Overview

This continues the patterns set by #15532 and #15646

Before

Complex logic used to include BillingName and also credit card details

After

Less complex logic

ping @eileenmcnaughton @mattwire

civibot · 2019-10-29T19:25:39Z

(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 · 2019-10-29T20:29:35Z

Oh dang - I missed that you did this

eileenmcnaughton · 2019-10-29T20:31:59Z

xml/templates/message_templates/membership_online_receipt_text.tpl

 {if $is_pay_later}
 ===========================================================
 {ts}Registered Email{/ts}

 ===========================================================
 {$email}
-{elseif $amount GT 0 OR $membership_amount GT 0 }
+{elseif $billingName}
 ===========================================================


So I left this out to think about it more but as you see it is possible to have email AND billing name. I think we need to reverse it to be

{if billingName}

{elseif $email}

{/if}

(you might want to merge mine & rebase this & focus in on the small number of trickier ones)

@eileenmcnaughton that will still work in mine because note the elseif i believe is linked to the {if $is_pay_later} not the {if $email}

eileenmcnaughton · 2019-10-29T20:44:03Z

Hmm - I think mine will hit the same :-(

eileenmcnaughton · 2019-10-31T22:02:18Z

xml/templates/message_templates/contribution_online_receipt_html.tpl

@@ -311,7 +311,7 @@
      </tr>
     {/if}

-     {if ! ($contributeMode eq 'notify' OR $contributeMode eq 'directIPN') and $is_monetary}
+     {if ($is_pay_later || $billingName)}


If you remove this file's changes for later the tests won't fail & we can do the others

eileenmcnaughton · 2019-10-31T22:02:43Z

xml/templates/message_templates/contribution_online_receipt_text.tpl

@@ -126,14 +126,14 @@
 {/foreach}
 {/if}

-{if !( $contributeMode eq 'notify' OR $contributeMode eq 'directIPN' ) and $is_monetary}
+{if ($is_pay_later || $billingName)}


I think on this one we need to reverse the IF order

so we have 2 if statements here L129 an L130 and the elseif in L136 only relates to the if in L130 as far as i can tell

eileenmcnaughton · 2019-10-31T22:03:00Z

xml/templates/message_templates/event_offline_receipt_html.tpl

@@ -374,7 +374,7 @@
        </tr>
       {/if}

-       {if $contributeMode ne 'notify' and !$isAmountzero and !$is_pay_later and !$isOnWaitlist and !$isRequireApproval}
+       {if $billingName}


Yay - straight forward - we can get this merged easily

eileenmcnaughton · 2019-10-31T22:03:09Z

xml/templates/message_templates/event_offline_receipt_text.tpl

@@ -192,7 +192,7 @@
 {if $checkNumber}
 {ts}Check Number{/ts}: {$checkNumber}
 {/if}
-{if $contributeMode ne 'notify' and !$isAmountzero and !$is_pay_later and !$isOnWaitlist and !$isRequireApproval}
+{if $billingName}


straightforward

eileenmcnaughton · 2019-10-31T22:03:14Z

xml/templates/message_templates/membership_offline_receipt_html.tpl

@@ -222,7 +222,7 @@
    <td>
     <table style="border: 1px solid #999; margin: 1em 0em 1em; border-collapse: collapse; width:100%;">

-      {if $contributeMode ne 'notify' and !$isAmountzero and !$is_pay_later }
+      {if $billingName}


eileenmcnaughton · 2019-10-31T22:03:21Z

xml/templates/message_templates/membership_offline_receipt_text.tpl

@@ -79,7 +79,7 @@
 {/if}

 {if $isPrimary }
-{if $contributeMode ne 'notify' and !$isAmountzero and !$is_pay_later  }
+{if $billingName}


eileenmcnaughton · 2019-10-31T22:03:33Z

xml/templates/message_templates/membership_online_receipt_text.tpl

@@ -154,14 +154,14 @@
 {/foreach}
 {/if}

-{if !( $contributeMode eq 'notify' OR $contributeMode eq 'directIPN' ) and $is_monetary}
+{if ($is_pay_later || $billingName)}


we need to reverse the if order on this one

why would that help with this if tho, this is the if saying are we printing either the Registered Email or Billing Name sections. Then we do or are you suggesting we completely get rid of the outer if all together?

Right - the outer if is not required we want

{if $billingName}
.... show billing name & email
{elseif $email}
... show email
{/if}

I actually already did this on the same receipt but the html version

eileenmcnaughton · 2019-10-31T22:03:43Z

xml/templates/message_templates/membership_online_receipt_text.tpl

@@ -172,7 +172,7 @@
 {$email}
 {/if} {* End ! is_pay_later condition. *}
 {/if}
-{if $contributeMode eq 'direct' AND !$is_pay_later AND ( $amount GT 0 OR $membership_amount GT 0 ) }
+{if $credit_card_type}


eileenmcnaughton · 2019-10-31T22:04:37Z

@seamuslee001 3 patterns here

specific tested template requiring test changes
billingName + email combos
straight forward

Maybe pull 1 & 2 out & we'll get all the 3s merged first?

…ections as agreed in civicrm#15651

…tails in email Receipts

seamuslee001 · 2019-11-01T04:56:59Z

@eileenmcnaughton rebased after latest template fixes, It will need to be re-run after we merge your change.

eileenmcnaughton · 2019-11-01T08:29:31Z

test this please

…ections as agreed in civicrm#15651

mattwire · 2019-11-01T11:36:15Z

Tests passed - merging per tag

…ections as agreed in civicrm#15651

civibot bot added the master label Oct 29, 2019

eileenmcnaughton reviewed Oct 29, 2019

View reviewed changes

seamuslee001 force-pushed the dev_core_1344 branch 2 times, most recently from 05fc34f to b2439ec Compare October 30, 2019 21:38

eileenmcnaughton mentioned this pull request Oct 31, 2019

Simplify billingName logic on xml/templates/message_templates/members… #15674

Merged

seamuslee001 force-pushed the dev_core_1344 branch from b2439ec to 4efb011 Compare October 31, 2019 21:57

eileenmcnaughton reviewed Oct 31, 2019

View reviewed changes

seamuslee001 added a commit to seamuslee001/civicrm-core that referenced this pull request Oct 31, 2019

dev/core#1344 Fix complicated logic on credit card and billing name s…

c6c66ed

…ections as agreed in civicrm#15651

This was referenced Oct 31, 2019

dev/core#1344 Fix complicated logic on credit card and billing name s… #15680

Merged

dev/core#1344 Re-work if statements and remove excess ifs #15682

Merged

dev/core#1344 Fix logic for displaying BillingName and Credit Card De…

1016611

…tails in email Receipts

seamuslee001 force-pushed the dev_core_1344 branch from 4efb011 to 1016611 Compare November 1, 2019 04:50

eileenmcnaughton added the merge on pass label Nov 1, 2019

magnolia61 pushed a commit to magnolia61/civicrm-core that referenced this pull request Nov 1, 2019

dev/core#1344 Fix complicated logic on credit card and billing name s…

0d9bc8e

…ections as agreed in civicrm#15651

mattwire merged commit 5dced02 into civicrm:master Nov 1, 2019

seamuslee001 deleted the dev_core_1344 branch November 1, 2019 19:59

eileenmcnaughton pushed a commit to eileenmcnaughton/civicrm-core that referenced this pull request Nov 2, 2019

dev/core#1344 Fix complicated logic on credit card and billing name s…

ba94095

…ections as agreed in civicrm#15651

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dev/core#1344 Fix logic for displaying BillingName and Credit Card De… #15651

dev/core#1344 Fix logic for displaying BillingName and Credit Card De… #15651

seamuslee001 commented Oct 29, 2019

civibot bot commented Oct 29, 2019

eileenmcnaughton commented Oct 29, 2019

eileenmcnaughton Oct 29, 2019

seamuslee001 Oct 29, 2019

eileenmcnaughton commented Oct 29, 2019

eileenmcnaughton Oct 31, 2019

eileenmcnaughton Oct 31, 2019

seamuslee001 Oct 31, 2019

eileenmcnaughton Oct 31, 2019

eileenmcnaughton Oct 31, 2019

eileenmcnaughton Oct 31, 2019

eileenmcnaughton Oct 31, 2019

eileenmcnaughton Oct 31, 2019

seamuslee001 Oct 31, 2019

eileenmcnaughton Oct 31, 2019

eileenmcnaughton Oct 31, 2019

eileenmcnaughton commented Oct 31, 2019

seamuslee001 commented Nov 1, 2019

eileenmcnaughton commented Nov 1, 2019

mattwire commented Nov 1, 2019

dev/core#1344 Fix logic for displaying BillingName and Credit Card De… #15651

dev/core#1344 Fix logic for displaying BillingName and Credit Card De… #15651

Conversation

seamuslee001 commented Oct 29, 2019

Overview

Before

After

civibot bot commented Oct 29, 2019

eileenmcnaughton commented Oct 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eileenmcnaughton commented Oct 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eileenmcnaughton commented Oct 31, 2019

seamuslee001 commented Nov 1, 2019

eileenmcnaughton commented Nov 1, 2019

mattwire commented Nov 1, 2019