From 27d31a0ff5d38d9b7a7693a347edac5db087530e Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Thu, 27 May 2021 14:17:52 -0400 Subject: [PATCH] SearchKit - Improve descriptions for Relationship & GroupContact joins This changes the 'bridge' metadata property to a multi-dimensional array to hold extra info like description. --- Civi/Api4/Address.php | 4 ++-- Civi/Api4/EntityFinancialAccount.php | 13 ++++++++++++- Civi/Api4/EntityFinancialTrxn.php | 14 ++++++++++++-- Civi/Api4/Generic/Traits/EntityBridge.php | 7 +++---- Civi/Api4/GroupContact.php | 13 ++++++++++++- Civi/Api4/Query/Api4SelectQuery.php | 2 +- Civi/Api4/RelationshipCache.php | 13 ++++++++++++- Civi/Api4/Utils/ReflectionUtils.php | 3 --- ext/search_kit/Civi/Search/Admin.php | 9 +++++---- tests/phpunit/api/v4/Entity/ConformanceTest.php | 4 ++++ 10 files changed, 63 insertions(+), 19 deletions(-) diff --git a/Civi/Api4/Address.php b/Civi/Api4/Address.php index 242afbe0bd..22c16323c9 100644 --- a/Civi/Api4/Address.php +++ b/Civi/Api4/Address.php @@ -22,11 +22,11 @@ namespace Civi\Api4; /** * Address Entity. * - * This entity holds the address informatiom of a contact. Each contact may hold + * This entity holds the address information of a contact. Each contact may hold * one or more addresses but must have different location types respectively. * * Creating a new address requires at minimum a contact's ID and location type ID - * and other attributes (although optional) like street address, city, country etc. + * and other attributes (although optional) like street address, city, country etc. * * @ui_join_filters location_type_id * diff --git a/Civi/Api4/EntityFinancialAccount.php b/Civi/Api4/EntityFinancialAccount.php index 0963f73a93..4b9b3d537b 100644 --- a/Civi/Api4/EntityFinancialAccount.php +++ b/Civi/Api4/EntityFinancialAccount.php @@ -23,7 +23,6 @@ namespace Civi\Api4; * * @see https://docs.civicrm.org/dev/en/latest/financial/financialentities/#financial-accounts * - * @bridge entity_id financial_account_id * @ui_join_filters account_relationship * * @package Civi\Api4 @@ -31,4 +30,16 @@ namespace Civi\Api4; class EntityFinancialAccount extends Generic\DAOEntity { use Generic\Traits\EntityBridge; + /** + * @return array + */ + public static function getInfo() { + $info = parent::getInfo(); + $info['bridge'] = [ + 'entity_id' => [], + 'financial_account_id' => [], + ]; + return $info; + } + } diff --git a/Civi/Api4/EntityFinancialTrxn.php b/Civi/Api4/EntityFinancialTrxn.php index 3ad135bfdc..6fd015de30 100644 --- a/Civi/Api4/EntityFinancialTrxn.php +++ b/Civi/Api4/EntityFinancialTrxn.php @@ -24,11 +24,21 @@ namespace Civi\Api4; * * @see https://docs.civicrm.org/dev/en/latest/financial/financialentities/ * - * @bridge entity_id financial_trxn_id - * * @package Civi\Api4 */ class EntityFinancialTrxn extends Generic\DAOEntity { use Generic\Traits\EntityBridge; + /** + * @return array + */ + public static function getInfo() { + $info = parent::getInfo(); + $info['bridge'] = [ + 'entity_id' => [], + 'financial_trxn_id' => [], + ]; + return $info; + } + } diff --git a/Civi/Api4/Generic/Traits/EntityBridge.php b/Civi/Api4/Generic/Traits/EntityBridge.php index ff0d4120d2..2b5807b67e 100644 --- a/Civi/Api4/Generic/Traits/EntityBridge.php +++ b/Civi/Api4/Generic/Traits/EntityBridge.php @@ -23,17 +23,16 @@ trait EntityBridge { /** * Adds "bridge" info, which should specify an array of two field names from this entity * - * This automatic function can be overridden by annotating the APIv4 entity like - * `@bridge contact_id group_id` + * This automatic function can be overridden by adding a getInfo function to the api entity class. * * @return array */ public static function getInfo() { $info = parent::getInfo(); - if (!empty($info['dao']) && empty($info['bridge'])) { + if (!empty($info['dao'])) { foreach (($info['dao'])::fields() as $field) { if (!empty($field['FKClassName']) || $field['name'] === 'entity_id') { - $info['bridge'][] = $field['name']; + $info['bridge'][$field['name']] = []; } } } diff --git a/Civi/Api4/GroupContact.php b/Civi/Api4/GroupContact.php index 1f489c5134..fd988878bd 100644 --- a/Civi/Api4/GroupContact.php +++ b/Civi/Api4/GroupContact.php @@ -26,7 +26,6 @@ namespace Civi\Api4; * * @ui_join_filters status * - * @bridge group_id contact_id * @see \Civi\Api4\Group * @package Civi\Api4 */ @@ -60,4 +59,16 @@ class GroupContact extends Generic\DAOEntity { ->setCheckPermissions($checkPermissions); } + /** + * @return array + */ + public static function getInfo() { + $info = parent::getInfo(); + $info['bridge'] = [ + 'group_id' => ['description' => ts('Static (non-smart) group contacts')], + 'contact_id' => ['description' => ts('Static (non-smart) group contacts')], + ]; + return $info; + } + } diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php index 83033154f8..20c0f0ec64 100644 --- a/Civi/Api4/Query/Api4SelectQuery.php +++ b/Civi/Api4/Query/Api4SelectQuery.php @@ -795,7 +795,7 @@ class Api4SelectQuery { // Get the 2 bridge reference columns as CRM_Core_Reference_* objects $joinRef = $baseRef = NULL; foreach ($bridgeDAO::getReferenceColumns() as $ref) { - if (in_array($ref->getReferenceKey(), $bridgeFields)) { + if (array_key_exists($ref->getReferenceKey(), $bridgeFields)) { if (!$joinRef && in_array($joinEntity, $ref->getTargetEntities())) { $joinRef = $ref; } diff --git a/Civi/Api4/RelationshipCache.php b/Civi/Api4/RelationshipCache.php index 2f5ad02cc3..c878b8056e 100644 --- a/Civi/Api4/RelationshipCache.php +++ b/Civi/Api4/RelationshipCache.php @@ -23,7 +23,6 @@ namespace Civi\Api4; * RelationshipCache - readonly table to facilitate joining and finding contacts by relationship. * * @see \Civi\Api4\Relationship - * @bridge near_contact_id far_contact_id * @ui_join_filters near_relation * @package Civi\Api4 */ @@ -48,4 +47,16 @@ class RelationshipCache extends Generic\AbstractEntity { ->setCheckPermissions($checkPermissions); } + /** + * @return array + */ + public static function getInfo() { + $info = parent::getInfo(); + $info['bridge'] = [ + 'near_contact_id' => ['description' => ts('One or more contacts with a relationship to this contact')], + 'far_contact_id' => ['description' => ts('One or more contacts with a relationship to this contact')], + ]; + return $info; + } + } diff --git a/Civi/Api4/Utils/ReflectionUtils.php b/Civi/Api4/Utils/ReflectionUtils.php index 781a87b127..102b1bc40e 100644 --- a/Civi/Api4/Utils/ReflectionUtils.php +++ b/Civi/Api4/Utils/ReflectionUtils.php @@ -98,9 +98,6 @@ class ReflectionUtils { elseif ($key == 'searchable') { $info[$key] = strtolower($words[0]) !== 'false'; } - elseif ($key == 'bridge') { - $info[$key] = $words; - } elseif ($key == 'param' && $words) { $type = $words[0][0] !== '$' ? explode('|', array_shift($words)) : NULL; $param = rtrim(array_shift($words), '-:()/'); diff --git a/ext/search_kit/Civi/Search/Admin.php b/ext/search_kit/Civi/Search/Admin.php index ad606704f4..020c7db676 100644 --- a/ext/search_kit/Civi/Search/Admin.php +++ b/ext/search_kit/Civi/Search/Admin.php @@ -190,6 +190,7 @@ class Admin { }); $fields = array_column($entity['fields'], NULL, 'name'); $bridge = in_array('EntityBridge', $entity['type']) ? $entity['name'] : NULL; + $bridgeFields = array_keys($entity['bridge'] ?? []); foreach ($references as $reference) { $keyField = $fields[$reference->getReferenceKey()] ?? NULL; if ( @@ -198,7 +199,7 @@ class Admin { // Exclude any joins that are better represented by pseudoconstants is_a($reference, 'CRM_Core_Reference_OptionValue') || (!$bridge && !empty($keyField['options'])) || // Limit bridge joins to just the first - ($bridge && array_search($keyField['name'], $entity['bridge']) !== 0) || + ($bridge && array_search($keyField['name'], $bridgeFields) !== 0) || // Sanity check - table should match $daoClass::getTableName() !== $reference->getReferenceTable() ) { @@ -240,7 +241,7 @@ class Admin { // Bridge joins (sanity check - bridge must specify exactly 2 FK fields) elseif (count($entity['bridge']) === 2) { // Get the other entity being linked through this bridge - $baseKey = array_search($reference->getReferenceKey(), $entity['bridge']) ? $entity['bridge'][0] : $entity['bridge'][1]; + $baseKey = array_search($reference->getReferenceKey(), $bridgeFields) ? $bridgeFields[0] : $bridgeFields[1]; $baseEntity = $allowedEntities[$fields[$baseKey]['fk_entity']] ?? NULL; if (!$baseEntity) { continue; @@ -251,7 +252,7 @@ class Admin { $alias = $baseEntity['name'] . "_{$bridge}_" . $targetEntityName; $joins[$baseEntity['name']][] = [ 'label' => $baseEntity['title'] . ' ' . $targetsTitle, - 'description' => E::ts('Multiple %1 per %2', [1 => $targetsTitle, 2 => $baseEntity['title']]), + 'description' => $entity['bridge'][$baseKey]['description'] ?? E::ts('Multiple %1 per %2', [1 => $targetsTitle, 2 => $baseEntity['title']]), 'entity' => $targetEntityName, 'conditions' => array_merge( [$bridge], @@ -266,7 +267,7 @@ class Admin { $alias = $targetEntityName . "_{$bridge}_" . $baseEntity['name']; $joins[$targetEntityName][] = [ 'label' => $targetEntity['title'] . ' ' . $baseEntity['title_plural'], - 'description' => E::ts('Multiple %1 per %2', [1 => $baseEntity['title_plural'], 2 => $targetEntity['title']]), + 'description' => $entity['bridge'][$reference->getReferenceKey()]['description'] ?? E::ts('Multiple %1 per %2', [1 => $baseEntity['title_plural'], 2 => $targetEntity['title']]), 'entity' => $baseEntity['name'], 'conditions' => array_merge( [$bridge], diff --git a/tests/phpunit/api/v4/Entity/ConformanceTest.php b/tests/phpunit/api/v4/Entity/ConformanceTest.php index ea870e93bb..c4f48446f9 100644 --- a/tests/phpunit/api/v4/Entity/ConformanceTest.php +++ b/tests/phpunit/api/v4/Entity/ConformanceTest.php @@ -156,6 +156,10 @@ class ConformanceTest extends UnitTestCase { $this->assertNotEmpty($info['title_plural']); $this->assertNotEmpty($info['type']); $this->assertNotEmpty($info['description']); + // Bridge must be between exactly 2 entities + if (in_array('EntityBridge', $info['type'], TRUE)) { + $this->assertCount(2, $info['bridge']); + } } /** -- 2.25.1