From edc2ebcfac7fd745198f317180c67048e7a3a850 Mon Sep 17 00:00:00 2001 From: Michael McAndrew Date: Thu, 11 Mar 2021 15:38:43 +0000 Subject: [PATCH] dev/core#1845 Change FK on civicrm_group to delete the associated group if a saved search is deleted Add in delete function on Group BAO to handle deleting associated saved search if appropriate when group is deleted and move upgrade step appropriately Add unit test and move code to the discard function --- CRM/Contact/BAO/Group.php | 14 ++++ CRM/Contact/DAO/Group.php | 2 +- .../Incremental/php/FiveThirtySeven.php | 22 +++++++ CRM/Upgrade/Incremental/php/FiveThirtySix.php | 1 - tests/phpunit/api/v3/SavedSearchTest.php | 64 +++++++++++++++++-- xml/schema/Contact/Group.xml | 2 +- 6 files changed, 97 insertions(+), 8 deletions(-) diff --git a/CRM/Contact/BAO/Group.php b/CRM/Contact/BAO/Group.php index 138fd9e2d3..81feb0f2e3 100644 --- a/CRM/Contact/BAO/Group.php +++ b/CRM/Contact/BAO/Group.php @@ -9,6 +9,8 @@ +--------------------------------------------------------------------+ */ +use Civi\Api4\Group; + /** * * @package CRM @@ -94,6 +96,18 @@ class CRM_Contact_BAO_Group extends CRM_Contact_DAO_Group { $query = "DELETE FROM civicrm_acl_entity_role where entity_table = 'civicrm_group' AND entity_id = %1"; CRM_Core_DAO::executeQuery($query, $params); + //check whether this group contains any saved searches and check if that saved search is appropriate to delete. + $groupDetails = Group::get(FALSE)->addWhere('id', '=', $id)->execute(); + if (!empty($groupDetails[0]['saved_search_id'])) { + $savedSearch = new CRM_Contact_DAO_SavedSearch(); + $savedSearch->id = $groupDetails[0]['saved_search_id']; + $savedSearch->find(TRUE); + // If it is a traditional saved search i.e has form values and there is no linked api_entity then delete the saved search as well. + if (!empty($savedSearch->form_values) && empty($savedSearch->api_entity) && empty($savedSearch->api_params)) { + $savedSearch->delete(); + } + } + // delete from group table $group = new CRM_Contact_DAO_Group(); $group->id = $id; diff --git a/CRM/Contact/DAO/Group.php b/CRM/Contact/DAO/Group.php index 56f2c52c24..b06e64a168 100644 --- a/CRM/Contact/DAO/Group.php +++ b/CRM/Contact/DAO/Group.php @@ -6,7 +6,7 @@ * * Generated from xml/schema/CRM/Contact/Group.xml * DO NOT EDIT. Generated by CRM_Core_CodeGen - * (GenCodeChecksum:31478cb7fd1eee1299fb22225020be77) + * (GenCodeChecksum:8f7306d4427fc261d17944ad601bb422) */ /** diff --git a/CRM/Upgrade/Incremental/php/FiveThirtySeven.php b/CRM/Upgrade/Incremental/php/FiveThirtySeven.php index bad13a5b51..24edc4a44e 100644 --- a/CRM/Upgrade/Incremental/php/FiveThirtySeven.php +++ b/CRM/Upgrade/Incremental/php/FiveThirtySeven.php @@ -52,6 +52,16 @@ class CRM_Upgrade_Incremental_php_FiveThirtySeven extends CRM_Upgrade_Incrementa * (change the x in the function name): */ + /** + * Upgrade function. + * + * @param string $rev + */ + public function upgrade_5_37_alpha1($rev) { + $this->addTask(ts('Upgrade DB to %1: SQL', [1 => $rev]), 'runSql', $rev); + $this->addTask('core-issue#1845 - Alter Foreign key on civicrm_group to delete when the associated group when the saved search is deleted', 'alterSavedSearchFK'); + } + // /** // * Upgrade function. // * @@ -69,4 +79,16 @@ class CRM_Upgrade_Incremental_php_FiveThirtySeven extends CRM_Upgrade_Incrementa // return TRUE; // } + /** + * @param \CRM_Queue_TaskContext $ctx + * + * @return bool + */ + public static function alterSavedSearchFK(CRM_Queue_TaskContext $ctx) { + CRM_Core_BAO_SchemaHandler::safeRemoveFK('civicrm_group', 'FK_civicrm_group_saved_search_id'); + CRM_Core_DAO::executeQuery('DELETE civicrm_saved_search FROM civicrm_saved_search LEFT JOIN civicrm_group ON civicrm_saved_search.id = civicrm_group.saved_search_id WHERE civicrm_group.id IS NULL AND form_values IS NOT NULL and api_params IS NULL'); + CRM_Core_DAO::executeQuery('ALTER TABLE civicrm_group ADD CONSTRAINT `FK_civicrm_group_saved_search_id` FOREIGN KEY (`saved_search_id`) REFERENCES `civicrm_saved_search`(`id`) ON DELETE CASCADE', [], TRUE, NULL, FALSE, FALSE); + return TRUE; + } + } diff --git a/CRM/Upgrade/Incremental/php/FiveThirtySix.php b/CRM/Upgrade/Incremental/php/FiveThirtySix.php index d1dfe3112a..89a65a6c77 100644 --- a/CRM/Upgrade/Incremental/php/FiveThirtySix.php +++ b/CRM/Upgrade/Incremental/php/FiveThirtySix.php @@ -83,7 +83,6 @@ class CRM_Upgrade_Incremental_php_FiveThirtySix extends CRM_Upgrade_Incremental_ 'civicrm_saved_search', 'description', "text DEFAULT NULL"); $this->addTask('core-issue#2422 - Add constraints to civicrm_saved_search', 'taskAddConstraints'); - } /** diff --git a/tests/phpunit/api/v3/SavedSearchTest.php b/tests/phpunit/api/v3/SavedSearchTest.php index 2b227ca07d..ed303c8ae4 100644 --- a/tests/phpunit/api/v3/SavedSearchTest.php +++ b/tests/phpunit/api/v3/SavedSearchTest.php @@ -40,7 +40,7 @@ class api_v3_SavedSearchTest extends CiviUnitTestCase { protected $_entity; public $DBResetRequired = FALSE; - public function setUp() { + public function setUp(): void { parent::setUp(); // The line below makes it unneccessary to do cleanup after a test, @@ -68,7 +68,7 @@ class api_v3_SavedSearchTest extends CiviUnitTestCase { /** * Create a saved search, and see whether the returned values make sense. */ - public function testCreateSavedSearch() { + public function testCreateSavedSearch(): void { $contactID = $this->createLoggedInUser(); $result = $this->callAPIAndDocument( $this->_entity, 'create', $this->params, __FUNCTION__, __FILE__)['values']; @@ -91,7 +91,7 @@ class api_v3_SavedSearchTest extends CiviUnitTestCase { * Create a saved search, retrieve it again, and check for ID and one of * the field values. */ - public function testCreateAndGetSavedSearch() { + public function testCreateAndGetSavedSearch(): void { // Arrange: // (create a saved search) $create_result = $this->callAPISuccess( @@ -115,7 +115,7 @@ class api_v3_SavedSearchTest extends CiviUnitTestCase { * Create a saved search, and test whether it can be used for a smart * group. */ - public function testCreateSavedSearchWithSmartGroup() { + public function testCreateSavedSearchWithSmartGroup(): void { // First create a volunteer for the default organization $result = $this->callAPISuccess('Contact', 'create', [ @@ -161,7 +161,61 @@ class api_v3_SavedSearchTest extends CiviUnitTestCase { $this->assertEquals($contact_id, $get_result['values'][$contact_id]['id']); } - public function testDeleteSavedSearch() { + /** + * Create a saved search, and test whether it can be used for a smart + * group. Also check that when the Group is deleted the associated saved search gets deleted. + * @dataProvider versionThreeAndFour + */ + public function testSavedSearchIsDeletedWhenSmartGroupIs($apiVersion): void { + $this->_apiVersion = $apiVersion; + // First create a volunteer for the default organization + + $result = $this->callAPISuccess('Contact', 'create', [ + 'first_name' => 'Joe', + 'last_name' => 'Schmoe', + 'contact_type' => 'Individual', + 'api.Relationship.create' => [ + 'contact_id_a' => '$value.id', + // default organization: + 'contact_id_b' => 1, + // volunteer relationship: + 'relationship_type_id' => 6, + 'is_active' => 1, + ], + ]); + $contact_id = $result['id']; + + // Now create our saved search, and chain the creation of a smart group. + $params = $this->params; + $params['api.Group.create'] = [ + 'name' => 'my_smartgroup', + 'title' => 'my smartgroup', + 'description' => 'Volunteers for the default organization', + 'saved_search_id' => '$value.id', + 'is_active' => 1, + 'visibility' => 'User and User Admin Only', + 'is_hidden' => 0, + 'is_reserved' => 0, + ]; + + $create_result = $this->callAPISuccess($this->_entity, 'create', $params); + + $created_search = CRM_Utils_Array::first($create_result['values']); + $group_id = $created_search['api.Group.create']['id']; + + // Search for contacts in our new smart group + $get_result = $this->callAPISuccess('Contact', 'get', ['group' => $group_id]); + + // Expect our contact to be there. + $this->assertEquals(1, $get_result['count']); + $this->assertEquals($contact_id, $get_result['values'][$contact_id]['id']); + + $this->callAPISuccess('Group', 'delete', ['id' => $group_id]); + $savedSearch = $this->callAPISuccess('SavedSearch', 'get', ['id' => $created_search['id']]); + $this->assertCount(0, $savedSearch['values']); + } + + public function testDeleteSavedSearch(): void { // Create saved search, delete it again, and try to get it $create_result = $this->callAPISuccess($this->_entity, 'create', $this->params); $delete_params = ['id' => $create_result['id']]; diff --git a/xml/schema/Contact/Group.xml b/xml/schema/Contact/Group.xml index a53625142a..7970845139 100644 --- a/xml/schema/Contact/Group.xml +++ b/xml/schema/Contact/Group.xml @@ -82,7 +82,7 @@ civicrm_saved_search
id 1.1 - SET NULL + CASCADE is_active -- 2.25.1