Afform - reset managed entities when deleting a dashlet

Fixes dev/core#3122
civicrm · Mar 16, 2022 · cc9b2a9 · cc9b2a9
1 parent d74a5e0
commit cc9b2a9
Show file tree

Hide file tree

Showing 6 changed files with 145 additions and 38 deletions.
diff --git a/CRM/Core/ManagedEntities.php b/CRM/Core/ManagedEntities.php
@@ -667,6 +667,7 @@ protected function loadDeclarations(): void {
 
   protected function loadManagedEntityActions(): void {
     $managedEntities = Managed::get(FALSE)->addSelect('*')->execute();
+    $this->managedActions = [];
     foreach ($managedEntities as $managedEntity) {
       $key = "{$managedEntity['module']}_{$managedEntity['name']}_{$managedEntity['entity_type']}";
       // Set to 'delete' - it will be overwritten below if it is to be updated.

diff --git a/Civi/Api4/Generic/BasicBatchAction.php b/Civi/Api4/Generic/BasicBatchAction.php
@@ -59,16 +59,36 @@ public function __construct($entityName, $actionName, $doer = NULL) {
   }
 
   /**
-   * We pass the doTask function an array representing one item to update.
-   * We expect to get the same format back.
+   * Checks permissions and then delegates to processBatch.
+   *
+   * Note: Unconditional logic must go here in the run function, as delegated functions may be overridden.
    *
    * @param \Civi\Api4\Generic\Result $result
    */
   public function _run(Result $result) {
-    foreach ($this->getBatchRecords() as $item) {
+    $items = $this->getBatchRecords();
+    foreach ($items as $item) {
       if ($this->checkPermissions && !CoreUtil::checkAccessRecord($this, $item, \CRM_Core_Session::getLoggedInContactID() ?: 0)) {
         throw new UnauthorizedException("ACL check failed");
       }
+    }
+    $this->processBatch($result, $items);
+  }
+
+  /**
+   * Calls doTask once per item and stores the result.
+   *
+   * We pass the doTask function an array representing one item to process.
+   * We expect to get the same format back.
+   *
+   * Note: This function may be overridden by the end api.
+   *
+   * @param Result $result
+   * @param array $items
+   * @throws NotImplementedException
+   */
+  protected function processBatch(Result $result, array $items) {
+    foreach ($items as $item) {
       $result[] = $this->doTask($item);
     }
   }

diff --git a/ext/afform/core/Civi/Api4/Action/Afform/Revert.php b/ext/afform/core/Civi/Api4/Action/Afform/Revert.php
@@ -0,0 +1,92 @@
+<?php
+
+namespace Civi\Api4\Action\Afform;
+
+use Civi\Api4\Generic\Result;
+
+/**
+ * @inheritDoc
+ * @package Civi\Api4\Action\Afform
+ */
+class Revert extends \Civi\Api4\Generic\BasicBatchAction {
+
+  /**
+   * @var bool
+   */
+  private $flushManaged = FALSE;
+
+  /**
+   * @var bool
+   */
+  private $flushMenu = FALSE;
+
+  /**
+   * Revert every record, and flush caches at the end.
+   *
+   * @inheritDoc
+   */
+  protected function processBatch(Result $result, array $items) {
+    parent::processBatch($result, $items);
+
+    // We may have changed list of files covered by the cache.
+    _afform_clear();
+
+    if ($this->flushManaged) {
+      // FIXME: more targeted reconciliation
+      \CRM_Core_ManagedEntities::singleton()->reconcile();
+    }
+    if ($this->flushMenu) {
+      \CRM_Core_Menu::store();
+    }
+  }
+
+  /**
+   * Revert (delete) a record.
+   *
+   * @inheritDoc
+   */
+  protected function doTask($item) {
+    /** @var \CRM_Afform_AfformScanner $scanner */
+    $scanner = \Civi::service('afform_scanner');
+    $files = [
+      \CRM_Afform_AfformScanner::METADATA_FILE,
+      \CRM_Afform_AfformScanner::LAYOUT_FILE,
+    ];
+
+    foreach ($files as $file) {
+      $metaPath = $scanner->createSiteLocalPath($item['name'], $file);
+      if (file_exists($metaPath)) {
+        if (!@unlink($metaPath)) {
+          throw new \API_Exception("Failed to remove afform overrides in $file");
+        }
+      }
+    }
+
+    $original = (array) $scanner->getMeta($item['name']);
+
+    // If the dashlet setting changed, managed entities must be reconciled
+    if (
+      (empty($item['is_dashlet']) !== empty($original['is_dashlet'])) ||
+      ($item['is_dashlet'] && ($item['title'] ?? '') !== ($original['title'] ?? ''))
+    ) {
+      $this->flushManaged = TRUE;
+    }
+
+    // If the server_route changed, reset menu cache
+    if (($item['server_route'] ?? '') !== ($original['server_route'] ?? '')) {
+      $this->flushMenu = TRUE;
+    }
+
+    return $item;
+  }
+
+  /**
+   * Adds extra return params so caches can be conditionally flushed.
+   *
+   * @return string[]
+   */
+  protected function getSelect() {
+    return ['name', 'title', 'is_dashlet', 'server_route'];
+  }
+
+}
diff --git a/ext/afform/core/Civi/Api4/Afform.php b/ext/afform/core/Civi/Api4/Afform.php
@@ -2,7 +2,6 @@
 
 namespace Civi\Api4;
 
-use Civi\Api4\Generic\BasicBatchAction;
 use Civi\Api4\Generic\BasicGetFieldsAction;
 
 /**
@@ -105,33 +104,11 @@ public static function getOptions($checkPermissions = TRUE) {
 
   /**
    * @param bool $checkPermissions
-   * @return Generic\BasicBatchAction
+   * @return Action\Afform\Revert
    */
   public static function revert($checkPermissions = TRUE) {
-    return (new BasicBatchAction('Afform', __FUNCTION__, function($item, BasicBatchAction $action) {
-      $scanner = \Civi::service('afform_scanner');
-      $files = [
-        \CRM_Afform_AfformScanner::METADATA_FILE,
-        \CRM_Afform_AfformScanner::LAYOUT_FILE,
-      ];
-
-      foreach ($files as $file) {
-        $metaPath = $scanner->createSiteLocalPath($item['name'], $file);
-        if (file_exists($metaPath)) {
-          if (!@unlink($metaPath)) {
-            throw new \API_Exception("Failed to remove afform overrides in $file");
-          }
-        }
-      }
-
-      // We may have changed list of files covered by the cache.
-      _afform_clear();
-
-      // FIXME if `server_route` changes, then flush the menu cache.
-      // FIXME if asset-caching is enabled, then flush the asset cache
-
-      return $item;
-    }))->setCheckPermissions($checkPermissions);
+    return (new Action\Afform\Revert('Afform', __FUNCTION__))
+      ->setCheckPermissions($checkPermissions);
   }
 
   /**

diff --git a/ext/afform/core/Civi/Api4/Utils/AfformSaveTrait.php b/ext/afform/core/Civi/Api4/Utils/AfformSaveTrait.php
@@ -65,19 +65,18 @@ protected function writeRecord($item) {
       return ($item[$field] ?? NULL) !== ($orig[$field] ?? NULL);
     };
 
-    if ($isChanged('is_dashlet')) {
-      // FIXME: more targetted reconciliation
-      \CRM_Core_ManagedEntities::singleton()->reconcile();
-    }
-    elseif (array_key_exists('is_dashlet', (array) $orig) && $orig['is_dashlet'] && $isChanged('title')) {
-      // FIXME: more targetted reconciliation
+    // If the dashlet setting changed, managed entities must be reconciled
+    if (
+      $isChanged('is_dashlet') ||
+      (!empty($meta['is_dashlet']) && $isChanged('title'))
+    ) {
+      // FIXME: more targeted reconciliation
       \CRM_Core_ManagedEntities::singleton()->reconcile();
     }
 
     // Right now, permission-checks are completely on-demand.
     if ($isChanged('server_route') /* || $isChanged('permission') */) {
       \CRM_Core_Menu::store();
-      \CRM_Core_BAO_Navigation::resetNavigation();
     }
 
     $item['module_name'] = _afform_angular_module_name($item['name'], 'camel');

diff --git a/ext/afform/mock/tests/phpunit/api/v4/AfformTest.php b/ext/afform/mock/tests/phpunit/api/v4/AfformTest.php
@@ -1,4 +1,5 @@
 <?php
+use Civi\Api4\Dashboard;
 
 /**
  * Afform.Get API Test Case
@@ -50,6 +51,18 @@ public function testGetUpdateRevert($formName, $originalMetadata): void {
       return isset($arr[$key]) ? $arr[$key] : NULL;
     };
 
+    $checkDashlet = function($afform) use ($formName) {
+      $dashlet = Dashboard::get(FALSE)
+        ->addWhere('name', '=', $formName)
+        ->execute();
+      if (!empty($afform['is_dashlet'])) {
+        $this->assertCount(1, $dashlet);
+      }
+      else {
+        $this->assertCount(0, $dashlet);
+      }
+    };
+
     Civi\Api4\Afform::revert()->addWhere('name', '=', $formName)->execute();
 
     $message = 'The initial Afform.get should return default data';
@@ -66,13 +79,14 @@ public function testGetUpdateRevert($formName, $originalMetadata): void {
     $this->assertEquals(TRUE, $get($result[0], 'has_base'), $message);
     $this->assertEquals(FALSE, $get($result[0], 'has_local'), $message);
     $this->assertEquals('org.civicrm.afform-mock', $get($result[0], 'base_module'), $message);
+    $checkDashlet($originalMetadata);
 
     $message = 'After updating with Afform.create, the revised data should be returned';
     $result = Civi\Api4\Afform::update()
       ->addWhere('name', '=', $formName)
       ->addValue('description', 'The temporary description')
       ->addValue('permission', 'access foo')
-      ->addValue('is_dashlet', TRUE)
+      ->addValue('is_dashlet', empty($originalMetadata['is_dashlet']))
       ->execute();
     $this->assertEquals($formName, $result[0]['name'], $message);
     $this->assertEquals('The temporary description', $result[0]['description'], $message);
@@ -84,13 +98,14 @@ public function testGetUpdateRevert($formName, $originalMetadata): void {
     $this->assertEquals($formName, $result[0]['name'], $message);
     $this->assertEquals($get($originalMetadata, 'title'), $get($result[0], 'title'), $message);
     $this->assertEquals('The temporary description', $get($result[0], 'description'), $message);
-    $this->assertEquals(TRUE, $get($result[0], 'is_dashlet'), $message);
+    $this->assertEquals(empty($originalMetadata['is_dashlet']), $get($result[0], 'is_dashlet'), $message);
     $this->assertEquals($get($originalMetadata, 'server_route'), $get($result[0], 'server_route'), $message);
     $this->assertEquals('access foo', $get($result[0], 'permission'), $message);
     $this->assertTrue(is_array($result[0]['layout']), $message);
     $this->assertEquals(TRUE, $get($result[0], 'has_base'), $message);
     $this->assertEquals(TRUE, $get($result[0], 'has_local'), $message);
     $this->assertEquals('org.civicrm.afform-mock', $get($result[0], 'base_module'), $message);
+    $checkDashlet($result[0]);
 
     Civi\Api4\Afform::revert()->addWhere('name', '=', $formName)->execute();
     $message = 'After reverting, the final Afform.get should return default data';
@@ -102,10 +117,13 @@ public function testGetUpdateRevert($formName, $originalMetadata): void {
     $this->assertEquals($get($originalMetadata, 'description'), $get($result[0], 'description'), $message);
     $this->assertEquals($get($originalMetadata, 'server_route'), $get($result[0], 'server_route'), $message);
     $this->assertEquals($get($originalMetadata, 'permission'), $get($result[0], 'permission'), $message);
+    $this->assertEquals($get($originalMetadata, 'is_dashlet'), $get($result[0], 'is_dashlet'), $message);
     $this->assertTrue(is_array($result[0]['layout']), $message);
     $this->assertEquals(TRUE, $get($result[0], 'has_base'), $message);
     $this->assertEquals(FALSE, $get($result[0], 'has_local'), $message);
     $this->assertEquals('org.civicrm.afform-mock', $get($result[0], 'base_module'), $message);
+
+    $checkDashlet($originalMetadata);
   }
 
   public function getFormatExamples() {