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

dev/financial#152 Pass contribution directly to completeOrder #18728

Merged
merged 1 commit into from
Oct 11, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Oct 10, 2020

Overview

Simplify function signature on internal completeOrder function

This addresses item 2 on https://lab.civicrm.org/dev/financial/-/issues/152

Before

 public static function completeOrder($input, $ids, $contribution, $isPostPaymentCreate = FALSE) {
    $contribution = $objects['contribution'];
    // Unset objects just to make it clear it's not used again.
    unset($objects);

After

public static function completeOrder($input, $ids, $contribution, $isPostPaymentCreate = FALSE) {

Technical Details

We have done quite a a bit of cleanup on this and the only value in objects now used is
contribution - this is really explicit in the code as we actually unset objects after
extracting contribution.

This alters the function signature such that it receives contribution directly rather than
in an array

Comments

These are the only places it is accessed from

Screen Shot 2020-10-11 at 8 48 08 AM

All have test cover

@mattwire

We have done quite a a bit of cleanup on this and the only value in objects now used is
contribution - this is really explict in the code as we actually unset objects after
extracting contribution.

This alters the function signature such that it receives contribution directly rather than
in an array
@civibot
Copy link

civibot bot commented Oct 10, 2020

(Standard links)

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 do you want to MOP this too - it's straight forward & will permit further cleanup

@mattwire
Copy link
Contributor

Yes, we already discussed - merging.

@mattwire mattwire merged commit 97c29aa into civicrm:master Oct 11, 2020
@eileenmcnaughton eileenmcnaughton deleted the aip branch October 11, 2020 20:05
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