From 4f94e3fac75687b050154ec84413796249bb4fb5 Mon Sep 17 00:00:00 2001 From: Mattias Michaux Date: Tue, 17 May 2016 21:23:25 +0200 Subject: [PATCH] Fixed API test failures due to correct check on error message. --- Civi/API/Kernel.php | 2 +- api/v3/MembershipStatus.php | 12 +++++ api/v3/Profile.php | 13 +++++- api/v3/utils.php | 46 ++++++++++++------- tests/phpunit/api/v3/APITest.php | 2 +- tests/phpunit/api/v3/LoggingTest.php | 2 +- tests/phpunit/api/v3/MailingTest.php | 2 +- tests/phpunit/api/v3/NoteTest.php | 4 +- tests/phpunit/api/v3/PaymentTest.php | 6 +-- tests/phpunit/api/v3/RelationshipTest.php | 4 +- tests/phpunit/api/v3/RelationshipTypeTest.php | 2 +- tests/phpunit/api/v3/UFGroupTest.php | 2 +- 12 files changed, 67 insertions(+), 30 deletions(-) diff --git a/Civi/API/Kernel.php b/Civi/API/Kernel.php index dc2de659ff..a22c3fcf63 100644 --- a/Civi/API/Kernel.php +++ b/Civi/API/Kernel.php @@ -348,7 +348,7 @@ class Kernel { if ($msg == 'DB Error: constraint violation' || substr($msg, 0, 9) == 'DB Error:' || $msg == 'DB Error: already exists') { try { $fields = _civicrm_api3_api_getfields($apiRequest); - _civicrm_api3_validate_fields($apiRequest['entity'], $apiRequest['action'], $apiRequest['params'], $fields, TRUE); + _civicrm_api3_validate_foreign_keys($apiRequest['entity'], $apiRequest['action'], $apiRequest['params'], $fields); } catch (\Exception $e) { $msg = $e->getMessage(); diff --git a/api/v3/MembershipStatus.php b/api/v3/MembershipStatus.php index fd57834281..499c823fe3 100644 --- a/api/v3/MembershipStatus.php +++ b/api/v3/MembershipStatus.php @@ -43,6 +43,18 @@ function civicrm_api3_membership_status_create($params) { return _civicrm_api3_basic_create(_civicrm_api3_get_BAO(__FUNCTION__), $params); } +/** + * Adjust Metadata for Create action. + * + * The metadata is used for setting defaults, documentation & validation. + * + * @param array $params + * Array of parameters determined by getfields. + */ +function _civicrm_api3_membership_status_create_spec(&$params) { + $params['name']['api.required'] = 1; +} + /** * Get a membership status. * diff --git a/api/v3/Profile.php b/api/v3/Profile.php index 3607b42849..ab7f1c092d 100644 --- a/api/v3/Profile.php +++ b/api/v3/Profile.php @@ -348,12 +348,23 @@ function civicrm_api3_profile_apply($params) { ); if (empty($data)) { - throw new API_Exception('Enable to format profile parameters.'); + throw new API_Exception('Unable to format profile parameters.'); } return civicrm_api3_create_success($data); } +/** + * Adjust Metadata for Apply action. + * + * The metadata is used for setting defaults, documentation & validation. + * + * @param array $params + * Array of parameters determined by getfields. + */ +function _civicrm_api3_profile_apply_spec(&$params) { + $params['profile_id']['api.required'] = 1; +} /** * Get pseudo profile 'billing'. diff --git a/api/v3/utils.php b/api/v3/utils.php index 916eae4c04..c6b1255650 100644 --- a/api/v3/utils.php +++ b/api/v3/utils.php @@ -1539,8 +1539,7 @@ function _civicrm_api3_custom_data_get(&$returnArray, $checkPermission, $entity, * Validate fields being passed into API. * * This function relies on the getFields function working accurately - * for the given API. If error mode is set to TRUE then it will also check - * foreign keys + * for the given API. * * As of writing only date was implemented. * @@ -1550,8 +1549,6 @@ function _civicrm_api3_custom_data_get(&$returnArray, $checkPermission, $entity, * -. * @param array $fields * Response from getfields all variables are the same as per civicrm_api. - * @param bool $errorMode - * ErrorMode do intensive post fail checks?. * * @throws Exception */ @@ -1611,22 +1608,39 @@ function _civicrm_api3_validate_fields($entity, $action, &$params, $fields, $err } break; } + } +} - // intensive checks - usually only called after DB level fail - if (!empty($errorMode) && strtolower($action) == 'create') { - if (!empty($fieldInfo['FKClassName'])) { - if (!empty($fieldValue)) { - _civicrm_api3_validate_constraint($params, $fieldName, $fieldInfo); - } - elseif (!empty($fieldInfo['required'])) { - throw new Exception("DB Constraint Violation - possibly $fieldName should possibly be marked as mandatory for this API. If so, please raise a bug report."); - } +/** + * Validate foreign key values of fields being passed into API. + * + * This function relies on the getFields function working accurately + * for the given API. + * + * @param string $entity + * @param string $action + * @param array $params + * + * @param array $fields + * Response from getfields all variables are the same as per civicrm_api. + * + * @throws Exception + */ +function _civicrm_api3_validate_foreign_keys($entity, $action, &$params, $fields) { + // intensive checks - usually only called after DB level fail + foreach ($fields as $fieldName => $fieldInfo) { + if (!empty($fieldInfo['FKClassName'])) { + if (!empty($params[$fieldName])) { + _civicrm_api3_validate_constraint($params[$fieldName], $fieldName, $fieldInfo); } - if (!empty($fieldInfo['api.unique'])) { - $params['entity'] = $entity; - _civicrm_api3_validate_unique_key($params, $fieldName); + elseif (!empty($fieldInfo['required'])) { + throw new Exception("DB Constraint Violation - possibly $fieldName should possibly be marked as mandatory for this API. If so, please raise a bug report."); } } + if (!empty($fieldInfo['api.unique'])) { + $params['entity'] = $entity; + _civicrm_api3_validate_unique_key($params, $fieldName); + } } } diff --git a/tests/phpunit/api/v3/APITest.php b/tests/phpunit/api/v3/APITest.php index 8bcbea43a7..cfa2eae595 100644 --- a/tests/phpunit/api/v3/APITest.php +++ b/tests/phpunit/api/v3/APITest.php @@ -74,7 +74,7 @@ class api_v3_APITest extends CiviUnitTestCase { 'RandomFile', 'get', array(), - 'API (RandomFile, get) does not exist (join the API team and implement it!)' + 'API (RandomFile, get) does not exist (join the API team and implement it!)' ); } diff --git a/tests/phpunit/api/v3/LoggingTest.php b/tests/phpunit/api/v3/LoggingTest.php index 308622cc15..2c8c16f123 100644 --- a/tests/phpunit/api/v3/LoggingTest.php +++ b/tests/phpunit/api/v3/LoggingTest.php @@ -307,7 +307,7 @@ class api_v3_LoggingTest extends CiviUnitTestCase { 'Logging', 'revert', array('log_conn_id' => 'Wopity woot'), - 'Failure in api call for Logging revert: The connection date must be passed in to disambiguate this logging entry per CRM-18193' + 'The connection date must be passed in to disambiguate this logging entry per CRM-18193' ); } diff --git a/tests/phpunit/api/v3/MailingTest.php b/tests/phpunit/api/v3/MailingTest.php index 5859825af8..7f5f21c4e5 100644 --- a/tests/phpunit/api/v3/MailingTest.php +++ b/tests/phpunit/api/v3/MailingTest.php @@ -594,7 +594,7 @@ SELECT event_queue_id, time_stamp FROM mail_{$type}_temp"; 'time_stamp' => '20111111010101', ); $this->callAPIFailure('mailing_event', 'confirm', $params, - 'Confirmation failed' + 'contact_id is not a valid integer' ); } diff --git a/tests/phpunit/api/v3/NoteTest.php b/tests/phpunit/api/v3/NoteTest.php index 2496a282f1..e0e6fd63b6 100644 --- a/tests/phpunit/api/v3/NoteTest.php +++ b/tests/phpunit/api/v3/NoteTest.php @@ -241,9 +241,9 @@ class api_v3_NoteTest extends CiviUnitTestCase { */ public function testDeleteWithWrongID() { $params = array( - 'id' => 0, + 'id' => 99999, ); - $this->callAPIFailure('note', 'delete', $params, 'Mandatory key(s) missing from params array: id'); + $this->callAPIFailure('note', 'delete', $params, 'Error while deleting Note'); } /** diff --git a/tests/phpunit/api/v3/PaymentTest.php b/tests/phpunit/api/v3/PaymentTest.php index 78f1fa7b38..52b1198f38 100644 --- a/tests/phpunit/api/v3/PaymentTest.php +++ b/tests/phpunit/api/v3/PaymentTest.php @@ -310,7 +310,7 @@ class api_v3_PaymentTest extends CiviUnitTestCase { 'id' => $payment['id'], 'check_permissions' => TRUE, ); - $payment = $this->callAPIFailure('payment', 'cancel', $cancelParams, 'API permission check failed for Payment/cancel call; insufficient permission: require access CiviCRM and edit contributions'); + $payment = $this->callAPIFailure('payment', 'cancel', $cancelParams, 'API permission check failed for Payment/cancel call; insufficient permission: require access CiviCRM and access CiviContribute and edit contributions'); array_push(CRM_Core_Config::singleton()->userPermissionClass->permissions, 'access CiviCRM', 'edit contributions'); @@ -346,7 +346,7 @@ class api_v3_PaymentTest extends CiviUnitTestCase { 'id' => $payment['id'], 'check_permissions' => TRUE, ); - $payment = $this->callAPIFailure('payment', 'delete', $deleteParams, 'API permission check failed for Payment/delete call; insufficient permission: require access CiviCRM and delete in CiviContribute'); + $payment = $this->callAPIFailure('payment', 'delete', $deleteParams, 'API permission check failed for Payment/delete call; insufficient permission: require access CiviCRM and access CiviContribute and delete in CiviContribute'); array_push(CRM_Core_Config::singleton()->userPermissionClass->permissions, 'access CiviCRM', 'delete in CiviContribute'); $this->callAPIAndDocument('payment', 'delete', $deleteParams, __FUNCTION__, __FILE__); @@ -402,7 +402,7 @@ class api_v3_PaymentTest extends CiviUnitTestCase { 'id' => $payment['id'], 'check_permissions' => TRUE, ); - $payment = $this->callAPIFailure('payment', 'create', $params, 'API permission check failed for Payment/create call; insufficient permission: require access CiviCRM and edit contributions'); + $payment = $this->callAPIFailure('payment', 'create', $params, 'API permission check failed for Payment/create call; insufficient permission: require access CiviCRM and access CiviContribute and edit contributions'); array_push(CRM_Core_Config::singleton()->userPermissionClass->permissions, 'access CiviCRM', 'edit contributions'); $payment = $this->callAPIAndDocument('payment', 'create', $params, __FUNCTION__, __FILE__, 'Update Payment', 'UpdatePayment'); diff --git a/tests/phpunit/api/v3/RelationshipTest.php b/tests/phpunit/api/v3/RelationshipTest.php index aadfa70850..e10da921e3 100644 --- a/tests/phpunit/api/v3/RelationshipTest.php +++ b/tests/phpunit/api/v3/RelationshipTest.php @@ -550,7 +550,7 @@ class api_v3_RelationshipTest extends CiviUnitTestCase { $this->callAPIFailure('relationship', 'delete', $params, 'Mandatory key(s) missing from params array: id'); $params['id'] = "Invalid"; - $this->callAPIFailure('relationship', 'delete', $params, 'Invalid value for relationship ID'); + $this->callAPIFailure('relationship', 'delete', $params, 'id is not a valid integer'); } /** @@ -611,7 +611,7 @@ class api_v3_RelationshipTest extends CiviUnitTestCase { 'is_active' => 0, ); - $this->callAPIFailure('relationship', 'create', $params, 'Relationship already exists'); + $this->callAPIFailure('relationship', 'create', $params, 'Duplicate Relationship'); $this->callAPISuccess('relationship', 'delete', array('id' => $result['id'])); $this->relationshipTypeDelete($this->_relTypeID); diff --git a/tests/phpunit/api/v3/RelationshipTypeTest.php b/tests/phpunit/api/v3/RelationshipTypeTest.php index 2389bf9777..58518f69d0 100644 --- a/tests/phpunit/api/v3/RelationshipTypeTest.php +++ b/tests/phpunit/api/v3/RelationshipTypeTest.php @@ -165,7 +165,7 @@ class api_v3_RelationshipTypeTest extends CiviUnitTestCase { 'is_active' => 0, ); $result = $this->callAPIFailure('relationship_type', 'delete', $params, - 'Invalid value for relationship type ID' + 'id is not a valid integer' ); } diff --git a/tests/phpunit/api/v3/UFGroupTest.php b/tests/phpunit/api/v3/UFGroupTest.php index eaf4a562b3..9afc25bdf9 100644 --- a/tests/phpunit/api/v3/UFGroupTest.php +++ b/tests/phpunit/api/v3/UFGroupTest.php @@ -196,7 +196,7 @@ class api_v3_UFGroupTest extends CiviUnitTestCase { } public function testUFGroupUpdateWithEmptyParams() { - $result = $this->callAPIFailure('uf_group', 'create', array(), $this->_ufGroupId); + $result = $this->callAPIFailure('uf_group', 'create', array(), 'Mandatory key(s) missing from params array: version, title'); } public function testUFGroupDelete() { -- 2.25.1