From a2bf5923cd5bb6dedac554931190598e7a6b8f2e Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Fri, 5 Nov 2021 17:45:24 -0400
Subject: [PATCH] ManagedEntities - Always delete managed record when deleting
 an entity

This uses hooks to ensure managed records are always cleared out when an entity is deleted.
Fixes OptionValue::delete which was previously not calling hooks.
---
 CRM/Core/BAO/Managed.php                      | 46 +++++++++++++++++++
 CRM/Core/BAO/OptionValue.php                  |  3 ++
 CRM/Core/ManagedEntities.php                  | 12 ++++-
 .../phpunit/CRM/Core/ManagedEntitiesTest.php  |  4 +-
 .../api/v4/Entity/ManagedEntityTest.php       | 24 ++++++++++
 5 files changed, 85 insertions(+), 4 deletions(-)
 create mode 100644 CRM/Core/BAO/Managed.php

diff --git a/CRM/Core/BAO/Managed.php b/CRM/Core/BAO/Managed.php
new file mode 100644
index 0000000000..fcbb8b0960
--- /dev/null
+++ b/CRM/Core/BAO/Managed.php
@@ -0,0 +1,46 @@
+<?php
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ *
+ * @package CRM
+ * @copyright CiviCRM LLC https://civicrm.org/licensing
+ */
+
+/**
+ * This class contains functions for managed entities.
+ */
+class CRM_Core_BAO_Managed extends CRM_Core_DAO_Managed implements Civi\Test\HookInterface {
+
+  /**
+   * Callback for hook_civicrm_post().
+   * @param \Civi\Core\Event\PostEvent $event
+   */
+  public static function on_hook_civicrm_post(\Civi\Core\Event\PostEvent $event) {
+    // When an entity is deleted, delete the corresponding Managed record
+    if ($event->action === 'delete' && $event->id && self::isApi4ManagedType($event->entity)) {
+      \Civi\Api4\Managed::delete(FALSE)
+        ->addWhere('entity_type', '=', $event->entity)
+        ->addWhere('entity_id', '=', $event->id)
+        ->execute();
+    }
+  }
+
+  /**
+   * @param string $entityName
+   * @return bool
+   */
+  public static function isApi4ManagedType(string $entityName) {
+    $type = \Civi\Api4\Utils\CoreUtil::getInfoItem($entityName, 'type');
+    return $type && in_array('ManagedEntity', $type, TRUE);
+  }
+
+}
diff --git a/CRM/Core/BAO/OptionValue.php b/CRM/Core/BAO/OptionValue.php
index 4e3a22533d..5414431659 100644
--- a/CRM/Core/BAO/OptionValue.php
+++ b/CRM/Core/BAO/OptionValue.php
@@ -264,9 +264,12 @@ class CRM_Core_BAO_OptionValue extends CRM_Core_DAO_OptionValue {
     if (!$optionValue->find()) {
       return FALSE;
     }
+    $hookParams = ['id' => $optionValueId];
+    CRM_Utils_Hook::pre('delete', 'OptionValue', $optionValueId, $hookParams);
     if (self::updateRecords($optionValueId, CRM_Core_Action::DELETE)) {
       CRM_Core_PseudoConstant::flush();
       $optionValue->delete();
+      CRM_Utils_Hook::post('delete', 'OptionValue', $optionValueId, $optionValue);
       return TRUE;
     }
     return FALSE;
diff --git a/CRM/Core/ManagedEntities.php b/CRM/Core/ManagedEntities.php
index a4d2096ae7..2978393b35 100644
--- a/CRM/Core/ManagedEntities.php
+++ b/CRM/Core/ManagedEntities.php
@@ -439,13 +439,21 @@ class CRM_Core_ManagedEntities {
         throw new CRM_Core_Exception('Unrecognized cleanup policy: ' . $policy);
     }
 
-    if ($doDelete) {
+    // APIv4 delete - deletion from `civicrm_managed` will be taken care of by
+    // CRM_Core_BAO_Managed::on_hook_civicrm_post()
+    if ($doDelete && CRM_Core_BAO_Managed::isApi4ManagedType($dao->entity_type)) {
+      civicrm_api4($dao->entity_type, 'delete', [
+        'where' => [['id', '=', $dao->entity_id]],
+      ]);
+    }
+    // APIv3 delete
+    elseif ($doDelete) {
       $params = [
         'version' => 3,
         'id' => $dao->entity_id,
       ];
       $check = civicrm_api3($dao->entity_type, 'get', $params);
-      if ((bool) $check['count']) {
+      if ($check['count']) {
         $result = civicrm_api($dao->entity_type, 'delete', $params);
         if ($result['is_error']) {
           if (isset($dao->name)) {
diff --git a/tests/phpunit/CRM/Core/ManagedEntitiesTest.php b/tests/phpunit/CRM/Core/ManagedEntitiesTest.php
index 548ecb5f43..90e1b3240d 100644
--- a/tests/phpunit/CRM/Core/ManagedEntitiesTest.php
+++ b/tests/phpunit/CRM/Core/ManagedEntitiesTest.php
@@ -263,8 +263,8 @@ class CRM_Core_ManagedEntitiesTest extends CiviUnitTestCase {
 
   /**
    * Set up an active module with one managed-entity using the
-   * policy "cleanup=>never". When the managed-entity goes away,
-   * ensure that the policy is followed (ie the entity is not
+   * policy "cleanup=>unused". When the managed-entity goes away,
+   * ensure that the policy is followed (ie the entity is conditionally
    * deleted).
    *
    * @throws \CRM_Core_Exception
diff --git a/tests/phpunit/api/v4/Entity/ManagedEntityTest.php b/tests/phpunit/api/v4/Entity/ManagedEntityTest.php
index 2ce5341520..f959940f8f 100644
--- a/tests/phpunit/api/v4/Entity/ManagedEntityTest.php
+++ b/tests/phpunit/api/v4/Entity/ManagedEntityTest.php
@@ -119,4 +119,28 @@ class ManagedEntityTest extends UnitTestCase implements TransactionalInterface,
     $this->assertFalse($search['has_base']);
   }
 
+  /**
+   * @dataProvider sampleEntityTypes
+   * @param string $entityName
+   * @param bool $expected
+   */
+  public function testIsApi4ManagedType($entityName, $expected) {
+    $this->assertEquals($expected, \CRM_Core_BAO_Managed::isAPi4ManagedType($entityName));
+  }
+
+  public function sampleEntityTypes() {
+    return [
+      // v3 pseudo-entity
+      'ActivityType' => ['ActivityType', FALSE],
+      // v3 pseudo-entity
+      'CustomSearch' => ['CustomSearch', FALSE],
+      // Not a dao entity, can't be managed
+      'Entity' => ['Entity', FALSE],
+      // v4 entity not using ManagedEntity trait
+      'UFJoin' => ['UFJoin', FALSE],
+      // v4 entity using ManagedEntity trait
+      'SavedSearch' => ['SavedSearch', TRUE],
+    ];
+  }
+
 }
-- 
2.25.1