From 9c961a3ae053ab117b0a07d6a7417a16502946a5 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Sat, 12 Jun 2021 21:36:39 -0400 Subject: [PATCH] APIv4 - add is_current as a pseudo (calculated) field This adds the first calculated field to APIv4. It works by injecting a SQL function and aliasing it with the name of the fake field. This approach allows it to work as part of the WHERE clause as well as the SELECT clause, even across joins. Marking the old "is_current" api param deprecated because the field works better. --- Civi/Api4/Action/Campaign/Get.php | 2 - Civi/Api4/Action/Event/Get.php | 2 - Civi/Api4/Action/Relationship/Get.php | 2 - .../Event/Subscriber/IsCurrentSubscriber.php | 3 +- Civi/Api4/Generic/DAOGetFieldsAction.php | 5 ++ Civi/Api4/Generic/Traits/IsCurrentTrait.php | 14 ++-- Civi/Api4/Query/Api4SelectQuery.php | 13 ++-- Civi/Api4/Query/SqlField.php | 4 ++ .../Api4/Service/Schema/Joinable/Joinable.php | 22 ++---- Civi/Api4/Service/Spec/FieldSpec.php | 15 ++++ .../Provider/IsCurrentFieldSpecProvider.php | 70 +++++++++++++++++++ Civi/Api4/Service/Spec/RequestSpec.php | 22 +++++- Civi/Api4/Service/Spec/SpecGatherer.php | 4 ++ Civi/Test/Api3TestTrait.php | 3 +- .../api/v4/Action/CurrentFilterTest.php | 16 ++--- 15 files changed, 150 insertions(+), 47 deletions(-) create mode 100644 Civi/Api4/Service/Spec/Provider/IsCurrentFieldSpecProvider.php diff --git a/Civi/Api4/Action/Campaign/Get.php b/Civi/Api4/Action/Campaign/Get.php index cdb66de7b6..4861f8dc8f 100644 --- a/Civi/Api4/Action/Campaign/Get.php +++ b/Civi/Api4/Action/Campaign/Get.php @@ -13,8 +13,6 @@ namespace Civi\Api4\Action\Campaign; /** * @inheritDoc - * - * Set current = true to get active, non past campaigns. */ class Get extends \Civi\Api4\Generic\DAOGetAction { use \Civi\Api4\Generic\Traits\IsCurrentTrait; diff --git a/Civi/Api4/Action/Event/Get.php b/Civi/Api4/Action/Event/Get.php index bc02338ade..3f8c667aa9 100644 --- a/Civi/Api4/Action/Event/Get.php +++ b/Civi/Api4/Action/Event/Get.php @@ -13,8 +13,6 @@ namespace Civi\Api4\Action\Event; /** * @inheritDoc - * - * Set current = true to get active, non past events. */ class Get extends \Civi\Api4\Generic\DAOGetAction { use \Civi\Api4\Generic\Traits\IsCurrentTrait; diff --git a/Civi/Api4/Action/Relationship/Get.php b/Civi/Api4/Action/Relationship/Get.php index e9460283c4..21a832b5c1 100644 --- a/Civi/Api4/Action/Relationship/Get.php +++ b/Civi/Api4/Action/Relationship/Get.php @@ -14,8 +14,6 @@ namespace Civi\Api4\Action\Relationship; /** * @inheritDoc - * - * Set current = true to get active, non past relationships. */ class Get extends \Civi\Api4\Generic\DAOGetAction { use \Civi\Api4\Generic\Traits\IsCurrentTrait; diff --git a/Civi/Api4/Event/Subscriber/IsCurrentSubscriber.php b/Civi/Api4/Event/Subscriber/IsCurrentSubscriber.php index 2a80539c80..fcede7f824 100644 --- a/Civi/Api4/Event/Subscriber/IsCurrentSubscriber.php +++ b/Civi/Api4/Event/Subscriber/IsCurrentSubscriber.php @@ -16,8 +16,7 @@ use Civi\API\Event\PrepareEvent; use Civi\Api4\Utils\ReflectionUtils; /** - * Process $current api param for Get actions - * + * @deprecated * @see \Civi\Api4\Generic\Traits\IsCurrentTrait */ class IsCurrentSubscriber extends Generic\AbstractPrepareSubscriber { diff --git a/Civi/Api4/Generic/DAOGetFieldsAction.php b/Civi/Api4/Generic/DAOGetFieldsAction.php index 62165a2f1e..e11ac0c8f7 100644 --- a/Civi/Api4/Generic/DAOGetFieldsAction.php +++ b/Civi/Api4/Generic/DAOGetFieldsAction.php @@ -118,6 +118,11 @@ class DAOGetFieldsAction extends BasicGetFieldsAction { 'data_type' => 'Array', '@internal' => TRUE, ]; + $fields[] = [ + 'name' => 'sql_renderer', + 'data_type' => 'Array', + '@internal' => TRUE, + ]; return $fields; } diff --git a/Civi/Api4/Generic/Traits/IsCurrentTrait.php b/Civi/Api4/Generic/Traits/IsCurrentTrait.php index b85301f748..8853d25651 100644 --- a/Civi/Api4/Generic/Traits/IsCurrentTrait.php +++ b/Civi/Api4/Generic/Traits/IsCurrentTrait.php @@ -12,26 +12,21 @@ namespace Civi\Api4\Generic\Traits; /** - * This trait adds the $current param to a Get action. - * + * @deprecated * @see \Civi\Api4\Event\Subscriber\IsCurrentSubscriber */ trait IsCurrentTrait { /** - * Convenience filter for selecting items that are enabled and are currently within their start/end dates. - * - * Adding current = TRUE is a shortcut for - * WHERE is_active = 1 AND (end_date IS NULL OR end_date >= now) AND (start_date IS NULL OR start_DATE <= now) - * - * Adding current = FALSE is a shortcut for - * WHERE is_active = 0 OR start_date > now OR end_date < now + * Param deprecated in favor of is_current filter. * * @var bool + * @deprecated */ protected $current; /** + * @deprecated * @return bool */ public function getCurrent() { @@ -39,6 +34,7 @@ trait IsCurrentTrait { } /** + * @deprecated * @param bool $current * @return $this */ diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php index 4136698f13..7ecad8b827 100644 --- a/Civi/Api4/Query/Api4SelectQuery.php +++ b/Civi/Api4/Query/Api4SelectQuery.php @@ -246,7 +246,7 @@ class Api4SelectQuery { foreach ($expr->getFields() as $fieldName) { $field = $this->getField($fieldName); // Remove expressions with unknown fields without raising an error - if (!$field || !in_array($field['type'], ['Field', 'Custom'], TRUE)) { + if (!$field || $field['type'] === 'Filter') { $select = array_diff($select, [$item]); $valid = FALSE; } @@ -439,7 +439,7 @@ class Api4SelectQuery { if ($type === 'WHERE') { $field = $this->getField($expr, TRUE); FormattingUtil::formatInputValue($value, $expr, $field, $operator); - $fieldAlias = $field['sql_name']; + $fieldAlias = $this->getExpression($expr)->render($this->apiFieldSpec); } // For HAVING, expr must be an item in the SELECT clause elseif ($type === 'HAVING') { @@ -563,6 +563,9 @@ class Api4SelectQuery { return "($fieldAlias $isEmptyClause $fieldAlias $operator)"; } } + if (is_bool($value)) { + $value = (int) $value; + } return \CRM_Core_DAO::createSQLFilter($fieldAlias, [$operator => $value]); } @@ -816,7 +819,9 @@ class Api4SelectQuery { else { $joinEntityClass = CoreUtil::getApiClass($joinEntity); foreach ($joinEntityClass::get($this->getCheckPermissions())->entityFields() as $name => $field) { - $bridgeFields[$field['column_name']] = '`' . $joinAlias . '`.`' . $field['column_name'] . '`'; + if ($field['type'] === 'Field') { + $bridgeFields[$field['column_name']] = '`' . $joinAlias . '`.`' . $field['column_name'] . '`'; + } } $select = implode(',', $bridgeFields); $joinConditions = array_merge($joinConditions, $bridgeConditions); @@ -898,7 +903,7 @@ class Api4SelectQuery { $bridgeFkFields = [$joinRef->getReferenceKey(), $joinRef->getTypeColumn(), $baseRef->getReferenceKey(), $baseRef->getTypeColumn()]; $bridgeEntityClass = CoreUtil::getApiClass($bridgeEntity); foreach ($bridgeEntityClass::get($this->getCheckPermissions())->entityFields() as $name => $field) { - if ($name === 'id' || ($side === 'INNER' && in_array($name, $bridgeFkFields, TRUE))) { + if ($field['type'] !== 'Field' || $name === 'id' || ($side === 'INNER' && in_array($name, $bridgeFkFields, TRUE))) { continue; } // For INNER joins, these fields get a sql alias pointing to the bridge entity, diff --git a/Civi/Api4/Query/SqlField.php b/Civi/Api4/Query/SqlField.php index 6bd8c3c2c9..6180fbf1b4 100644 --- a/Civi/Api4/Query/SqlField.php +++ b/Civi/Api4/Query/SqlField.php @@ -34,6 +34,10 @@ class SqlField extends SqlExpression { if ($fieldList[$this->expr] === FALSE) { throw new UnauthorizedException("Unauthorized field '{$this->expr}'"); } + if (!empty($fieldList[$this->expr]['sql_renderer'])) { + $renderer = $fieldList[$this->expr]['sql_renderer']; + return $renderer($fieldList[$this->expr]); + } return $fieldList[$this->expr]['sql_name']; } diff --git a/Civi/Api4/Service/Schema/Joinable/Joinable.php b/Civi/Api4/Service/Schema/Joinable/Joinable.php index e0ced42e5f..d98c6b5bb5 100644 --- a/Civi/Api4/Service/Schema/Joinable/Joinable.php +++ b/Civi/Api4/Service/Schema/Joinable/Joinable.php @@ -13,7 +13,6 @@ namespace Civi\Api4\Service\Schema\Joinable; use Civi\Api4\Utils\CoreUtil; -use CRM_Core_DAO_AllCoreTables as AllCoreTables; class Joinable { @@ -275,17 +274,13 @@ class Joinable { } /** - * @return \Civi\Api4\Service\Spec\FieldSpec[] + * @return \Civi\Api4\Service\Spec\RequestSpec */ public function getEntityFields() { - $entityFields = []; - $bao = AllCoreTables::getClassForTable($this->getTargetTable()); - if ($bao) { - foreach ($bao::getSupportedFields() as $field) { - $entityFields[] = \Civi\Api4\Service\Spec\SpecFormatter::arrayToField($field, $this->getEntity()); - } - } - return $entityFields; + /** @var \Civi\Api4\Service\Spec\SpecGatherer $gatherer */ + $gatherer = \Civi::container()->get('spec_gatherer'); + $spec = $gatherer->getSpec($this->entity, 'get', FALSE); + return $spec; } /** @@ -303,12 +298,7 @@ class Joinable { * @return \Civi\Api4\Service\Spec\FieldSpec|NULL */ public function getField($fieldName) { - foreach ($this->getEntityFields() as $field) { - if ($field->getName() === $fieldName) { - return $field; - } - } - return NULL; + return $this->getEntityFields()->getFieldByName($fieldName); } } diff --git a/Civi/Api4/Service/Spec/FieldSpec.php b/Civi/Api4/Service/Spec/FieldSpec.php index fcaf67ed55..f5733bac8a 100644 --- a/Civi/Api4/Service/Spec/FieldSpec.php +++ b/Civi/Api4/Service/Spec/FieldSpec.php @@ -133,12 +133,17 @@ class FieldSpec { */ public $outputFormatters; + /** + * @var callable + */ + public $sqlRenderer; /** * @var callable[] */ public $sqlFilters; + /** * Aliases for the valid data types * @@ -432,6 +437,16 @@ class FieldSpec { return $this; } + /** + * @param callable $sqlRenderer + * @return $this + */ + public function setSqlRenderer($sqlRenderer) { + $this->sqlRenderer = $sqlRenderer; + + return $this; + } + /** * @param callable[] $sqlFilters * @return $this diff --git a/Civi/Api4/Service/Spec/Provider/IsCurrentFieldSpecProvider.php b/Civi/Api4/Service/Spec/Provider/IsCurrentFieldSpecProvider.php new file mode 100644 index 0000000000..03598ac640 --- /dev/null +++ b/Civi/Api4/Service/Spec/Provider/IsCurrentFieldSpecProvider.php @@ -0,0 +1,70 @@ +getEntity(), 'Boolean'); + $field->setLabel(ts('Is Current')) + ->setTitle(ts('Current')) + // This pseudo-field is like is_active with some extra criteria + ->setColumnName('is_current') + ->setDescription(ts('Is active with a non-past end-date')) + ->setType('Extra') + ->setSqlRenderer([__CLASS__, 'renderIsCurrentSql']); + $spec->addFieldSpec($field); + } + + /** + * @param string $entity + * @param string $action + * + * @return bool + */ + public function applies($entity, $action) { + if ($action !== 'get') { + return FALSE; + } + // TODO: If we wanted this to not be a hard-coded list, we could always return TRUE here + // and then in the `modifySpec` function check for the 3 fields `is_active`, `start_date`, and `end_date` + return in_array($entity, ['Relationship', 'RelationshipCache', 'Event', 'Campaign'], TRUE); + } + + /** + * @param array $field + * return string + */ + public static function renderIsCurrentSql(array $field): string { + $startDate = substr_replace($field['sql_name'], 'start_date', -11, -1); + $endDate = substr_replace($field['sql_name'], 'end_date', -11, -1); + $isActive = substr_replace($field['sql_name'], 'is_active', -11, -1); + $todayStart = date('Ymd', strtotime('now')); + $todayEnd = date('Ymd', strtotime('now')); + return "IF($isActive = 1 AND ($startDate <= '$todayStart' OR $startDate IS NULL) AND ($endDate >= '$todayEnd' OR $endDate IS NULL), '1', '0')"; + } + +} diff --git a/Civi/Api4/Service/Spec/RequestSpec.php b/Civi/Api4/Service/Spec/RequestSpec.php index 78b27386ca..a66b244a63 100644 --- a/Civi/Api4/Service/Spec/RequestSpec.php +++ b/Civi/Api4/Service/Spec/RequestSpec.php @@ -14,7 +14,7 @@ namespace Civi\Api4\Service\Spec; use Civi\Api4\Utils\CoreUtil; -class RequestSpec { +class RequestSpec implements \Iterator { /** * @var string @@ -133,4 +133,24 @@ class RequestSpec { return $this->action; } + public function rewind() { + return reset($this->fields); + } + + public function current() { + return current($this->fields); + } + + public function key() { + return key($this->fields); + } + + public function next() { + return next($this->fields); + } + + public function valid() { + return key($this->fields) !== NULL; + } + } diff --git a/Civi/Api4/Service/Spec/SpecGatherer.php b/Civi/Api4/Service/Spec/SpecGatherer.php index eee3974134..f4a9eacd9e 100644 --- a/Civi/Api4/Service/Spec/SpecGatherer.php +++ b/Civi/Api4/Service/Spec/SpecGatherer.php @@ -16,6 +16,10 @@ use Civi\Api4\CustomField; use Civi\Api4\Service\Spec\Provider\Generic\SpecProviderInterface; use Civi\Api4\Utils\CoreUtil; +/** + * Class SpecGatherer + * @package Civi\Api4\Service\Spec + */ class SpecGatherer { /** diff --git a/Civi/Test/Api3TestTrait.php b/Civi/Test/Api3TestTrait.php index 953e484b71..ca7ba747ce 100644 --- a/Civi/Test/Api3TestTrait.php +++ b/Civi/Test/Api3TestTrait.php @@ -315,7 +315,8 @@ trait Api3TestTrait { $onlyId = !empty($v3Params['format.only_id']); $onlySuccess = !empty($v3Params['format.is_success']); if (!empty($v3Params['filters']['is_current']) || !empty($v3Params['isCurrent'])) { - $v4Params['current'] = TRUE; + $v3Params['is_current'] = 1; + unset($v3Params['filters']['is_current'], $v3Params['isCurrent']); } $language = $v3Params['options']['language'] ?? $v3Params['option.language'] ?? NULL; if ($language) { diff --git a/tests/phpunit/api/v4/Action/CurrentFilterTest.php b/tests/phpunit/api/v4/Action/CurrentFilterTest.php index b4d50b3a86..febac7383b 100644 --- a/tests/phpunit/api/v4/Action/CurrentFilterTest.php +++ b/tests/phpunit/api/v4/Action/CurrentFilterTest.php @@ -62,15 +62,15 @@ class CurrentFilterTest extends UnitTestCase { 'is_active' => 0, ])->execute()->first(); - $getCurrent = (array) Relationship::get()->setCurrent(TRUE)->execute()->indexBy('id'); - $notCurrent = (array) Relationship::get()->setCurrent(FALSE)->execute()->indexBy('id'); - $getAll = (array) Relationship::get()->execute()->indexBy('id'); + $getCurrent = Relationship::get()->addWhere('is_current', '=', TRUE)->execute()->indexBy('id'); + $notCurrent = Relationship::get()->addWhere('is_current', '=', FALSE)->execute()->indexBy('id'); + $getAll = Relationship::get()->addSelect('is_current')->execute()->indexBy('id'); - $this->assertArrayHasKey($current['id'], $getAll); - $this->assertArrayHasKey($indefinite['id'], $getAll); - $this->assertArrayHasKey($expiring['id'], $getAll); - $this->assertArrayHasKey($past['id'], $getAll); - $this->assertArrayHasKey($inactive['id'], $getAll); + $this->assertTrue($getAll[$current['id']]['is_current']); + $this->assertTrue($getAll[$indefinite['id']]['is_current']); + $this->assertTrue($getAll[$expiring['id']]['is_current']); + $this->assertFalse($getAll[$past['id']]['is_current']); + $this->assertFalse($getAll[$inactive['id']]['is_current']); $this->assertArrayHasKey($current['id'], $getCurrent); $this->assertArrayHasKey($indefinite['id'], $getCurrent); -- 2.25.1