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] Remove transaction from BaseIPN completeTransaction call #18042

Merged
merged 1 commit into from
Aug 3, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Aug 2, 2020

Overview

[REF] Remove transaction from BaseIPN completeTransaction call

A grep looks like this - note the Contribution/Confirm class has a different function with the same name

Screen Shot 2020-08-03 at 9 42 19 AM

Before

public function completeTransaction(&$input, &$ids, &$objects, $transaction = NULL) {

After

public function completeTransaction(&$input, &$ids, &$objects) {

Technical Details

This is no longer ever passed in

Comments

@civibot
Copy link

civibot bot commented Aug 2, 2020

(Standard links)

@civibot civibot bot added the master label Aug 2, 2020
@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 this is now possible with those other PRs meaning it no longer reaches this function

@seamuslee001
Copy link
Contributor

@eileenmcnaughton I did a grep around the universe and the only one that turned up something interesting was sagepay https://github.com/circleinteractive/uk.co.circleinteractive.payment.sagepay/blob/master/custom/php/CRM/Core/Payment/Sagepay/IPN.php#L403

@seamuslee001
Copy link
Contributor

I think this is probably safe none the less and it seems the sagepay extension hasn't been updated for a while

@seamuslee001 seamuslee001 merged commit ebb02d1 into civicrm:master Aug 3, 2020
@seamuslee001 seamuslee001 deleted the transact branch August 3, 2020 00:15
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