From 0419bf7b41a6c5b5ac22a31bf0b0589dc649e1b3 Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Tue, 12 May 2020 12:55:26 -0400
Subject: [PATCH] Various BAO create/add cleanups to use writeRecord()

---
 CRM/Batch/BAO/Batch.php                       | 14 +----
 CRM/Campaign/BAO/Campaign.php                 | 25 ++-------
 CRM/Campaign/BAO/Survey.php                   | 31 +++--------
 CRM/Case/BAO/CaseContact.php                  |  9 +---
 CRM/Contact/BAO/RelationshipType.php          | 10 +---
 CRM/Core/BAO/ActionLog.php                    | 25 +--------
 CRM/Core/BAO/Phone.php                        | 10 +---
 CRM/Pledge/BAO/PledgeBlock.php                | 52 +++----------------
 CRM/Pledge/BAO/PledgePayment.php              | 26 ++--------
 .../CRM/Pledge/BAO/PledgeBlockTest.php        | 10 +++-
 10 files changed, 35 insertions(+), 177 deletions(-)

diff --git a/CRM/Batch/BAO/Batch.php b/CRM/Batch/BAO/Batch.php
index c777fa4acb..f44c0e4999 100644
--- a/CRM/Batch/BAO/Batch.php
+++ b/CRM/Batch/BAO/Batch.php
@@ -42,20 +42,10 @@ class CRM_Batch_BAO_Batch extends CRM_Batch_DAO_Batch {
    *   $batch batch object
    */
   public static function create(&$params) {
-    $op = 'edit';
-    $batchId = $params['id'] ?? NULL;
-    if (!$batchId) {
-      $op = 'create';
+    if (empty($params['id']) && empty($params['name'])) {
       $params['name'] = CRM_Utils_String::titleToVar($params['title']);
     }
-    CRM_Utils_Hook::pre($op, 'Batch', $batchId, $params);
-    $batch = new CRM_Batch_DAO_Batch();
-    $batch->copyValues($params);
-    $batch->save();
-
-    CRM_Utils_Hook::post($op, 'Batch', $batch->id, $batch);
-
-    return $batch;
+    return self::writeRecord($params);
   }
 
   /**
diff --git a/CRM/Campaign/BAO/Campaign.php b/CRM/Campaign/BAO/Campaign.php
index 4ceeec2ea8..7800cf925d 100644
--- a/CRM/Campaign/BAO/Campaign.php
+++ b/CRM/Campaign/BAO/Campaign.php
@@ -34,10 +34,8 @@ class CRM_Campaign_BAO_Campaign extends CRM_Campaign_DAO_Campaign {
     }
 
     if (empty($params['id'])) {
-
       if (empty($params['created_id'])) {
-        $session = CRM_Core_Session::singleton();
-        $params['created_id'] = $session->get('userID');
+        $params['created_id'] = CRM_Core_Session::getLoggedInContactID();
       }
 
       if (empty($params['created_date'])) {
@@ -47,26 +45,11 @@ class CRM_Campaign_BAO_Campaign extends CRM_Campaign_DAO_Campaign {
       if (empty($params['name'])) {
         $params['name'] = CRM_Utils_String::titleToVar($params['title'], 64);
       }
-
-      CRM_Utils_Hook::pre('create', 'Campaign', NULL, $params);
-    }
-    else {
-      CRM_Utils_Hook::pre('edit', 'Campaign', $params['id'], $params);
     }
 
-    $campaign = new CRM_Campaign_DAO_Campaign();
-    $campaign->copyValues($params);
-    $campaign->save();
-
-    if (!empty($params['id'])) {
-      CRM_Utils_Hook::post('edit', 'Campaign', $campaign->id, $campaign);
-    }
-    else {
-      CRM_Utils_Hook::post('create', 'Campaign', $campaign->id, $campaign);
-    }
+    $campaign = self::writeRecord($params);
 
     /* Create the campaign group record */
-
     $groupTableName = CRM_Contact_BAO_Group::getTableName();
 
     if (isset($params['groups']) && !empty($params['groups']['include']) && is_array($params['groups']['include'])) {
@@ -81,9 +64,7 @@ class CRM_Campaign_BAO_Campaign extends CRM_Campaign_DAO_Campaign {
     }
 
     //store custom data
-    if (!empty($params['custom']) &&
-      is_array($params['custom'])
-    ) {
+    if (!empty($params['custom']) && is_array($params['custom'])) {
       CRM_Core_BAO_CustomValueTable::store($params['custom'], 'civicrm_campaign', $campaign->id);
     }
 
diff --git a/CRM/Campaign/BAO/Survey.php b/CRM/Campaign/BAO/Survey.php
index 9277e4f040..93b487982b 100644
--- a/CRM/Campaign/BAO/Survey.php
+++ b/CRM/Campaign/BAO/Survey.php
@@ -62,36 +62,19 @@ class CRM_Campaign_BAO_Survey extends CRM_Campaign_DAO_Survey {
       CRM_Core_DAO::executeQuery($query);
     }
 
-    if (!(CRM_Utils_Array::value('id', $params))) {
-
-      if (!(CRM_Utils_Array::value('created_id', $params))) {
-        $session = CRM_Core_Session::singleton();
-        $params['created_id'] = $session->get('userID');
+    if (empty($params['id'])) {
+      if (empty($params['created_id'])) {
+        $params['created_id'] = CRM_Core_Session::getLoggedInContactID();
       }
-      if (!(CRM_Utils_Array::value('created_date', $params))) {
+
+      if (empty($params['created_date'])) {
         $params['created_date'] = date('YmdHis');
       }
-
-      CRM_Utils_Hook::pre('create', 'Survey', NULL, $params);
     }
-    else {
-      CRM_Utils_Hook::pre('edit', 'Survey', $params['id'], $params);
-    }
-
-    $dao = new CRM_Campaign_DAO_Survey();
-    $dao->copyValues($params);
-    $dao->save();
 
-    if (!empty($params['id'])) {
-      CRM_Utils_Hook::post('edit', 'Survey', $dao->id, $dao);
-    }
-    else {
-      CRM_Utils_Hook::post('create', 'Survey', $dao->id, $dao);
-    }
+    $dao = self::writeRecord($params);
 
-    if (!empty($params['custom']) &&
-      is_array($params['custom'])
-    ) {
+    if (!empty($params['custom']) && is_array($params['custom'])) {
       CRM_Core_BAO_CustomValueTable::store($params['custom'], 'civicrm_survey', $dao->id);
     }
     return $dao;
diff --git a/CRM/Case/BAO/CaseContact.php b/CRM/Case/BAO/CaseContact.php
index 6e5ed5779b..3b409bb65c 100644
--- a/CRM/Case/BAO/CaseContact.php
+++ b/CRM/Case/BAO/CaseContact.php
@@ -29,14 +29,7 @@ class CRM_Case_BAO_CaseContact extends CRM_Case_DAO_CaseContact {
    * @return CRM_Case_BAO_CaseContact
    */
   public static function create($params) {
-    $hook = empty($params['id']) ? 'create' : 'edit';
-    CRM_Utils_Hook::pre($hook, 'CaseContact', CRM_Utils_Array::value('id', $params), $params);
-
-    $caseContact = new self();
-    $caseContact->copyValues($params);
-    $caseContact->save();
-
-    CRM_Utils_Hook::post($hook, 'CaseContact', $caseContact->id, $caseContact);
+    $caseContact = self::writeRecord($params);
 
     // add to recently viewed
     $caseType = CRM_Case_BAO_Case::getCaseType($caseContact->case_id);
diff --git a/CRM/Contact/BAO/RelationshipType.php b/CRM/Contact/BAO/RelationshipType.php
index 6c44ea5900..08b0d81032 100644
--- a/CRM/Contact/BAO/RelationshipType.php
+++ b/CRM/Contact/BAO/RelationshipType.php
@@ -85,15 +85,7 @@ class CRM_Contact_BAO_RelationshipType extends CRM_Contact_DAO_RelationshipType
     }
 
     // action is taken depending upon the mode
-    $relationshipType = new CRM_Contact_DAO_RelationshipType();
-
-    $hook = empty($params['id']) ? 'create' : 'edit';
-    CRM_Utils_Hook::pre($hook, 'RelationshipType', CRM_Utils_Array::value('id', $params), $params);
-
-    $relationshipType->copyValues($params);
-    $relationshipType->save();
-
-    CRM_Utils_Hook::post($hook, 'RelationshipType', $relationshipType->id, $relationshipType);
+    $relationshipType = self::writeRecord($params);
 
     CRM_Core_PseudoConstant::relationshipType('label', TRUE);
     CRM_Core_PseudoConstant::relationshipType('name', TRUE);
diff --git a/CRM/Core/BAO/ActionLog.php b/CRM/Core/BAO/ActionLog.php
index c04e3f30cc..877c88b8ae 100644
--- a/CRM/Core/BAO/ActionLog.php
+++ b/CRM/Core/BAO/ActionLog.php
@@ -30,30 +30,9 @@ class CRM_Core_BAO_ActionLog extends CRM_Core_DAO_ActionLog {
    * @return array
    */
   public static function create($params) {
-    $actionLog = new CRM_Core_DAO_ActionLog();
+    $params['action_date_time'] = $params['action_date_time'] ?? date('YmdHis');
 
-    $params['action_date_time'] = CRM_Utils_Array::value('action_date_time', $params, date('YmdHis'));
-
-    $actionLog->copyValues($params);
-
-    $edit = (bool) $actionLog->id;
-    if ($edit) {
-      CRM_Utils_Hook::pre('edit', 'ActionLog', $actionLog->id, $actionLog);
-    }
-    else {
-      CRM_Utils_Hook::pre('create', 'ActionLog', NULL, $actionLog);
-    }
-
-    $actionLog->save();
-
-    if ($edit) {
-      CRM_Utils_Hook::post('edit', 'ActionLog', $actionLog->id, $actionLog);
-    }
-    else {
-      CRM_Utils_Hook::post('create', 'ActionLog', NULL, $actionLog);
-    }
-
-    return $actionLog;
+    return self::writeRecord($params);
   }
 
 }
diff --git a/CRM/Core/BAO/Phone.php b/CRM/Core/BAO/Phone.php
index bf4dd2c043..f5851eee26 100644
--- a/CRM/Core/BAO/Phone.php
+++ b/CRM/Core/BAO/Phone.php
@@ -57,15 +57,7 @@ class CRM_Core_BAO_Phone extends CRM_Core_DAO_Phone {
     // Ensure mysql phone function exists
     CRM_Core_DAO::checkSqlFunctionsExist();
 
-    $hook = empty($params['id']) ? 'create' : 'edit';
-    CRM_Utils_Hook::pre($hook, 'Phone', $params['id'] ?? NULL, $params);
-
-    $phone = new CRM_Core_DAO_Phone();
-    $phone->copyValues($params);
-    $phone->save();
-
-    CRM_Utils_Hook::post($hook, 'Phone', $phone->id, $phone);
-    return $phone;
+    return self::writeRecord($params);
   }
 
   /**
diff --git a/CRM/Pledge/BAO/PledgeBlock.php b/CRM/Pledge/BAO/PledgeBlock.php
index 8d363e06f9..a3bac518b5 100644
--- a/CRM/Pledge/BAO/PledgeBlock.php
+++ b/CRM/Pledge/BAO/PledgeBlock.php
@@ -70,58 +70,18 @@ class CRM_Pledge_BAO_PledgeBlock extends CRM_Pledge_DAO_PledgeBlock {
   }
 
   /**
-   * Add pledgeBlock.
+   * Add or update pledgeBlock.
    *
    * @param array $params
-   *   Reference array contains the values submitted by the form.
-   *
    *
    * @return object
    */
-  public static function add(&$params) {
-
-    if (!empty($params['id'])) {
-      CRM_Utils_Hook::pre('edit', 'PledgeBlock', $params['id'], $params);
-    }
-    else {
-      CRM_Utils_Hook::pre('create', 'PledgeBlock', NULL, $params);
-    }
-
-    $pledgeBlock = new CRM_Pledge_DAO_PledgeBlock();
-
-    // fix for pledge_frequency_unit
-    $freqUnits = $params['pledge_frequency_unit'] ?? NULL;
-
-    if ($freqUnits && is_array($freqUnits)) {
-      unset($params['pledge_frequency_unit']);
-      $newFreqUnits = array();
-      foreach ($freqUnits as $k => $v) {
-        if ($v) {
-          $newFreqUnits[$k] = $v;
-        }
-      }
-
-      $freqUnits = $newFreqUnits;
-      if (is_array($freqUnits) && !empty($freqUnits)) {
-        $freqUnits = implode(CRM_Core_DAO::VALUE_SEPARATOR, array_keys($freqUnits));
-        $pledgeBlock->pledge_frequency_unit = $freqUnits;
-      }
-      else {
-        $pledgeBlock->pledge_frequency_unit = '';
-      }
+  public static function add($params) {
+    // FIXME: This is assuming checkbox input like ['foo' => 1, 'bar' => 0, 'baz' => 1]. Not API friendly.
+    if (!empty($params['pledge_frequency_unit']) && is_array($params['pledge_frequency_unit'])) {
+      $params['pledge_frequency_unit'] = array_keys(array_filter($params['pledge_frequency_unit']));
     }
-
-    $pledgeBlock->copyValues($params);
-    $result = $pledgeBlock->save();
-
-    if (!empty($params['id'])) {
-      CRM_Utils_Hook::post('edit', 'PledgeBlock', $pledgeBlock->id, $pledgeBlock);
-    }
-    else {
-      CRM_Utils_Hook::post('create', 'Pledge', $pledgeBlock->id, $pledgeBlock);
-    }
-
-    return $result;
+    return self::writeRecord($params);
   }
 
   /**
diff --git a/CRM/Pledge/BAO/PledgePayment.php b/CRM/Pledge/BAO/PledgePayment.php
index 38f947cc69..d049f9e55a 100644
--- a/CRM/Pledge/BAO/PledgePayment.php
+++ b/CRM/Pledge/BAO/PledgePayment.php
@@ -154,31 +154,11 @@ WHERE     pledge_id = %1
    *   pledge payment id
    */
   public static function add($params) {
-    if (!empty($params['id'])) {
-      CRM_Utils_Hook::pre('edit', 'PledgePayment', $params['id'], $params);
-    }
-    else {
-      CRM_Utils_Hook::pre('create', 'PledgePayment', NULL, $params);
-    }
-
-    $payment = new CRM_Pledge_DAO_PledgePayment();
-    $payment->copyValues($params);
-
     // set currency for CRM-1496
-    if (!isset($payment->currency)) {
-      $payment->currency = CRM_Core_Config::singleton()->defaultCurrency;
+    if (empty($params['id']) && !isset($params['currency'])) {
+      $params['currency'] = CRM_Core_Config::singleton()->defaultCurrency;
     }
-
-    $result = $payment->save();
-
-    if (!empty($params['id'])) {
-      CRM_Utils_Hook::post('edit', 'PledgePayment', $payment->id, $payment);
-    }
-    else {
-      CRM_Utils_Hook::post('create', 'PledgePayment', $payment->id, $payment);
-    }
-
-    return $result;
+    return self::writeRecord($params);
   }
 
   /**
diff --git a/tests/phpunit/CRM/Pledge/BAO/PledgeBlockTest.php b/tests/phpunit/CRM/Pledge/BAO/PledgeBlockTest.php
index 2a9cc2cf77..6832bb8965 100644
--- a/tests/phpunit/CRM/Pledge/BAO/PledgeBlockTest.php
+++ b/tests/phpunit/CRM/Pledge/BAO/PledgeBlockTest.php
@@ -74,9 +74,10 @@ class CRM_Pledge_BAO_PledgeBlockTest extends CiviUnitTestCase {
 
     $pledgeFrequencyUnit = [
       'week' => 1,
-      'month' => 1,
+      'month' => 0,
       'year' => 1,
     ];
+    $pledgeFrequencySerialized = implode(CRM_Core_DAO::VALUE_SEPARATOR, array_keys(array_filter($pledgeFrequencyUnit)));
 
     $params = [
       'entity_id' => $this->_contributionPageId,
@@ -89,6 +90,8 @@ class CRM_Pledge_BAO_PledgeBlockTest extends CiviUnitTestCase {
 
     // check for add pledge block
     $pledgeBlock = CRM_Pledge_BAO_PledgeBlock::add($params);
+    // This param is expected to get serialized
+    $params['pledge_frequency_unit'] = $pledgeFrequencySerialized;
     foreach ($params as $param => $value) {
       $this->assertEquals($value, $pledgeBlock->$param);
     }
@@ -106,6 +109,8 @@ class CRM_Pledge_BAO_PledgeBlockTest extends CiviUnitTestCase {
 
     // also check for edit pledge block
     $pledgeBlock = CRM_Pledge_BAO_PledgeBlock::add($params);
+    // This param is expected to get serialized
+    $params['pledge_frequency_unit'] = $pledgeFrequencySerialized;
     foreach ($params as $param => $value) {
       $this->assertEquals($value, $pledgeBlock->$param);
     }
@@ -121,6 +126,7 @@ class CRM_Pledge_BAO_PledgeBlockTest extends CiviUnitTestCase {
       'month' => 1,
       'year' => 1,
     ];
+    $pledgeFrequencySerialized = implode(CRM_Core_DAO::VALUE_SEPARATOR, array_keys(array_filter($pledgeFrequencyUnit)));
 
     $params = [
       'entity_id' => $this->_contributionPageId,
@@ -144,6 +150,8 @@ class CRM_Pledge_BAO_PledgeBlockTest extends CiviUnitTestCase {
     // use getPledgeBlock() method
     $getPledgeBlock = CRM_Pledge_BAO_PledgeBlock::getPledgeBlock($this->_contributionPageId);
 
+    // This param is expected to get serialized
+    $params['pledge_frequency_unit'] = $pledgeFrequencySerialized;
     // check on both retrieve and getPledgeBlock values
     foreach ($params as $param => $value) {
       $this->assertEquals($value, $retrievePledgeBlock->$param);
-- 
2.25.1