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

Separate loadRelatedObjects params in complete/repeattransaction #18253

Conversation

mattwire
Copy link
Contributor

Overview

Separate params being passed to loadRelatedObjects from params being passed through later functions.

Before

More difficult to disentangle.

After

Easier to disentangleify.

Technical Details

  • loadRelatedObjects accepts an input array and reads payment_processor_id and component from that array. For repeat/completetransaction we are not passing component.
  • $input is passed through to _ipn_process_transaction and completeOrder but payment_processor_id is never used (it's filtered out in the case of repeattransaction but not in completetransaction. But code inspection shows it's not used.

Comments

@eileenmcnaughton Further simplification.

@civibot
Copy link

civibot bot commented Aug 25, 2020

(Standard links)

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

eileenmcnaughton commented Aug 25, 2020

@mattwire per dm - do we even need to loadRelatedObjects anymore - we only use $contribution->_relatedObjects['paymentProcessor'] once - it's at the bottom of this snippet but I included the lines a little earlier because they relate

    $paymentProcessorId = '';
    if (isset($objects['paymentProcessor'])) {
      if (is_array($objects['paymentProcessor'])) {
        $paymentProcessorId = $objects['paymentProcessor']['id'];
      }
      else {
        $paymentProcessorId = $objects['paymentProcessor']->id;
      }
    }

....

    // 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'];
    }


(note I did #18257 on this theme)

@eileenmcnaughton
Copy link
Contributor

That gave me an idea - let's see what tests hit that line - #18259

@eileenmcnaughton
Copy link
Contributor

Nice - the line has test cover! #18259 (comment) - including from the api & BaseIPN class.

@mattwire
Copy link
Contributor Author

mattwire commented Sep 1, 2020

Superseded

@mattwire mattwire closed this Sep 1, 2020
@mattwire mattwire deleted the repeattransaction_loadrelatedobjectsparams branch September 1, 2020 22:24
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