Fix campaign_id handling for batch entry

Fixes a bug whereby campaign_id is not updated on batch entry if it has been added to the profile. The underlying issue appears to be an attack of copy and paste. I tidied up the variable in both the places that call the processMembership function such that it is passed in as an array of pass-through params. The function can be simplified & eventually removed.
civicrm · Oct 19, 2020 · f7e70bd · f7e70bd
1 parent 137e207
commit f7e70bd
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 37 deletions.
diff --git a/CRM/Batch/Form/Entry.php b/CRM/Batch/Form/Entry.php
@@ -821,13 +821,6 @@ private function processMembership(&$params) {
           $this->_params = $params;
           $value['is_renew'] = TRUE;
           $isPayLater = $params['is_pay_later'] ?? NULL;
-          $campaignId = NULL;
-          if (isset($this->_values) && is_array($this->_values) && !empty($this->_values)) {
-            $campaignId = $this->_params['campaign_id'] ?? NULL;
-            if (!array_key_exists('campaign_id', $this->_params)) {
-              $campaignId = $this->_values['campaign_id'] ?? NULL;
-            }
-          }
 
           $formDates = [
             'end_date' => $value['membership_end_date'] ?? NULL,
@@ -838,7 +831,7 @@ private function processMembership(&$params) {
             $value['contact_id'], $value['membership_type_id'], FALSE,
             //$numTerms should be default to 1.
             NULL, NULL, $value['custom'], 1, NULL, FALSE,
-            NULL, $membershipSource, $isPayLater, $campaignId, $formDates
+            NULL, $membershipSource, $isPayLater, ['campaign_id' => $value['campaign_id'] ?? NULL], $formDates
           );
 
           // make contribution entry

diff --git a/CRM/Contribute/Form/Contribution/Confirm.php b/CRM/Contribute/Form/Contribution/Confirm.php
@@ -1527,13 +1527,9 @@ protected function postProcessMembership(
         if (isset($form->_params)) {
           $isPayLater = $form->_params['is_pay_later'] ?? NULL;
         }
-        $campaignId = NULL;
-        if (isset($form->_values) && is_array($form->_values) && !empty($form->_values)) {
-          $campaignId = $form->_params['campaign_id'] ?? NULL;
-          if (!array_key_exists('campaign_id', $form->_params)) {
-            $campaignId = $form->_values['campaign_id'] ?? NULL;
-          }
-        }
+        $memParams = [
+          'campaign_id' => $form->_params['campaign_id'] ?? ($form->_values['campaign_id'] ?? NULL),
+        ];
 
         // @todo Move this into CRM_Member_BAO_Membership::processMembership
         if (!empty($membershipContribution)) {
@@ -1557,7 +1553,7 @@ protected function postProcessMembership(
           date('YmdHis'), $membershipParams['cms_contactID'] ?? NULL,
           $customFieldsFormatted,
           $numTerms, $membershipID, $pending,
-          $contributionRecurID, $membershipSource, $isPayLater, $campaignId, [], $membershipContribution,
+          $contributionRecurID, $membershipSource, $isPayLater, $memParams, [], $membershipContribution,
           $membershipLineItems
         );
 

diff --git a/CRM/Member/BAO/Membership.php b/CRM/Member/BAO/Membership.php
@@ -1751,7 +1751,7 @@ public static function getMembershipRenewals($membershipTypeId, $startDate, $end
    * @param int $contributionRecurID
    * @param $membershipSource
    * @param $isPayLater
-   * @param int $campaignId
+   * @param array $memParams
    * @param array $formDates
    * @param null|CRM_Contribute_BAO_Contribution $contribution
    * @param array $lineItems
@@ -1760,7 +1760,7 @@ public static function getMembershipRenewals($membershipTypeId, $startDate, $end
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
    */
-  public static function processMembership($contactID, $membershipTypeID, $is_test, $changeToday, $modifiedID, $customFieldsFormatted, $numRenewTerms, $membershipID, $pending, $contributionRecurID, $membershipSource, $isPayLater, $campaignId, $formDates = [], $contribution = NULL, $lineItems = []) {
+  public static function processMembership($contactID, $membershipTypeID, $is_test, $changeToday, $modifiedID, $customFieldsFormatted, $numRenewTerms, $membershipID, $pending, $contributionRecurID, $membershipSource, $isPayLater, $memParams = [], $formDates = [], $contribution = NULL, $lineItems = []) {
     $renewalMode = $updateStatusId = FALSE;
     $allStatus = CRM_Member_PseudoConstant::membershipStatus();
     $format = '%Y%m%d';
@@ -1786,7 +1786,7 @@ public static function processMembership($contactID, $membershipTypeID, $is_test
         array_search('Cancelled', CRM_Member_PseudoConstant::membershipStatus(NULL, " name = 'Cancelled' ", 'name', FALSE, TRUE)),
       ])) {
 
-        $memParams = [
+        $memParams = array_merge([
           'id' => $currentMembership['id'],
           'contribution' => $contribution,
           'status_id' => $currentMembership['status_id'],
@@ -1797,7 +1797,7 @@ public static function processMembership($contactID, $membershipTypeID, $is_test
           'membership_type_id' => $membershipTypeID,
           'max_related' => !empty($membershipTypeDetails['max_related']) ? $membershipTypeDetails['max_related'] : NULL,
           'membership_activity_status' => ($pending || $isPayLater) ? 'Scheduled' : 'Completed',
-        ];
+        ], $memParams);
         if ($contributionRecurID) {
           $memParams['contribution_recur_id'] = $contributionRecurID;
         }
@@ -1836,7 +1836,7 @@ public static function processMembership($contactID, $membershipTypeID, $is_test
         if (!empty($currentMembership['id'])) {
           $ids['membership'] = $currentMembership['id'];
         }
-        $memParams = $currentMembership;
+        $memParams = array_merge($currentMembership, $memParams);
         $memParams['membership_type_id'] = $membershipTypeID;
 
         //set the log start date.
@@ -1856,7 +1856,6 @@ public static function processMembership($contactID, $membershipTypeID, $is_test
         );
 
         // Insert renewed dates for CURRENT membership
-        $memParams = [];
         $memParams['join_date'] = CRM_Utils_Date::isoToMysql($membership->join_date);
         $memParams['start_date'] = $formDates['start_date'] ?? CRM_Utils_Date::isoToMysql($membership->start_date);
         $memParams['end_date'] = $formDates['end_date'] ?? NULL;
@@ -1887,10 +1886,10 @@ public static function processMembership($contactID, $membershipTypeID, $is_test
     }
     else {
       // NEW Membership
-      $memParams = [
+      $memParams = array_merge([
         'contact_id' => $contactID,
         'membership_type_id' => $membershipTypeID,
-      ];
+      ], $memParams);
 
       if (!$pending) {
         $dates = CRM_Member_BAO_MembershipType::getDatesForMembershipType($membershipTypeID, NULL, NULL, NULL, $numRenewTerms);
@@ -1953,11 +1952,6 @@ public static function processMembership($contactID, $membershipTypeID, $is_test
     }
     $params['modified_id'] = $modifiedID ?? $contactID;
 
-    //inherit campaign from contrib page.
-    if (isset($campaignId)) {
-      $memParams['campaign_id'] = $campaignId;
-    }
-
     $memParams['contribution'] = $contribution;
     $memParams['custom'] = $customFieldsFormatted;
     // Load all line items & process all in membership. Don't do in contribution.

diff --git a/tests/phpunit/CRM/Batch/Form/EntryTest.php b/tests/phpunit/CRM/Batch/Form/EntryTest.php
@@ -24,6 +24,8 @@
  *   <http://www.gnu.org/licenses/>.
  */
 
+use Civi\Api4\Campaign;
+
 /**
  *  Test CRM/Member/BAO Membership Log add , delete functions
  *
@@ -238,6 +240,7 @@ public function testProcessContribution($thousandSeparator) {
    */
   public function testMembershipRenewalDates() {
     $form = new CRM_Batch_Form_Entry();
+    $campaignID = Campaign::create()->setValues(['name' => 'blah', 'title' => 'blah'])->execute()->first()['id'];
     foreach ([$this->_contactID, $this->_contactID2] as $contactID) {
       $membershipParams = [
         'membership_type_id' => $this->_membershipTypeID2,
@@ -257,6 +260,7 @@ public function testMembershipRenewalDates() {
     ];
     $params['field'][1]['membership_type'] = [0 => $this->_orgContactID2, 1 => $this->_membershipTypeID2];
     $params['field'][1]['receive_date'] = date('Y-m-d');
+    $params['field'][1]['campaign_id'] = $campaignID;
 
     // explicitly specify start and end dates
     $params['field'][2]['membership_type'] = [0 => $this->_orgContactID2, 1 => $this->_membershipTypeID2];
@@ -265,16 +269,18 @@ public function testMembershipRenewalDates() {
     $params['field'][2]['receive_date'] = "2016-04-01";
 
     $this->assertTrue($form->testProcessMembership($params));
-    $result = $this->callAPISuccess('membership', 'get', []);
+    $result = $this->callAPISuccess('membership', 'get')['values'];
 
     // renewal dates should be from current if start_date and end_date is passed as NULL
-    $this->assertEquals(date('Y-m-d'), $result['values'][1]['start_date']);
+    $this->assertEquals(date('Y-m-d'), $result[1]['start_date']);
     $endDate = date("Y-m-d", strtotime(date("Y-m-d") . " +1 year -1 day"));
-    $this->assertEquals($endDate, $result['values'][1]['end_date']);
+    $this->assertEquals($endDate, $result[1]['end_date']);
+    $this->assertEquals(1, $result[1]['campaign_id']);
 
     // verify if the modified dates asserts with the dates passed above
-    $this->assertEquals('2016-04-01', $result['values'][2]['start_date']);
-    $this->assertEquals('2017-03-31', $result['values'][2]['end_date']);
+    $this->assertEquals('2016-04-01', $result[2]['start_date']);
+    $this->assertEquals('2017-03-31', $result[2]['end_date']);
+    $this->assertTrue(empty($result[2]['campaign_id']));
   }
 
   /**

diff --git a/tests/phpunit/CRM/Member/BAO/MembershipTest.php b/tests/phpunit/CRM/Member/BAO/MembershipTest.php
@@ -445,7 +445,6 @@ public function testRenewMembership() {
       NULL,
       NULL,
       FALSE,
-      NULL,
       NULL
     );
     $endDate = date("Y-m-d", strtotime($membership->end_date . " +1 year"));
@@ -527,8 +526,7 @@ public function testStaleMembership() {
       NULL,
       NULL,
       NULL,
-      FALSE,
-      NULL
+      FALSE
     );
 
     $this->assertDBNotNull('CRM_Member_BAO_MembershipLog',