Simply buildMembershipTypeValues & access cached values

This simplifies this function & switches it to use a cached function to get the MembershipTypes. In general I would
argue that we should be caching metadata entities (membership types, statues, price fields, etc) and accessing
the cached versions & ideally the v4 api would be able to access cached versions filter-dependent.

In order to keep the test passing I worked a little on standardisation

CRM/Core/Config.php

@@ -242,7 +242,7 @@ public static function domainID($domainID = NULL, $reset = FALSE) {
       $domain = defined('CIVICRM_DOMAIN_ID') ? CIVICRM_DOMAIN_ID : 1;
     }
 
-    return $domain;
+    return (int) $domain;
   }
 
   /**

CRM/Member/BAO/Membership.php

@@ -1537,55 +1537,19 @@ public static function deleteMembershipPayment($membershipId, $preserveContrib =
    *   behaviour unchanged).
    *
    * @return array
+   *
+   * @throws \CiviCRM_API3_Exception
    */
-  public static function buildMembershipTypeValues(&$form, $membershipTypeID = [], $activeOnly = FALSE) {
-    $whereClause = " WHERE domain_id = " . CRM_Core_Config::domainID();
+  public static function buildMembershipTypeValues($form, $membershipTypeID = [], $activeOnly = FALSE) {
     $membershipTypeIDS = (array) $membershipTypeID;
+    $membershipTypeValues = CRM_Member_BAO_MembershipType::getPermissionedMembershipTypes();
 
-    if ($activeOnly) {
-      $whereClause .= " AND is_active = 1 ";
-    }
-    if (!empty($membershipTypeIDS)) {
-      $allIDs = implode(',', $membershipTypeIDS);
-      $whereClause .= " AND id IN ( $allIDs )";
-    }
-    CRM_Financial_BAO_FinancialType::getAvailableFinancialTypes($financialTypes, CRM_Core_Action::ADD);
-
-    if ($financialTypes) {
-      $whereClause .= " AND financial_type_id IN (" . implode(',', array_keys($financialTypes)) . ")";
-    }
-    else {
-      $whereClause .= " AND financial_type_id IN (0)";
-    }
-
-    $query = "
-SELECT *
-FROM   civicrm_membership_type
-       $whereClause;
-";
-    $dao = CRM_Core_DAO::executeQuery($query);
-
-    $membershipTypeValues = [];
-    $membershipTypeFields = [
-      'id',
-      'minimum_fee',
-      'name',
-      'is_active',
-      'description',
-      'financial_type_id',
-      'auto_renew',
-      'member_of_contact_id',
-      'relationship_type_id',
-      'relationship_direction',
-      'max_related',
-      'duration_unit',
-      'duration_interval',
-    ];
-
-    while ($dao->fetch()) {
-      $membershipTypeValues[$dao->id] = [];
-      foreach ($membershipTypeFields as $mtField) {
-        $membershipTypeValues[$dao->id][$mtField] = $dao->$mtField;
+    // MembershipTypes are already filtered by domain, filter as appropriate by is_active & a passed in list of ids.
+    foreach ($membershipTypeValues as $id => $type) {
+      if (($activeOnly && empty($type['is_active']))
+        || (!empty($membershipTypeIDS) && !in_array($id, $membershipTypeIDS, FALSE))
+      ) {
+        unset($membershipTypeValues[$id]);
       }
     }
 
@@ -1594,11 +1558,10 @@ public static function buildMembershipTypeValues(&$form, $membershipTypeID = [],
     if (is_numeric($membershipTypeID) &&
       $membershipTypeID > 0
     ) {
+      CRM_Core_Error::deprecatedFunctionWarning('Non arrays deprecated');
       return $membershipTypeValues[$membershipTypeID];
     }
-    else {
-      return $membershipTypeValues;
-    }
+    return $membershipTypeValues;
   }
 
   /**

CRM/Member/BAO/MembershipType.php

@@ -818,7 +818,22 @@ public static function updateAllPriceFieldValue($membershipTypeId, $params) {
    */
   public static function getAllMembershipTypes() {
     if (!Civi::cache('metadata')->has(__CLASS__ . __FUNCTION__)) {
-      Civi::cache('metadata')->set(__CLASS__ . __FUNCTION__, civicrm_api3('MembershipType', 'get', ['options' => ['limit' => 0, 'sort' => 'weight']])['values']);
+      $types = civicrm_api3('MembershipType', 'get', ['options' => ['limit' => 0, 'sort' => 'weight']])['values'];
+      $keys = ['description', 'relationship_type_id', 'relationship_direction', 'max_related'];
+      // In order to avoid down-stream e-notices we undo api v3 filtering of NULL values. This is covered
+      // in Unit tests & ideally we might switch to apiv4 but I would argue we should build caching
+      // of metadata entities like this directly into apiv4.
+      foreach ($types as $id => $type) {
+        foreach ($keys as $key) {
+          if (!isset($type[$key])) {
+            $types[$id][$key] = NULL;
+          }
+        }
+        if (isset($type['contribution_type_id'])) {
+          unset($types[$id]['contribution_type_id']);
+        }
+      }
+      Civi::cache('metadata')->set(__CLASS__ . __FUNCTION__, $types);
     }
     return Civi::cache('metadata')->get(__CLASS__ . __FUNCTION__);
   }
@@ -835,4 +850,21 @@ public static function getMembershipType($id) {
     return self::getAllMembershipTypes()[$id];
   }
 
+  /**
+   * Get an array of all membership types the contact is permitted to access.
+   *
+   * @throws \CiviCRM_API3_Exception
+   */
+  public static function getPermissionedMembershipTypes() {
+    $types = self::getAllMembershipTypes();
+    $financialTypes = NULL;
+    $financialTypes = CRM_Financial_BAO_FinancialType::getAvailableFinancialTypes($financialTypes, CRM_Core_Action::ADD);
+    foreach ($types as $id => $type) {
+      if (!isset($financialTypes[$type['financial_type_id']])) {
+        unset($types[$id]);
+      }
+    }
+    return $types;
+  }
+
 }

tests/phpunit/CRM/Member/BAO/MembershipTest.php

@@ -770,8 +770,11 @@ public function testMembershipPaymentForSingleContributionMultipleMembership() {
 
   /**
    * Test the buildMembershipTypeValues function.
+   *
+   * @throws \CiviCRM_API3_Exception
    */
   public function testBuildMembershipTypeValues() {
+    $this->restoreMembershipTypes();
     $form = new CRM_Core_Form();
     $values = CRM_Member_BAO_Membership::buildMembershipTypeValues($form);
     $this->assertEquals([
@@ -783,11 +786,15 @@ public function testBuildMembershipTypeValues() {
       'financial_type_id' => '2',
       'auto_renew' => '0',
       'member_of_contact_id' => $values[1]['member_of_contact_id'],
-      'relationship_type_id' => 7,
-      'relationship_direction' => 'b_a',
+      'relationship_type_id' => [7],
+      'relationship_direction' => ['b_a'],
       'max_related' => NULL,
       'duration_unit' => 'year',
       'duration_interval' => '2',
+      'domain_id' => '1',
+      'period_type' => 'rolling',
+      'visibility' => 'Public',
+      'weight' => '1',
     ], $values[1]);
   }