From f3601685862ca56157dcc027acd000c910d271c3 Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Tue, 12 May 2020 11:56:56 -0400
Subject: [PATCH] Cleanup PriceFieldValue BAO

- Use writeRecord() helper function
- Remove $ids param from add()
- Make sense of previously unused $priceFieldID variable
---
 CRM/Member/BAO/MembershipType.php |  5 ++---
 CRM/Price/BAO/PriceFieldValue.php | 28 ++++++----------------------
 2 files changed, 8 insertions(+), 25 deletions(-)

diff --git a/CRM/Member/BAO/MembershipType.php b/CRM/Member/BAO/MembershipType.php
index 8a56e9bde5..09b3eef0c4 100644
--- a/CRM/Member/BAO/MembershipType.php
+++ b/CRM/Member/BAO/MembershipType.php
@@ -740,7 +740,6 @@ class CRM_Member_BAO_MembershipType extends CRM_Member_DAO_MembershipType {
       if (!empty($results)) {
         $results['label'] = $results['name'] = $params['name'];
         $results['amount'] = empty($params['minimum_fee']) ? 0 : $params['minimum_fee'];
-        $optionsIds['id'] = $results['id'];
       }
       else {
         $results = [
@@ -756,12 +755,12 @@ class CRM_Member_BAO_MembershipType extends CRM_Member_DAO_MembershipType {
       if ($previousID) {
         CRM_Member_Form_MembershipType::checkPreviousPriceField($previousID, $priceSetId, $membershipTypeId, $optionsIds);
         if (!empty($optionsIds['option_id'])) {
-          $optionsIds['id'] = current(CRM_Utils_Array::value('option_id', $optionsIds));
+          $results['id'] = current($optionsIds['option_id']);
         }
       }
       $results['financial_type_id'] = $params['financial_type_id'] ?? NULL;
       $results['description'] = $params['description'] ?? NULL;
-      CRM_Price_BAO_PriceFieldValue::add($results, $optionsIds);
+      CRM_Price_BAO_PriceFieldValue::add($results);
     }
   }
 
diff --git a/CRM/Price/BAO/PriceFieldValue.php b/CRM/Price/BAO/PriceFieldValue.php
index 34d431210a..7806a6c4dc 100644
--- a/CRM/Price/BAO/PriceFieldValue.php
+++ b/CRM/Price/BAO/PriceFieldValue.php
@@ -26,35 +26,18 @@ class CRM_Price_BAO_PriceFieldValue extends CRM_Price_DAO_PriceFieldValue {
    *
    * @param array $params
    *
-   * @param array $ids
-   *  Deprecated variable.
-   *
    * @return CRM_Price_DAO_PriceFieldValue
    */
-  public static function add(&$params, $ids = []) {
-    $hook = empty($params['id']) ? 'create' : 'edit';
-    CRM_Utils_Hook::pre($hook, 'PriceFieldValue', CRM_Utils_Array::value('id', $params), $params);
-
-    $fieldValueBAO = new CRM_Price_BAO_PriceFieldValue();
-    $fieldValueBAO->copyValues($params);
-
-    // CRM-16189
-    $priceFieldID = $params['price_field_id'] ?? NULL;
+  public static function add($params) {
+    $fieldValueBAO = self::writeRecord($params);
 
-    $id = CRM_Utils_Array::value('id', $ids, CRM_Utils_Array::value('id', $params));
-
-    if (!$priceFieldID) {
-      $priceFieldID = CRM_Core_DAO::getFieldValue('CRM_Price_BAO_PriceFieldValue', $id, 'price_field_id');
-    }
     if (!empty($params['is_default'])) {
+      $priceFieldID = $params['price_field_id'] ?? CRM_Core_DAO::getFieldValue('CRM_Price_BAO_PriceFieldValue', $fieldValueBAO->id, 'price_field_id');
       $query = 'UPDATE civicrm_price_field_value SET is_default = 0 WHERE  price_field_id = %1';
-      $p = [1 => [$params['price_field_id'], 'Integer']];
+      $p = [1 => [$priceFieldID, 'Integer']];
       CRM_Core_DAO::executeQuery($query, $p);
     }
 
-    $fieldValueBAO->save();
-    CRM_Utils_Hook::post($hook, 'PriceFieldValue', $fieldValueBAO->id, $fieldValueBAO);
-
     // Reset the cached values in this function.
     CRM_Price_BAO_PriceField::getOptions(CRM_Utils_Array::value('price_field_id', $params), FALSE, TRUE);
     return $fieldValueBAO;
@@ -110,7 +93,8 @@ class CRM_Price_BAO_PriceFieldValue extends CRM_Price_DAO_PriceFieldValue {
     if (!empty($financialType) && !array_key_exists($financialType, $financialTypes) && $params['is_active']) {
       throw new CRM_Core_Exception("Financial Type for Price Field Option is either disabled or does not exist");
     }
-    return self::add($params, $ids);
+    $params['id'] = $id;
+    return self::add($params);
   }
 
   /**
-- 
2.25.1