From c96c0d5ec589c21835c6a10e3a9fbab04caa76b4 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Fri, 19 Aug 2022 15:21:20 -0400 Subject: [PATCH] APIv4 - Support updating activity.case_id --- CRM/Activity/BAO/Activity.php | 15 ++--- CRM/Case/BAO/Case.php | 51 +++++++++++----- CRM/Case/Page/AJAX.php | 18 +----- CRM/Case/XMLProcessor/Process.php | 7 --- CRM/Core/BAO/ActionSchedule.php | 9 +-- api/v3/Activity.php | 5 ++ tests/phpunit/CRM/Case/BAO/CaseTest.php | 8 +-- tests/phpunit/api/v4/Entity/CaseTest.php | 75 ++++++++++++++++++++++++ 8 files changed, 126 insertions(+), 62 deletions(-) diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php index 1f40d7b3e2..35f779e726 100644 --- a/CRM/Activity/BAO/Activity.php +++ b/CRM/Activity/BAO/Activity.php @@ -310,7 +310,7 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { // CRM-8708, preserve case ID even though it's not part of the SQL model $activity->case_id = $params['case_id']; } - elseif (is_numeric($activity->id)) { + elseif ($action === 'edit' && CRM_Core_Component::isEnabled('CiviCase')) { // CRM-8708, preserve case ID even though it's not part of the SQL model $activity->case_id = CRM_Case_BAO_Case::getCaseIdByActivityId($activity->id); } @@ -456,9 +456,10 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { if (empty($params['skipRecentView'])) { $recentOther = []; if (!empty($params['case_id'])) { - $caseContactID = CRM_Core_DAO::getFieldValue('CRM_Case_DAO_CaseContact', $params['case_id'], 'contact_id', 'case_id'); + $caseId = CRM_Utils_Array::first((array) $params['case_id']); + $caseContactID = CRM_Core_DAO::getFieldValue('CRM_Case_DAO_CaseContact', $caseId, 'contact_id', 'case_id'); $url = CRM_Utils_System::url('civicrm/case/activity/view', - "reset=1&aid={$activity->id}&cid={$caseContactID}&caseID={$params['case_id']}&context=home" + "reset=1&aid={$activity->id}&cid={$caseContactID}&caseID={$caseId}&context=home" ); } else { @@ -522,12 +523,8 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { CRM_Contact_BAO_GroupContactCache::opportunisticCacheFlush(); // Add to case - if (!empty($params['case_id']) && $action === 'create') { - // If this is a brand new case activity, add to case(s) - foreach ((array) $params['case_id'] as $singleCaseId) { - $caseActivityParams = ['activity_id' => $activity->id, 'case_id' => $singleCaseId]; - CRM_Case_BAO_Case::processCaseActivity($caseActivityParams); - } + if (isset($params['case_id'])) { + CRM_Case_BAO_Case::updateCaseActivity($activity->id, $params['case_id']); } CRM_Utils_Hook::post($action, 'Activity', $activity->id, $activity); diff --git a/CRM/Case/BAO/Case.php b/CRM/Case/BAO/Case.php index 129ff21297..21f0128f0f 100644 --- a/CRM/Case/BAO/Case.php +++ b/CRM/Case/BAO/Case.php @@ -62,10 +62,15 @@ class CRM_Case_BAO_Case extends CRM_Case_DAO_Case implements \Civi\Core\HookInte * @param \Civi\Core\Event\PostEvent $e */ public static function on_hook_civicrm_post(\Civi\Core\Event\PostEvent $e): void { + // FIXME: The EventScanner ought to skip over disabled components when registering HookInterface + if (!CRM_Core_Component::isEnabled('CiviCase')) { + return; + } if ($e->entity === 'Activity' && in_array($e->action, ['create', 'edit'])) { - // If subject contains a ‘[case #…]’ string, file activity on the related case (CRM-5916) /** @var CRM_Activity_DAO_Activity $activity */ $activity = $e->object; + + // If subject contains a ‘[case #…]’ string, file activity on the related case (CRM-5916) $matches = []; $subjectToMatch = $activity->subject ?? ''; if (preg_match('/\[case #([0-9a-h]{7})\]/', $subjectToMatch, $matches)) { @@ -158,6 +163,26 @@ class CRM_Case_BAO_Case extends CRM_Case_DAO_Case implements \Civi\Core\HookInte $caseActivityDAO->save(); } + /** + * Associate an activity with 0 or more cases. + * + * @param int $activityId + * @param array|int $caseIds + */ + public static function updateCaseActivity(int $activityId, $caseIds): void { + $actionName = empty($caseIds) ? 'delete' : 'replace'; + $action = \Civi\Api4\CaseActivity::$actionName(FALSE) + ->addWhere('activity_id', '=', $activityId); + + if (!empty($caseIds)) { + foreach ((array) $caseIds as $caseId) { + $action->addRecord(['case_id' => $caseId]); + } + } + + $action->execute(); + } + /** * Get the case subject for Activity. * @@ -308,22 +333,27 @@ WHERE civicrm_case.id = %1"; * Look up a case using an activity ID. * * @param int $activityId + * @param bool $getSingle * - * @return int|null, case ID + * @return array|int|null */ - public static function getCaseIdByActivityId($activityId) { + public static function getCaseIdByActivityId($activityId, $getSingle = TRUE) { $originalId = CRM_Core_DAO::singleValueQuery( 'SELECT original_id FROM civicrm_activity WHERE id = %1', ['1' => [$activityId, 'Integer']] ); - $caseId = CRM_Core_DAO::singleValueQuery( + $caseIds = []; + $query = CRM_Core_DAO::executeQuery( 'SELECT case_id FROM civicrm_case_activity WHERE activity_id in (%1,%2)', [ '1' => [$activityId, 'Integer'], - '2' => [$originalId ? $originalId : $activityId, 'Integer'], + '2' => [$originalId ?: $activityId, 'Integer'], ] ); - return $caseId; + while ($query->fetch()) { + $caseIds[] = $query->case_id; + } + return $getSingle ? CRM_Utils_Array::first($caseIds) : $caseIds; } /** @@ -1404,15 +1434,6 @@ HERESQL; if (!empty($recordedActivityParams)) { $activity = CRM_Activity_BAO_Activity::create($recordedActivityParams); - - //create case_activity record if its case activity. - if ($caseId) { - $caseParams = [ - 'activity_id' => $activity->id, - 'case_id' => $caseId, - ]; - self::processCaseActivity($caseParams); - } } return $result; diff --git a/CRM/Case/Page/AJAX.php b/CRM/Case/Page/AJAX.php index dcb75c10ff..b692655b40 100644 --- a/CRM/Case/Page/AJAX.php +++ b/CRM/Case/Page/AJAX.php @@ -65,17 +65,10 @@ class CRM_Case_Page_AJAX { $activityParams['status_id'] = CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_status_id', 'Completed'); $activityParams['case_id'] = $caseId; $activityParams['is_auto'] = 0; - $activityParams['subject'] = 'Change Case Tags'; + $activityParams['subject'] = ts('Change Case Tags'); $activity = CRM_Activity_BAO_Activity::create($activityParams); - $caseParams = [ - 'activity_id' => $activity->id, - 'case_id' => $caseId, - ]; - - CRM_Case_BAO_Case::processCaseActivity($caseParams); - echo 'true'; CRM_Utils_System::civiExit(); } @@ -138,16 +131,9 @@ class CRM_Case_Page_AJAX { $activityParams['status_id'] = CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_status_id', 'Completed'); $activityParams['case_id'] = $caseId; $activityParams['is_auto'] = 0; - $activityParams['subject'] = 'Client Added To Case'; + $activityParams['subject'] = ts('Client Added To Case'); $activity = CRM_Activity_BAO_Activity::create($activityParams); - - $caseParams = [ - 'activity_id' => $activity->id, - 'case_id' => $caseId, - ]; - - CRM_Case_BAO_Case::processCaseActivity($caseParams); CRM_Utils_JSON::output(TRUE); } diff --git a/CRM/Case/XMLProcessor/Process.php b/CRM/Case/XMLProcessor/Process.php index 2e3463d54b..ac0592cf03 100644 --- a/CRM/Case/XMLProcessor/Process.php +++ b/CRM/Case/XMLProcessor/Process.php @@ -540,13 +540,6 @@ AND a.is_deleted = 0 if (!$activity) { throw new CRM_Core_Exception('Unable to create Activity'); } - - // create case activity record - $caseParams = [ - 'activity_id' => $activity->id, - 'case_id' => $params['caseID'], - ]; - CRM_Case_BAO_Case::processCaseActivity($caseParams); return TRUE; } diff --git a/CRM/Core/BAO/ActionSchedule.php b/CRM/Core/BAO/ActionSchedule.php index 2c49bc33b6..0785fc3907 100644 --- a/CRM/Core/BAO/ActionSchedule.php +++ b/CRM/Core/BAO/ActionSchedule.php @@ -464,17 +464,10 @@ FROM civicrm_action_schedule cas 'status_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'status_id', 'Completed'), 'activity_type_id' => $activityTypeID, 'source_record_id' => $entityID, + 'case_id' => $caseID, ]; // @todo use api, remove all the above wrangling $activity = CRM_Activity_BAO_Activity::create($activityParams); - - //file reminder on case if source activity is a case activity - if (!empty($caseID)) { - $caseActivityParams = []; - $caseActivityParams['case_id'] = $caseID; - $caseActivityParams['activity_id'] = $activity->id; - CRM_Case_BAO_Case::processCaseActivity($caseActivityParams); - } } /** diff --git a/api/v3/Activity.php b/api/v3/Activity.php index a93c443ea8..878304bac4 100644 --- a/api/v3/Activity.php +++ b/api/v3/Activity.php @@ -63,6 +63,11 @@ function civicrm_api3_activity_create($params) { // needs testing $params['skipRecentView'] = TRUE; + // In APIv3, only new activities can be filed on a case. + if (!$isNew && isset($params['case_id'])) { + unset($params['case_id']); + } + // create activity $activityBAO = CRM_Activity_BAO_Activity::create($params); diff --git a/tests/phpunit/CRM/Case/BAO/CaseTest.php b/tests/phpunit/CRM/Case/BAO/CaseTest.php index a412cb5363..c1b852c1ff 100644 --- a/tests/phpunit/CRM/Case/BAO/CaseTest.php +++ b/tests/phpunit/CRM/Case/BAO/CaseTest.php @@ -749,7 +749,7 @@ class CRM_Case_BAO_CaseTest extends CiviUnitTestCase { $case1 = $this->createCase($clientId1, $loggedInUser); $case2 = $this->createCase($clientId2, $loggedInUser); $linkActivity = $this->callAPISuccess('Activity', 'create', [ - 'case_id' => $case1->id, + 'case_id' => [$case1->id, $case2->id], 'source_contact_id' => $loggedInUser, 'target_contact' => $clientId1, 'activity_type_id' => 'Link Cases', @@ -757,12 +757,6 @@ class CRM_Case_BAO_CaseTest extends CiviUnitTestCase { 'status_id' => 'Completed', ]); - // Put it in the format needed for endPostProcess - $activity = new StdClass(); - $activity->id = $linkActivity['id']; - $params = ['link_to_case_id' => $case2->id]; - CRM_Case_Form_Activity_LinkCases::endPostProcess(NULL, $params, $activity); - // Get the option_value.value for case status Closed $closedStatusResult = $this->callAPISuccess('OptionValue', 'get', [ 'option_group_id' => 'case_status', diff --git a/tests/phpunit/api/v4/Entity/CaseTest.php b/tests/phpunit/api/v4/Entity/CaseTest.php index 636e80ff8d..27cd65f1a5 100644 --- a/tests/phpunit/api/v4/Entity/CaseTest.php +++ b/tests/phpunit/api/v4/Entity/CaseTest.php @@ -21,6 +21,7 @@ namespace api\v4\Entity; use api\v4\Api4TestBase; use Civi\Api4\Activity; +use Civi\Api4\CaseActivity; use Civi\Api4\Relationship; /** @@ -87,6 +88,80 @@ class CaseTest extends Api4TestBase { $this->assertContains($activity1['id'], $get1); $this->assertNotContains($activity2['id'], $get1); + + Activity::update(FALSE) + ->addWhere('id', '=', $activity1['id']) + ->addValue('case_id', $case2['id']) + ->execute(); + + // Both activities now belong to case 2 + $get2 = CaseActivity::get(FALSE) + ->addWhere('case_id', '=', $case2['id']) + ->execute() + ->column('activity_id'); + $this->assertContains($activity1['id'], $get2); + $this->assertContains($activity2['id'], $get2); + + // Ensure it's been moved out of case 1 + $get1 = CaseActivity::get(FALSE) + ->addWhere('case_id', '=', $case1['id']) + ->execute() + ->column('activity_id'); + $this->assertNotContains($activity1['id'], $get1); + + Activity::update(FALSE) + ->addWhere('id', '=', $activity1['id']) + ->addValue('case_id', NULL) + ->execute(); + + // Activity 1 has been removed + $get2 = CaseActivity::get(FALSE) + ->addWhere('case_id', '=', $case2['id']) + ->execute() + ->column('activity_id'); + $this->assertNotContains($activity1['id'], $get2); + $this->assertContains($activity2['id'], $get2); + } + + public function testMultipleCaseActivity(): void { + $case1 = $this->createTestRecord('Case'); + $case2 = $this->createTestRecord('Case'); + + $activity = $this->createTestRecord('Activity', [ + 'case_id' => [$case1['id'], $case2['id']], + ]); + + $get1 = CaseActivity::get(FALSE) + ->addWhere('activity_id', '=', $activity['id']) + ->execute() + ->column('case_id'); + $this->assertCount(2, $get1); + $this->assertContains($case1['id'], $get1); + $this->assertContains($case2['id'], $get1); + + // Ensure updating the activity doesn't change the case assoc + Activity::update(FALSE) + ->addValue('id', $activity['id']) + ->execute(); + + $get1 = CaseActivity::get(FALSE) + ->addWhere('activity_id', '=', $activity['id']) + ->execute() + ->column('case_id'); + $this->assertCount(2, $get1); + $this->assertContains($case1['id'], $get1); + $this->assertContains($case2['id'], $get1); + + // Delete the case assoc + Activity::update(FALSE) + ->addValue('id', $activity['id']) + ->addValue('case_id', []) + ->execute(); + + $get1 = CaseActivity::get(FALSE) + ->addWhere('activity_id', '=', $activity['id']) + ->execute(); + $this->assertCount(0, $get1); } } -- 2.25.1