additional improvement

diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php
index 95f646d2017e33412149d70cc2ee3b9b90b306db..d12afdc0b075d120cfa63078625ba01902c5ecbd 100644 (file)
--- a/CRM/Activity/BAO/Activity.php
+++ b/CRM/Activity/BAO/Activity.php
@@ -2068,6 +2068,14 @@ WHERE      activity.id IN ($activityIds)";
         return NULL;
       }
 
+      // retrieve existing activity based on source_record_id and activity_type
+      if (empty($params['id'])) {
+        $params['id'] = CRM_Utils_Array::value('id', civicrm_api3('Activity', 'Get', array(
+          'source_record_id' => $activity->id,
+          'activity_type_id' => $activityType,
+        )));
+      }
+
       $date = CRM_Utils_Date::isoToMysql($activity->receive_date);
       $activityType = $component = 'Contribution';
     }

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index 22c3e625f733cca0626962ca46e57c576ebc0f8f..fb5a7c95537109514128646e88e4c94945a47ec6 100644 (file)
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -579,7 +579,7 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution {
     if (empty($contribution->contact_id)) {
       $contribution->find(TRUE);
     }
-    CRM_Activity_BAO_Activity::addActivity($contribution, 'Offline');
+    CRM_Activity_BAO_Activity::addActivity($contribution, 'Contribution');
 
     // do not add to recent items for import, CRM-4399
     if (empty($params['skipRecentView'])) {
@@ -1882,6 +1882,7 @@ LEFT JOIN  civicrm_contribution contribution ON ( componentPayment.contribution_
             }
 
             if ($currentMembership) {
+              $oldStatus = $currentMembership['status_id'];
               CRM_Member_BAO_Membership::fixMembershipStatusBeforeRenew($currentMembership, NULL);
               $dates = CRM_Member_BAO_MembershipType::getRenewalDatesForMembershipType($membership->id, NULL, NULL, $numterms);
               $dates['join_date'] = CRM_Utils_Date::customFormat($currentMembership['join_date'], $format);
@@ -1931,8 +1932,37 @@ LEFT JOIN  civicrm_contribution contribution ON ( componentPayment.contribution_
             //update related Memberships.
             CRM_Member_BAO_Membership::updateRelatedMemberships($membership->id, $formattedParams);
 
-            //CRM-19678: No Membership Renewal Activity is created when a Pay Later is set to Completed
-            CRM_Activity_BAO_Activity::addActivity($membership, 'Membership Renewal', $membership->contact_id);
+            foreach (array('Membership Signup', 'Membership Renewal') as $activityType) {
+              $scheduledActivityID = CRM_Utils_Array::value('id',
+                civicrm_api3('Activity', 'Get',
+                  array(
+                    'source_record_id' => $membership->id,
+                    'activity_type_id' => $activityType,
+                    'status_id' => 'Scheduled',
+                  )
+                )
+              );
+              // 1. Update Schedule Membership Signup/Renwal activity to completed on successful payment of pending membership
+              // 2. OR Create renewal activity scheduled if its membership renewal will be paid later
+              if ($scheduledActivityID) {
+                CRM_Activity_BAO_Activity::addActivity($membership, $activityType, $membership->contact_id, array('id' => $scheduledActivityID));
+                break;
+              }
+            }
+
+            // track membership status change if any
+            if (!empty($oldStatus) && $membership->status_id != $oldStatus) {
+              $allStatus = CRM_Member_BAO_Membership::buildOptions('status_id', 'get');
+              CRM_Activity_BAO_Activity::addActivity($membership,
+                'Change Membership Status',
+                NULL,
+                array(
+                  'subject' => "Status changed from {$allStatus[$oldStatus]} to {$allStatus[$membership->status_id]}",
+                  'source_contact_id' => $membershipLog['modified_id'],
+                  'priority_id' => 2,
+                )
+              );
+            }
 
             $updateResult['membership_end_date'] = CRM_Utils_Date::customFormat($dates['end_date'],
               '%B %E%f, %Y'

diff --git a/CRM/Member/BAO/Membership.php b/CRM/Member/BAO/Membership.php
index 03b70d128c2c8f7c76eed185e3d3b7e0802cca56..f04602158f4218e06ab6dea4bedb245f79557167 100644 (file)
--- a/CRM/Member/BAO/Membership.php
+++ b/CRM/Member/BAO/Membership.php
@@ -139,13 +139,20 @@ class CRM_Member_BAO_Membership extends CRM_Member_DAO_Membership {
     // reset the group contact cache since smart groups might be affected due to this
     CRM_Contact_BAO_GroupContactCache::opportunisticCacheFlush();
 
+    $activityParams = array();
+    $allStatus = CRM_Member_BAO_Membership::buildOptions('status_id', 'get');
+    if (!empty($params['is_pay_later']) ||
+      in_array($allStatus[$membership->status_id], array('Pending', 'Grace'))
+    ) {
+      $activityParams['status_id'] = CRM_Core_OptionGroup::getValue('activity_status', 'Scheduled', 'name');
+    }
+
     $targetContactID = $membership->contact_id;
     if (!empty($params['is_for_organization'])) {
       $targetContactID = CRM_Utils_Array::value('userId', $ids);
     }
     if ($id) {
       if ($membership->status_id != $oldStatus) {
-        $allStatus = CRM_Member_BAO_Membership::buildOptions('status_id', 'get');
         CRM_Activity_BAO_Activity::addActivity($membership,
           'Change Membership Status',
           NULL,
@@ -168,16 +175,29 @@ class CRM_Member_BAO_Membership extends CRM_Member_DAO_Membership {
           )
         );
       }
-      // via Contribution.completeOrder membership.create is called again to ensure correct data has been
-      // copied to the related membership. but there is a sideffect of this new membership workflow as a
-      // 'Membership Renewal' Activity is created. We can avoid this on basis of is_override=FALSE condition
-      if (CRM_Utils_Array::value('is_override', $params) != 'null') {
-        CRM_Activity_BAO_Activity::addActivity($membership, 'Membership Renewal', $targetContactID);
+
+      foreach (array('Membership Signup', 'Membership Renewal') as $activityType) {
+        $activityParams['id'] = CRM_Utils_Array::value('id',
+          civicrm_api3('Activity', 'Get',
+            array(
+              'source_record_id' => $membership->id,
+              'activity_type_id' => $activityType,
+              'status_id' => 'Scheduled',
+            )
+          )
+        );
+        // 1. Update Schedule Membership Signup/Renwal activity to completed on successful payment of pending membership
+        // 2. OR Create renewal activity scheduled if its membership renewal will be paid later
+        if (!empty($activityParams['id']) || $activityType == 'Membership Renewal') {
+          CRM_Activity_BAO_Activity::addActivity($membership, $activityType, $targetContactID, $activityParams);
+          break;
+        }
       }
+
       CRM_Utils_Hook::post('edit', 'Membership', $membership->id, $membership);
     }
     else {
-      CRM_Activity_BAO_Activity::addActivity($membership, 'Membership Signup', $targetContactID);
+      CRM_Activity_BAO_Activity::addActivity($membership, 'Membership Signup', $targetContactID, $activityParams);
       CRM_Utils_Hook::post('create', 'Membership', $membership->id, $membership);
     }
 
@@ -803,7 +823,7 @@ INNER JOIN  civicrm_membership_type type ON ( type.id = membership.membership_ty
     }
 
     //avoid pending membership as current membership: CRM-3027
-    $statusIds[] = array_search('Pending', CRM_Member_PseudoConstant::membershipStatus());
+    $statusIds = array(array_search('Pending', CRM_Member_PseudoConstant::membershipStatus()));
     if (!$membershipId) {
       // CRM-15475
       $statusIds[] = array_search(
@@ -1833,6 +1853,7 @@ INNER JOIN  civicrm_contact contact ON ( contact.id = membership.contact_id AND
           'join_date' => $currentMembership['join_date'],
           'membership_type_id' => $membershipTypeID,
           'max_related' => !empty($membershipTypeDetails['max_related']) ? $membershipTypeDetails['max_related'] : NULL,
+          'is_pay_later' => $isPayLater,
         );
         if ($contributionRecurID) {
           $memParams['contribution_recur_id'] = $contributionRecurID;

diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php
index d7f79e519a75a066d5ca88d4164bfd14bd0b79ef..b287d815f514d4980fb080b9eef2c38e9dcc43d5 100644 (file)
--- a/tests/phpunit/api/v3/ContributionTest.php
+++ b/tests/phpunit/api/v3/ContributionTest.php
@@ -2765,6 +2765,13 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
     );
     $this->setUpPendingContribution($this->_ids['price_field_value'][0]);
     $this->callAPISuccess('membership', 'getsingle', array('id' => $this->_ids['membership']));
+    // Case 1: Assert that Membership Signup Activity is created on Pending (Pay later) to Completed Contribution via backoffice
+    $activity = $this->callAPISuccess('Activity', 'get', array(
+      'activity_type_id' => 'Membership Signup',
+      'source_record_id' => $this->_ids['membership'],
+      'status_id' => 'Scheduled',
+    ));
+    $this->assertEquals(1, $activity['count']);
 
     // change pending contribution to completed
     $form = new CRM_Contribute_Form_Contribution();
@@ -2806,34 +2813,29 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
     catch (Civi\Payment\Exception\PaymentProcessorException $e) {
       $error = TRUE;
     }
-    // Case 1: Assert that Membership Renewal Activity is created on Pending (Pay later) to Completed Contribution via backoffice
-    $activity = $this->callAPISuccess('Activity', 'get', array(
-      'activity_type_id' => 'Membership Renewal',
-      'source_record_id' => $this->_ids['contribution'],
-    ));
-    $this->assertEquals(1, $activity['count']);
-
-    // Case 2: Assert that Membership Signup Activity is created on Pending (Pay later) Contribution on current membership
-    $this->setUpPendingContribution($this->_ids['price_field_value'][0], array('trxn_id' => 'xoa12', 'invoice_id' => '12mv'));
+    // Case 2: After successful payment for Pending (Pay later) backoffice there are three activities created
+    //  2.a Update status of existing Scheduled Membership Signup (created in step 1) to Completed
     $activity = $this->callAPISuccess('Activity', 'get', array(
       'activity_type_id' => 'Membership Signup',
-      'source_record_id' => $this->_ids['contribution'],
+      'source_record_id' => $this->_ids['membership'],
+      'status_id' => 'Completed',
     ));
     $this->assertEquals(1, $activity['count']);
-    // Case 3: Assert that no Membership Renewal Activity is created on Pending (Pay later) Contribution on current membership
+    // 2.b Contribution activity created to record successful payment
     $activity = $this->callAPISuccess('Activity', 'get', array(
-      'activity_type_id' => 'Membership Renewal',
+      'activity_type_id' => 'Contribution',
       'source_record_id' => $this->_ids['contribution'],
+      'status_id' => 'Completed',
     ));
-    $this->assertEquals(0, $activity['count']);
-
-    //Case 4 (CRM-19600): Assert that Membership Renewal Activity is created via online contribution
-    $this->callAPISuccess('contribution', 'completetransaction', array('id' => $this->_ids['contribution']));
+    $this->assertEquals(1, $activity['count']);
+    // 2.c 'Change membership type' activity created to record Membership status change from Grace to Current
     $activity = $this->callAPISuccess('Activity', 'get', array(
-      'activity_type_id' => 'Membership Renewal',
-      'source_record_id' => $this->_ids['contribution'],
+      'activity_type_id' => 'Change Membership Status',
+      'source_record_id' => $this->_ids['membership'],
+      'status_id' => 'Completed',
     ));
     $this->assertEquals(1, $activity['count']);
+    $this->assertEquals('Status changed from Grace to Current', $activity['values'][$activity['id']]['subject']);
   }
 
   /**