Fix missing membershiplog entries and add tests following #30493

civicrm · Dec 24, 2024 · 5339cc5 · 5339cc5
1 parent 72cdaf2
commit 5339cc5
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 24 deletions.
diff --git 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,7 @@ 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);
+        self::createMembershipLog($dao2);
         $updateCount++;
       }
     }
@@ -2571,4 +2552,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
@@ -600,6 +600,20 @@ public function testUpdateAllMembershipStatusConvertExpiredOverriddenStatusToNor
 
     $this->assertEquals($createdMembershipID, $membershipAfterProcess['id']);
     $this->assertArrayNotHasKey('status_override_end_date', $membershipAfterProcess);
+
+    // Check that MembershipLog was created and is correct
+    $latestMembershipLog = \Civi\Api4\MembershipLog::get(FALSE)
+      ->addWhere('membership_id', '=', $createdMembershipID)
+      ->addOrderBy('id', 'DESC')
+      ->execute()
+      ->first();
+    $currentMembershipStatus = \Civi\Api4\MembershipStatus::get(FALSE)
+      ->addSelect('id')
+      ->addWhere('name', '=', 'Current')
+      ->execute()
+      ->first();
+    $this->assertEquals($currentMembershipStatus['id'], $latestMembershipLog['status_id']);
+
   }
 
   /**