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

Smarty modifier - stop using isset to check taxTerm #22323

Merged
merged 2 commits into from
Jan 6, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Dec 27, 2021

Overview

Smarty modifier - stop using isset to check taxTerm. This works by adding a new domain token (domain.tax_term) and exporting it as the smarty variable $taxTerm to the smarty layer.

Before

Use of isset - which doesn't work with default modifiers

After

$taxTerm is always assigned (as an empty string if none exists). In addition it is available as a tomain token.

Technical Details

Adding it as a domain token helps to keep our logic in the tokens layer and makes it avialable from non-workflow templates. Realistically people likely would write it in in letters but if being copied from one site to another or used in a multisite it would make sense.

Comments

@civibot
Copy link

civibot bot commented Dec 27, 2021

(Standard links)

@eileenmcnaughton
Copy link
Contributor Author

As expected a swag of test fails on notices (since I haven't added the assign yet)

api_v3_ContributionPageTest.testSubmitContributionPageWithPriceSetQuantity with data set #0
api_v3_ContributionPageTest.testSubmitContributionPageWithPriceSetQuantity with data set #1
api_v3_ContributionTest.testCompleteTransaction
api_v3_ContributionTest.testCompleteTransactionEuro
api_v3_ContributionTest.testPayLater
api_v3_ContributionTest.testBillingAddress
api_v3_ContributionTest.testCompleteTransactionFeeAmount
api_v3_ContributionTest.testRepeatTransaction
api_v3_ContributionTest.testRepeatTransactionWithCustomData
api_v3_ContributionTest.testRepeatTransactionLineItems
api_v3_ContributionTest.testRepeatTransactionIsTest
api_v3_ContributionTest.testRepeatTransactionPassedInStatus
api_v3_ContributionTest.testRepeatTransactionAcceptRecurID
api_v3_ContributionTest.testRepeatTransactionTestRecurId
api_v3_ContributionTest.testRepeatTransactionAlteredAmount
api_v3_ContributionTest.testRepeatTransactionPassedInFinancialType
api_v3_ContributionTest.testRepeatTransactionPassedInFinancialTypeTwoLineItems
api_v3_ContributionTest.testRepeatTransactionUpdatedFinancialType
api_v3_ContributionTest.testRepeatTransactionPassedInCampaign
api_v3_ContributionTest.testRepeatTransactionUpdatedCampaign
api_v3_ContributionTest.testRepeatTransactionUpdatedFinancialTypeAndNotEquals
api_v3_ContributionTest.testCompleteTransactionNetAmountOK
api_v3_ContributionTest.testCompleteTransactionWithReceiptDateSet
api_v3_ContributionTest.testCompleteTransactionForRecurring
api_v3_ContributionTest.testCompleteTransactionWithEmailReceiptInputTrue
api_v3_ContributionTest.testCompleteTransactionWithTestTemplate
api_v3_ContributionTest.testCompleteTransactionContributionPageFromAddress
api_v3_ContributionTest.testCompleteTransactionUpdatePledgePayment
api_v3_ContributionTest.testCompleteTransactionMembershipPriceSet
api_v3_ContributionTest.testCompleteTransactionMembershipPriceSetTwoTerms
api_v3_ContributionTest.testSendMail
api_v3_ContributionTest.testSendConfirmationPayLater
api_v3_ContributionTest.testRepeatTransactionWithNonCreditCardDefault
api_v3_ContributionTest.testSendMailWithAPISetFromDetails
api_v3_ContributionTest.testSendMailWithNoFromSetFallToDomain
api_v3_ContributionTest.testSendMailWithRepeatTransactionAPIFalltoDomain
api_v3_ContributionTest.testSendMailWithRepeatTransactionAPIFalltoContributionPage
api_v3_ContributionTest.testSendMailWithRepeatTransactionAPIFalltoSystemFromNoDefaultFrom
api_v3_ContributionTest.testRepeatTransactionWithDifferenceCurrency
api_v3_ContributionTest.testCheckTaxAmount with data set #0
api_v3_ContributionTest.testCheckTaxAmount with data set #1
api_v3_ContributionTest.testCompleteTransactionSetStatusToInProgress with data set #0
api_v3_ContributionTest.testCompleteTransactionSetStatusToInProgress with data set #1
api_v3_ContributionTest.testCompleteTransactionSetStatusToInProgress with data set #2
api_v3_ContributionTest.testCompleteTransactionSetStatusToInProgress with data set "receive_date_includes_time"
api_v3_JobTest.testMailReportForPrint
api_v3_JobTest.testMailReportForPdf
api_v3_JobTest.testMailReportForCsv

I'm thinking this should fail on some notices on CI - so will follow with the fix
@eileenmcnaughton eileenmcnaughton force-pushed the taxterm branch 2 times, most recently from 7f05fb6 to 2e9b470 Compare January 2, 2022 22:03
I do wonder about making this a domain token - I go back & forth a bit but as
a domain token it would be available in non-smarty contexts too & could be exported
@colemanw
Copy link
Member

colemanw commented Jan 6, 2022

This looks good. Merging per discusson with @seamuslee001

@colemanw colemanw merged commit 755ef42 into civicrm:master Jan 6, 2022
@colemanw colemanw deleted the taxterm branch January 6, 2022 23:01
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