From 6623e554386d7d69b5d12ec172f0b996f87e718d Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Sat, 21 Aug 2021 12:51:41 -0400 Subject: [PATCH] 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 dev/core#2757 --- CRM/Contact/Page/View/Note.php | 8 ++- CRM/Contribute/BAO/Contribution.php | 2 +- CRM/Contribute/Form/AdditionalInfo.php | 4 +- CRM/Core/BAO/Note.php | 78 ++++++++---------------- CRM/Event/BAO/Participant.php | 4 +- CRM/Note/Form/Note.php | 4 +- api/v3/Note.php | 4 +- tests/phpunit/api/v4/Entity/NoteTest.php | 77 +++++++++++++++++++++++ 8 files changed, 120 insertions(+), 61 deletions(-) create mode 100644 tests/phpunit/api/v4/Entity/NoteTest.php diff --git a/CRM/Contact/Page/View/Note.php b/CRM/Contact/Page/View/Note.php index f7ffb06e7a..55938c425e 100644 --- a/CRM/Contact/Page/View/Note.php +++ b/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'); } /** diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index 840878bc6c..b420f885a9 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/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(); diff --git a/CRM/Contribute/Form/AdditionalInfo.php b/CRM/Contribute/Form/AdditionalInfo.php index fdbaaade14..c1ce9bfd7e 100644 --- a/CRM/Contribute/Form/AdditionalInfo.php +++ b/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 diff --git a/CRM/Core/BAO/Note.php b/CRM/Core/BAO/Note.php index 85cc99bd2b..3f6080a7a6 100644 --- a/CRM/Core/BAO/Note.php +++ b/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]); } } diff --git a/CRM/Event/BAO/Participant.php b/CRM/Event/BAO/Participant.php index 4d73cf2566..5edcb9ea7a 100644 --- a/CRM/Event/BAO/Participant.php +++ b/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(); diff --git a/CRM/Note/Form/Note.php b/CRM/Note/Form/Note.php index 2104fd3245..c0ec7fe0d5 100644 --- a/CRM/Note/Form/Note.php +++ b/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; } diff --git a/api/v3/Note.php b/api/v3/Note.php index a361620c94..e1866d1196 100644 --- a/api/v3/Note.php +++ b/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'); } /** diff --git a/tests/phpunit/api/v4/Entity/NoteTest.php b/tests/phpunit/api/v4/Entity/NoteTest.php new file mode 100644 index 0000000000..89eef7b835 --- /dev/null +++ b/tests/phpunit/api/v4/Entity/NoteTest.php @@ -0,0 +1,77 @@ +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); + } + +} -- 2.25.1