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

ensure correct payment_instrument_id #11768

Merged
merged 1 commit into from
Mar 6, 2018

Conversation

jmcclelland
Copy link
Contributor

https://lab.civicrm.org/dev/core/issues/7

Otherwise, after first recurring contribution, contributions are coded
as check, not credit card.

Overview

Ensure the correct payment instrument is assigned to recurring authorize.net contributions (after the initial one).

Before

After the initial recurring contribution, subsequent contributions are coded as payment instrument check, instead of credit card.

After

Subsequent contributions are coded as payment instrument credit card.

Technical Details

I took the path of least resistance and I'm not sure it's the correct one. In particular I'm suspicious of the code in CRM/Contribute/BAO/Contribution.php on line 4571:

    // If paymentProcessor is not set then the payment_instrument_id would not be correct.
    // not clear when or if this would occur if you encounter this please fix here & add a unit test.
    if (empty($contributionParams['payment_instrument_id']) && isset($contribution->_relatedObjects['paymentProcessor']['payment_instrument_id'])) {
      $contributionParams['payment_instrument_id'] = $contribution->_relatedObjects['paymentProcessor']['payment_instrument_id'];
    }

Maybe this code should just check for $contribution->payment_instrument_id and use that value if present?

https://lab.civicrm.org/dev/core/issues/7

Otherwise, after first recurring contribution, contributions are coded
as check, not credit card.
@seamuslee001
Copy link
Contributor

ping @eileenmcnaughton

@eileenmcnaughton
Copy link
Contributor

It's not pretty but I think it makes sense & the test is adequate IMHO.

@eileenmcnaughton eileenmcnaughton merged commit 73499ef into civicrm:master Mar 6, 2018
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.

4 participants