Merge pull request #21232 from colemanw/APIDelete

APIv4 - Use correct BAO delete function (fixes dev/core#2757)

CRM/Core/BAO/EntityTag.php

@@ -96,7 +96,7 @@ public static function dataExists($params) {
    * Delete the tag for a contact.
    *
    * @param array $params
-   *
+   * @deprecated
    * WARNING: Nonstandard params searches by tag_id rather than id!
    */
   public static function del(&$params) {

Civi/Api4/Action/Contact/Delete.php

@@ -0,0 +1,37 @@
+<?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       |
+ +--------------------------------------------------------------------+
+ */
+
+namespace Civi\Api4\Action\Contact;
+
+/**
+ * Deletes a contact, by default moving to trash. Set `useTrash = FALSE` for permanent deletion.
+ * @inheritDoc
+ */
+class Delete extends \Civi\Api4\Generic\DAODeleteAction {
+  use \Civi\Api4\Generic\Traits\SoftDelete;
+
+  /**
+   * @param $items
+   * @return array
+   * @throws \API_Exception
+   */
+  protected function deleteObjects($items) {
+    foreach ($items as $item) {
+      if (!\CRM_Contact_BAO_Contact::deleteContact($item['id'], FALSE, !$this->useTrash, $this->checkPermissions)) {
+        throw new \API_Exception("Could not delete {$this->getEntityName()} id {$item['id']}");
+      }
+      $ids[] = ['id' => $item['id']];
+    }
+    return $ids;
+  }
+
+}

Civi/Api4/Contact.php

@@ -26,6 +26,15 @@
  */
 class Contact extends Generic\DAOEntity {
 
+  /**
+   * @param bool $checkPermissions
+   * @return Action\Contact\Delete
+   */
+  public static function delete($checkPermissions = TRUE) {
+    return (new Action\Contact\Delete(__CLASS__, __FUNCTION__))
+      ->setCheckPermissions($checkPermissions);
+  }
+
   /**
    * @param bool $checkPermissions
    * @return Action\Contact\GetChecksum

Civi/Api4/Generic/DAODeleteAction.php

@@ -14,6 +14,7 @@
 
 use Civi\API\Exception\UnauthorizedException;
 use Civi\Api4\Utils\CoreUtil;
+use Civi\Api4\Utils\ReflectionUtils;
 
 /**
  * Delete one or more $ENTITIES.
@@ -56,7 +57,8 @@ protected function deleteObjects($items) {
     $ids = [];
     $baoName = $this->getBaoName();
 
-    if ($this->getEntityName() !== 'EntityTag' && method_exists($baoName, 'del')) {
+    // Use BAO::del() method if it is not deprecated
+    if (method_exists($baoName, 'del') && !ReflectionUtils::isMethodDeprecated($baoName, 'del')) {
       foreach ($items as $item) {
         $args = [$item['id']];
         $bao = call_user_func_array([$baoName, 'del'], $args);

Civi/Api4/Generic/Traits/SoftDelete.php

@@ -0,0 +1,27 @@
+<?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       |
+ +--------------------------------------------------------------------+
+ */
+
+namespace Civi\Api4\Generic\Traits;
+
+/**
+ * This trait is used by entities with a "move to trash" option.
+ * @method $this setUseTrash(bool $useTrash) Pass FALSE to force delete and bypass trash
+ * @method bool getUseTrash()
+ */
+trait SoftDelete {
+
+  /**
+   * Should $ENTITY be moved to the trash instead of permanently deleted?
+   * @var bool
+   */
+  protected $useTrash = TRUE;
+
+}

Civi/Api4/Utils/ReflectionUtils.php

@@ -179,6 +179,20 @@ public static function findStandardProperties($class): iterable {
     }
   }
 
+  /**
+   * Check if a class method is deprecated
+   *
+   * @param string $className
+   * @param string $methodName
+   * @return bool
+   * @throws \ReflectionException
+   */
+  public static function isMethodDeprecated(string $className, string $methodName): bool {
+    $reflection = new \ReflectionClass($className);
+    $docBlock = $reflection->getMethod($methodName)->getDocComment();
+    return strpos($docBlock, "@deprecated") !== FALSE;
+  }
+
   /**
    * Find any methods in this class which match the given prefix.
    *

Civi/Test/Api3TestTrait.php

@@ -510,6 +510,10 @@ public function runApi4Legacy($v3Entity, $v3Action, $v3Params = []) {
       }
     }
 
+    if (isset($actionInfo[0]['params']['useTrash'])) {
+      $v4Params['useTrash'] = empty($v3Params['skip_undelete']);
+    }
+
     // Build where clause for 'getcount', 'getsingle', 'getvalue', 'get' & 'replace'
     if ($v4Action == 'get' || $v3Action == 'replace') {
       foreach ($v3Params as $key => $val) {

tests/phpunit/api/v3/ContactTest.php

@@ -3154,27 +3154,32 @@ public function testContactCreationPermissions(int $version): void {
 
   /**
    * Test that delete with skip undelete respects permissions.
-   * TODO: Api4
+   *
+   * @param int $version
    *
    * @throws \CRM_Core_Exception
-   * @throws \CiviCRM_API3_Exception
+   * @dataProvider versionThreeAndFour
    */
-  public function testContactDeletePermissions(): void {
+  public function testContactDeletePermissions(int $version): void {
+    $this->_apiversion = $version;
     $contactID = $this->individualCreate();
-    $tag = $this->callAPISuccess('Tag', 'create', ['name' => 'to be deleted']);
+    $this->quickCleanup(['civicrm_entity_tag', 'civicrm_tag']);
+    $tag = $this->callAPISuccess('Tag', 'create', ['name' => uniqid('to be deleted')]);
     $this->callAPISuccess('EntityTag', 'create', ['entity_id' => $contactID, 'tag_id' => $tag['id']]);
     CRM_Core_Config::singleton()->userPermissionClass->permissions = ['access CiviCRM'];
     $this->callAPIFailure('Contact', 'delete', [
       'id' => $contactID,
       'check_permissions' => 1,
       'skip_undelete' => 1,
     ]);
+    $this->callAPISuccessGetCount('EntityTag', ['entity_id' => $contactID], 1);
     $this->callAPISuccess('Contact', 'delete', [
       'id' => $contactID,
       'check_permissions' => 0,
       'skip_undelete' => 1,
     ]);
     $this->callAPISuccessGetCount('EntityTag', ['entity_id' => $contactID], 0);
+    $this->quickCleanup(['civicrm_entity_tag', 'civicrm_tag']);
   }
 
   /**

tests/phpunit/api/v4/Entity/ConformanceTest.php

@@ -424,10 +424,15 @@ protected function checkDeletionAllowed($entityClass, $id, $entity) {
     $this->assertEquals(0, $this->checkAccessCounts["{$entity}::delete"]);
     $isReadOnly = $this->isReadOnly($entityClass);
 
-    $deleteResult = $entityClass::delete()
+    $deleteAction = $entityClass::delete()
       ->setCheckPermissions(!$isReadOnly)
-      ->addWhere('id', '=', $id)
-      ->execute();
+      ->addWhere('id', '=', $id);
+
+    if (property_exists($deleteAction, 'useTrash')) {
+      $deleteAction->setUseTrash(FALSE);
+    }
+
+    $deleteResult = $deleteAction->execute();
 
     // should get back an array of deleted id
     $this->assertEquals([['id' => $id]], (array) $deleteResult);

tests/phpunit/api/v4/Mock/MockV4ReflectionGrandchild.php

@@ -31,4 +31,21 @@
  */
 class MockV4ReflectionGrandchild extends MockV4ReflectionChild {
 
+  /**
+   * Function marked deprecated
+   * @see \api\v4\Utils\ReflectionUtilsTest::testIsMethodDeprecated
+   * @deprecated
+   */
+  public static function deprecatedFn() {
+
+  }
+
+  /**
+   * Function not marked deprecated
+   * @see \api\v4\Utils\ReflectionUtilsTest::testIsMethodDeprecated
+   */
+  public static function nonDeprecatedFn() {
+
+  }
+
 }

tests/phpunit/api/v4/Utils/ReflectionUtilsTest.php

@@ -109,4 +109,10 @@ public function testParseDocBlock($input, $expected) {
     $this->assertEquals($expected, ReflectionUtils::parseDocBlock($input));
   }
 
+  public function testIsMethodDeprecated() {
+    $mockClass = 'api\v4\Mock\MockV4ReflectionGrandchild';
+    $this->assertTrue(ReflectionUtils::isMethodDeprecated($mockClass, 'deprecatedFn'));
+    $this->assertFalse(ReflectionUtils::isMethodDeprecated($mockClass, 'nonDeprecatedFn'));
+  }
+
 }