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] Interim code cleanup - make the usage of addPayments clearer #16441

Merged
merged 1 commit into from
Feb 3, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Minor code cleanup - separates some narrow handling for Partially Paid from a main loop

Before

Not clear what addPayments does

After

The status check is called before addPayments - it is clearly specific to 'Partially paid'

Technical Details

The intention is to switch this to creating pending & adding payments per our preferred flow. This is just a step on that path - tests have been added here #16437

Comments

@civibot
Copy link

civibot bot commented Jan 31, 2020

(Standard links)

@civibot civibot bot added the master label Jan 31, 2020
@yashodha
Copy link
Contributor

@eileenmcnaughton Doc comment for parameter $contributions on line no 4830 does not match actual variable name $contribution

This brings the condition back to the calling function which clarifies what is going on with it
@seamuslee001
Copy link
Contributor

This looks straightforward to me, confirmed doing a grep this affects all the places it is called in the code and makes sense to me and tests have passed merging

@seamuslee001 seamuslee001 merged commit 67b7cfa into civicrm:master Feb 3, 2020
@seamuslee001 seamuslee001 deleted the event_pre branch February 3, 2020 20:58
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.

3 participants