From 3357fb76934d5c5957e263830811007e1206845e Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Sun, 7 Nov 2021 11:54:54 -0500 Subject: [PATCH] ManagedEntity - Add update mode 'unmodified' and fix cleanup mode 'unused' for APIv4 Update mode 'unmodified' will only update a record if it has not been locally edited. This new setting works only for entities opted-in to the APIv4 ManagedEntity trait, and will emit a warning and fall back on 'always' for others. Cleanup mode 'unmodified' now works for APIv4 managed entities, and they are cleaned up in reverse order to ensure references are deleted before their parents. --- CRM/Core/ManagedEntities.php | 27 ++- .../v4/SearchDisplay/ManagedSearchTest.php | 106 ++++++++++++ .../api/v4/Entity/ManagedEntityTest.php | 159 ++++++++++++++++-- 3 files changed, 271 insertions(+), 21 deletions(-) create mode 100644 ext/search_kit/tests/phpunit/api/v4/SearchDisplay/ManagedSearchTest.php diff --git a/CRM/Core/ManagedEntities.php b/CRM/Core/ManagedEntities.php index 80f79e639a1e..0598fb1ebec4 100644 --- a/CRM/Core/ManagedEntities.php +++ b/CRM/Core/ManagedEntities.php @@ -185,6 +185,7 @@ protected function reconcileEnabledModule(string $module): void { $dao->name = $todo['name']; $dao->entity_type = $todo['entity_type']; $dao->entity_id = $todo['entity_id']; + $dao->entity_modified_date = $todo['entity_modified_date']; $dao->id = $todo['id']; $this->updateExistingEntity($dao, $todo); } @@ -234,7 +235,8 @@ protected function getManagedEntitiesToUpdate(array $filters = []): array { * @return array */ protected function getManagedEntitiesToDelete(array $filters = []): array { - return $this->getManagedEntities(array_merge($filters, ['managed_action' => 'delete'])); + // Return array in reverse-order so that child entities are cleaned up before their parents + return array_reverse($this->getManagedEntities(array_merge($filters, ['managed_action' => 'delete']))); } /** @@ -339,6 +341,14 @@ protected function updateExistingEntity($dao, $todo) { $policy = $todo['update'] ?? 'always'; $doUpdate = ($policy === 'always'); + if ($policy === 'unmodified') { + // If this is not an APIv4 managed entity, the entity_modidfied_date will always be null + if (!CRM_Core_BAO_Managed::isApi4ManagedType($dao->entity_type)) { + Civi::log()->warning('ManagedEntity update policy "unmodified" specified for entity type ' . $dao->entity_type . ' which is not an APIv4 ManagedEntity. Falling back to policy "always".'); + } + $doUpdate = empty($dao->entity_modified_date); + } + if ($doUpdate && $todo['params']['version'] == 3) { $defaults = ['id' => $dao->entity_id]; if ($this->isActivationSupported($dao->entity_type)) { @@ -427,13 +437,18 @@ protected function removeStaleEntity($dao) { break; case 'unused': - $getRefCount = civicrm_api3($dao->entity_type, 'getrefcount', [ - 'debug' => 1, - 'id' => $dao->entity_id, - ]); + if (CRM_Core_BAO_Managed::isApi4ManagedType($dao->entity_type)) { + $getRefCount = \Civi\Api4\Utils\CoreUtil::getRefCount($dao->entity_type, $dao->entity_id); + } + else { + $getRefCount = civicrm_api3($dao->entity_type, 'getrefcount', [ + 'id' => $dao->entity_id, + ])['values']; + } + // FIXME: This extra counting should be unnecessary, because getRefCount only returns values if count > 0 $total = 0; - foreach ($getRefCount['values'] as $refCount) { + foreach ($getRefCount as $refCount) { $total += $refCount['count']; } diff --git a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/ManagedSearchTest.php b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/ManagedSearchTest.php new file mode 100644 index 000000000000..62989cda7e4c --- /dev/null +++ b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/ManagedSearchTest.php @@ -0,0 +1,106 @@ +_managedEntities = []; + parent::setUp(); + } + + public function setUpHeadless() { + // Civi\Test has many helpers, like install(), uninstall(), sql(), and sqlFile(). + // See: https://docs.civicrm.org/dev/en/latest/testing/phpunit/#civitest + return \Civi\Test::headless() + ->installMe(__DIR__) + ->apply(); + } + + public function hook_civicrm_managed(array &$entities): void { + $entities = array_merge($entities, $this->_managedEntities); + } + + public function testDeleteUnusedSearch() { + $savedSearch = [ + 'module' => 'civicrm', + 'name' => 'testDeleteUnusedSearch', + 'entity' => 'SavedSearch', + 'cleanup' => 'unused', + 'update' => 'unmodified', + 'params' => [ + 'version' => 4, + 'values' => [ + 'name' => 'testDeleteUnusedSearch', + 'label' => 'Test Search', + 'description' => 'Original state', + 'api_entity' => 'Contact', + 'api_params' => [ + 'version' => 4, + 'select' => ['id'], + ], + ], + ], + ]; + $searchDisplay = [ + 'module' => 'civicrm', + 'name' => 'testDeleteUnusedDisplay', + 'entity' => 'SearchDisplay', + 'cleanup' => 'unused', + 'update' => 'unmodified', + 'params' => [ + 'version' => 4, + 'values' => [ + 'type' => 'table', + 'name' => 'testDeleteUnusedDisplay', + 'label' => 'testDeleteUnusedDisplay', + 'saved_search_id.name' => 'testDeleteUnusedSearch', + 'settings' => [ + 'limit' => 20, + 'pager' => TRUE, + 'columns' => [ + [ + 'key' => 'id', + 'label' => 'Contact ID', + 'dataType' => 'Integer', + 'type' => 'field', + ], + ], + ], + ], + ], + ]; + // Add managed search + display + $this->_managedEntities[] = $savedSearch; + $this->_managedEntities[] = $searchDisplay; + \CRM_Core_ManagedEntities::singleton(TRUE)->reconcile(); + + $search = SavedSearch::get(FALSE) + ->selectRowCount() + ->addWhere('name', '=', 'testDeleteUnusedSearch') + ->execute(); + $this->assertCount(1, $search); + + $this->_managedEntities = []; + \CRM_Core_ManagedEntities::singleton(TRUE)->reconcile(); + + $search = SavedSearch::get(FALSE) + ->selectRowCount() + ->addWhere('name', '=', 'testDeleteUnusedSearch') + ->execute(); + $this->assertCount(0, $search); + } + +} diff --git a/tests/phpunit/api/v4/Entity/ManagedEntityTest.php b/tests/phpunit/api/v4/Entity/ManagedEntityTest.php index bc86da428dd7..fb3d8ee35d98 100644 --- a/tests/phpunit/api/v4/Entity/ManagedEntityTest.php +++ b/tests/phpunit/api/v4/Entity/ManagedEntityTest.php @@ -27,9 +27,33 @@ * @group headless */ class ManagedEntityTest extends UnitTestCase implements TransactionalInterface, HookInterface { + /** + * @var array[] + */ + private $_managedEntities = []; + + public function setUp(): void { + $this->_managedEntities = []; + parent::setUp(); + } public function hook_civicrm_managed(array &$entities): void { - $entities[] = [ + $entities = array_merge($entities, $this->_managedEntities); + } + + public function testGetFields() { + $fields = SavedSearch::getFields(FALSE) + ->addWhere('type', '=', 'Extra') + ->setLoadOptions(TRUE) + ->execute()->indexBy('name'); + + $this->assertEquals('Boolean', $fields['has_base']['data_type']); + // If this core extension ever goes away or gets renamed, just pick a different one here + $this->assertArrayHasKey('org.civicrm.flexmailer', $fields['base_module']['options']); + } + + public function testRevertSavedSearch() { + $this->_managedEntities[] = [ // Setting module to 'civicrm' works for the test but not sure we should actually support that // as it's probably better to package stuff in a core extension instead of core itself. 'module' => 'civicrm', @@ -52,20 +76,7 @@ public function hook_civicrm_managed(array &$entities): void { ], ], ]; - } - - public function testGetFields() { - $fields = SavedSearch::getFields(FALSE) - ->addWhere('type', '=', 'Extra') - ->setLoadOptions(TRUE) - ->execute()->indexBy('name'); - $this->assertEquals('Boolean', $fields['has_base']['data_type']); - // If this core extension ever goes away or gets renamed, just pick a different one here - $this->assertArrayHasKey('org.civicrm.flexmailer', $fields['base_module']['options']); - } - - public function testRevertSavedSearch() { \CRM_Core_ManagedEntities::singleton(TRUE)->reconcile(); $search = SavedSearch::get(FALSE) @@ -101,7 +112,6 @@ public function testRevertSavedSearch() { $result = SavedSearch::get(FALSE) ->addWhere('name', '=', 'TestManagedSavedSearch') ->addSelect('description', 'has_base', 'base_module', 'local_modified_date') - ->setDebug(TRUE) ->execute(); $search = $result->single(); $this->assertEquals('Original state', $search['description']); @@ -128,6 +138,125 @@ public function testRevertSavedSearch() { $this->assertNull($search['local_modified_date']); } + public function testAutoUpdateSearch() { + $autoUpdateSearch = [ + 'module' => 'civicrm', + 'name' => 'testAutoUpdate', + 'entity' => 'SavedSearch', + 'cleanup' => 'unused', + 'update' => 'unmodified', + 'params' => [ + 'version' => 4, + 'values' => [ + 'name' => 'TestAutoUpdateSavedSearch', + 'label' => 'Test AutoUpdate Search', + 'description' => 'Original state', + 'api_entity' => 'Email', + 'api_params' => [ + 'version' => 4, + 'select' => ['id'], + 'orderBy' => ['id', 'ASC'], + ], + ], + ], + ]; + // Add managed search + $this->_managedEntities[] = $autoUpdateSearch; + \CRM_Core_ManagedEntities::singleton(TRUE)->reconcile(); + + $search = SavedSearch::get(FALSE) + ->addWhere('name', '=', 'TestAutoUpdateSavedSearch') + ->addSelect('description', 'local_modified_date') + ->execute()->single(); + $this->assertEquals('Original state', $search['description']); + $this->assertNull($search['local_modified_date']); + + // Remove managed search + $this->_managedEntities = []; + \CRM_Core_ManagedEntities::singleton(TRUE)->reconcile(); + + // Because the search has no displays, it will be deleted (cleanup = unused) + $search = SavedSearch::get(FALSE) + ->addWhere('name', '=', 'TestAutoUpdateSavedSearch') + ->execute(); + $this->assertCount(0, $search); + + // Restore managed entity + $this->_managedEntities = []; + $this->_managedEntities[] = $autoUpdateSearch; + \CRM_Core_ManagedEntities::singleton(TRUE)->reconcile(); + + // Entity should be restored + $result = SavedSearch::get(FALSE) + ->addWhere('name', '=', 'TestAutoUpdateSavedSearch') + ->addSelect('description', 'has_base', 'base_module', 'local_modified_date') + ->execute(); + $search = $result->single(); + $this->assertEquals('Original state', $search['description']); + // Check calculated fields + $this->assertTrue($search['has_base']); + $this->assertEquals('civicrm', $search['base_module']); + $this->assertNull($search['local_modified_date']); + + $search = SavedSearch::get(FALSE) + ->addWhere('name', '=', 'TestAutoUpdateSavedSearch') + ->addSelect('description', 'local_modified_date') + ->execute()->single(); + $this->assertEquals('Original state', $search['description']); + $this->assertNull($search['local_modified_date']); + + // Update packaged version + $autoUpdateSearch['params']['values']['description'] = 'New packaged state'; + $this->_managedEntities = []; + $this->_managedEntities[] = $autoUpdateSearch; + \CRM_Core_ManagedEntities::singleton(TRUE)->reconcile(); + + // Because the entity was not modified, it will be updated to match the new packaged version + $search = SavedSearch::get(FALSE) + ->addWhere('name', '=', 'TestAutoUpdateSavedSearch') + ->addSelect('description', 'local_modified_date') + ->execute()->single(); + $this->assertEquals('New packaged state', $search['description']); + $this->assertNull($search['local_modified_date']); + + // Update local + SavedSearch::update(FALSE) + ->addValue('id', $search['id']) + ->addValue('description', 'Altered state') + ->execute(); + + // Update packaged version + $autoUpdateSearch['params']['values']['description'] = 'Newer packaged state'; + $this->_managedEntities = []; + $this->_managedEntities[] = $autoUpdateSearch; + \CRM_Core_ManagedEntities::singleton(TRUE)->reconcile(); + + // Because the entity was modified, it will not be updated + $search = SavedSearch::get(FALSE) + ->addWhere('name', '=', 'TestAutoUpdateSavedSearch') + ->addSelect('description', 'local_modified_date') + ->execute()->single(); + $this->assertEquals('Altered state', $search['description']); + $this->assertNotNull($search['local_modified_date']); + + SavedSearch::revert(FALSE) + ->addWhere('name', '=', 'TestAutoUpdateSavedSearch') + ->execute(); + + // Entity should be revered to newer packaged state + $result = SavedSearch::get(FALSE) + ->addWhere('name', '=', 'TestAutoUpdateSavedSearch') + ->addSelect('description', 'has_base', 'base_module', 'local_modified_date') + ->execute(); + $search = $result->single(); + $this->assertEquals('Newer packaged state', $search['description']); + // Check calculated fields + $this->assertTrue($search['has_base']); + $this->assertEquals('civicrm', $search['base_module']); + // local_modified_date should be reset by the revert action + $this->assertNull($search['local_modified_date']); + } + /** * @dataProvider sampleEntityTypes * @param string $entityName