From 5a443458b6e14de9c21ded8cc3f1976228a47524 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Thu, 6 Jan 2022 17:46:10 -0500 Subject: [PATCH] APIv4 - Refactor writeObjects to choose BAO function based on annotations Instead of using a hardcoded list of BAO functions to call, with extra hardcoded params to e.g. the Contact & Address BAOs, this switches to picking the BAO's create/add only if it's not `@deprecated`, and falling back to WriteRecords. Also refactors the large `writeObjects` function into two smaller functions so individual action classes can more easily override them. --- Civi/Api4/Action/Address/AddressSaveTrait.php | 8 +- .../Action/CiviCase/CiviCaseSaveTrait.php | 17 ++-- Civi/Api4/Action/Contact/Save.php | 36 +++++++++ Civi/Api4/Action/Contact/Update.php | 34 ++++++++ Civi/Api4/Action/EntityTag/Create.php | 21 +++++ .../Action/EntityTag/EntityTagSaveTrait.php | 34 ++++++++ Civi/Api4/Action/EntityTag/Save.php | 21 +++++ Civi/Api4/Action/EntityTag/Update.php | 21 +++++ .../GroupContact/GroupContactSaveTrait.php | 4 +- Civi/Api4/Contact.php | 18 +++++ Civi/Api4/EntityTag.php | 27 +++++++ .../Generic/Traits/CustomValueActionTrait.php | 2 +- Civi/Api4/Generic/Traits/DAOActionTrait.php | 77 ++++++++----------- 13 files changed, 261 insertions(+), 59 deletions(-) create mode 100644 Civi/Api4/Action/Contact/Save.php create mode 100644 Civi/Api4/Action/Contact/Update.php create mode 100644 Civi/Api4/Action/EntityTag/Create.php create mode 100644 Civi/Api4/Action/EntityTag/EntityTagSaveTrait.php create mode 100644 Civi/Api4/Action/EntityTag/Save.php create mode 100644 Civi/Api4/Action/EntityTag/Update.php diff --git a/Civi/Api4/Action/Address/AddressSaveTrait.php b/Civi/Api4/Action/Address/AddressSaveTrait.php index 1a2746f1db..de055b08f5 100644 --- a/Civi/Api4/Action/Address/AddressSaveTrait.php +++ b/Civi/Api4/Action/Address/AddressSaveTrait.php @@ -47,14 +47,16 @@ trait AddressSaveTrait { /** * @inheritDoc */ - protected function writeObjects(&$items) { - foreach ($items as &$item) { + protected function write(array $items) { + $saved = []; + foreach ($items as $item) { if ($this->streetParsing && !empty($item['street_address'])) { $item = array_merge($item, \CRM_Core_BAO_Address::parseStreetAddress($item['street_address'])); } $item['skip_geocode'] = $this->skipGeocode; + $saved[] = \CRM_Core_BAO_Address::add($item, $this->fixAddress); } - return parent::writeObjects($items); + return $saved; } } diff --git a/Civi/Api4/Action/CiviCase/CiviCaseSaveTrait.php b/Civi/Api4/Action/CiviCase/CiviCaseSaveTrait.php index b112a4a7c0..6b50e75936 100644 --- a/Civi/Api4/Action/CiviCase/CiviCaseSaveTrait.php +++ b/Civi/Api4/Action/CiviCase/CiviCaseSaveTrait.php @@ -18,20 +18,19 @@ namespace Civi\Api4\Action\CiviCase; trait CiviCaseSaveTrait { /** - * @param array $cases + * @param array $items * @return array */ - protected function writeObjects(&$cases) { - $cases = array_values($cases); - $result = parent::writeObjects($cases); - - // If the case doesn't have an id, it's new & needs to be opened. - foreach ($cases as $idx => $case) { + protected function write(array $items) { + $saved = []; + foreach ($items as $case) { + $saved[] = $result = \CRM_Case_BAO_Case::create($case); + // If the case doesn't have an id, it's new & needs to be opened. if (empty($case['id'])) { - $this->openCase($case, $result[$idx]['id']); + $this->openCase($case, $result->id); } } - return $result; + return $saved; } /** diff --git a/Civi/Api4/Action/Contact/Save.php b/Civi/Api4/Action/Contact/Save.php new file mode 100644 index 0000000000..5c08909395 --- /dev/null +++ b/Civi/Api4/Action/Contact/Save.php @@ -0,0 +1,36 @@ +method; $item['tracking'] = $this->tracking; } - return parent::writeObjects($items); + return \CRM_Contact_BAO_GroupContact::writeRecords($items); } } diff --git a/Civi/Api4/Contact.php b/Civi/Api4/Contact.php index 34f6463f55..800d8f6aec 100644 --- a/Civi/Api4/Contact.php +++ b/Civi/Api4/Contact.php @@ -26,6 +26,24 @@ namespace Civi\Api4; */ class Contact extends Generic\DAOEntity { + /** + * @param bool $checkPermissions + * @return Action\Contact\Update + */ + public static function update($checkPermissions = TRUE) { + return (new Action\Contact\Update(__CLASS__, __FUNCTION__)) + ->setCheckPermissions($checkPermissions); + } + + /** + * @param bool $checkPermissions + * @return Action\Contact\Save + */ + public static function save($checkPermissions = TRUE) { + return (new Action\Contact\Save(__CLASS__, __FUNCTION__)) + ->setCheckPermissions($checkPermissions); + } + /** * @param bool $checkPermissions * @return Action\Contact\Delete diff --git a/Civi/Api4/EntityTag.php b/Civi/Api4/EntityTag.php index 5707098187..d0d8f22a8f 100644 --- a/Civi/Api4/EntityTag.php +++ b/Civi/Api4/EntityTag.php @@ -21,4 +21,31 @@ namespace Civi\Api4; class EntityTag extends Generic\DAOEntity { use Generic\Traits\EntityBridge; + /** + * @param bool $checkPermissions + * @return Action\EntityTag\Create + */ + public static function create($checkPermissions = TRUE) { + return (new Action\EntityTag\Create('EntityTag', __FUNCTION__)) + ->setCheckPermissions($checkPermissions); + } + + /** + * @param bool $checkPermissions + * @return Action\EntityTag\Save + */ + public static function save($checkPermissions = TRUE) { + return (new Action\EntityTag\Save('EntityTag', __FUNCTION__)) + ->setCheckPermissions($checkPermissions); + } + + /** + * @param bool $checkPermissions + * @return Action\EntityTag\Update + */ + public static function update($checkPermissions = TRUE) { + return (new Action\EntityTag\Update('EntityTag', __FUNCTION__)) + ->setCheckPermissions($checkPermissions); + } + } diff --git a/Civi/Api4/Generic/Traits/CustomValueActionTrait.php b/Civi/Api4/Generic/Traits/CustomValueActionTrait.php index 280f5bd75b..e194ef6af4 100644 --- a/Civi/Api4/Generic/Traits/CustomValueActionTrait.php +++ b/Civi/Api4/Generic/Traits/CustomValueActionTrait.php @@ -63,7 +63,7 @@ trait CustomValueActionTrait { /** * @inheritDoc */ - protected function writeObjects(&$items) { + protected function writeObjects($items) { $fields = $this->entityFields(); foreach ($items as $idx => $item) { FormattingUtil::formatWriteParams($item, $fields); diff --git a/Civi/Api4/Generic/Traits/DAOActionTrait.php b/Civi/Api4/Generic/Traits/DAOActionTrait.php index ced33ad1ab..201f4c2ab9 100644 --- a/Civi/Api4/Generic/Traits/DAOActionTrait.php +++ b/Civi/Api4/Generic/Traits/DAOActionTrait.php @@ -16,6 +16,7 @@ use Civi\Api4\CustomField; use Civi\Api4\Service\Schema\Joinable\CustomGroupJoinable; use Civi\Api4\Utils\FormattingUtil; use Civi\Api4\Utils\CoreUtil; +use Civi\Api4\Utils\ReflectionUtils; /** * @method string getLanguage() @@ -105,24 +106,8 @@ trait DAOActionTrait { * @throws \API_Exception * @throws \CRM_Core_Exception */ - protected function writeObjects(&$items) { - $baoName = $this->getBaoName(); + protected function writeObjects($items) { $updateWeights = FALSE; - - // TODO: Opt-in more entities to use the new writeRecords BAO method. - $functionNames = [ - 'Address' => 'add', - 'CustomField' => 'writeRecords', - 'EntityTag' => 'add', - 'GroupContact' => 'add', - 'Navigation' => 'writeRecords', - 'WordReplacement' => 'writeRecords', - ]; - $method = $functionNames[$this->getEntityName()] ?? NULL; - if (!isset($method)) { - $method = method_exists($baoName, 'create') ? 'create' : (method_exists($baoName, 'add') ? 'add' : 'writeRecords'); - } - // Adjust weights for sortable entities if (in_array('SortableEntity', CoreUtil::getInfoItem($this->getEntityName(), 'type'))) { $weightField = CoreUtil::getInfoItem($this->getEntityName(), 'order_by'); @@ -145,39 +130,18 @@ trait DAOActionTrait { $this->updateWeight($item); } - // Skip individual processing if using writeRecords - if ($method === 'writeRecords') { - continue; - } $item['check_permissions'] = $this->getCheckPermissions(); + } - // For some reason the contact bao requires this - if ($entityId && $this->getEntityName() === 'Contact') { - $item['contact_id'] = $entityId; - } - - if ($this->getEntityName() === 'Address') { - $createResult = $baoName::$method($item, $this->fixAddress); - } - else { - $createResult = $baoName::$method($item); - } + // Ensure array keys start at 0 + $items = array_values($items); - if (!$createResult) { + foreach ($this->write($items) as $index => $dao) { + if (!$dao) { $errMessage = sprintf('%s write operation failed', $this->getEntityName()); throw new \API_Exception($errMessage); } - - $result[] = $this->baoToArray($createResult, $item); - } - - // Use bulk `writeRecords` method if the BAO doesn't have a create or add method - // TODO: reverse this from opt-in to opt-out and default to using `writeRecords` for all BAOs - if ($method === 'writeRecords') { - $items = array_values($items); - foreach ($baoName::writeRecords($items) as $i => $createResult) { - $result[] = $this->baoToArray($createResult, $items[$i]); - } + $result[] = $this->baoToArray($dao, $items[$index]); } \CRM_Utils_API_HTMLInputCoder::singleton()->decodeRows($result); @@ -185,6 +149,31 @@ trait DAOActionTrait { return $result; } + /** + * Overrideable function to save items using the appropriate BAO function + * + * @param array[] $items + * Items already formatted by self::writeObjects + * @return \CRM_Core_DAO[] + * Array of saved DAO records + */ + protected function write(array $items) { + $saved = []; + $baoName = $this->getBaoName(); + + $method = method_exists($baoName, 'create') ? 'create' : (method_exists($baoName, 'add') ? 'add' : NULL); + // Use BAO create or add method if not deprecated + if ($method && !ReflectionUtils::isMethodDeprecated($baoName, $method)) { + foreach ($items as $item) { + $saved[] = $baoName::$method($item); + } + } + else { + $saved = $baoName::writeRecords($items); + } + return $saved; + } + /** * @inheritDoc */ -- 2.25.1