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/membership#21 fix bug where inherited memberships get inappropriately deleted #16139

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 0 additions & 4 deletions CRM/Contribute/BAO/Contribution.php
Expand Up @@ -5401,10 +5401,6 @@ public static function updateMembershipBasedOnCompletionOfContribution($contribu
$membershipParams['is_override'] = FALSE;
$membershipParams['status_override_end_date'] = 'null';

//CRM-17723 - reset static $relatedContactIds array()
// @todo move it to Civi Statics.
$var = TRUE;
CRM_Member_BAO_Membership::createRelatedMemberships($var, $var, TRUE);
civicrm_api3('Membership', 'create', $membershipParams);
}
}
Expand Down
18 changes: 4 additions & 14 deletions CRM/Member/BAO/Membership.php
Expand Up @@ -1324,33 +1324,22 @@ public static function sortName($id) {
* @param CRM_Core_DAO $dao
* Membership object.
*
* @param bool $reset
*
* @return array|null
* Membership details, if created.
*
* @throws \CRM_Core_Exception
* @throws \CiviCRM_API3_Exception
*/
public static function createRelatedMemberships(&$params, &$dao, $reset = FALSE) {
// CRM-4213 check for loops, using static variable to record contacts already processed.
if (!isset(\Civi::$statics[__CLASS__]['related_contacts'])) {
\Civi::$statics[__CLASS__]['related_contacts'] = [];
}
if ($reset) {
// CRM-17723.
unset(\Civi::$statics[__CLASS__]['related_contacts']);
return FALSE;
}
$relatedContactIds = &\Civi::$statics[__CLASS__]['related_contacts'];
public static function createRelatedMemberships(&$params, &$dao) {
$relatedContactIds = [];

$membership = new CRM_Member_DAO_Membership();
$membership->id = $dao->id;

// required since create method doesn't return all the
// parameters in the returned membership object
if (!$membership->find(TRUE)) {
return;
return NULL;
}
$deceasedStatusId = array_search('Deceased', CRM_Member_PseudoConstant::membershipStatus());
// FIXME : While updating/ renewing the
Expand Down Expand Up @@ -1404,6 +1393,7 @@ public static function createRelatedMemberships(&$params, &$dao, $reset = FALSE)

//lets cleanup related membership if any.
if (empty($relatedContacts)) {
// This is deeply truely madly wrong - refer to function name....
self::deleteRelatedMemberships($membership->id);
}
else {
Expand Down
41 changes: 20 additions & 21 deletions tests/phpunit/CRM/Member/Form/MembershipTest.php
Expand Up @@ -1038,57 +1038,56 @@ public function testTwoInheritedMembershipsViaPriceSetInBackend() {
$this->createTwoMembershipsViaPriceSetInBackEnd($orgID);

// Check the primary memberships on the organization.
$orgMembershipResult = $this->callAPISuccess('membership', 'get', [
'contact_id' => $orgID,
]);
$this->assertEquals(2, $orgMembershipResult['count'], "2 primary memberships should have been created on the organization.");
$orgMembershipResult = $this->callAPISuccess('membership', 'get', ['contact_id' => $orgID])['values'];
$this->assertCount(2, $orgMembershipResult, '2 primary memberships should have been created on the organization.');
$primaryMembershipIds = [];
foreach ($orgMembershipResult['values'] as $membership) {
foreach ($orgMembershipResult as $membership) {
$primaryMembershipIds[] = $membership['id'];
$this->assertTrue(empty($membership['owner_membership_id']), "Membership on the organization has owner_membership_id so is inherited.");
$this->assertTrue(empty($membership['owner_membership_id']), 'Membership on the organization has owner_membership_id so is inherited.');
}

// CRM-20955: check that correct inherited memberships were created for the individual,
// for both of the primary memberships.
$individualMembershipResult = $this->callAPISuccess('membership', 'get', [
'contact_id' => $this->_individualId,
]);
$this->assertEquals(2, $individualMembershipResult['count'], "2 inherited memberships should have been created on the individual.");
$this->assertEquals(2, $individualMembershipResult['count'], '2 inherited memberships should have been created on the individual.');
foreach ($individualMembershipResult['values'] as $membership) {
$this->assertNotEmpty($membership['owner_membership_id'], "Membership on the individual lacks owner_membership_id so is not inherited.");
$this->assertNotContains($membership['id'], $primaryMembershipIds, "Inherited membership id should not be the id of a primary membership.");
$this->assertContains($membership['owner_membership_id'], $primaryMembershipIds, "Inherited membership owner_membership_id should be the id of a primary membership.");
$this->assertNotEmpty($membership['owner_membership_id'], 'Membership on the individual lacks owner_membership_id so is not inherited.');
$this->assertNotContains($membership['id'], $primaryMembershipIds, 'Inherited membership id should not be the id of a primary membership.');
$this->assertContains($membership['owner_membership_id'], $primaryMembershipIds, 'Inherited membership owner_membership_id should be the id of a primary membership.');
}

// CRM-20966: check that the correct membership contribution, line items
// & membership_payment records were created for the organization.
$contributionResult = $this->callAPISuccess('contribution', 'get', [
$contributionResult = $this->callAPISuccessGetSingle('contribution', [
'contact_id' => $orgID,
'sequential' => 1,
'api.line_item.get' => [],
'api.membership_payment.get' => [],
]);
$this->assertEquals(1, $contributionResult['count'], "One contribution should have been created for the organization's memberships.");

$this->assertEquals(2, $contributionResult['values'][0]['api.line_item.get']['count'], "2 line items should have been created for the organization's memberships.");
foreach ($contributionResult['values'][0]['api.line_item.get']['values'] as $lineItem) {
$this->assertEquals(2, $contributionResult['api.line_item.get']['count'], "2 line items should have been created for the organization's memberships.");
foreach ($contributionResult['api.line_item.get']['values'] as $lineItem) {
$this->assertEquals('civicrm_membership', $lineItem['entity_table'], "Membership line item's entity_table should be 'civicrm_membership'.");
$this->assertContains($lineItem['entity_id'], $primaryMembershipIds, "Membership line item's entity_id should be the id of a primary membership.");
}

$this->assertEquals(2, $contributionResult['values'][0]['api.membership_payment.get']['count'], "2 membership payment records should have been created for the organization's memberships.");
foreach ($contributionResult['values'][0]['api.membership_payment.get']['values'] as $membershipPayment) {
$this->assertEquals($contributionResult['values'][0]['id'], $membershipPayment['contribution_id'], "membership payment's contribution ID should be the ID of the organization's membership contribution.");
$this->assertEquals(2, $contributionResult['api.membership_payment.get']['count'], "2 membership payment records should have been created for the organization's memberships.");
foreach ($contributionResult['api.membership_payment.get']['values'] as $membershipPayment) {
$this->assertEquals($contributionResult['id'], $membershipPayment['contribution_id'], "membership payment's contribution ID should be the ID of the organization's membership contribution.");
$this->assertContains($membershipPayment['membership_id'], $primaryMembershipIds, "membership payment's membership ID should be the ID of a primary membership.");
}

$this->callAPISuccessGetCount('Membership', [], 4);
$this->callAPISuccess('Membership', 'create', ['id' => $primaryMembershipIds[0]]);
$this->callAPISuccessGetCount('Membership', [], 4);

// CRM-20966: check that deleting relationship used for inheritance does not delete contribution.
$this->callAPISuccess('relationship', 'delete', [
'id' => $relationship['id'],
]);
$this->callAPISuccess('relationship', 'delete', ['id' => $relationship['id']]);

$contributionResultAfterRelationshipDelete = $this->callAPISuccess('contribution', 'get', [
'id' => $contributionResult['values'][0]['id'],
'id' => $contributionResult['id'],
'contact_id' => $orgID,
]);
$this->assertEquals(1, $contributionResultAfterRelationshipDelete['count'], "Contribution has been wrongly deleted.");
Expand Down
4 changes: 0 additions & 4 deletions tests/phpunit/api/v3/JobTest.php
Expand Up @@ -2041,10 +2041,6 @@ public function testProcessMembershipUpdateStatus() {
$this->assertEquals($organizationMembershipID, $expiredInheritedRelationship['owner_membership_id']);
$this->assertMembershipStatus('Grace', (int) $expiredInheritedRelationship['status_id']);

// Reset static $relatedContactIds array in createRelatedMemberships(),
// to avoid bug where inherited membership gets deleted.
$var = TRUE;
CRM_Member_BAO_Membership::createRelatedMemberships($var, $var, TRUE);
// Check that after running process_membership job, statuses are correct.
$this->callAPISuccess('Job', 'process_membership', []);

Expand Down