From a60c0bc84a8db9a4123f53879109576524393ff9 Mon Sep 17 00:00:00 2001 From: Seamus Lee Date: Wed, 28 Sep 2016 07:47:30 +1000 Subject: [PATCH 1/1] Enable api_v3_syntaxConformaceTest::testInvalidID_delete (#9068) * Enable api_v3_SyntaxConformanceTest delete with wrong id * Remove unneded if and remove debugging * Remove debugging * Tidy up fix for CustomValue tear down --- CRM/Contribute/BAO/ContributionSoft.php | 6 ++++++ CRM/Core/BAO/Dashboard.php | 3 +++ CRM/Core/BAO/OptionValue.php | 6 +++++- CRM/Core/ManagedEntities.php | 17 ++++++++++------- CRM/Event/BAO/Participant.php | 7 +++++-- CRM/Event/BAO/ParticipantStatusType.php | 4 +++- CRM/Member/BAO/MembershipStatus.php | 3 +++ api/v3/ActivityType.php | 6 +++++- api/v3/ContributionSoft.php | 7 +++++-- api/v3/Dashboard.php | 2 +- api/v3/Group.php | 5 ++++- api/v3/GroupContact.php | 4 ++++ api/v3/MembershipStatus.php | 5 ++++- api/v3/OptionValue.php | 5 +++-- api/v3/ParticipantStatusType.php | 2 +- api/v3/SystemLog.php | 2 +- api/v3/utils.php | 11 ++++++++--- tests/phpunit/CRM/Batch/Form/EntryTest.php | 4 +++- tests/phpunit/CiviTest/CiviUnitTestCase.php | 10 ++++++++-- tests/phpunit/api/v3/ActivityTest.php | 5 ++++- tests/phpunit/api/v3/CustomSearchTest.php | 10 ++++++---- tests/phpunit/api/v3/CustomValueTest.php | 5 ++++- tests/phpunit/api/v3/SyntaxConformanceTest.php | 5 +---- 23 files changed, 97 insertions(+), 37 deletions(-) diff --git a/CRM/Contribute/BAO/ContributionSoft.php b/CRM/Contribute/BAO/ContributionSoft.php index 756c22a37c..5d835e9570 100644 --- a/CRM/Contribute/BAO/ContributionSoft.php +++ b/CRM/Contribute/BAO/ContributionSoft.php @@ -230,10 +230,16 @@ class CRM_Contribute_BAO_ContributionSoft extends CRM_Contribute_DAO_Contributio public static function del($params) { //delete from contribution soft table $contributionSoft = new CRM_Contribute_DAO_ContributionSoft(); + $contributionSoft->id = $params['id']; + if (!$contributionSoft->find()) { + return FALSE; + } + unset($params['id']); foreach ($params as $column => $value) { $contributionSoft->$column = $value; } $contributionSoft->delete(); + return TRUE; } /** diff --git a/CRM/Core/BAO/Dashboard.php b/CRM/Core/BAO/Dashboard.php index ccc82d105c..d64620e3f6 100644 --- a/CRM/Core/BAO/Dashboard.php +++ b/CRM/Core/BAO/Dashboard.php @@ -493,6 +493,9 @@ class CRM_Core_BAO_Dashboard extends CRM_Core_DAO_Dashboard { public static function deleteDashlet($dashletID) { $dashlet = new CRM_Core_DAO_Dashboard(); $dashlet->id = $dashletID; + if (!$dashlet->find(TRUE)) { + return FALSE; + } $dashlet->delete(); return TRUE; } diff --git a/CRM/Core/BAO/OptionValue.php b/CRM/Core/BAO/OptionValue.php index a529d1b18d..0fc76b3121 100644 --- a/CRM/Core/BAO/OptionValue.php +++ b/CRM/Core/BAO/OptionValue.php @@ -240,9 +240,13 @@ class CRM_Core_BAO_OptionValue extends CRM_Core_DAO_OptionValue { public static function del($optionValueId) { $optionValue = new CRM_Core_DAO_OptionValue(); $optionValue->id = $optionValueId; + if (!$optionValue->find()) { + return FALSE; + } if (self::updateRecords($optionValueId, CRM_Core_Action::DELETE)) { CRM_Core_PseudoConstant::flush(); - return $optionValue->delete(); + $optionValue->delete(); + return TRUE; } return FALSE; } diff --git a/CRM/Core/ManagedEntities.php b/CRM/Core/ManagedEntities.php index 4de184a28c..5021859b01 100644 --- a/CRM/Core/ManagedEntities.php +++ b/CRM/Core/ManagedEntities.php @@ -334,14 +334,17 @@ class CRM_Core_ManagedEntities { 'version' => 3, 'id' => $dao->entity_id, ); - $result = civicrm_api($dao->entity_type, 'delete', $params); - if ($result['is_error']) { - $this->onApiError($dao->entity_type, 'delete', $params, $result); - } + $check = civicrm_api3($dao->entity_type, 'get', $params); + if ((bool) $check['count']) { + $result = civicrm_api($dao->entity_type, 'delete', $params); + if ($result['is_error']) { + $this->onApiError($dao->entity_type, 'delete', $params, $result); + } - CRM_Core_DAO::executeQuery('DELETE FROM civicrm_managed WHERE id = %1', array( - 1 => array($dao->id, 'Integer'), - )); + CRM_Core_DAO::executeQuery('DELETE FROM civicrm_managed WHERE id = %1', array( + 1 => array($dao->id, 'Integer'), + )); + } } } diff --git a/CRM/Event/BAO/Participant.php b/CRM/Event/BAO/Participant.php index 49921a411e..b2a608f466 100644 --- a/CRM/Event/BAO/Participant.php +++ b/CRM/Event/BAO/Participant.php @@ -870,6 +870,11 @@ WHERE civicrm_participant.id = {$participantId} * @return \CRM_Event_DAO_Participant */ public static function deleteParticipant($id) { + $participant = new CRM_Event_DAO_Participant(); + $participant->id = $id; + if (!$participant->find()) { + return FALSE; + } CRM_Utils_Hook::pre('delete', 'Participant', $id, CRM_Core_DAO::$_nullArray); $transaction = new CRM_Core_Transaction(); @@ -902,8 +907,6 @@ WHERE civicrm_participant.id = {$participantId} CRM_Core_BAO_Note::del($noteId, FALSE); } - $participant = new CRM_Event_DAO_Participant(); - $participant->id = $id; $participant->delete(); $transaction->commit(); diff --git a/CRM/Event/BAO/ParticipantStatusType.php b/CRM/Event/BAO/ParticipantStatusType.php index 802c7363d3..05308d69b8 100644 --- a/CRM/Event/BAO/ParticipantStatusType.php +++ b/CRM/Event/BAO/ParticipantStatusType.php @@ -80,7 +80,9 @@ class CRM_Event_BAO_ParticipantStatusType extends CRM_Event_DAO_ParticipantStatu $dao = new CRM_Event_DAO_ParticipantStatusType(); $dao->id = $id; - $dao->find(TRUE); + if (!$dao->find()) { + return FALSE; + } $dao->delete(); return TRUE; } diff --git a/CRM/Member/BAO/MembershipStatus.php b/CRM/Member/BAO/MembershipStatus.php index 3e9033b14a..5cdd7b9810 100644 --- a/CRM/Member/BAO/MembershipStatus.php +++ b/CRM/Member/BAO/MembershipStatus.php @@ -207,6 +207,9 @@ class CRM_Member_BAO_MembershipStatus extends CRM_Member_DAO_MembershipStatus { //delete from membership Type table $membershipStatus = new CRM_Member_DAO_MembershipStatus(); $membershipStatus->id = $membershipStatusId; + if (!$membershipStatus->find()) { + throw new CRM_Core_Exception(ts('Cannot delete membership status ' . $membershipStatusId)); + } $membershipStatus->delete(); CRM_Member_PseudoConstant::flush('membershipStatus'); $membershipStatus->free(); diff --git a/api/v3/ActivityType.php b/api/v3/ActivityType.php index f54c8bf9be..ad15847d1d 100644 --- a/api/v3/ActivityType.php +++ b/api/v3/ActivityType.php @@ -117,5 +117,9 @@ function _civicrm_api3_activity_type_create_spec(&$params) { * @deprecated use OptionValue api */ function civicrm_api3_activity_type_delete($params) { - return civicrm_api3_create_success(CRM_Core_BAO_OptionValue::del($params['id']), $params); + $result = CRM_Core_BAO_OptionValue::del($params['id']); + if ($result) { + return civicrm_api3_create_success(TRUE, $params); + } + throw new API_Exception("Failure to delete activity type id {$params['id']}"); } diff --git a/api/v3/ContributionSoft.php b/api/v3/ContributionSoft.php index 3bfe307f2f..30b7a46e68 100644 --- a/api/v3/ContributionSoft.php +++ b/api/v3/ContributionSoft.php @@ -65,8 +65,11 @@ function _civicrm_api3_contribution_soft_create_spec(&$params) { */ function civicrm_api3_contribution_soft_delete($params) { // Non standard BAO - we have to write custom code to cope. - CRM_Contribute_BAO_ContributionSoft::del(array('id' => $params['id'])); - + $result = CRM_Contribute_BAO_ContributionSoft::del(array('id' => $params['id'])); + if (!$result) { + throw new API_Exception('Cannot delete contributionSoft ' . $params['id']); + } + civicrm_api3_create_success(TRUE); } /** diff --git a/api/v3/Dashboard.php b/api/v3/Dashboard.php index 904fc837cf..77f6d30bd6 100644 --- a/api/v3/Dashboard.php +++ b/api/v3/Dashboard.php @@ -93,6 +93,6 @@ function civicrm_api3_dashboard_delete($params) { return civicrm_api3_create_success(1, $params, 'Dashboard', 'delete'); } else { - return civicrm_api3_create_error('Could not delete dashlet'); + throw new API_Exception('Could not delete dashlet'); } } diff --git a/api/v3/Group.php b/api/v3/Group.php index c0e1188ea7..f53172961e 100644 --- a/api/v3/Group.php +++ b/api/v3/Group.php @@ -97,7 +97,10 @@ function civicrm_api3_group_get($params) { * API result array */ function civicrm_api3_group_delete($params) { - + $group = civicrm_api3_group_get(array('id' => $params['id'])); + if ($group['count'] == 0) { + throw new API_Exception('Could not delete group ' . $params['id']); + } CRM_Contact_BAO_Group::discard($params['id']); return civicrm_api3_create_success(TRUE); } diff --git a/api/v3/GroupContact.php b/api/v3/GroupContact.php index 1546b692a9..e296e18297 100644 --- a/api/v3/GroupContact.php +++ b/api/v3/GroupContact.php @@ -146,6 +146,10 @@ function civicrm_api3_group_contact_create($params) { * @deprecated */ function civicrm_api3_group_contact_delete($params) { + $groupContact = civicrm_api3('GroupContact', 'get', $params); + if ($groupContact['count'] == 0) { + throw new API_Exception('Cannot Delete GroupContact'); + } $params['status'] = CRM_Utils_Array::value('status', $params, empty($params['skip_undelete']) ? 'Removed' : 'Deleted'); // "Deleted" isn't a real option so skip the api wrapper to avoid pseudoconstant validation return civicrm_api3_group_contact_create($params); diff --git a/api/v3/MembershipStatus.php b/api/v3/MembershipStatus.php index 499c823fe3..696de0ad4a 100644 --- a/api/v3/MembershipStatus.php +++ b/api/v3/MembershipStatus.php @@ -126,7 +126,10 @@ function civicrm_api3_membership_status_update($params) { function civicrm_api3_membership_status_delete($params) { $memberStatusDelete = CRM_Member_BAO_MembershipStatus::del($params['id'], TRUE); - return $memberStatusDelete ? civicrm_api3_create_error($memberStatusDelete['error_message']) : civicrm_api3_create_success(); + if ($memberStatusDelete) { + throw new API_Exception($memberStatusDelete['error_message']); + } + return civicrm_api3_create_success(); } /** diff --git a/api/v3/OptionValue.php b/api/v3/OptionValue.php index e34f3ec28b..2e5cd6d4a1 100644 --- a/api/v3/OptionValue.php +++ b/api/v3/OptionValue.php @@ -106,11 +106,12 @@ function _civicrm_api3_option_value_create_spec(&$params) { function civicrm_api3_option_value_delete($params) { // We will get the option group id before deleting so we can flush pseudoconstants. $optionGroupID = civicrm_api('option_value', 'getvalue', array('version' => 3, 'id' => $params['id'], 'return' => 'option_group_id')); - if (CRM_Core_BAO_OptionValue::del((int) $params['id'])) { + $result = CRM_Core_BAO_OptionValue::del($params['id']); + if ($result) { civicrm_api('option_value', 'getfields', array('version' => 3, 'cache_clear' => 1, 'option_group_id' => $optionGroupID)); return civicrm_api3_create_success(); } else { - civicrm_api3_create_error('Could not delete OptionValue ' . $params['id']); + throw new API_Exception('Could not delete OptionValue ' . $params['id']); } } diff --git a/api/v3/ParticipantStatusType.php b/api/v3/ParticipantStatusType.php index fc3549f995..c433fc5b9a 100644 --- a/api/v3/ParticipantStatusType.php +++ b/api/v3/ParticipantStatusType.php @@ -77,5 +77,5 @@ function civicrm_api3_participant_status_type_delete($params) { return civicrm_api3_create_success(TRUE); } - return civicrm_api3_create_error(TRUE); + throw new API_Exception('Could not delete participant status type id ' . $params['id']); } diff --git a/api/v3/SystemLog.php b/api/v3/SystemLog.php index 9b624639e4..b1248f6c2b 100644 --- a/api/v3/SystemLog.php +++ b/api/v3/SystemLog.php @@ -39,7 +39,7 @@ * @return array */ function civicrm_api3_system_log_delete($params) { - return _civicrm_api3_basic_get(_civicrm_api3_get_BAO(__FUNCTION__), $params, TRUE, 'SystemLog'); + return _civicrm_api3_basic_delete(_civicrm_api3_get_BAO(__FUNCTION__), $params); } /** diff --git a/api/v3/utils.php b/api/v3/utils.php index 078d9717e2..96f616b32e 100644 --- a/api/v3/utils.php +++ b/api/v3/utils.php @@ -1457,9 +1457,14 @@ function _civicrm_api3_basic_delete($bao_name, &$params) { _civicrm_api3_check_edit_permissions($bao_name, array('id' => $params['id'])); $args = array(&$params['id']); if (method_exists($bao_name, 'del')) { - $bao = call_user_func_array(array($bao_name, 'del'), $args); - if ($bao !== FALSE) { - return civicrm_api3_create_success(TRUE); + $dao = new $bao_name(); + $dao->id = $params['id']; + if ($dao->find()) { + $bao = call_user_func_array(array($bao_name, 'del'), $args); + if ($bao !== FALSE) { + return civicrm_api3_create_success(); + } + throw new API_Exception('Could not delete entity id ' . $params['id']); } throw new API_Exception('Could not delete entity id ' . $params['id']); } diff --git a/tests/phpunit/CRM/Batch/Form/EntryTest.php b/tests/phpunit/CRM/Batch/Form/EntryTest.php index ad5e4c696e..ef40bb28e6 100644 --- a/tests/phpunit/CRM/Batch/Form/EntryTest.php +++ b/tests/phpunit/CRM/Batch/Form/EntryTest.php @@ -146,7 +146,9 @@ class CRM_Batch_Form_EntryTest extends CiviUnitTestCase { if ($this->callAPISuccessGetCount('membership', array('id' => $this->_membershipTypeID))) { $this->membershipTypeDelete(array('id' => $this->_membershipTypeID)); } - $this->membershipStatusDelete($this->_membershipStatusID); + if ($this->callAPISuccessGetCount('MembershipStatus', array('id' => $this->_membershipStatusID))) { + $this->membershipStatusDelete($this->_membershipStatusID); + } $this->contactDelete($this->_contactID); $this->contactDelete($this->_contactID2); $this->contactDelete($this->_orgContactID); diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index 1d952657b9..0aab1a3d55 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -1266,7 +1266,10 @@ class CiviUnitTestCase extends PHPUnit_Extensions_Database_TestCase { */ public function relationshipTypeDelete($relationshipTypeID) { $params['id'] = $relationshipTypeID; - $this->callAPISuccess('relationship_type', 'delete', $params); + $check = $this->callAPISuccess('relationship_type', 'get', $params); + if (!empty($check['count'])) { + $this->callAPISuccess('relationship_type', 'delete', $params); + } } /** @@ -1621,7 +1624,10 @@ class CiviUnitTestCase extends PHPUnit_Extensions_Database_TestCase { $params = array( 'id' => $participantID, ); - return $this->callAPISuccess('Participant', 'delete', $params); + $check = $this->callAPISuccess('Participant', 'get', $params); + if ($check['count'] > 0) { + return $this->callAPISuccess('Participant', 'delete', $params); + } } /** diff --git a/tests/phpunit/api/v3/ActivityTest.php b/tests/phpunit/api/v3/ActivityTest.php index 27611c362d..21e7f236cf 100644 --- a/tests/phpunit/api/v3/ActivityTest.php +++ b/tests/phpunit/api/v3/ActivityTest.php @@ -106,7 +106,10 @@ class api_v3_ActivityTest extends CiviUnitTestCase { 'civicrm_uf_match', ); $this->quickCleanup($tablesToTruncate, TRUE); - $this->callAPISuccess('option_value', 'delete', array('id' => $this->test_activity_type_id)); + $type = $this->callAPISuccess('optionValue', 'get', array('id' => $this->test_activity_type_id)); + if (!empty($type['count'])) { + $this->callAPISuccess('option_value', 'delete', array('id' => $this->test_activity_type_id)); + } } /** diff --git a/tests/phpunit/api/v3/CustomSearchTest.php b/tests/phpunit/api/v3/CustomSearchTest.php index e6c38ea2be..ebf7cc3113 100644 --- a/tests/phpunit/api/v3/CustomSearchTest.php +++ b/tests/phpunit/api/v3/CustomSearchTest.php @@ -57,10 +57,12 @@ class api_v3_CustomSearchTest extends CiviUnitTestCase { AND option_group_id IN (SELECT id from civicrm_option_group WHERE name = "custom_search") '); $this->assertDBQuery(1, 'SELECT is_active FROM civicrm_option_value WHERE name = "CRM_Contact_Form_Search_Custom_Examplez"'); - - $result = $this->callAPISuccess('CustomSearch', 'delete', array( - 'id' => $entityId, - )); + $check = $this->callAPISuccess('CustomSearch', 'get', array('id' => $entityId)); + if (!empty($check['count'])) { + $result = $this->callAPISuccess('CustomSearch', 'delete', array( + 'id' => $entityId, + )); + } $this->assertEquals(1, $result['count']); $this->assertDBQuery(0, 'SELECT count(*) FROM civicrm_option_value WHERE name = "CRM_Contact_Form_Search_Custom_Examplez" diff --git a/tests/phpunit/api/v3/CustomValueTest.php b/tests/phpunit/api/v3/CustomValueTest.php index 6e63b6e798..7bfd07f052 100644 --- a/tests/phpunit/api/v3/CustomValueTest.php +++ b/tests/phpunit/api/v3/CustomValueTest.php @@ -94,7 +94,10 @@ class api_v3_CustomValueTest extends CiviUnitTestCase { if (!empty($this->optionGroup)) { foreach ($this->optionGroup as $type => $value) { if (!empty($value['id'])) { - $this->callAPISuccess('OptionGroup', 'delete', array('id' => $value['id'])); + $count = $this->callAPISuccess('OptionGroup', 'get', array('id' => $value['id'])); + if ((bool) $count['count']) { + $this->callAPISuccess('OptionGroup', 'delete', array('id' => $value['id'])); + } } } } diff --git a/tests/phpunit/api/v3/SyntaxConformanceTest.php b/tests/phpunit/api/v3/SyntaxConformanceTest.php index 78cfdfe20f..c32b983005 100644 --- a/tests/phpunit/api/v3/SyntaxConformanceTest.php +++ b/tests/phpunit/api/v3/SyntaxConformanceTest.php @@ -1363,11 +1363,8 @@ class api_v3_SyntaxConformanceTest extends CiviUnitTestCase { * @throws \PHPUnit_Framework_IncompleteTestError */ public function testInvalidID_delete($Entity) { - // turn test off for now - $this->markTestIncomplete("Entity [ $Entity ] cannot be mocked - no known DAO"); - return; if (in_array($Entity, $this->toBeImplemented['delete'])) { - // $this->markTestIncomplete("civicrm_api3_{$Entity}_delete to be implemented"); + $this->markTestIncomplete("civicrm_api3_{$Entity}_delete to be implemented"); return; } $result = $this->callAPIFailure($Entity, 'Delete', array('id' => 999)); -- 2.25.1