From e420fe8d59795a5106ce7a07c68cc34213b2a0b2 Mon Sep 17 00:00:00 2001 From: colemanw Date: Mon, 4 Sep 2023 20:56:17 -0400 Subject: [PATCH] APIv4 - Fix setting nullable/required/default_value field metadata --- .../ACLEntityRoleCreationSpecProvider.php | 37 ------------------- .../Spec/Provider/CustomValueSpecProvider.php | 3 ++ .../Provider/EmailCreationSpecProvider.php | 2 +- .../Provider/GetActionDefaultsProvider.php | 6 +-- Civi/Api4/Service/Spec/SpecFormatter.php | 9 ++++- Civi/Api4/Service/Spec/SpecGatherer.php | 3 -- tests/phpunit/api/v4/Action/GetFieldsTest.php | 37 +++++++++++++++++++ .../phpunit/api/v4/Custom/CustomValueTest.php | 2 + 8 files changed, 53 insertions(+), 46 deletions(-) delete mode 100644 Civi/Api4/Service/Spec/Provider/ACLEntityRoleCreationSpecProvider.php diff --git a/Civi/Api4/Service/Spec/Provider/ACLEntityRoleCreationSpecProvider.php b/Civi/Api4/Service/Spec/Provider/ACLEntityRoleCreationSpecProvider.php deleted file mode 100644 index c6da836a71..0000000000 --- a/Civi/Api4/Service/Spec/Provider/ACLEntityRoleCreationSpecProvider.php +++ /dev/null @@ -1,37 +0,0 @@ -getFieldByName('is_active')->setDefaultValue(1); - } - - /** - * @inheritDoc - */ - public function applies($entity, $action) { - return $entity === 'ACLEntityRole' && $action === 'create'; - } - -} diff --git a/Civi/Api4/Service/Spec/Provider/CustomValueSpecProvider.php b/Civi/Api4/Service/Spec/Provider/CustomValueSpecProvider.php index 2cb57becd1..1da2127af6 100644 --- a/Civi/Api4/Service/Spec/Provider/CustomValueSpecProvider.php +++ b/Civi/Api4/Service/Spec/Provider/CustomValueSpecProvider.php @@ -31,8 +31,10 @@ class CustomValueSpecProvider extends \Civi\Core\Service\AutoService implements $idField->setType('Field'); $idField->setInputType('Number'); $idField->setColumnName('id'); + $idField->setNullable('false'); $idField->setTitle(ts('Custom Value ID')); $idField->setReadonly(TRUE); + $idField->setNullable(FALSE); $spec->addFieldSpec($idField); $entityField = new FieldSpec('entity_id', $spec->getEntity(), 'Integer'); @@ -43,6 +45,7 @@ class CustomValueSpecProvider extends \Civi\Core\Service\AutoService implements $entityField->setRequired($action === 'create'); $entityField->setFkEntity('Contact'); $entityField->setReadonly(TRUE); + $entityField->setNullable(FALSE); $entityField->setInputType('EntityRef'); $spec->addFieldSpec($entityField); } diff --git a/Civi/Api4/Service/Spec/Provider/EmailCreationSpecProvider.php b/Civi/Api4/Service/Spec/Provider/EmailCreationSpecProvider.php index 761df4da39..83598ae350 100644 --- a/Civi/Api4/Service/Spec/Provider/EmailCreationSpecProvider.php +++ b/Civi/Api4/Service/Spec/Provider/EmailCreationSpecProvider.php @@ -30,7 +30,7 @@ class EmailCreationSpecProvider extends \Civi\Core\Service\AutoService implement $spec->getFieldByName('is_primary')->setRequired(FALSE); $defaultLocationType = \CRM_Core_BAO_LocationType::getDefault()->id ?? NULL; - $spec->getFieldByName('location_type_id')->setDefaultValue($defaultLocationType); + $spec->getFieldByName('location_type_id')->setDefaultValue($defaultLocationType ? (int) $defaultLocationType : NULL); } /** diff --git a/Civi/Api4/Service/Spec/Provider/GetActionDefaultsProvider.php b/Civi/Api4/Service/Spec/Provider/GetActionDefaultsProvider.php index a8e8616531..45b14c25a5 100644 --- a/Civi/Api4/Service/Spec/Provider/GetActionDefaultsProvider.php +++ b/Civi/Api4/Service/Spec/Provider/GetActionDefaultsProvider.php @@ -27,18 +27,18 @@ class GetActionDefaultsProvider extends \Civi\Core\Service\AutoService implement // Exclude deleted records from api Get by default $isDeletedField = $spec->getFieldByName('is_deleted'); if ($isDeletedField) { - $isDeletedField->setDefaultValue('0'); + $isDeletedField->setDefaultValue(FALSE); } // Exclude test records from api Get by default $isTestField = $spec->getFieldByName('is_test'); if ($isTestField) { - $isTestField->setDefaultValue('0'); + $isTestField->setDefaultValue(FALSE); } $isTemplateField = $spec->getFieldByName('is_template'); if ($isTemplateField) { - $isTemplateField->setDefaultValue('0'); + $isTemplateField->setDefaultValue(FALSE); } } diff --git a/Civi/Api4/Service/Spec/SpecFormatter.php b/Civi/Api4/Service/Spec/SpecFormatter.php index d44fd43f50..1b21b781e2 100644 --- a/Civi/Api4/Service/Spec/SpecFormatter.php +++ b/Civi/Api4/Service/Spec/SpecFormatter.php @@ -26,6 +26,8 @@ class SpecFormatter { public static function arrayToField(array $data, $entity) { $dataTypeName = self::getDataType($data); + $hasDefault = isset($data['default']) && $data['default'] !== ''; + // Custom field if (!empty($data['custom_group_id'])) { $field = new CustomFieldSpec($data['name'], $entity, $dataTypeName); if (strpos($entity, 'Custom_') !== 0) { @@ -57,13 +59,14 @@ class SpecFormatter { } $field->setReadonly($data['is_view']); } + // Core field else { $name = $data['name'] ?? NULL; $field = new FieldSpec($name, $entity, $dataTypeName); $field->setType('Field'); $field->setColumnName($name); $field->setNullable(empty($data['required'])); - $field->setRequired(!empty($data['required']) && empty($data['default'])); + $field->setRequired(!empty($data['required']) && !$hasDefault && $name !== 'id'); $field->setTitle($data['title'] ?? NULL); $field->setLabel($data['html']['label'] ?? NULL); $field->setLocalizable($data['localizable'] ?? FALSE); @@ -93,8 +96,10 @@ class SpecFormatter { } $field->setReadonly(!empty($data['readonly'])); } + if ($hasDefault) { + $field->setDefaultValue(FormattingUtil::convertDataType($data['default'], $dataTypeName)); + } $field->setSerialize($data['serialize'] ?? NULL); - $field->setDefaultValue($data['default'] ?? NULL); $field->setDescription($data['description'] ?? NULL); $field->setDeprecated($data['deprecated'] ?? FALSE); self::setInputTypeAndAttrs($field, $data, $dataTypeName); diff --git a/Civi/Api4/Service/Spec/SpecGatherer.php b/Civi/Api4/Service/Spec/SpecGatherer.php index 75222965df..80ce71555a 100644 --- a/Civi/Api4/Service/Spec/SpecGatherer.php +++ b/Civi/Api4/Service/Spec/SpecGatherer.php @@ -91,9 +91,6 @@ class SpecGatherer extends AutoService { $DAOFields = $this->getDAOFields($entity); foreach ($DAOFields as $DAOField) { - if ($DAOField['name'] == 'id' && $action == 'create') { - $DAOField['required'] = FALSE; - } if (array_key_exists('contactType', $DAOField) && $spec->getValue('contact_type') && $DAOField['contactType'] != $spec->getValue('contact_type')) { continue; } diff --git a/tests/phpunit/api/v4/Action/GetFieldsTest.php b/tests/phpunit/api/v4/Action/GetFieldsTest.php index 359dfab28a..f919dfec0b 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\Api4TestBase; +use Civi\Api4\ACLEntityRole; use Civi\Api4\Activity; use Civi\Api4\Address; use Civi\Api4\Campaign; @@ -68,6 +69,13 @@ class GetFieldsTest extends Api4TestBase implements TransactionalInterface { // Check suffixes $this->assertEquals(['name', 'label', 'icon'], $fields['contact_type']['suffixes']); $this->assertEquals(['name', 'label', 'icon'], $fields['contact_sub_type']['suffixes']); + + // Check `required` and `nullable` + $this->assertFalse($fields['is_deleted']['required']); + $this->assertFalse($fields['is_deleted']['nullable']); + $this->assertFalse($fields['id']['nullable']); + $this->assertFalse($fields['id']['required']); + $this->assertNull($fields['id']['default_value']); } public function testComponentFields(): void { @@ -95,6 +103,15 @@ class GetFieldsTest extends Api4TestBase implements TransactionalInterface { ->execute()->indexBy('name'); $this->assertEquals('Email', $createFields['email']['input_type']); + $this->assertIsInt($createFields['location_type_id']['default_value']); + + // Check `required` and `nullable` + $this->assertFalse($createFields['is_primary']['required']); + $this->assertFalse($createFields['is_primary']['nullable']); + $this->assertFalse($createFields['is_primary']['default_value']); + $this->assertFalse($createFields['id']['required']); + $this->assertFalse($createFields['id']['nullable']); + $this->assertNull($createFields['id']['default_value']); } public function testInternalPropsAreHidden(): void { @@ -131,12 +148,32 @@ class GetFieldsTest extends Api4TestBase implements TransactionalInterface { ->execute()->indexBy('name'); $this->assertFalse($actFields['id']['required']); + $this->assertFalse($actFields['id']['nullable']); $this->assertTrue($actFields['activity_type_id']['required']); $this->assertFalse($actFields['activity_type_id']['nullable']); + $this->assertNull($actFields['activity_type_id']['default_value']); + $this->assertFalse($actFields['is_deleted']['required']); + $this->assertFalse($actFields['is_deleted']['nullable']); + $this->assertFalse($actFields['is_deleted']['default_value']); $this->assertFalse($actFields['subject']['required']); $this->assertTrue($actFields['subject']['nullable']); $this->assertFalse($actFields['subject']['deprecated']); $this->assertTrue($actFields['phone_id']['deprecated']); + + $getFields = Activity::getFields(FALSE) + ->setAction('get') + ->execute()->indexBy('name'); + + $this->assertFalse($getFields['is_deleted']['required']); + $this->assertFalse($getFields['is_deleted']['nullable']); + $this->assertFalse($getFields['is_deleted']['default_value']); + + $aclFields = ACLEntityRole::getFields(FALSE) + ->setAction('create') + ->execute()->indexBy('name'); + $this->assertTrue($aclFields['is_active']['default_value']); + $this->assertFalse($aclFields['is_active']['nullable']); + $this->assertFalse($aclFields['is_active']['required']); } public function testGetSuffixes(): void { diff --git a/tests/phpunit/api/v4/Custom/CustomValueTest.php b/tests/phpunit/api/v4/Custom/CustomValueTest.php index 1c81316d3c..d2ec8bde91 100644 --- a/tests/phpunit/api/v4/Custom/CustomValueTest.php +++ b/tests/phpunit/api/v4/Custom/CustomValueTest.php @@ -138,6 +138,7 @@ class CustomValueTest extends CustomTestBase { 'entity' => "Custom_$group", 'table_name' => $customGroup['table_name'], 'column_name' => 'id', + 'nullable' => FALSE, 'data_type' => 'Integer', 'fk_entity' => NULL, ], @@ -148,6 +149,7 @@ class CustomValueTest extends CustomTestBase { 'table_name' => $customGroup['table_name'], 'column_name' => 'entity_id', 'entity' => "Custom_$group", + 'nullable' => FALSE, 'data_type' => 'Integer', 'fk_entity' => 'Contact', ], -- 2.25.1