From 3506b6cde754f02cb968e546b9d18ab44d962423 Mon Sep 17 00:00:00 2001 From: Dave Greenberg Date: Fri, 26 Jul 2013 16:36:41 -0700 Subject: [PATCH] Fixed bug where delete membership task does not also delete related memberships. Created a new del method in Membership BAO. --- CRM/Member/BAO/Membership.php | 80 ++++++++++++------- CRM/Member/Form/Membership.php | 3 +- CRM/Member/Form/MembershipView.php | 2 +- CRM/Member/Form/Task/Delete.php | 2 +- api/v3/Membership.php | 4 +- .../phpunit/CRM/Member/BAO/MembershipTest.php | 2 +- tests/phpunit/api/v3/MembershipTest.php | 4 +- 7 files changed, 57 insertions(+), 40 deletions(-) diff --git a/CRM/Member/BAO/Membership.php b/CRM/Member/BAO/Membership.php index 549f70b4f7..b34e822db7 100644 --- a/CRM/Member/BAO/Membership.php +++ b/CRM/Member/BAO/Membership.php @@ -565,6 +565,23 @@ INNER JOIN civicrm_membership_type type ON ( type.id = membership.membership_ty return $values; } + /** + * Function to delete membership. + * Wrapper for most delete calls. Use this unless you JUST want to delete memberships w/o deleting the parent. + * + * @param int $membershipId membership id that needs to be deleted + * + * @static + * + * @return $results no of deleted Membership on success, false otherwise + * @access public + */ + static function del($membershipId) { + //delete related first and then delete parent. + self::deleteRelatedMemberships($membershipId); + return self::deleteMembership($membershipId); + } + /** * Function to delete membership. * @@ -580,6 +597,7 @@ INNER JOIN civicrm_membership_type type ON ( type.id = membership.membership_ty $params = array('id' => $membershipId); $memValues = array(); $memberships = self::getValues($params, $memValues); + $membership = $memberships[$membershipId]; CRM_Utils_Hook::pre('delete', 'Membership', $membershipId, $memValues); @@ -627,6 +645,36 @@ INNER JOIN civicrm_membership_type type ON ( type.id = membership.membership_ty return $results; } + /** + * Function to delete related memberships + * + * @param int $ownerMembershipId + * @param int $contactId + * + * @return null + * @static + */ + static function deleteRelatedMemberships($ownerMembershipId, $contactId = NULL) { + if (!$ownerMembershipId && !$contactId) { + return; + } + + $membership = new CRM_Member_DAO_Membership(); + $membership->owner_membership_id = $ownerMembershipId; + + if ($contactId) { + $membership->contact_id = $contactId; + } + + $membership->find(); + while ($membership->fetch()) { + //delete related first and then delete parent. + self::deleteRelatedMemberships($membership->id); + self::deleteMembership($membership->id); + } + $membership->free(); + } + /** * Function to obtain active/inactive memberships from the list of memberships passed to it. * @@ -1879,36 +1927,6 @@ SELECT c.contribution_page_id as pageID ); } - /** - * Function to delete related memberships - * - * @param int $ownerMembershipId - * @param int $contactId - * - * @return null - * @static - */ - static function deleteRelatedMemberships($ownerMembershipId, $contactId = NULL) { - if (!$ownerMembershipId && !$contactId) { - return; - } - - $membership = new CRM_Member_DAO_Membership(); - $membership->owner_membership_id = $ownerMembershipId; - - if ($contactId) { - $membership->contact_id = $contactId; - } - - $membership->find(); - while ($membership->fetch()) { - //delete related first and then delete parent. - self::deleteRelatedMemberships($membership->id); - self::deleteMembership($membership->id); - } - $membership->free(); - } - /** * Function to updated related memberships * @@ -2061,7 +2079,7 @@ WHERE civicrm_membership.contact_id = civicrm_contact.id //lets cleanup related membership if any. if (empty($relatedContacts)) { - CRM_Member_BAO_Membership::deleteRelatedMemberships($membership->id); + self::deleteRelatedMemberships($membership->id); } else { // Edit the params array diff --git a/CRM/Member/Form/Membership.php b/CRM/Member/Form/Membership.php index 6eb400e0cd..dae3cb4c06 100644 --- a/CRM/Member/Form/Membership.php +++ b/CRM/Member/Form/Membership.php @@ -1067,8 +1067,7 @@ WHERE id IN ( ' . implode(' , ', array_keys($membershipType)) . ' )'; */ public function postProcess() { if ($this->_action & CRM_Core_Action::DELETE) { - CRM_Member_BAO_Membership::deleteRelatedMemberships($this->_id); - CRM_Member_BAO_Membership::deleteMembership($this->_id); + CRM_Member_BAO_Membership::del($this->_id); return; } diff --git a/CRM/Member/Form/MembershipView.php b/CRM/Member/Form/MembershipView.php index 64aa72d219..451514fa75 100644 --- a/CRM/Member/Form/MembershipView.php +++ b/CRM/Member/Form/MembershipView.php @@ -100,7 +100,7 @@ class CRM_Member_Form_MembershipView extends CRM_Core_Form { $id = CRM_Utils_Request::retrieve('mid', 'Positive', $this); $relatedContactId = CRM_Utils_Request::retrieve('cid', 'Positive', $this); $relatedDisplayName = CRM_Contact_BAO_Contact::displayName($relatedContactId); - CRM_Member_BAO_Membership::deleteMembership($id); + CRM_Member_BAO_Membership::del($id); CRM_Core_Session::setStatus(ts('Related membership for %1 has been deleted.', array(1 => $relatedDisplayName)), ts('Membership Deleted'), 'success'); break; case 'create': diff --git a/CRM/Member/Form/Task/Delete.php b/CRM/Member/Form/Task/Delete.php index 12152e6970..11136c0d94 100644 --- a/CRM/Member/Form/Task/Delete.php +++ b/CRM/Member/Form/Task/Delete.php @@ -83,7 +83,7 @@ class CRM_Member_Form_Task_Delete extends CRM_Member_Form_Task { public function postProcess() { $deletedMembers = 0; foreach ($this->_memberIds as $memberId) { - if (CRM_Member_BAO_Membership::deleteMembership($memberId)) { + if (CRM_Member_BAO_Membership::del($memberId)) { $deletedMembers++; } } diff --git a/api/v3/Membership.php b/api/v3/Membership.php index 95345c1c91..f327649bb7 100644 --- a/api/v3/Membership.php +++ b/api/v3/Membership.php @@ -56,10 +56,8 @@ function civicrm_api3_membership_delete($params) { return civicrm_api3_create_error('Input parameter should be numeric'); } - CRM_Member_BAO_Membership::deleteRelatedMemberships($params['id']); - $membership = new CRM_Member_BAO_Membership(); - $result = $membership->deleteMembership($params['id']); + $result = $membership->del($params['id']); return $result ? civicrm_api3_create_success() : civicrm_api3_create_error('Error while deleting Membership'); diff --git a/tests/phpunit/CRM/Member/BAO/MembershipTest.php b/tests/phpunit/CRM/Member/BAO/MembershipTest.php index d5aedbec50..a24ad5170f 100644 --- a/tests/phpunit/CRM/Member/BAO/MembershipTest.php +++ b/tests/phpunit/CRM/Member/BAO/MembershipTest.php @@ -295,7 +295,7 @@ class CRM_Member_BAO_MembershipTest extends CiviUnitTestCase { $membershipId = $this->assertDBNotNull('CRM_Member_BAO_Membership', $contactId, 'id', 'contact_id', 'Database check for created membership.' ); - CRM_Member_BAO_Membership::deleteMembership($membershipId); + CRM_Member_BAO_Membership::del($membershipId); $membershipId = $this->assertDBNull('CRM_Member_BAO_Membership', $contactId, 'id', 'contact_id', 'Database check for deleted membership.' diff --git a/tests/phpunit/api/v3/MembershipTest.php b/tests/phpunit/api/v3/MembershipTest.php index a552d4d667..36e81afe30 100644 --- a/tests/phpunit/api/v3/MembershipTest.php +++ b/tests/phpunit/api/v3/MembershipTest.php @@ -90,10 +90,12 @@ class api_v3_MembershipTest extends CiviUnitTestCase { */ function testMembershipDelete() { $membershipID = $this->contactMembershipCreate($this->_params); + $this->assertDBRowExist('CRM_Member_DAO_Membership', $membershipID); $params = array( - 'id' => $membershipID, + 'id' => $membershipID ); $result = $this->callAPIAndDocument('membership', 'delete', $params, __FUNCTION__, __FILE__); + $this->assertDBRowNotExist('CRM_Member_DAO_Membership', $membershipID); } function testMembershipDeleteEmpty() { -- 2.25.1