Use line items to look up memberships #20495
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(Standard links)
|
seamuslee001
reviewed
Jun 4, 2021
CRM/Contribute/BAO/Contribution.php
Outdated
| $membershipPayments = civicrm_api3('MembershipPayment', 'get', [ | ||
| 'return' => 'membership_id', | ||
| 'contribution_id' => (int) $contributionID, | ||
| 'contribution_id' => $contributionID, | ||
| 'membership_id' => ['NOT IN' => $membershipIDs], |
There was a problem hiding this comment.
I'm guessing it is complaining about this but not 100% sure.
|
@seamuslee001 yeah I didn't handle 'empty' |
|
I'll dig into these tests - not sure if they are real (pre-existing) bugs or just bad test set up |
This addresses a poor set up issue where the membership + contribution was being set up incorrectly & hence the line items were wrong, along with the ability to validate the financials. It was blocking civicrm#20495 along with the efforts to get financial validation on all tests
|
#20521 fixes the first one - checking the others
|
This still checks the membership payment but creates some noise if there are validity issues
Fix for civicrm#20495 - resolve invalid test config
|
OK - the fails seem to be entirely issues with the tests not creating things in a way that valid contribution membership orders are created |
eileenmcnaughton
commented
Jun 7, 2021
| @@ -145,7 +158,6 @@ public function testAddPaymentUsingCreditCardForPartiallyPaidContribution() { | |||
|
|
|||
| $mut->stop(); | |||
| $mut->clearMessages(); | |||
| $this->validateAllPayments(); | |||
There was a problem hiding this comment.
Not needed as now in tearDown for the whole class
|
Looks good to me merging |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Use line items to look up memberships
Before
Checks (deprecated) membership payments
After
Checks line items, still checks membership payments & adds them in as a deprecated temporary behaviour
Technical Details
This still checks the membership payment but creates some noise if there are validity issues
Comments
@seamuslee001 as suggested - note there are many tests you can step through this with - eg in the PaypalIPNTest class