From b675a45723292d4cd5ff2c4b74d2358794067544 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Thu, 1 Jul 2021 22:10:52 -0400 Subject: [PATCH] APIv4 - Rename `id_field` to `primary_key` and make it an array Some entities have a combination of primary keys instead of a single field. This is allowed in the schema so the API needs to be able to handle it too. Renaming it for consistency with the schema. --- Civi/Api4/CustomValue.php | 2 +- Civi/Api4/Entity.php | 5 +++-- Civi/Api4/Generic/AbstractEntity.php | 2 +- Civi/Api4/Generic/BasicEntity.php | 2 +- Civi/Api4/Query/Api4SelectQuery.php | 10 +++++----- ext/afform/core/Civi/Api4/Afform.php | 2 +- ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php | 4 ++-- tests/phpunit/api/v4/Action/BasicActionsTest.php | 2 +- tests/phpunit/api/v4/Entity/ConformanceTest.php | 3 ++- 9 files changed, 17 insertions(+), 15 deletions(-) diff --git a/Civi/Api4/CustomValue.php b/Civi/Api4/CustomValue.php index bd1705d065..f08ffcb582 100644 --- a/Civi/Api4/CustomValue.php +++ b/Civi/Api4/CustomValue.php @@ -142,7 +142,7 @@ class CustomValue { 'class' => __CLASS__, 'type' => ['CustomValue'], 'searchable' => 'secondary', - 'id_field' => 'id', + 'primary_key' => ['id'], 'see' => [ 'https://docs.civicrm.org/user/en/latest/organising-your-data/creating-custom-fields/#multiple-record-fieldsets', '\Civi\Api4\CustomGroup', diff --git a/Civi/Api4/Entity.php b/Civi/Api4/Entity.php index aa8e3fe284..b693895572 100644 --- a/Civi/Api4/Entity.php +++ b/Civi/Api4/Entity.php @@ -79,8 +79,9 @@ class Entity extends Generic\AbstractEntity { 'description' => 'Class name for dao-based entities', ], [ - 'name' => 'id_field', - 'description' => 'Name of unique identifier field (e.g. "id")', + 'name' => 'primary_key', + 'type' => 'Array', + 'description' => 'Name of unique identifier field(s) (e.g. [id])', ], [ 'name' => 'label_field', diff --git a/Civi/Api4/Generic/AbstractEntity.php b/Civi/Api4/Generic/AbstractEntity.php index 1e17d1337f..b218cec75e 100644 --- a/Civi/Api4/Generic/AbstractEntity.php +++ b/Civi/Api4/Generic/AbstractEntity.php @@ -143,7 +143,7 @@ abstract class AbstractEntity { 'type' => [self::stripNamespace(get_parent_class(static::class))], 'paths' => static::getEntityPaths(), 'class' => static::class, - 'id_field' => 'id', + 'primary_key' => ['id'], // Entities without a @searchable annotation will default to secondary, // which makes them visible in SearchKit but not at the top of the list. 'searchable' => 'secondary', diff --git a/Civi/Api4/Generic/BasicEntity.php b/Civi/Api4/Generic/BasicEntity.php index 66ea70c00e..dbe4ad1a53 100644 --- a/Civi/Api4/Generic/BasicEntity.php +++ b/Civi/Api4/Generic/BasicEntity.php @@ -141,7 +141,7 @@ abstract class BasicEntity extends AbstractEntity { */ public static function getInfo() { $info = parent::getInfo(); - $info['id_field'] = static::$idField; + $info['primary_key'] = (array) static::$idField; return $info; } diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php index 7ecad8b827..882331a073 100644 --- a/Civi/Api4/Query/Api4SelectQuery.php +++ b/Civi/Api4/Query/Api4SelectQuery.php @@ -89,8 +89,8 @@ class Api4SelectQuery { $this->api = $apiGet; // Always select ID of main table unless grouping by something else - $id = CoreUtil::getInfoItem($this->getEntity(), 'id_field'); - $this->forceSelectId = !$this->isAggregateQuery() || $this->getGroupBy() === [$id]; + $keys = CoreUtil::getInfoItem($this->getEntity(), 'primary_key'); + $this->forceSelectId = !$this->isAggregateQuery() || array_intersect($this->getGroupBy(), $keys); // Build field lists foreach ($this->api->entityFields() as $field) { @@ -205,8 +205,8 @@ class Api4SelectQuery { } else { if ($this->forceSelectId) { - $id = CoreUtil::getInfoItem($this->getEntity(), 'id_field'); - $select = array_merge([$id], $select); + $keys = CoreUtil::getInfoItem($this->getEntity(), 'primary_key'); + $select = array_merge($keys, $select); } // Expand the superstar 'custom.*' to select all fields in all custom groups @@ -233,7 +233,7 @@ class Api4SelectQuery { // If the joined_entity.id isn't in the fieldspec already, autoJoinFK will attempt to add the entity. $fkField = substr($wildField, 0, strrpos($wildField, '.')); $fkEntity = $this->getField($fkField)['fk_entity'] ?? NULL; - $id = $fkEntity ? CoreUtil::getInfoItem($fkEntity, 'id_field') : 'id'; + $id = $fkEntity ? CoreUtil::getInfoItem($fkEntity, 'primary_key')[0] : 'id'; $this->autoJoinFK($fkField . ".$id"); $matches = $this->selectMatchingFields($wildField); array_splice($select, $pos, 1, $matches); diff --git a/ext/afform/core/Civi/Api4/Afform.php b/ext/afform/core/Civi/Api4/Afform.php index a9b38c5005..1c8e3275e0 100644 --- a/ext/afform/core/Civi/Api4/Afform.php +++ b/ext/afform/core/Civi/Api4/Afform.php @@ -229,7 +229,7 @@ class Afform extends Generic\AbstractEntity { */ public static function getInfo() { $info = parent::getInfo(); - $info['id_field'] = 'name'; + $info['primary_key'] = ['name']; return $info; } diff --git a/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php b/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php index e27b6f1101..ea0f915603 100644 --- a/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php +++ b/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php @@ -419,8 +419,8 @@ class Run extends \Civi\Api4\Generic\AbstractAction { */ private function getExtraEntityFields(string $entityName): array { if (!isset($this->_extraEntityFields[$entityName])) { - $id = CoreUtil::getInfoItem($entityName, 'id_field'); - $this->_extraEntityFields[$entityName] = [$id]; + $id = CoreUtil::getInfoItem($entityName, 'primary_key'); + $this->_extraEntityFields[$entityName] = $id; foreach (CoreUtil::getInfoItem($entityName, 'paths') ?? [] as $path) { $matches = []; preg_match_all('#\[(\w+)]#', $path, $matches); diff --git a/tests/phpunit/api/v4/Action/BasicActionsTest.php b/tests/phpunit/api/v4/Action/BasicActionsTest.php index 68fcf7c341..78215925cb 100644 --- a/tests/phpunit/api/v4/Action/BasicActionsTest.php +++ b/tests/phpunit/api/v4/Action/BasicActionsTest.php @@ -37,7 +37,7 @@ class BasicActionsTest extends UnitTestCase { public function testGetInfo() { $info = MockBasicEntity::getInfo(); $this->assertEquals('MockBasicEntity', $info['name']); - $this->assertEquals('identifier', $info['id_field']); + $this->assertEquals(['identifier'], $info['primary_key']); } public function testCrud() { diff --git a/tests/phpunit/api/v4/Entity/ConformanceTest.php b/tests/phpunit/api/v4/Entity/ConformanceTest.php index 34d0b7cee6..48794e0589 100644 --- a/tests/phpunit/api/v4/Entity/ConformanceTest.php +++ b/tests/phpunit/api/v4/Entity/ConformanceTest.php @@ -164,7 +164,8 @@ class ConformanceTest extends UnitTestCase implements HookInterface { $this->assertNotEmpty($info['title_plural']); $this->assertNotEmpty($info['type']); $this->assertNotEmpty($info['description']); - $this->assertNotEmpty($info['id_field']); + $this->assertIsArray($info['primary_key']); + $this->assertNotEmpty($info['primary_key']); $this->assertRegExp(';^\d\.\d+$;', $info['since']); $this->assertContains($info['searchable'], ['primary', 'secondary', 'bridge', 'none']); // Bridge must be between exactly 2 entities -- 2.25.1