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

[REF] Complete order Unset contribution once we have finished with it. #19527

Merged
merged 1 commit into from
Feb 4, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

[REF] Complete order Unset contribution once we have finished with it.

Before

A whole lotta stuff is happening with the contribution object - but we can't see to what end

After

Checked whether it IS still used after repeatransaction & altered the one place that does use it, unset it to make that clear & avoid that checking work needing to be re-done

Technical Details

This patch does very little but it takes a bit of searching to check the handful of places
that call completeOrder and check that none of them use the contribution
object after this function call. With this merged it will be possible to
cleanup the references to the contribution in the repeattransaction
code safe in the knowledge that the object is not needed after that
function (or within it from my reading). It seems likely we can switch to
passing in contributionID rather than the whole object.

Comments

Unset contribution once we have finished with it.

This patch does very little but it takes a bit of searching to check the handful of places
that call completeOrder and check that none of them use the contribution
object after this function call. With this merged it will be possible to
cleanup the references to the contribution in the repeattransaction
code safe in the knowledge that the object is not needed after that
function (or within it from my reading). It seems likely we can switch to
passing in contributionID rather than the whole object.
@civibot
Copy link

civibot bot commented Feb 4, 2021

(Standard links)

@civibot civibot bot added the master label Feb 4, 2021
@mattwire
Copy link
Contributor

mattwire commented Feb 4, 2021

Agreed

@mattwire mattwire merged commit 69bb8c7 into civicrm:master Feb 4, 2021
@eileenmcnaughton eileenmcnaughton deleted the complete branch February 4, 2021 12:09
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