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

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

Merged

Conversation

seamuslee001
Copy link
Contributor

Overview

This removes some excess ifs and re-works the order of the if logic as per #156764 this is a split off from #15651

Before

If logic complicated

After

If logic simplified

ping @eileenmcnaughton

@civibot
Copy link

civibot bot commented Oct 31, 2019

(Standard links)

@civibot civibot bot added the master label Oct 31, 2019
@@ -170,8 +163,14 @@
{$address}

{$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 ) }
Copy link
Contributor

Choose a reason for hiding this comment

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

This if can go

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe after the other is merged because I forsee a conflict below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@seamuslee001 seamuslee001 force-pushed the dev_core_1344_aditional_if branch from 0969c6c to d88185a Compare October 31, 2019 23:57
@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton i have now rebased this after #15651 has been merged

@eileenmcnaughton
Copy link
Contributor

Looks good to me!

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

{if ! ($contributeMode eq 'notify' OR $contributeMode eq 'directIPN') and $is_monetary}
{if $is_pay_later && !$isBillingAddressRequiredForPayLater}
Copy link
Contributor

Choose a reason for hiding this comment

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

Dang - this template is the one that needs a test change

@seamuslee001 seamuslee001 force-pushed the dev_core_1344_aditional_if branch from d88185a to 1a7a41e Compare November 1, 2019 03:34
@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton have pulled out the changes to that template

@seamuslee001 seamuslee001 force-pushed the dev_core_1344_aditional_if branch from 1a7a41e to 66af4b3 Compare November 1, 2019 03:35
@eileenmcnaughton
Copy link
Contributor

Cool getting close now - only one tpl left here & only one search jcalendar!

@seamuslee001
Copy link
Contributor Author

Test fail unrelated

@seamuslee001 seamuslee001 merged commit be95ef1 into civicrm:master Nov 1, 2019
@seamuslee001 seamuslee001 deleted the dev_core_1344_aditional_if branch November 1, 2019 04:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants