Skip to content

[REF] Fully remove contribution object from repeattransaction function #19547

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

Merged
merged 1 commit into from
Feb 8, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Feb 5, 2021

Overview

[REF] Fully remove contribution object from repeattransaction function

Before

Object passed in but not used in any meaningful way

After

Poof

Technical Details

We can now see it is no longer used & remove it

This slightly grew in scope - with the overrides handling being tidied up & moved to getTemplateContribution because I hit a few test errors & the easiest way seemed to be to finish the cleanup

Comments

test cover on some of the tricksier things

testRepeatTransactionPassedInFinancialType
testRepeatTransactionPassedInCampaign
testRepeatTransactionUpdatedCampaign
testRepeatTransactionAlteredAmount

@civibot
Copy link

civibot bot commented Feb 5, 2021

(Standard links)

@civibot civibot bot added the master label Feb 5, 2021
@eileenmcnaughton eileenmcnaughton force-pushed the cont_obj branch 4 times, most recently from b04001d to 854e923 Compare February 7, 2021 01:25
@mattwire
Copy link
Contributor

mattwire commented Feb 7, 2021

Looks like test fails relate:

CRM_Contribute_Form_Contribution_ConfirmTest.testPayNowPayment
api_v3_ContributionTest.testRepeatTransactionWithCustomData

We can now see it is no longer used & remove it
@eileenmcnaughton
Copy link
Contributor Author

Yep - I can see what the error was that they caught - the 'id' field was leaking

@mattwire mattwire merged commit 75a5609 into civicrm:master Feb 8, 2021
@eileenmcnaughton eileenmcnaughton deleted the cont_obj branch February 8, 2021 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants