From 095e8ae41f6c71a8c7372828046fc012545b85a6 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/SearchDisplay/SearchAfformTest.php | 2 +- .../api/v4/Entity/ManagedEntityTest.php | 159 ++++++++++++++++-- 4 files changed, 272 insertions(+), 22 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 80f79e639a..0598fb1ebe 100644 --- a/CRM/Core/ManagedEntities.php +++ b/CRM/Core/ManagedEntities.php @@ -185,6 +185,7 @@ class CRM_Core_ManagedEntities { $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 @@ class CRM_Core_ManagedEntities { * @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 @@ class CRM_Core_ManagedEntities { $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 @@ class CRM_Core_ManagedEntities { 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 0000000000..244d19ce13 --- /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/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchAfformTest.php b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchAfformTest.php index 131e223347..91a7ecf8a2 100644 --- a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchAfformTest.php +++ b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchAfformTest.php @@ -24,7 +24,7 @@ class SearchAfformTest extends \PHPUnit\Framework\TestCase implements HeadlessIn ->apply(); } - public function tearDown() { + public function tearDown(): void { Afform::revert(FALSE)->addWhere('has_local', '=', TRUE)->execute(); parent::tearDown(); } diff --git a/tests/phpunit/api/v4/Entity/ManagedEntityTest.php b/tests/phpunit/api/v4/Entity/ManagedEntityTest.php index bc86da428d..fb3d8ee35d 100644 --- a/tests/phpunit/api/v4/Entity/ManagedEntityTest.php +++ b/tests/phpunit/api/v4/Entity/ManagedEntityTest.php @@ -27,9 +27,33 @@ use Civi\Test\TransactionalInterface; * @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 @@ class ManagedEntityTest extends UnitTestCase implements TransactionalInterface, ], ], ]; - } - - 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 @@ class ManagedEntityTest extends UnitTestCase implements TransactionalInterface, $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 @@ class ManagedEntityTest extends UnitTestCase implements TransactionalInterface, $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 -- 2.25.1