From da9aa6ae21765e7b951b280918d8fef4bb5cdf96 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Sun, 8 May 2022 20:45:21 -0400 Subject: [PATCH] SearchKit - ensure only relevant fields are editable Fixes dev/core#3423 Ensures that custom fields for one entity sub-type (e.g. Individual) are not presented as editable for other sub-types (e.g. Organiztion). --- .../SearchDisplay/AbstractRunAction.php | 63 ++++++++-- .../api/v4/SearchDisplay/SearchRunTest.php | 103 +++++++++++++++ .../SearchRunWithCustomFieldTest.php | 118 +++++++++++++++++- 3 files changed, 272 insertions(+), 12 deletions(-) diff --git a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php index 1b592602a7..7a60d61ffe 100644 --- a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php +++ b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php @@ -7,6 +7,7 @@ use Civi\Api4\Generic\Traits\ArrayQueryActionTrait; use Civi\Api4\Query\SqlField; use Civi\Api4\SearchDisplay; use Civi\Api4\Utils\CoreUtil; +use Civi\Api4\Utils\FormattingUtil; /** * Base class for running a search. @@ -535,8 +536,8 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction { } /** - * @param $column - * @param $data + * @param array $column + * @param array $data * @return array{entity: string, action: string, input_type: string, data_type: string, options: bool, serialize: bool, nullable: bool, fk_entity: string, value_key: string, record: array, value: mixed}|null */ private function formatEditableColumn($column, $data) { @@ -547,6 +548,12 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction { $editable['action'] = 'update'; $editable['record'][$editable['id_key']] = $data[$editable['id_path']]; $editable['value'] = $data[$editable['value_path']]; + // Ensure field is appropriate to this entity sub-type + $field = $this->getField($column['key']); + $entityValues = FormattingUtil::filterByPrefix($data, $editable['id_path'], $editable['id_key']); + if (!$this->fieldBelongsToEntity($editable['entity'], $field['name'], $entityValues)) { + return NULL; + } } // Generate params to create new record, if applicable elseif ($editable['explicit_join'] && !$this->getJoin($editable['explicit_join'])['bridge']) { @@ -600,18 +607,45 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction { 'values' => $editable['record'], ], 0)['access']; if ($access) { - \CRM_Utils_Array::remove($editable, 'id_key', 'id_path', 'value_path', 'explicit_join'); + // Remove info that's for internal use only + \CRM_Utils_Array::remove($editable, 'id_key', 'id_path', 'value_path', 'explicit_join', 'grouping_fields'); return $editable; } } return NULL; } + /** + * Check if a field is appropriate for this entity type or sub-type. + * + * For example, the 'first_name' field does not belong to Contacts of type Organization. + * And custom data is sometimes limited to specific contact types, event types, case types, etc. + * + * @param string $entityName + * @param string $fieldName + * @param array $entityValues + * @param bool $checkPermissions + * @return bool + */ + private function fieldBelongsToEntity($entityName, $fieldName, $entityValues, $checkPermissions = TRUE) { + try { + return (bool) civicrm_api4($entityName, 'getFields', [ + 'checkPermissions' => $checkPermissions, + 'where' => [['name', '=', $fieldName]], + 'values' => $entityValues, + ])->count(); + } + catch (\API_Exception $e) { + return FALSE; + } + } + /** * @param $key - * @return array{entity: string, input_type: string, data_type: string, options: bool, serialize: bool, nullable: bool, fk_entity: string, value_key: string, value_path: string, id_key: string, id_path: string, explicit_join: string}|null + * @return array{entity: string, input_type: string, data_type: string, options: bool, serialize: bool, nullable: bool, fk_entity: string, value_key: string, value_path: string, id_key: string, id_path: string, explicit_join: string, grouping_fields: array}|null */ private function getEditableInfo($key) { + $result = NULL; // Strip pseudoconstant suffix [$key] = explode(':', $key); $field = $this->getField($key); @@ -621,13 +655,14 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction { } if ($field) { $idKey = CoreUtil::getIdFieldName($field['entity']); - $idPath = ($field['explicit_join'] ? $field['explicit_join'] . '.' : '') . $idKey; + $path = ($field['explicit_join'] ? $field['explicit_join'] . '.' : ''); + $idPath = $path . $idKey; // Hack to support editing relationships if ($field['entity'] === 'RelationshipCache') { $field['entity'] = 'Relationship'; - $idPath = ($field['explicit_join'] ? $field['explicit_join'] . '.' : '') . 'relationship_id'; + $idPath = $path . 'relationship_id'; } - return [ + $result = [ 'entity' => $field['entity'], 'input_type' => $field['input_type'], 'data_type' => $field['data_type'], @@ -640,9 +675,18 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction { 'id_key' => $idKey, 'id_path' => $idPath, 'explicit_join' => $field['explicit_join'], + 'grouping_fields' => [], ]; + // Grouping fields get added to the query so that contact sub-type and entity type (for custom fields) + // are available to filter fields specific to an entity sub-type. See self::fieldBelongsToEntity() + if ($field['type'] === 'Custom' || $field['entity'] === 'Contact') { + $customInfo = \Civi\Api4\Utils\CoreUtil::getCustomGroupExtends($field['entity']); + foreach ((array) ($customInfo['grouping'] ?? []) as $grouping) { + $result['grouping_fields'][] = $path . $grouping; + } + } } - return NULL; + return $result; } /** @@ -937,8 +981,7 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction { if (!empty($column['editable'])) { $editable = $this->getEditableInfo($column['key']); if ($editable) { - $additions[] = $editable['value_path']; - $additions[] = $editable['id_path']; + $additions = array_merge($additions, $editable['grouping_fields'], [$editable['value_path'], $editable['id_path']]); } } // Add style & icon conditions for the column diff --git a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php index f5d8deb1b7..62e43010b0 100644 --- a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php +++ b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php @@ -1253,4 +1253,107 @@ class SearchRunTest extends \PHPUnit\Framework\TestCase implements HeadlessInter $this->assertEquals([1, 2], $data); } + public function testEditableContactFields() { + $source = uniqid(__FUNCTION__); + $sampleData = [ + ['contact_type' => 'Individual', 'first_name' => 'One'], + ['contact_type' => 'Individual'], + ['contact_type' => 'Organization'], + ['contact_type' => 'Household'], + ]; + $contact = Contact::save(FALSE) + ->addDefault('source', $source) + ->setRecords($sampleData) + ->execute(); + + $params = [ + 'checkPermissions' => FALSE, + 'return' => 'page:1', + 'savedSearch' => [ + 'api_entity' => 'Contact', + 'api_params' => [ + 'version' => 4, + 'select' => ['first_name', 'organization_name', 'household_name'], + 'where' => [['source', '=', $source]], + ], + ], + 'display' => [ + 'type' => 'table', + 'label' => '', + 'settings' => [ + 'actions' => TRUE, + 'pager' => [], + 'columns' => [ + [ + 'key' => 'first_name', + 'label' => 'First', + 'dataType' => 'String', + 'type' => 'field', + 'editable' => TRUE, + ], + [ + 'key' => 'organization_name', + 'label' => 'First', + 'dataType' => 'String', + 'type' => 'field', + 'editable' => TRUE, + ], + [ + 'key' => 'household_name', + 'label' => 'First', + 'dataType' => 'String', + 'type' => 'field', + 'editable' => TRUE, + ], + ], + 'sort' => [ + ['id', 'ASC'], + ], + ], + ], + 'afform' => NULL, + ]; + + $result = civicrm_api4('SearchDisplay', 'run', $params); + // First Individual + $expectedFirstNameEdit = [ + 'entity' => 'Contact', + 'input_type' => 'Text', + 'data_type' => 'String', + 'options' => FALSE, + 'serialize' => FALSE, + 'nullable' => TRUE, + 'fk_entity' => NULL, + 'value_key' => 'first_name', + 'record' => ['id' => $contact[0]['id']], + 'action' => 'update', + 'value' => 'One', + ]; + // Ensure first_name is editable but not organization_name or household_name + $this->assertEquals($expectedFirstNameEdit, $result[0]['columns'][0]['edit']); + $this->assertTrue(!isset($result[0]['columns'][1]['edit'])); + $this->assertTrue(!isset($result[0]['columns'][2]['edit'])); + + // Second Individual + $expectedFirstNameEdit['record']['id'] = $contact[1]['id']; + $expectedFirstNameEdit['value'] = NULL; + $this->assertEquals($expectedFirstNameEdit, $result[1]['columns'][0]['edit']); + $this->assertTrue(!isset($result[1]['columns'][1]['edit'])); + $this->assertTrue(!isset($result[1]['columns'][2]['edit'])); + + // Third contact: Organization + $expectedFirstNameEdit['record']['id'] = $contact[2]['id']; + $expectedFirstNameEdit['value_key'] = 'organization_name'; + $this->assertTrue(!isset($result[2]['columns'][0]['edit'])); + $this->assertEquals($expectedFirstNameEdit, $result[2]['columns'][1]['edit']); + $this->assertTrue(!isset($result[2]['columns'][2]['edit'])); + + // Third contact: Household + $expectedFirstNameEdit['record']['id'] = $contact[3]['id']; + $expectedFirstNameEdit['value_key'] = 'household_name'; + $this->assertTrue(!isset($result[3]['columns'][0]['edit'])); + $this->assertTrue(!isset($result[3]['columns'][1]['edit'])); + $this->assertEquals($expectedFirstNameEdit, $result[3]['columns'][2]['edit']); + } + } diff --git a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunWithCustomFieldTest.php b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunWithCustomFieldTest.php index d4d51d1b43..3b5bda71fc 100644 --- a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunWithCustomFieldTest.php +++ b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunWithCustomFieldTest.php @@ -6,6 +6,8 @@ require_once 'tests/phpunit/api/v4/Api4TestBase.php'; require_once 'tests/phpunit/api/v4/Custom/CustomTestBase.php'; use api\v4\Custom\CustomTestBase; +use Civi\Api4\Activity; +use Civi\Api4\Contact; use Civi\Api4\CustomField; use Civi\Api4\CustomGroup; @@ -246,7 +248,7 @@ class SearchRunWithCustomFieldTest extends CustomTestBase { $this->assertEquals('abc', $result[0]['columns'][3]['val']); $this->assertEquals($childRel, $result[0]['columns'][3]['edit']['record']['id']); $this->assertNull($result[0]['columns'][4]['val']); - // $this->assertArrayNotHasKey('edit', $result[0]['columns'][4]); + $this->assertArrayNotHasKey('edit', $result[0]['columns'][4]); // Second contact has a spouse relation but not a child $this->assertEquals('s', $result[1]['columns'][1]['val']); @@ -254,7 +256,7 @@ class SearchRunWithCustomFieldTest extends CustomTestBase { $this->assertEquals('Married', $result[1]['columns'][2]['val']); $this->assertEquals($spouseRel, $result[1]['columns'][2]['edit']['record']['id']); $this->assertNull($result[1]['columns'][3]['val']); - // $this->assertArrayNotHasKey('edit', $result[1]['columns'][3]); + $this->assertArrayNotHasKey('edit', $result[1]['columns'][3]); $this->assertNull($result[1]['columns'][4]['val']); $this->assertEquals($spouseRel, $result[1]['columns'][4]['edit']['record']['id']); @@ -269,4 +271,116 @@ class SearchRunWithCustomFieldTest extends CustomTestBase { $this->assertArrayNotHasKey('edit', $result[2]['columns'][4]); } + public function testEditableCustomFields() { + $subject = uniqid(__FUNCTION__); + + $contact = Contact::create(FALSE) + ->execute()->single(); + + // CustomGroup based on Activity Type + CustomGroup::create(FALSE) + ->addValue('extends', 'Activity') + ->addValue('extends_entity_column_value:name', ['Meeting', 'Phone Call']) + ->addValue('title', 'meeting_phone') + ->addChain('field', CustomField::create() + ->addValue('custom_group_id', '$id') + ->addValue('label', 'sub_field') + ->addValue('html_type', 'Text') + ) + ->execute(); + + $sampleData = [ + ['activity_type_id:name' => 'Meeting', 'meeting_phone.sub_field' => 'Abc'], + ['activity_type_id:name' => 'Phone Call'], + ['activity_type_id:name' => 'Email'], + ]; + $activity = $this->saveTestRecords('Activity', [ + 'defaults' => ['subject' => $subject, 'source_contact_id', $contact['id']], + 'records' => $sampleData, + ]); + + $activityTypes = array_column( + Activity::getFields(FALSE)->setLoadOptions(['id', 'name'])->addWhere('name', '=', 'activity_type_id')->execute()->single()['options'], + 'id', + 'name' + ); + + $params = [ + 'checkPermissions' => FALSE, + 'return' => 'page:1', + 'savedSearch' => [ + 'api_entity' => 'Activity', + 'api_params' => [ + 'version' => 4, + 'select' => ['subject', 'meeting_phone.sub_field'], + 'where' => [['subject', '=', $subject]], + ], + ], + 'display' => [ + 'type' => 'table', + 'label' => '', + 'settings' => [ + 'actions' => TRUE, + 'pager' => [], + 'columns' => [ + [ + 'key' => 'subject', + 'label' => 'First', + 'dataType' => 'String', + 'type' => 'field', + 'editable' => TRUE, + ], + [ + 'key' => 'meeting_phone.sub_field', + 'label' => 'First', + 'dataType' => 'String', + 'type' => 'field', + 'editable' => TRUE, + ], + ], + 'sort' => [ + ['id', 'ASC'], + ], + ], + ], + 'afform' => NULL, + ]; + + $result = civicrm_api4('SearchDisplay', 'run', $params); + // Custom field editable + $expectedCustomFieldEdit = [ + 'entity' => 'Activity', + 'input_type' => 'Text', + 'data_type' => 'String', + 'options' => FALSE, + 'serialize' => FALSE, + 'nullable' => TRUE, + 'fk_entity' => NULL, + 'value_key' => 'meeting_phone.sub_field', + 'record' => ['id' => $activity[0]['id']], + 'action' => 'update', + 'value' => 'Abc', + ]; + $expectedSubjectEdit = ['value_key' => 'subject', 'value' => $subject] + $expectedCustomFieldEdit; + + // First Activity + $this->assertEquals($expectedSubjectEdit, $result[0]['columns'][0]['edit']); + $this->assertEquals($expectedCustomFieldEdit, $result[0]['columns'][1]['edit']); + $this->assertEquals($activityTypes['Meeting'], $result[0]['data']['activity_type_id']); + + // Second Activity + $expectedSubjectEdit['record']['id'] = $activity[1]['id']; + $expectedCustomFieldEdit['record']['id'] = $activity[1]['id']; + $expectedCustomFieldEdit['value'] = NULL; + $this->assertEquals($expectedSubjectEdit, $result[1]['columns'][0]['edit']); + $this->assertEquals($expectedCustomFieldEdit, $result[1]['columns'][1]['edit']); + $this->assertEquals($activityTypes['Phone Call'], $result[1]['data']['activity_type_id']); + + // Third Activity + $expectedSubjectEdit['record']['id'] = $activity[2]['id']; + $this->assertEquals($expectedSubjectEdit, $result[2]['columns'][0]['edit']); + $this->assertTrue(!isset($result[2]['columns'][1]['edit'])); + $this->assertEquals($activityTypes['Email'], $result[2]['data']['activity_type_id']); + } + } -- 2.25.1