Fixed bug where delete membership task does not also delete related memberships....

diff --git a/CRM/Member/BAO/Membership.php b/CRM/Member/BAO/Membership.php
index 549f70b4f72d014e69cc7661401df969e7dec90c..b34e822db75298acdef101a74fddbec30f963658 100644 (file)
--- 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 6eb400e0cd0ebeab610fa2e22e60773178777867..dae3cb4c06be2b9e9a56f2406211c434c383e548 100644 (file)
--- 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 64aa72d219a610d721fe3112c1efd782a8bbb680..451514fa75f016eb336d0e79549619fc1b66f87c 100644 (file)
--- 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 12152e69705852183f57640a8369bd85f2ed9bd1..11136c0d9410de8dc3ffdab22546948c0fdb45b5 100644 (file)
--- 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 95345c1c9106d589c59036bc8cdece9977a6d08c..f327649bb7d36d8e355a1d79791c5cffe83ae1cb 100644 (file)
--- 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 d5aedbec5078ca8c4db37ad76a11726b6af4bcb4..a24ad5170fcf920b6ffa404f782cd60dd748ff59 100644 (file)
--- 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 a552d4d66743d2b775df7ec1864e952c219bb34a..36e81afe3000e7c10c66ff1f40d73ca34bbb9932 100644 (file)
--- 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() {