From: Coleman Watts Date: Mon, 3 Jan 2022 16:48:37 +0000 (-0500) Subject: APIv4 - Add `nullable` property to getFields; improve SearchKit editable UX X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=f8d39baf91d81e4f668a08e7c8a066e0616fb9da;p=civicrm-core.git APIv4 - Add `nullable` property to getFields; improve SearchKit editable UX Unlike the 'required' field property, which only determines if the API requires a value to Create, the 'nullable' property tells a UI whether a field is allowed to be set to NULL in Create OR Update. SearchKit uses this property during in-place edit and bulk edit operations to determine whether a field can be left blank. --- diff --git a/Civi/Api4/Generic/BasicGetFieldsAction.php b/Civi/Api4/Generic/BasicGetFieldsAction.php index 7bb18411ef..483cf7353e 100644 --- a/Civi/Api4/Generic/BasicGetFieldsAction.php +++ b/Civi/Api4/Generic/BasicGetFieldsAction.php @@ -282,9 +282,16 @@ class BasicGetFieldsAction extends BasicGetAction { ], [ 'name' => 'required', + 'description' => 'Is this field required when creating a new entity', 'data_type' => 'Boolean', 'default_value' => FALSE, ], + [ + 'name' => 'nullable', + 'description' => 'Whether a null value is allowed in this field', + 'data_type' => 'Boolean', + 'default_value' => TRUE, + ], [ 'name' => 'required_if', 'data_type' => 'String', diff --git a/Civi/Api4/Service/Spec/FieldSpec.php b/Civi/Api4/Service/Spec/FieldSpec.php index e1da6ce9c1..89f1386416 100644 --- a/Civi/Api4/Service/Spec/FieldSpec.php +++ b/Civi/Api4/Service/Spec/FieldSpec.php @@ -63,6 +63,11 @@ class FieldSpec { /** * @var bool */ + public $nullable = TRUE; + + /** + * @var string + */ public $requiredIf; /** @@ -127,6 +132,24 @@ class FieldSpec { return $this->entity; } + /** + * @return bool + */ + public function getNullable() { + return $this->nullable; + } + + /** + * @param bool $nullable + * + * @return $this + */ + public function setNullable(bool $nullable) { + $this->nullable = $nullable; + + return $this; + } + /** * @return bool */ @@ -146,14 +169,14 @@ class FieldSpec { } /** - * @return bool + * @return string */ public function getRequiredIf() { return $this->requiredIf; } /** - * @param bool $requiredIf + * @param string $requiredIf * * @return $this */ diff --git a/Civi/Api4/Service/Spec/SpecFormatter.php b/Civi/Api4/Service/Spec/SpecFormatter.php index cee41e1ac1..058f83dea2 100644 --- a/Civi/Api4/Service/Spec/SpecFormatter.php +++ b/Civi/Api4/Service/Spec/SpecFormatter.php @@ -37,6 +37,7 @@ class SpecFormatter { $field->setTableName($data['custom_group_id.table_name']); } $field->setColumnName($data['column_name']); + $field->setNullable(empty($data['is_required'])); $field->setCustomFieldId($data['id'] ?? NULL); $field->setCustomGroupName($data['custom_group_id.name']); $field->setTitle($data['label']); @@ -58,7 +59,8 @@ class SpecFormatter { $field = new FieldSpec($name, $entity, $dataTypeName); $field->setType('Field'); $field->setColumnName($name); - $field->setRequired(!empty($data['required'])); + $field->setNullable(empty($data['required'])); + $field->setRequired(!empty($data['required']) && empty($data['default'])); $field->setTitle($data['title'] ?? NULL); $field->setLabel($data['html']['label'] ?? NULL); if (!empty($data['pseudoconstant'])) { diff --git a/Civi/Api4/Service/Spec/SpecGatherer.php b/Civi/Api4/Service/Spec/SpecGatherer.php index c4f49b7c9b..d166207bda 100644 --- a/Civi/Api4/Service/Spec/SpecGatherer.php +++ b/Civi/Api4/Service/Spec/SpecGatherer.php @@ -97,9 +97,6 @@ class SpecGatherer { ) { continue; } - if ($action !== 'create' || isset($DAOField['default'])) { - $DAOField['required'] = FALSE; - } if ($DAOField['name'] == 'is_active' && empty($DAOField['default'])) { $DAOField['default'] = '1'; } diff --git a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php index 8f5a98c72b..28c92d7e16 100644 --- a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php +++ b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php @@ -497,7 +497,7 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction { /** * @param $key - * @return array{entity: string, input_type: string, data_type: string, options: bool, serialize: bool, fk_entity: string, value_key: string, value_path: string, id_key: string, id_path: 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}|null */ private function getEditableInfo($key) { [$key] = explode(':', $key); @@ -520,6 +520,7 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction { 'data_type' => $field['data_type'], 'options' => !empty($field['options']), 'serialize' => !empty($field['serialize']), + 'nullable' => !empty($field['nullable']), 'fk_entity' => $field['fk_entity'], 'value_key' => $field['name'], 'value_path' => $key, diff --git a/ext/search_kit/Civi/Search/Admin.php b/ext/search_kit/Civi/Search/Admin.php index f811dd772f..21f87cfff6 100644 --- a/ext/search_kit/Civi/Search/Admin.php +++ b/ext/search_kit/Civi/Search/Admin.php @@ -136,7 +136,7 @@ class Admin { $entity['links'] = array_values($links); } $getFields = civicrm_api4($entity['name'], 'getFields', [ - 'select' => ['name', 'title', 'label', 'description', 'type', 'options', 'input_type', 'input_attrs', 'data_type', 'serialize', 'entity', 'fk_entity', 'readonly', 'operators'], + 'select' => ['name', 'title', 'label', 'description', 'type', 'options', 'input_type', 'input_attrs', 'data_type', 'serialize', 'entity', 'fk_entity', 'readonly', 'operators', 'nullable'], 'where' => [['name', 'NOT IN', ['api_key', 'hash']]], 'orderBy' => ['label'], ]); diff --git a/ext/search_kit/ang/crmSearchDisplay/crmSearchDisplayEditable.component.js b/ext/search_kit/ang/crmSearchDisplay/crmSearchDisplayEditable.component.js index 052b9dcfbc..f205e4a3aa 100644 --- a/ext/search_kit/ang/crmSearchDisplay/crmSearchDisplayEditable.component.js +++ b/ext/search_kit/ang/crmSearchDisplay/crmSearchDisplayEditable.component.js @@ -29,6 +29,7 @@ options: col.edit.options, fk_entity: col.edit.fk_entity, serialize: col.edit.serialize, + nullable: col.edit.nullable }; $(document).on('keydown.crmSearchDisplayEditable', function(e) { @@ -36,8 +37,6 @@ $scope.$apply(function() { ctrl.cancel(); }); - } else if (e.key === 'Enter') { - $scope.$apply(ctrl.save); } }); @@ -71,7 +70,7 @@ action: 'update', select: ['options'], loadOptions: ['id', 'name', 'label', 'description', 'color', 'icon'], - where: [['name', '=', ctrl.field.name]], + where: [['name', '=', ctrl.field.name]] }, 0).then(function(field) { ctrl.field.options = optionsCache[cacheKey] = field.options; }); diff --git a/ext/search_kit/ang/crmSearchDisplay/crmSearchDisplayEditable.html b/ext/search_kit/ang/crmSearchDisplay/crmSearchDisplayEditable.html index d0d8eca08f..767c642294 100644 --- a/ext/search_kit/ang/crmSearchDisplay/crmSearchDisplayEditable.html +++ b/ext/search_kit/ang/crmSearchDisplay/crmSearchDisplayEditable.html @@ -1,9 +1,11 @@ - -
- - -
+
+ +
+ + +
+
diff --git a/ext/search_kit/ang/crmSearchTasks/crmSearchInput/date.html b/ext/search_kit/ang/crmSearchTasks/crmSearchInput/date.html index 78e4d03d6d..29d89573e5 100644 --- a/ext/search_kit/ang/crmSearchTasks/crmSearchInput/date.html +++ b/ext/search_kit/ang/crmSearchTasks/crmSearchInput/date.html @@ -10,8 +10,8 @@
- - + +
diff --git a/ext/search_kit/ang/crmSearchTasks/crmSearchInput/float.html b/ext/search_kit/ang/crmSearchTasks/crmSearchInput/float.html index 89b0079a78..f34a1184a9 100644 --- a/ext/search_kit/ang/crmSearchTasks/crmSearchInput/float.html +++ b/ext/search_kit/ang/crmSearchTasks/crmSearchInput/float.html @@ -1,6 +1,6 @@
- +
- +
diff --git a/ext/search_kit/ang/crmSearchTasks/crmSearchInput/integer.html b/ext/search_kit/ang/crmSearchTasks/crmSearchInput/integer.html index 0c5e75c49a..3f480c2b14 100644 --- a/ext/search_kit/ang/crmSearchTasks/crmSearchInput/integer.html +++ b/ext/search_kit/ang/crmSearchTasks/crmSearchInput/integer.html @@ -1,6 +1,6 @@
- +
- +
diff --git a/ext/search_kit/ang/crmSearchTasks/crmSearchInput/select.html b/ext/search_kit/ang/crmSearchTasks/crmSearchInput/select.html index 464c92e431..b46e8014b3 100644 --- a/ext/search_kit/ang/crmSearchTasks/crmSearchInput/select.html +++ b/ext/search_kit/ang/crmSearchTasks/crmSearchInput/select.html @@ -2,7 +2,7 @@
- +
diff --git a/ext/search_kit/ang/crmSearchTasks/crmSearchInput/text.html b/ext/search_kit/ang/crmSearchTasks/crmSearchInput/text.html index 61a4391c48..3718a11da7 100644 --- a/ext/search_kit/ang/crmSearchTasks/crmSearchInput/text.html +++ b/ext/search_kit/ang/crmSearchTasks/crmSearchInput/text.html @@ -1,6 +1,6 @@
- +
- +
diff --git a/ext/search_kit/ang/crmSearchTasks/crmSearchTaskUpdate.ctrl.js b/ext/search_kit/ang/crmSearchTasks/crmSearchTaskUpdate.ctrl.js index 5fd14a504c..01f41d16fd 100644 --- a/ext/search_kit/ang/crmSearchTasks/crmSearchTaskUpdate.ctrl.js +++ b/ext/search_kit/ang/crmSearchTasks/crmSearchTaskUpdate.ctrl.js @@ -13,7 +13,7 @@ crmApi4(this.entity, 'getFields', { action: 'update', - select: ['name', 'label', 'description', 'input_type', 'data_type', 'serialize', 'options', 'fk_entity'], + select: ['name', 'label', 'description', 'input_type', 'data_type', 'serialize', 'options', 'fk_entity', 'nullable'], loadOptions: ['id', 'name', 'label', 'description', 'color', 'icon'], where: [["readonly", "=", false]], }).then(function(fields) { diff --git a/ext/search_kit/ang/crmSearchTasks/crmSearchTaskUpdate.html b/ext/search_kit/ang/crmSearchTasks/crmSearchTaskUpdate.html index 9ae3811079..42a9fcc2ee 100644 --- a/ext/search_kit/ang/crmSearchTasks/crmSearchTaskUpdate.html +++ b/ext/search_kit/ang/crmSearchTasks/crmSearchTaskUpdate.html @@ -1,5 +1,5 @@
-
+

{{:: ts('Update the %1 selected %2 with the following values:', {1: model.ids.length, 2: $ctrl.entityTitle}) }}

@@ -18,7 +18,7 @@ {{:: ts('Cancel') }} - diff --git a/ext/search_kit/ang/crmSearchTasks/crmSearchTasks.component.js b/ext/search_kit/ang/crmSearchTasks/crmSearchTasks.component.js index 04dd42a820..d9953b202d 100644 --- a/ext/search_kit/ang/crmSearchTasks/crmSearchTasks.component.js +++ b/ext/search_kit/ang/crmSearchTasks/crmSearchTasks.component.js @@ -65,6 +65,7 @@ else if (action.uiDialog) { var options = CRM.utils.adjustDialogDefaults({ autoOpen: false, + dialogClass: 'crm-search-task-dialog', title: action.title }); dialogService.open('crmSearchTask', action.uiDialog.templateUrl, data, options) diff --git a/ext/search_kit/css/crmSearchAdmin.css b/ext/search_kit/css/crmSearchAdmin.css index ca9225ef3c..f69558afdc 100644 --- a/ext/search_kit/css/crmSearchAdmin.css +++ b/ext/search_kit/css/crmSearchAdmin.css @@ -33,9 +33,6 @@ cursor: not-allowed; } -#bootstrap-theme.crm-search input.ng-invalid { - border-color: #8a1f11; -} #bootstrap-theme.crm-search input.ng-invalid::placeholder { color: #8a1f11; } diff --git a/ext/search_kit/css/crmSearchDisplay.css b/ext/search_kit/css/crmSearchDisplay.css index 7326688ea2..a3a736e74f 100644 --- a/ext/search_kit/css/crmSearchDisplay.css +++ b/ext/search_kit/css/crmSearchDisplay.css @@ -10,6 +10,12 @@ opacity: .5; } +#bootstrap-theme.crm-search input.ng-invalid, +#bootstrap-theme.crm-search-display input.ng-invalid, +.crm-search-task-dialog #bootstrap-theme input.ng-invalid { + border-color: #8a1f11; +} + /* Loading placeholders */ #bootstrap-theme .crm-search-loading-placeholder { height: 2em; diff --git a/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php b/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php index 4cc32bbcf0..8c8c4c7df1 100644 --- a/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php +++ b/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php @@ -130,12 +130,22 @@ class BasicCustomFieldTest extends BaseCustomValueTest { ->addValue('label', 'FavFood') ->addValue('custom_group_id', '$id') ->addValue('html_type', 'Text') + ->addValue('is_required', TRUE) ->addValue('data_type', 'String')) ->execute(); // Test that no new option groups have been created (these are text fields with no options) $this->assertEquals($optionGroupCount, OptionGroup::get(FALSE)->selectRowCount()->execute()->count()); + // Check getFields output + $fields = Contact::getFields(FALSE)->execute()->indexBy('name'); + $this->assertFalse($fields['MyContactFields2.FavColor']['required']); + $this->assertTRUE($fields['MyContactFields2.FavColor']['nullable']); + // Custom fields are never actually *required* in the api, even if is_required = true + $this->assertFalse($fields['MyContactFields2.FavFood']['required']); + // But the api will report is_required as not nullable + $this->assertFalse($fields['MyContactFields2.FavFood']['nullable']); + $contactId1 = Contact::create(FALSE) ->addValue('first_name', 'Johann') ->addValue('last_name', 'Tester') diff --git a/tests/phpunit/api/v4/Action/GetFieldsTest.php b/tests/phpunit/api/v4/Action/GetFieldsTest.php index d5833dd9e7..e58f066520 100644 --- a/tests/phpunit/api/v4/Action/GetFieldsTest.php +++ b/tests/phpunit/api/v4/Action/GetFieldsTest.php @@ -20,6 +20,7 @@ namespace api\v4\Action; use api\v4\UnitTestCase; +use Civi\Api4\Activity; use Civi\Api4\Campaign; use Civi\Api4\Contact; use Civi\Api4\Contribution; @@ -93,4 +94,15 @@ class GetFieldsTest extends UnitTestCase { $this->assertEquals(['name', 'label'], $fields['campaign_id']['suffixes']); } + public function testRequiredAndNullable() { + $actFields = Activity::getFields(FALSE) + ->setAction('create') + ->execute()->indexBy('name'); + + $this->assertTrue($actFields['activity_type_id']['required']); + $this->assertFalse($actFields['activity_type_id']['nullable']); + $this->assertFalse($actFields['subject']['required']); + $this->assertTrue($actFields['subject']['nullable']); + } + }