From bbd802146b40491161f8688befbf80d1e0936966 Mon Sep 17 00:00:00 2001 From: Matthew Wire Date: Mon, 23 Dec 2024 12:43:31 +0000 Subject: [PATCH] Fix missing membershiplog entries and add tests following #30493 --- CRM/Member/BAO/Membership.php | 72 ++++++++++++------- .../phpunit/CRM/Member/BAO/MembershipTest.php | 37 ++++++++++ tests/phpunit/CiviTest/CiviUnitTestCase.php | 2 + 3 files changed, 87 insertions(+), 24 deletions(-) diff --git a/CRM/Member/BAO/Membership.php b/CRM/Member/BAO/Membership.php index b6599d768ceb..dc5e6c4933a6 100644 --- a/CRM/Member/BAO/Membership.php +++ b/CRM/Member/BAO/Membership.php @@ -88,30 +88,8 @@ public static function add(&$params) { //it is set during renewal of membership. $logStartDate = $params['log_start_date'] ?? NULL; $logStartDate = ($logStartDate) ? CRM_Utils_Date::isoToMysql($logStartDate) : CRM_Utils_Date::isoToMysql($membership->start_date); - $values = self::getStatusANDTypeValues($membership->id); - - $membershipLog = [ - 'membership_id' => $membership->id, - 'status_id' => $membership->status_id, - 'start_date' => $logStartDate, - 'end_date' => CRM_Utils_Date::isoToMysql($membership->end_date), - 'modified_date' => CRM_Utils_Time::date('Ymd'), - 'membership_type_id' => $values[$membership->id]['membership_type_id'], - 'max_related' => $membership->max_related, - ]; - - if (!empty($params['modified_id'])) { - $membershipLog['modified_id'] = $params['modified_id']; - } - // If we have an authenticated session, set modified_id to that user's contact_id, else set to membership.contact_id - elseif (CRM_Core_Session::getLoggedInContactID()) { - $membershipLog['modified_id'] = CRM_Core_Session::getLoggedInContactID(); - } - else { - $membershipLog['modified_id'] = $membership->contact_id; - } - - CRM_Member_BAO_MembershipLog::add($membershipLog); + $membershipTypeID = (int) self::getStatusANDTypeValues($membership->id)[$membership->id]['membership_type_id']; + $membershipLog = self::createMembershipLog($membership, $logStartDate, $membershipTypeID); // reset the group contact cache since smart groups might be affected due to this CRM_Contact_BAO_GroupContactCache::opportunisticCacheFlush(); @@ -2027,6 +2005,7 @@ public static function updateAllMembershipStatus($params = []) { // This query retrieves ALL memberships of active types. // Note: id, is_test, campaign_id expected by CRM_Activity_BAO_Activity::addActivity() // called by createChangeMembershipStatusActivity(). + // max_related expected by createMembershipLog(). $baseQuery = " SELECT civicrm_membership.id as membership_id, civicrm_membership.id as id, @@ -2040,6 +2019,7 @@ public static function updateAllMembershipStatus($params = []) { civicrm_membership.start_date as start_date, civicrm_membership.end_date as end_date, civicrm_membership.source as source, + civicrm_membership.max_related as max_related, civicrm_contact.id as contact_id, civicrm_membership.owner_membership_id as owner_membership_id, civicrm_membership.contribution_recur_id as recur_id @@ -2087,6 +2067,8 @@ public static function updateAllMembershipStatus($params = []) { $allStatusLabels = CRM_Member_BAO_Membership::buildOptions('status_id', 'get'); $changedByContactID = CRM_Core_Session::getLoggedInContactID() ?? $dao2->contact_id; self::createChangeMembershipStatusActivity($dao2, $allStatusLabels[$dao2->status_id], $allStatusLabels[$newStatusId], $changedByContactID); + $dao2->status_id = $newStatusId; + self::createMembershipLog($dao2); $updateCount++; } } @@ -2571,4 +2553,46 @@ private static function createChangeMembershipStatusActivity($membership, string ); } + /** + * Create the MembershipLog record. + * This was embedded deep in the ::add() function. + * Extracted here to it's own function so we have a single place to create it. + * + * @param CRM_Core_DAO $membership + * @param string $logStartDate + * @param ?int $membershipTypeID + * + * @return array + * + * @internal Signature may change + */ + private static function createMembershipLog($membership, string $logStartDate = '', ?int $membershipTypeID = NULL): array { + if (empty($logStartDate)) { + $logStartDate = CRM_Utils_Date::isoToMysql($membership->start_date); + } + $membershipLog = [ + 'membership_id' => $membership->id, + 'status_id' => $membership->status_id, + 'start_date' => $logStartDate, + 'end_date' => CRM_Utils_Date::isoToMysql($membership->end_date), + 'modified_date' => CRM_Utils_Time::date('Ymd'), + 'membership_type_id' => $membershipTypeID ?? $membership->membership_type_id, + 'max_related' => $membership->max_related, + ]; + + if (!empty($params['modified_id'])) { + $membershipLog['modified_id'] = $params['modified_id']; + } + // If we have an authenticated session, set modified_id to that user's contact_id, else set to membership.contact_id + elseif (CRM_Core_Session::getLoggedInContactID()) { + $membershipLog['modified_id'] = CRM_Core_Session::getLoggedInContactID(); + } + else { + $membershipLog['modified_id'] = $membership->contact_id; + } + // @todo maybe move this to an API4 call, or writeRecord() + CRM_Member_BAO_MembershipLog::add($membershipLog); + return $membershipLog; + } + } diff --git a/tests/phpunit/CRM/Member/BAO/MembershipTest.php b/tests/phpunit/CRM/Member/BAO/MembershipTest.php index d25cb4c9df1e..4c6a504d74b3 100644 --- a/tests/phpunit/CRM/Member/BAO/MembershipTest.php +++ b/tests/phpunit/CRM/Member/BAO/MembershipTest.php @@ -9,6 +9,9 @@ +--------------------------------------------------------------------+ */ +use Civi\Api4\MembershipLog; +use Civi\Api4\MembershipStatus; + /** * Class CRM_Member_BAO_MembershipTest * @group headless @@ -600,6 +603,19 @@ public function testUpdateAllMembershipStatusConvertExpiredOverriddenStatusToNor $this->assertEquals($createdMembershipID, $membershipAfterProcess['id']); $this->assertArrayNotHasKey('status_override_end_date', $membershipAfterProcess); + + // Check that MembershipLog was created and is correct + $latestMembershipLog = MembershipLog::get(FALSE) + ->addWhere('membership_id', '=', $createdMembershipID) + ->addOrderBy('id', 'DESC') + ->execute() + ->first(); + $newMembershipStatus = MembershipStatus::get(FALSE) + ->addSelect('id') + ->addWhere('name', '=', 'New') + ->execute() + ->first(); + $this->assertEquals($newMembershipStatus['id'], $latestMembershipLog['status_id']); } /** @@ -630,6 +646,19 @@ public function testUpdateAllMembershipStatusHandleOverriddenWithEndOverrideDate $this->assertEquals($createdMembershipID, $membershipAfterProcess['id']); $this->assertArrayNotHasKey('status_override_end_date', $membershipAfterProcess); + + // Check that MembershipLog was created and is correct + $latestMembershipLog = MembershipLog::get(FALSE) + ->addWhere('membership_id', '=', $createdMembershipID) + ->addOrderBy('id', 'DESC') + ->execute() + ->first(); + $newMembershipStatus = MembershipStatus::get(FALSE) + ->addSelect('id') + ->addWhere('name', '=', 'New') + ->execute() + ->first(); + $this->assertEquals($newMembershipStatus['id'], $latestMembershipLog['status_id']); } /** @@ -659,6 +688,14 @@ public function testUpdateAllMembershipStatusDoesNotConvertOverriddenMembershipW $this->assertEquals($createdMembershipID, $membershipAfterProcess['id']); $this->assertEquals(1, $membershipAfterProcess['is_override']); + + // Check that MembershipLog was created and is correct + $latestMembershipLog = MembershipLog::get(FALSE) + ->addWhere('membership_id', '=', $createdMembershipID) + ->addOrderBy('id', 'DESC') + ->execute() + ->first(); + $this->assertEquals($this->_membershipStatusID, $latestMembershipLog['status_id']); } /** diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index 280fec38515c..4c6d323676c8 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -708,6 +708,8 @@ public function membershipStatusCreate($name = 'test member status'): int { $params['end_event'] = 'end_date'; $params['is_current_member'] = 1; $params['is_active'] = 1; + // Make sure weight is after existing statuses (could be cleverer and get max(weight) first). + $params['weight'] = 100; $result = $this->callAPISuccess('MembershipStatus', 'Create', $params); CRM_Member_PseudoConstant::flush('membershipStatus');