From 076fe09aa31cb92fa9e254b06a32e17ce5c76e4b Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Thu, 25 Nov 2021 19:28:56 -0500 Subject: [PATCH] APIv4 - Add SortableEntity type which auto-adjusts weights This entity type will manage weight columns automatically, allowing items to be re-ordered easily. Simply by updating the weight of one record, others will auto-adjust to make room for it. --- CRM/Utils/String.php | 10 +++ CRM/Utils/Weight.php | 33 ++++---- Civi/Api4/Contact.php | 1 + Civi/Api4/CustomField.php | 2 + Civi/Api4/CustomGroup.php | 2 + Civi/Api4/Entity.php | 5 ++ Civi/Api4/Generic/AbstractEntity.php | 8 +- Civi/Api4/Generic/Traits/DAOActionTrait.php | 76 +++++++++++++++++++ Civi/Api4/Generic/Traits/SortableEntity.php | 21 +++++ Civi/Api4/MembershipStatus.php | 2 + Civi/Api4/MembershipType.php | 2 + Civi/Api4/OptionValue.php | 2 + Civi/Api4/PriceField.php | 2 + Civi/Api4/PriceFieldValue.php | 2 + Civi/Api4/UFField.php | 2 + .../api/v4/Action/BasicCustomFieldTest.php | 43 +++++++++++ .../phpunit/api/v4/Entity/MembershipTest.php | 76 +++++++++++++++++++ .../phpunit/api/v4/Entity/OptionValueTest.php | 68 +++++++++++++++++ 18 files changed, 339 insertions(+), 18 deletions(-) create mode 100644 Civi/Api4/Generic/Traits/SortableEntity.php create mode 100644 tests/phpunit/api/v4/Entity/MembershipTest.php diff --git a/CRM/Utils/String.php b/CRM/Utils/String.php index f98d4ec6d5..3400263aa0 100644 --- a/CRM/Utils/String.php +++ b/CRM/Utils/String.php @@ -99,6 +99,16 @@ class CRM_Utils_String { return $ucFirst ? $camel : lcfirst($camel); } + /** + * Inverse of above function, converts camelCase to snake_case + * + * @param string $str + * @return string + */ + public static function convertStringToSnakeCase(string $str): string { + return strtolower(ltrim(preg_replace('/(?=[A-Z])/', '_$0', $str), '_')); + } + /** * Takes a variable name and munges it randomly into another variable name. * diff --git a/CRM/Utils/Weight.php b/CRM/Utils/Weight.php index 345d6b1a80..b98353882f 100644 --- a/CRM/Utils/Weight.php +++ b/CRM/Utils/Weight.php @@ -122,14 +122,11 @@ class CRM_Utils_Weight { * @return int */ public static function updateOtherWeights($daoName, $oldWeight, $newWeight, $fieldValues = NULL, $weightField = 'weight') { - $oldWeight = (int ) $oldWeight; - $newWeight = (int ) $newWeight; + $oldWeight = (int) $oldWeight; + $newWeight = (int) $newWeight; // max weight is the highest current weight - $maxWeight = CRM_Utils_Weight::getMax($daoName, $fieldValues, $weightField); - if (!$maxWeight) { - $maxWeight = 1; - } + $maxWeight = self::getMax($daoName, $fieldValues, $weightField) ?: 1; if ($newWeight > $maxWeight) { // calculate new weight, CRM-4133 @@ -154,12 +151,18 @@ class CRM_Utils_Weight { return $newWeight; } + // Check for an existing record with this weight + $existing = self::query('SELECT', $daoName, $fieldValues, "id", "$weightField = $newWeight"); + // Nothing to do if no existing record has this weight + if (empty($existing->N)) { + return $newWeight; + } + // if oldWeight not present, indicates new weight is to be added. So create a gap for a new row to be inserted. if (!$oldWeight) { $additionalWhere = "$weightField >= $newWeight"; $update = "$weightField = ($weightField + 1)"; CRM_Utils_Weight::query('UPDATE', $daoName, $fieldValues, $update, $additionalWhere); - return $newWeight; } else { if ($newWeight > $oldWeight) { @@ -171,8 +174,8 @@ class CRM_Utils_Weight { $update = "$weightField = ($weightField + 1)"; } CRM_Utils_Weight::query('UPDATE', $daoName, $fieldValues, $update, $additionalWhere); - return $newWeight; } + return $newWeight; } /** @@ -262,7 +265,7 @@ class CRM_Utils_Weight { * * @param string $queryType * SELECT, UPDATE, DELETE. - * @param string $daoName + * @param CRM_Core_DAO|string $daoName * Full name of the DAO. * @param array $fieldValues * Field => value to be used in the WHERE. @@ -286,12 +289,8 @@ class CRM_Utils_Weight { $orderBy = NULL, $groupBy = NULL ) { - - require_once str_replace('_', DIRECTORY_SEPARATOR, $daoName) . ".php"; - - $dao = new $daoName(); - $table = $dao->getTablename(); - $fields = &$dao->fields(); + $table = $daoName::getTablename(); + $fields = $daoName::getSupportedFields(); $fieldlist = array_keys($fields); $whereConditions = []; @@ -304,7 +303,7 @@ class CRM_Utils_Weight { foreach ($fieldValues as $fieldName => $value) { if (!in_array($fieldName, $fieldlist)) { // invalid field specified. abort. - return FALSE; + throw new CRM_Core_Exception("Invalid field '$fieldName' for $daoName"); } $fieldNum++; $whereConditions[] = "$fieldName = %$fieldNum"; @@ -340,7 +339,7 @@ class CRM_Utils_Weight { break; default: - return FALSE; + throw new CRM_Core_Exception("Invalid query operation for $daoName"); } $resultDAO = CRM_Core_DAO::executeQuery($query, $params); diff --git a/Civi/Api4/Contact.php b/Civi/Api4/Contact.php index 9ec5679d95..1fe634a085 100644 --- a/Civi/Api4/Contact.php +++ b/Civi/Api4/Contact.php @@ -20,6 +20,7 @@ namespace Civi\Api4; * * @see https://docs.civicrm.org/user/en/latest/organising-your-data/contacts/ * @searchable primary + * @orderBy sort_name * @since 5.19 * @package Civi\Api4 */ diff --git a/Civi/Api4/CustomField.php b/Civi/Api4/CustomField.php index d92f51c97c..37be6eb2b4 100644 --- a/Civi/Api4/CustomField.php +++ b/Civi/Api4/CustomField.php @@ -15,10 +15,12 @@ namespace Civi\Api4; * * @see https://docs.civicrm.org/user/en/latest/organising-your-data/creating-custom-fields/ * @searchable secondary + * @orderBy weight * @since 5.19 * @package Civi\Api4 */ class CustomField extends Generic\DAOEntity { use Generic\Traits\ManagedEntity; + use Generic\Traits\SortableEntity; } diff --git a/Civi/Api4/CustomGroup.php b/Civi/Api4/CustomGroup.php index bee078777b..1aed113a48 100644 --- a/Civi/Api4/CustomGroup.php +++ b/Civi/Api4/CustomGroup.php @@ -15,10 +15,12 @@ namespace Civi\Api4; * * @see https://docs.civicrm.org/user/en/latest/organising-your-data/creating-custom-fields/ * @searchable secondary + * @orderBy weight * @since 5.19 * @package Civi\Api4 */ class CustomGroup extends Generic\DAOEntity { use Generic\Traits\ManagedEntity; + use Generic\Traits\SortableEntity; } diff --git a/Civi/Api4/Entity.php b/Civi/Api4/Entity.php index d6b0dfcc3c..52d22627c8 100644 --- a/Civi/Api4/Entity.php +++ b/Civi/Api4/Entity.php @@ -58,6 +58,7 @@ class Entity extends Generic\AbstractEntity { 'DAOEntity' => 'DAOEntity', 'CustomValue' => 'CustomValue', 'BasicEntity' => 'BasicEntity', + 'SortableEntity' => 'SortableEntity', 'ManagedEntity' => 'ManagedEntity', 'EntityBridge' => 'EntityBridge', 'OptionList' => 'OptionList', @@ -88,6 +89,10 @@ class Entity extends Generic\AbstractEntity { 'name' => 'label_field', 'description' => 'Field to show when displaying a record', ], + [ + 'name' => 'order_by', + 'description' => 'Default column to sort results', + ], [ 'name' => 'searchable', 'description' => 'How should this entity be presented in search UIs', diff --git a/Civi/Api4/Generic/AbstractEntity.php b/Civi/Api4/Generic/AbstractEntity.php index 7b4022a4d8..7211663024 100644 --- a/Civi/Api4/Generic/AbstractEntity.php +++ b/Civi/Api4/Generic/AbstractEntity.php @@ -163,7 +163,13 @@ abstract class AbstractEntity { $info['description'] = $dao::getEntityDescription() ?? $info['description'] ?? NULL; } unset($info['package'], $info['method']); - return $info; + + // Ensure all keys are snake_case + $keys = array_keys($info); + foreach ($keys as &$key) { + $key = \CRM_Utils_String::convertStringToSnakeCase($key); + } + return array_combine($keys, array_values($info)); } /** diff --git a/Civi/Api4/Generic/Traits/DAOActionTrait.php b/Civi/Api4/Generic/Traits/DAOActionTrait.php index 98707aff3b..277f2a727f 100644 --- a/Civi/Api4/Generic/Traits/DAOActionTrait.php +++ b/Civi/Api4/Generic/Traits/DAOActionTrait.php @@ -32,6 +32,8 @@ trait DAOActionTrait { */ protected $language; + private $_maxWeights = []; + /** * @return \CRM_Core_DAO|string */ @@ -105,6 +107,7 @@ trait DAOActionTrait { */ protected function writeObjects(&$items) { $baoName = $this->getBaoName(); + $updateWeights = FALSE; // TODO: Opt-in more entities to use the new writeRecords BAO method. $functionNames = [ @@ -118,6 +121,16 @@ trait DAOActionTrait { $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'); + // Only take action if updating a single record, or if no weights are specified in any record + // This avoids messing up a bulk update with multiple recalculations + if (count($items) === 1 || !array_filter(array_column($items, $weightField))) { + $updateWeights = TRUE; + } + } + $result = []; foreach ($items as &$item) { @@ -125,6 +138,11 @@ trait DAOActionTrait { FormattingUtil::formatWriteParams($item, $this->entityFields()); $this->formatCustomParams($item, $entityId); + // Adjust weights for sortable entities + if ($updateWeights) { + $this->updateWeight($item); + } + // Skip individual processing if using writeRecords if ($method === 'writeRecords') { continue; @@ -287,4 +305,62 @@ trait DAOActionTrait { return isset($info[$fieldName]) ? ['suffix' => $suffix] + $info[$fieldName] : NULL; } + /** + * Update weights when inserting or updating a sortable entity + * @param array $record + * @see SortableEntity + */ + protected function updateWeight(array &$record) { + /** @var \CRM_Core_DAO|string $daoName */ + $daoName = CoreUtil::getInfoItem($this->getEntityName(), 'dao'); + $weightField = CoreUtil::getInfoItem($this->getEntityName(), 'order_by'); + $idField = CoreUtil::getIdFieldName($this->getEntityName()); + // If updating an existing record without changing weight, do nothing + if (!isset($record[$weightField]) && !empty($record[$idField])) { + return; + } + $daoFields = $daoName::getSupportedFields(); + $newWeight = $record[$weightField] ?? NULL; + $oldWeight = empty($record[$idField]) ? NULL : \CRM_Core_DAO::getFieldValue($daoName, $record[$idField], $weightField); + + // FIXME: Need a more metadata-ish approach. For now here's a hardcoded list of the fields sortable entities use for grouping. + $guesses = ['option_group_id', 'price_set_id', 'price_field_id', 'premiums_id', 'uf_group_id', 'custom_group_id', 'domain_id']; + $filters = []; + foreach (array_intersect($guesses, array_keys($daoFields)) as $filter) { + $value = $record[$filter] ?? (empty($record[$idField]) ? NULL : \CRM_Core_DAO::getFieldValue($daoName, $record[$idField], $filter)); + // Ignore the db-formatted string 'null' and empty strings as well as NULL values + if (!\CRM_Utils_System::isNull($value)) { + $filters[$filter] = $value; + } + } + // Supply default weight for new record + if (!isset($record[$weightField]) && empty($record[$idField])) { + $record[$weightField] = $this->getMaxWeight($daoName, $filters, $weightField); + } + else { + $record[$weightField] = \CRM_Utils_Weight::updateOtherWeights($daoName, $oldWeight, $newWeight, $filters, $weightField); + } + } + + /** + * Looks up max weight for a set of sortable entities + * + * Keeps it in memory in case this operation is writing more than one record + * + * @param $daoName + * @param $filters + * @param $weightField + * @return int|mixed + */ + private function getMaxWeight($daoName, $filters, $weightField) { + $key = $daoName . json_encode($filters); + if (!isset($this->_maxWeights[$key])) { + $this->_maxWeights[$key] = \CRM_Utils_Weight::getMax($daoName, $filters, $weightField) + 1; + } + else { + ++$this->_maxWeights[$key]; + } + return $this->_maxWeights[$key]; + } + } diff --git a/Civi/Api4/Generic/Traits/SortableEntity.php b/Civi/Api4/Generic/Traits/SortableEntity.php new file mode 100644 index 0000000000..272b53d5f9 --- /dev/null +++ b/Civi/Api4/Generic/Traits/SortableEntity.php @@ -0,0 +1,21 @@ +assertEquals($optionGroupCount, OptionGroup::get(FALSE)->selectRowCount()->execute()->count()); } + public function testUpdateWeights() { + $getValues = function($groupName) { + return CustomField::get(FALSE) + ->addWhere('custom_group_id.name', '=', $groupName) + ->addOrderBy('weight') + ->execute()->indexBy('name')->column('weight'); + }; + + // Create 2 custom groups. Control group is to ensure updating one doesn't affect the other + foreach (['controlGroup', 'experimentalGroup'] as $groupName) { + $customGroups[$groupName] = CustomGroup::create(FALSE) + ->addValue('name', $groupName) + ->addValue('extends', 'Individual') + ->execute()->first(); + $sampleData = [ + ['label' => 'One'], + ['label' => 'Two'], + ['label' => 'Three'], + ['label' => 'Four'], + ]; + CustomField::save(FALSE) + ->setRecords($sampleData) + ->addDefault('custom_group_id.name', $groupName) + ->addDefault('html_type', 'Text') + ->execute(); + // Default weights should have been set during create + $this->assertEquals(['One' => 1, 'Two' => 2, 'Three' => 3, 'Four' => 4], $getValues($groupName)); + } + + // Ensure default weights were set for custom groups + $this->assertEquals($customGroups['controlGroup']['weight'] + 1, $customGroups['experimentalGroup']['weight']); + + // Move third option to second position + CustomField::update(FALSE) + ->addWhere('custom_group_id.name', '=', 'experimentalGroup') + ->addWhere('name', '=', 'Three') + ->addValue('weight', 2) + ->execute(); + // Experimental group should be updated, control group should not + $this->assertEquals(['One' => 1, 'Three' => 2, 'Two' => 3, 'Four' => 4], $getValues('experimentalGroup')); + $this->assertEquals(['One' => 1, 'Two' => 2, 'Three' => 3, 'Four' => 4], $getValues('controlGroup')); + } + } diff --git a/tests/phpunit/api/v4/Entity/MembershipTest.php b/tests/phpunit/api/v4/Entity/MembershipTest.php new file mode 100644 index 0000000000..5f553e3904 --- /dev/null +++ b/tests/phpunit/api/v4/Entity/MembershipTest.php @@ -0,0 +1,76 @@ +addWhere('domain_id.name', '=', $domain) + ->addOrderBy('weight') + ->execute()->indexBy('name')->column('weight'); + }; + + // Create 2 domains. Control domain is to ensure updating one doesn't affect the other + foreach (['controlDomain', 'experimentalDomain'] as $domain) { + Domain::create(FALSE) + ->addValue('name', $domain) + ->addValue('version', \CRM_Utils_System::version()) + ->execute(); + $sampleData = [ + ['name' => 'One'], + ['name' => 'Two'], + ['name' => 'Three'], + ['name' => 'Four'], + ]; + MembershipType::save(FALSE) + ->setRecords($sampleData) + ->addDefault('domain_id.name', $domain) + ->addDefault('financial_type_id', 1) + ->addDefault('duration_unit', 'day') + ->addDefault('period_type', 'rolling') + ->addDefault('member_of_contact_id', Contact::create(FALSE) + ->addValue('organization_name', $domain)->execute()->first()['id']) + ->execute(); + $this->assertEquals(['One' => 1, 'Two' => 2, 'Three' => 3, 'Four' => 4], $getValues($domain)); + } + + // Move first option to third position + MembershipType::update(FALSE) + ->addWhere('domain_id.name', '=', 'experimentalDomain') + ->addWhere('name', '=', 'One') + ->addValue('weight', 3) + ->execute(); + // Experimental domain should be updated, control domain should not + $this->assertEquals(['Two' => 1, 'Three' => 2, 'One' => 3, 'Four' => 4], $getValues('experimentalDomain')); + $this->assertEquals(['One' => 1, 'Two' => 2, 'Three' => 3, 'Four' => 4], $getValues('controlDomain')); + + } + +} diff --git a/tests/phpunit/api/v4/Entity/OptionValueTest.php b/tests/phpunit/api/v4/Entity/OptionValueTest.php index 470332c346..b8761ee2a7 100644 --- a/tests/phpunit/api/v4/Entity/OptionValueTest.php +++ b/tests/phpunit/api/v4/Entity/OptionValueTest.php @@ -55,4 +55,72 @@ class OptionValueTest extends UnitTestCase implements TransactionalInterface { $this->assertTrue(OptionValue::get(FALSE)->addWhere('id', '=', $defaultId)->execute()->first()['is_default']); } + public function testUpdateWeights() { + $getValues = function($groupName) { + return OptionValue::get(FALSE) + ->addWhere('option_group_id.name', '=', $groupName) + ->addOrderBy('weight') + ->execute()->indexBy('value')->column('weight'); + }; + + // Create 2 option groups. Control group is to ensure updating one doesn't affect the other + foreach (['controlGroup', 'experimentalGroup'] as $groupName) { + OptionGroup::create(FALSE) + ->addValue('name', $groupName) + ->execute(); + $sampleData = [ + ['label' => 'One', 'value' => 1], + ['label' => 'Two', 'value' => 2], + ['label' => 'Three', 'value' => 3], + ['label' => 'Four', 'value' => 4], + ]; + OptionValue::save(FALSE) + ->setRecords($sampleData) + ->addDefault('option_group_id.name', $groupName) + ->execute(); + // Default weights should have been set during create + $this->assertEquals([1 => 1, 2 => 2, 3 => 3, 4 => 4], $getValues($groupName)); + } + + // Move first option to last position + OptionValue::update(FALSE) + ->addWhere('option_group_id.name', '=', 'experimentalGroup') + ->addWhere('value', '=', 1) + ->addValue('weight', 4) + ->execute(); + // Experimental group should be updated, control group should not + $this->assertEquals([2 => 1, 3 => 2, 4 => 3, 1 => 4], $getValues('experimentalGroup')); + $this->assertEquals([1 => 1, 2 => 2, 3 => 3, 4 => 4], $getValues('controlGroup')); + + // Move 2nd (new first) option to last position + OptionValue::update(FALSE) + ->addWhere('option_group_id.name', '=', 'experimentalGroup') + ->addWhere('value', '=', 2) + ->addValue('weight', 4) + ->execute(); + // Experimental group should be updated, control group should not + $this->assertEquals([3 => 1, 4 => 2, 1 => 3, 2 => 4], $getValues('experimentalGroup')); + $this->assertEquals([1 => 1, 2 => 2, 3 => 3, 4 => 4], $getValues('controlGroup')); + + // Move last option to first position + OptionValue::update(FALSE) + ->addWhere('option_group_id.name', '=', 'experimentalGroup') + ->addWhere('value', '=', 2) + ->addValue('weight', 1) + ->execute(); + // Experimental group should be updated, control group should not + $this->assertEquals([2 => 1, 3 => 2, 4 => 3, 1 => 4], $getValues('experimentalGroup')); + $this->assertEquals([1 => 1, 2 => 2, 3 => 3, 4 => 4], $getValues('controlGroup')); + + // Same thing again - should have no impact + OptionValue::update(FALSE) + ->addWhere('option_group_id.name', '=', 'experimentalGroup') + ->addWhere('value', '=', 2) + ->addValue('weight', 1) + ->execute(); + // Nothing should have changed + $this->assertEquals([2 => 1, 3 => 2, 4 => 3, 1 => 4], $getValues('experimentalGroup')); + $this->assertEquals([1 => 1, 2 => 2, 3 => 3, 4 => 4], $getValues('controlGroup')); + } + } -- 2.25.1