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

Remove call to the dreaded replaceMultipleContributionTokens #21652

Merged
merged 2 commits into from
Sep 29, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Remove call to the dreaded replaceMultipleContributionTokens

Before

The horror the horror

After

Awful - but not calling legacy functions

Technical Details

This builds on #21524 to remove the last core usage of replaceContributionTokens

Comments

@demeritcowboy I worked up the courage to tackle it https://docs.civicrm.org/user/en/latest/contributions/manual-receipts-and-thank-yous/#grouped-contribution-thank-you-letters

The test I worked with is testGroupedThankYous but there is another in that class

@civibot
Copy link

civibot bot commented Sep 28, 2021

(Standard links)

@civibot civibot bot added the master label Sep 28, 2021
@eileenmcnaughton eileenmcnaughton force-pushed the cont_render_more branch 2 times, most recently from 2b093f3 to 5bcf707 Compare September 28, 2021 11:27
@demeritcowboy
Copy link
Contributor

Ok I take it this one supercedes #21524. Will take a look.

@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy it incorporates it - given it's mostly r-run testing might be better together. There is also a membership pdf tokens one - but that is probably a different testing process...

@demeritcowboy
Copy link
Contributor

Ok so a little part of the side of me that lives outside of civiworld is going to die with what I'm about to do with this PR, but I'm going to say merge-on-pass once the test is rebased which seems to be in conflict and not sure why github isn't showing that.

Jenkins retest this please

@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy also a little part of CRM_Utils_Token is going to die with that part of you...

@monishdeb
Copy link
Member

@demeritcowboy @eileenmcnaughton should I merge this PR based on tag? or needs to be tested again as there were some changes after the PR was marked with MoP?

@eileenmcnaughton
Copy link
Contributor Author

woo hoo - that is the last call to replaceContributionTokens gone

@eileenmcnaughton eileenmcnaughton merged commit 70fae04 into civicrm:master Sep 29, 2021
@eileenmcnaughton eileenmcnaughton deleted the cont_render_more branch September 29, 2021 04:04
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.

3 participants