Note API - Ensure child notes are deleted with parent, and hooks are called

Deprecates the CRM_Core_BAO_Note::del() function and refactors out all references to it.
Related to work on #2757

CRM/Contact/Page/View/Note.php

@@ -158,7 +158,7 @@ class CRM_Contact_Page_View_Note extends CRM_Core_Page {
     $session->pushUserContext($url);
 
     if (CRM_Utils_Request::retrieve('confirmed', 'Boolean')) {
-      CRM_Core_BAO_Note::del($this->_id);
+      $this->delete();
       CRM_Utils_System::redirect($url);
     }
 
@@ -233,10 +233,12 @@ class CRM_Contact_Page_View_Note extends CRM_Core_Page {
   }
 
   /**
-   * Delete the note object from the db.
+   * Delete the note object from the db and set a status msg.
    */
   public function delete() {
-    CRM_Core_BAO_Note::del($this->_id);
+    CRM_Core_BAO_Note::deleteRecord(['id' => $this->_id]);
+    $status = ts('Selected Note has been deleted successfully.');
+    CRM_Core_Session::setStatus($status, ts('Deleted'), 'success');
   }
 
   /**

CRM/Contribute/BAO/Contribution.php

@@ -1518,7 +1518,7 @@ INNER JOIN  civicrm_contact contact ON ( contact.id = c.contact_id )
     $note = CRM_Core_BAO_Note::getNote($id, 'civicrm_contribution');
     $noteId = key($note);
     if ($noteId) {
-      CRM_Core_BAO_Note::del($noteId, FALSE);
+      CRM_Core_BAO_Note::deleteRecord(['id' => $noteId]);
     }
 
     $dao = new CRM_Contribute_DAO_Contribution();

CRM/Contribute/Form/AdditionalInfo.php

@@ -231,7 +231,9 @@ class CRM_Contribute_Form_AdditionalInfo {
    */
   public static function processNote($params, $contactID, $contributionID, $contributionNoteID = NULL) {
     if (CRM_Utils_System::isNull($params['note']) && $contributionNoteID) {
-      CRM_Core_BAO_Note::del($contributionNoteID);
+      CRM_Core_BAO_Note::deleteRecord(['id' => $contributionNoteID]);
+      $status = ts('Selected Note has been deleted successfully.');
+      CRM_Core_Session::setStatus($status, ts('Deleted'), 'success');
       return;
     }
     //process note

CRM/Core/BAO/Note.php

@@ -18,7 +18,7 @@
 /**
  * BAO object for crm_note table.
  */
-class CRM_Core_BAO_Note extends CRM_Core_DAO_Note {
+class CRM_Core_BAO_Note extends CRM_Core_DAO_Note implements \Civi\Test\HookInterface {
   use CRM_Core_DynamicFKAccessTrait;
 
   /**
@@ -270,53 +270,34 @@ class CRM_Core_BAO_Note extends CRM_Core_DAO_Note {
     return $notes;
   }
 
+  /**
+   * Event fired prior to modifying a Note.
+   * @param \Civi\Core\Event\PreEvent $event
+   */
+  public static function self_hook_civicrm_pre(\Civi\Core\Event\PreEvent $event) {
+    if ($event->action === 'delete' && $event->id) {
+      // When deleting a note, also delete child notes
+      // This causes recursion as this hook is called again while deleting child notes,
+      // So the children of children, etc. will also be deleted.
+      foreach (self::getDescendentIds($event->id) as $child) {
+        self::deleteRecord(['id' => $child]);
+      }
+    }
+  }
+
   /**
    * Delete the notes.
    *
    * @param int $id
-   *   Note id.
-   * @param bool $showStatus
-   *   Do we need to set status or not.
    *
-   * @return int|null
-   *   no of deleted notes on success, null otherwise
+   * @deprecated
+   * @return int
    */
-  public static function del($id, $showStatus = TRUE) {
-    $return = NULL;
-    $recent = array($id);
-    $note = new CRM_Core_DAO_Note();
-    $note->id = $id;
-    $note->find();
-    $note->fetch();
-    if ($note->entity_table == 'civicrm_note') {
-      $status = ts('Selected Comment has been deleted successfully.');
-    }
-    else {
-      $status = ts('Selected Note has been deleted successfully.');
-    }
-
-    // Delete all descendents of this Note
-    foreach (self::getDescendentIds($id) as $childId) {
-      $childNote = new CRM_Core_DAO_Note();
-      $childNote->id = $childId;
-      $childNote->delete();
-      $recent[] = $childId;
-    }
-
-    $return = $note->delete();
-    if ($showStatus) {
-      CRM_Core_Session::setStatus($status, ts('Deleted'), 'success');
-    }
+  public static function del($id) {
+    // CRM_Core_Error::deprecatedFunctionWarning('deleteRecord');
+    self::deleteRecord(['id' => $id]);
 
-    // delete the recently created Note
-    foreach ($recent as $recentId) {
-      $noteRecent = array(
-        'id' => $recentId,
-        'type' => 'Note',
-      );
-      CRM_Utils_Recent::del($noteRecent);
-    }
-    return $return;
+    return 1;
   }
 
   /**
@@ -509,26 +490,21 @@ ORDER BY  modified_date desc";
   }
 
   /**
-   * Given a note id, get a list of the ids of all notes that are descendents of that note
+   * Get direct children of given parentId note
    *
    * @param int $parentId
-   *   Id of the given note.
-   * @param array $ids
-   *   (reference) one-dimensional array to store found descendent ids.
    *
    * @return array
-   *   One-dimensional array containing ids of all desendent notes
+   *   One-dimensional array containing ids of child notes
    */
-  public static function getDescendentIds($parentId, &$ids = []) {
-    // get direct children of given parentId note
+  public static function getDescendentIds($parentId) {
+    $ids = [];
     $note = new CRM_Core_DAO_Note();
     $note->entity_table = 'civicrm_note';
     $note->entity_id = $parentId;
     $note->find();
     while ($note->fetch()) {
-      // foreach child, add to ids list, and recurse
       $ids[] = $note->id;
-      self::getDescendentIds($note->id, $ids);
     }
     return $ids;
   }
@@ -561,7 +537,7 @@ WHERE participant.contact_id = %1 AND  note.entity_table = 'civicrm_participant'
 
     $contactNoteId = CRM_Core_DAO::executeQuery($contactQuery, $params);
     while ($contactNoteId->fetch()) {
-      self::del($contactNoteId->id, FALSE);
+      self::deleteRecord(['id' => $contactNoteId->id]);
     }
   }
 

CRM/Event/BAO/Participant.php

@@ -242,7 +242,7 @@ class CRM_Event_BAO_Participant extends CRM_Event_DAO_Participant {
         CRM_Core_BAO_Note::add($noteParams, $noteIDs);
       }
       elseif ($noteId && $hasNoteField) {
-        CRM_Core_BAO_Note::del($noteId, FALSE);
+        CRM_Core_BAO_Note::deleteRecord(['id' => $noteId]);
       }
     }
 
@@ -867,7 +867,7 @@ WHERE  civicrm_participant.id = {$participantId}
     $note = CRM_Core_BAO_Note::getNote($id, 'civicrm_participant');
     $noteId = key($note);
     if ($noteId) {
-      CRM_Core_BAO_Note::del($noteId, FALSE);
+      CRM_Core_BAO_Note::deleteRecord(['id' => $noteId]);
     }
 
     $participant->delete();

CRM/Note/Form/Note.php

@@ -173,7 +173,9 @@ class CRM_Note_Form_Note extends CRM_Core_Form {
     }
 
     if ($this->_action & CRM_Core_Action::DELETE) {
-      CRM_Core_BAO_Note::del($this->_id);
+      CRM_Core_BAO_Note::deleteRecord(['id' => $this->_id]);
+      $status = ts('Selected Note has been deleted successfully.');
+      CRM_Core_Session::setStatus($status, ts('Deleted'), 'success');
       return;
     }
 

api/v3/Note.php

@@ -57,8 +57,8 @@ function _civicrm_api3_note_create_spec(&$params) {
  * @return array
  */
 function civicrm_api3_note_delete($params) {
-  $result = new CRM_Core_BAO_Note();
-  return $result->del($params['id']) ? civicrm_api3_create_success() : civicrm_api3_create_error('Error while deleting Note');
+  $result = CRM_Core_BAO_Note::deleteRecord($params);
+  return $result ? civicrm_api3_create_success() : civicrm_api3_create_error('Error while deleting Note');
 }
 
 /**

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

+<?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
+ */
+
+namespace api\v4\Entity;
+
+use api\v4\UnitTestCase;
+use Civi\Api4\Note;
+use Civi\Test\TransactionalInterface;
+
+/**
+ * @group headless
+ */
+class NoteTest extends UnitTestCase implements TransactionalInterface {
+
+  public function testDeleteWithChildren() {
+    $c1 = $this->createEntity(['type' => 'Individual']);
+
+    $text = uniqid(__FUNCTION__, TRUE);
+
+    // Create 2 top-level notes.
+    $notes = Note::save(FALSE)
+      ->setRecords([['note' => $text], ['note' => $text]])
+      ->setDefaults([
+        'entity_id' => $c1['id'],
+        'entity_table' => 'civicrm_contact',
+      ])->execute();
+
+    // Add 2 children of the first note.
+    $children = Note::save(FALSE)
+      ->setRecords([['note' => $text], ['note' => $text]])
+      ->setDefaults([
+        'entity_id' => $notes->first()['id'],
+        'entity_table' => 'civicrm_note',
+      ])->execute();
+
+    // Add 2 children of the first child.
+    $grandChildren = Note::save(FALSE)
+      ->setRecords([['note' => $text], ['note' => $text]])
+      ->setDefaults([
+        'entity_id' => $children->first()['id'],
+        'entity_table' => 'civicrm_note',
+      ])->execute();
+
+    // We just created 2 top-level notes and 4 children. Ensure we have a total of 6.
+    $existing = Note::get(FALSE)
+      ->addWhere('note', '=', $text)
+      ->execute();
+    $this->assertCount(6, $existing);
+
+    // Delete parent
+    Note::delete(FALSE)
+      ->addWhere('id', '=', $notes->first()['id'])
+      ->execute();
+
+    // Should have deleted 1 parent + 4 child-notes, for a new total of 1 remaining.
+    $existing = Note::get(FALSE)
+      ->addWhere('note', '=', $text)
+      ->execute();
+    $this->assertCount(1, $existing);
+  }
+
+}