Skip to content

Commit

Permalink
APIv4 - Add nullable property to getFields; improve SearchKit edita…
Browse files Browse the repository at this point in the history
…ble 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.
  • Loading branch information
colemanw committed Jan 25, 2022
1 parent a90c387 commit 4db672c
Show file tree
Hide file tree
Showing 16 changed files with 83 additions and 30 deletions.
7 changes: 7 additions & 0 deletions Civi/Api4/Generic/BasicGetFieldsAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -282,9 +282,16 @@ public function fields() {
],
[
'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',
Expand Down
27 changes: 25 additions & 2 deletions Civi/Api4/Service/Spec/FieldSpec.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ class FieldSpec {
/**
* @var bool
*/
public $nullable = TRUE;

/**
* @var string
*/
public $requiredIf;

/**
Expand Down Expand Up @@ -127,6 +132,24 @@ public function getEntity() {
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
*/
Expand All @@ -146,14 +169,14 @@ public function setRequired($required) {
}

/**
* @return bool
* @return string
*/
public function getRequiredIf() {
return $this->requiredIf;
}

/**
* @param bool $requiredIf
* @param string $requiredIf
*
* @return $this
*/
Expand Down
4 changes: 3 additions & 1 deletion Civi/Api4/Service/Spec/SpecFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public static function arrayToField(array $data, $entity) {
$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']);
Expand All @@ -58,7 +59,8 @@ public static function arrayToField(array $data, $entity) {
$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'])) {
Expand Down
3 changes: 0 additions & 3 deletions Civi/Api4/Service/Spec/SpecGatherer.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,6 @@ private function addDAOFields($entity, $action, RequestSpec $spec) {
) {
continue;
}
if ($action !== 'create' || isset($DAOField['default'])) {
$DAOField['required'] = FALSE;
}
if ($DAOField['name'] == 'is_active' && empty($DAOField['default'])) {
$DAOField['default'] = '1';
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ private function formatEditableColumn($column, $data) {

/**
* @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);
Expand All @@ -520,6 +520,7 @@ private function getEditableInfo($key) {
'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,
Expand Down
2 changes: 1 addition & 1 deletion ext/search_kit/Civi/Search/Admin.php
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public static function getSchema(): array {
$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'],
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,14 @@
options: col.edit.options,
fk_entity: col.edit.fk_entity,
serialize: col.edit.serialize,
nullable: col.edit.nullable
};

$(document).on('keydown.crmSearchDisplayEditable', function(e) {
if (e.key === 'Escape') {
$scope.$apply(function() {
ctrl.cancel();
});
} else if (e.key === 'Enter') {
$scope.$apply(ctrl.save);
}
});

Expand Down Expand Up @@ -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;
});
Expand Down
20 changes: 11 additions & 9 deletions ext/search_kit/ang/crmSearchDisplay/crmSearchDisplayEditable.html
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
<crm-search-input class="form-inline" field="$ctrl.field" ng-model="$ctrl.value"></crm-search-input>
<div class="form-inline crm-search-display-editable-buttons">
<button type="button" ng-click="$ctrl.save()" class="btn btn-xs btn-success">
<i class="crm-i fa-check"></i>
</button>
<button type="button" ng-click="$ctrl.cancel()" class="btn btn-xs btn-danger">
<i class="crm-i fa-times"></i>
</button>
</div>
<form name="crmSearchDisplayEditableForm">
<crm-search-input class="form-inline" field="$ctrl.field" ng-model="$ctrl.value"></crm-search-input>
<div class="form-inline crm-search-display-editable-buttons">
<button type="submit" ng-disabled="!crmSearchDisplayEditableForm.$valid" ng-click="$ctrl.save()" class="btn btn-xs btn-success">
<i class="crm-i fa-check"></i>
</button>
<button type="button" ng-click="$ctrl.cancel()" class="btn btn-xs btn-danger">
<i class="crm-i fa-times"></i>
</button>
</div>
</form>
4 changes: 2 additions & 2 deletions ext/search_kit/ang/crmSearchTasks/crmSearchInput/date.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
<div class="form-group" ng-switch="$ctrl.dateType">

<div class="form-group" ng-switch-when="fixed">
<input class="form-control" crm-ui-datepicker="{time: $ctrl.field.data_type === 'Timestamp'}" ng-model="$ctrl.value" ng-if="!$ctrl.multi">
<input class="form-control" crm-multi-select-date ng-model="$ctrl.value" ng-if="$ctrl.multi">
<input class="form-control" crm-ui-datepicker="{time: $ctrl.field.data_type === 'Timestamp'}" ng-model="$ctrl.value" ng-required="!$ctrl.field.nullable" ng-if="!$ctrl.multi">
<input class="form-control" crm-multi-select-date ng-model="$ctrl.value" ng-required="!$ctrl.field.nullable" ng-if="$ctrl.multi">
</div>

<div class="form-group" ng-switch-when="range">
Expand Down
4 changes: 2 additions & 2 deletions ext/search_kit/ang/crmSearchTasks/crmSearchInput/float.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<div class="form-group" ng-if="!$ctrl.multi" >
<input type="number" step="any" class="form-control" ng-model="$ctrl.value">
<input type="number" step="any" class="form-control" ng-model="$ctrl.value" ng-required="!$ctrl.field.nullable">
</div>
<div class="form-group" ng-if="$ctrl.multi" >
<input class="form-control" ng-model="$ctrl.value" crm-ui-select="{multiple: true, tags: [], tokenSeparators: [','], formatNoMatches: ''}" ng-list>
<input class="form-control" ng-model="$ctrl.value" ng-required="!$ctrl.field.nullable" crm-ui-select="{multiple: true, tags: [], tokenSeparators: [','], formatNoMatches: ''}" ng-list>
</div>
4 changes: 2 additions & 2 deletions ext/search_kit/ang/crmSearchTasks/crmSearchInput/integer.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<div class="form-group" ng-if="!$ctrl.multi" >
<input type="number" step="1" class="form-control" ng-model="$ctrl.value">
<input type="number" step="1" class="form-control" ng-model="$ctrl.value" ng-required="!$ctrl.field.nullable">
</div>
<div class="form-group" ng-if="$ctrl.multi" >
<input class="form-control" ng-model="$ctrl.value" crm-ui-select="{multiple: true, tags: [], tokenSeparators: [','], formatNoMatches: ''}" ng-list>
<input class="form-control" ng-model="$ctrl.value" ng-required="!$ctrl.field.nullable" crm-ui-select="{multiple: true, tags: [], tokenSeparators: [','], formatNoMatches: ''}" ng-list>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<input disabled class="form-control loading" crm-ui-select="{data: []}">
</div>
<div class="form-group" ng-if="!$ctrl.multi && $ctrl.field.options !== true">
<input class="form-control" ng-model="$ctrl.value" crm-ui-select="{data: $ctrl.getFieldOptions}">
<input class="form-control" ng-model="$ctrl.value" crm-ui-select="{data: $ctrl.getFieldOptions, allowClear: $ctrl.field.nullable, placeholder: $ctrl.field.nullable ? ts('None') : ts('Select')}">
</div>
<div class="form-group" ng-if="$ctrl.multi && $ctrl.field.options !== true">
<input class="form-control" ng-model="$ctrl.value" crm-ui-select="{data: $ctrl.getFieldOptions, multiple: true}" ng-list >
Expand Down
4 changes: 2 additions & 2 deletions ext/search_kit/ang/crmSearchTasks/crmSearchInput/text.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<div class="form-group" ng-if="!$ctrl.multi" >
<input type="text" class="form-control" ng-model="$ctrl.value">
<input type="text" class="form-control" ng-model="$ctrl.value" ng-required="!$ctrl.field.nullable">
</div>
<div class="form-group" ng-if="$ctrl.multi" >
<input class="form-control" ng-model="$ctrl.value" crm-ui-select="{multiple: true, tags: [], tokenSeparators: [','], formatNoMatches: ''}" ng-list>
<input class="form-control" ng-model="$ctrl.value" ng-required="!$ctrl.field.nullable" crm-ui-select="{multiple: true, tags: [], tokenSeparators: [','], formatNoMatches: ''}" ng-list>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
10 changes: 10 additions & 0 deletions tests/phpunit/api/v4/Action/BasicCustomFieldTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,22 @@ public function testWithTwoFields() {
->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')
Expand Down
12 changes: 12 additions & 0 deletions tests/phpunit/api/v4/Action/GetFieldsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -93,4 +94,15 @@ public function testPreloadFalse() {
$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']);
}

}

0 comments on commit 4db672c

Please sign in to comment.