From 5fbbde1ffbe356e2b53eb2ac3deb22fb0a690f1b Mon Sep 17 00:00:00 2001 From: eileen Date: Mon, 19 Oct 2020 15:23:57 +1300 Subject: [PATCH] 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. --- CRM/Batch/Form/Entry.php | 9 +-------- CRM/Contribute/Form/Contribution/Confirm.php | 12 ++++------- CRM/Member/BAO/Membership.php | 20 +++++++------------ tests/phpunit/CRM/Batch/Form/EntryTest.php | 16 ++++++++++----- .../phpunit/CRM/Member/BAO/MembershipTest.php | 4 +--- 5 files changed, 24 insertions(+), 37 deletions(-) diff --git a/CRM/Batch/Form/Entry.php b/CRM/Batch/Form/Entry.php index aefa3a2b15..571d0ad2eb 100644 --- a/CRM/Batch/Form/Entry.php +++ b/CRM/Batch/Form/Entry.php @@ -821,13 +821,6 @@ class CRM_Batch_Form_Entry extends CRM_Core_Form { $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 @@ class CRM_Batch_Form_Entry extends CRM_Core_Form { $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['member_campaign_id'] ?? NULL], $formDates ); // make contribution entry diff --git a/CRM/Contribute/Form/Contribution/Confirm.php b/CRM/Contribute/Form/Contribution/Confirm.php index 246b669576..1d17d82837 100644 --- a/CRM/Contribute/Form/Contribution/Confirm.php +++ b/CRM/Contribute/Form/Contribution/Confirm.php @@ -1527,13 +1527,9 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr 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 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr 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 index 6e8a309958..a89e53a494 100644 --- a/CRM/Member/BAO/Membership.php +++ b/CRM/Member/BAO/Membership.php @@ -1743,7 +1743,7 @@ INNER JOIN civicrm_contact contact ON ( contact.id = membership.contact_id AND * @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 @@ -1752,7 +1752,7 @@ INNER JOIN civicrm_contact contact ON ( contact.id = membership.contact_id AND * @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'; @@ -1778,7 +1778,7 @@ INNER JOIN civicrm_contact contact ON ( contact.id = membership.contact_id AND 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'], @@ -1789,7 +1789,7 @@ INNER JOIN civicrm_contact contact ON ( contact.id = membership.contact_id AND '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; } @@ -1828,7 +1828,7 @@ INNER JOIN civicrm_contact contact ON ( contact.id = membership.contact_id AND 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. @@ -1848,7 +1848,6 @@ INNER JOIN civicrm_contact contact ON ( contact.id = membership.contact_id AND ); // 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; @@ -1879,10 +1878,10 @@ INNER JOIN civicrm_contact contact ON ( contact.id = membership.contact_id AND } 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); @@ -1945,11 +1944,6 @@ INNER JOIN civicrm_contact contact ON ( contact.id = membership.contact_id AND } $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 index 21ff503d6b..7999141ff0 100644 --- a/tests/phpunit/CRM/Batch/Form/EntryTest.php +++ b/tests/phpunit/CRM/Batch/Form/EntryTest.php @@ -24,6 +24,8 @@ * . */ +use Civi\Api4\Campaign; + /** * Test CRM/Member/BAO Membership Log add , delete functions * @@ -238,6 +240,7 @@ class CRM_Batch_Form_EntryTest extends CiviUnitTestCase { */ 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 @@ class CRM_Batch_Form_EntryTest extends CiviUnitTestCase { ]; $params['field'][1]['membership_type'] = [0 => $this->_orgContactID2, 1 => $this->_membershipTypeID2]; $params['field'][1]['receive_date'] = date('Y-m-d'); + $params['field'][1]['member_campaign_id'] = $campaignID; // explicitly specify start and end dates $params['field'][2]['membership_type'] = [0 => $this->_orgContactID2, 1 => $this->_membershipTypeID2]; @@ -265,16 +269,18 @@ class CRM_Batch_Form_EntryTest extends CiviUnitTestCase { $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 index 2425e1b824..83c6bbbc3d 100644 --- a/tests/phpunit/CRM/Member/BAO/MembershipTest.php +++ b/tests/phpunit/CRM/Member/BAO/MembershipTest.php @@ -444,7 +444,6 @@ class CRM_Member_BAO_MembershipTest extends CiviUnitTestCase { NULL, NULL, FALSE, - NULL, NULL ); $endDate = date("Y-m-d", strtotime($membership['end_date'] . " +1 year")); @@ -514,8 +513,7 @@ class CRM_Member_BAO_MembershipTest extends CiviUnitTestCase { NULL, NULL, NULL, - FALSE, - NULL + FALSE ); $this->assertDBNotNull('CRM_Member_BAO_MembershipLog', -- 2.25.1