From: Rich Lott / Artful Robot Date: Thu, 23 Jan 2020 11:08:30 +0000 (+0000) Subject: Add CiviCase option for showing case activities in normal views X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=3e120a63e1efb94a28408945f3c55585a3a0cfa0;p=civicrm-core.git Add CiviCase option for showing case activities in normal views Different aproach to tackling #16353 Fix array-to-string notice; Improve activity action links remove upgrader - as per Pradeep's suggestion update version added tweak way description is gotten update activity tests to check civicaseShowCaseActivities setting Fix another array-to-string conversion error Remove File On Case from activity View; add Manage Case fix case activity links when using caseShowCaseActivities remove ctx parameter for case activity links various US-Englishized Case settings texts. fix permission name --- diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php index 1f0e7a4fe8..59b9d81366 100644 --- a/CRM/Activity/BAO/Activity.php +++ b/CRM/Activity/BAO/Activity.php @@ -697,6 +697,9 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { 'case_id', 'campaign_id', ]; + // Q. What does the code below achieve? case_id and campaign_id are already + // in the array, defined above, and this code adds them in again if their + // component is enabled? @fixme remove case_id and campaign_id from the array above? foreach (['case_id' => 'CiviCase', 'campaign_id' => 'CiviCampaign'] as $attr => $component) { if (in_array($component, self::activityComponents())) { $activityParams['return'][] = $attr; @@ -780,6 +783,7 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { // Eventually this second iteration should just handle the target contacts. It's a bit muddled at // the moment as the bulk activity stuff needs unravelling & test coverage. + $caseIds = []; foreach ($result as $id => $activity) { $isBulkActivity = (!$bulkActivityTypeID || ($bulkActivityTypeID === $activity['activity_type_id'])); foreach ($mappingParams as $apiKey => $expectedName) { @@ -804,10 +808,9 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { // fetch case subject for case ID found if (!empty($activity['case_id'])) { - $activities[$id]['case_subject'] = civicrm_api3('Case', 'getvalue', [ - 'return' => 'subject', - 'id' => reset($activity['case_id']), - ]); + // Store cases; we'll look them up in one query below. We convert + // to int here so we can trust it for SQL. + $caseIds[$id] = (int) current($activity['case_id']); } } else { @@ -827,6 +830,15 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { $activities[$id]['is_recurring_activity'] = CRM_Core_BAO_RecurringEntity::getParentFor($id, 'civicrm_activity'); } + // Look up any case subjects we need in a single query and add them in the relevant activities under 'case_subject' + if ($caseIds) { + $subjects = CRM_Core_DAO::executeQuery('SELECT id, subject FROM civicrm_case WHERE id IN (' . implode(',', array_unique($caseIds)) . ')') + ->fetchMap('id', 'subject'); + foreach ($caseIds as $activityId => $caseId) { + $result[$activityId]['case_subject'] = $subjects[$caseId]; + } + } + return $activities; } @@ -2526,6 +2538,9 @@ INNER JOIN civicrm_option_group grp ON (grp.id = option_group_id AND grp.name = // Format params and add links. $contactActivities = []; + // View-only activity types + $viewOnlyCaseActivityTypeIDs = array_flip(CRM_Activity_BAO_Activity::getViewOnlyActivityTypeIDs()); + if (!empty($activities)) { $activityStatus = CRM_Core_PseudoConstant::activityStatus(); @@ -2538,6 +2553,7 @@ INNER JOIN civicrm_option_group grp ON (grp.id = option_group_id AND grp.name = } $mask = CRM_Core_Action::mask($permissions); + $userID = CRM_Core_Session::getLoggedInContactID(); foreach ($activities as $activityId => $values) { $activity = ['source_contact_name' => '', 'target_contact_name' => '']; @@ -2663,29 +2679,83 @@ INNER JOIN civicrm_option_group grp ON (grp.id = option_group_id AND grp.name = $accessMailingReport = TRUE; } - $actionLinks = CRM_Activity_Selector_Activity::actionLinks( - CRM_Utils_Array::value('activity_type_id', $values), - CRM_Utils_Array::value('source_record_id', $values), - $accessMailingReport, - CRM_Utils_Array::value('activity_id', $values) - ); + // Get action links. + + // If this is a case activity, then we hand off to Case's actionLinks instead. + if (!empty($values['case_id']) && Civi::settings()->get('civicaseShowCaseActivities')) { + // This activity belongs to a case. + $caseId = current($values['case_id']); + + $activity['subject'] = $values['subject']; + + // Get the view and edit (update) links: + $caseActionLinks = + $actionLinks = array_intersect_key( + CRM_Case_Selector_Search::actionLinks(), + array_fill_keys([CRM_Core_Action::VIEW, CRM_Core_Action::UPDATE], NULL)); + + // Create a Manage Case link (using ADVANCED as can't use two VIEW ones) + $actionLinks[CRM_Core_Action::ADVANCED] = [ + "name" => 'Manage Case', + "url" => 'civicrm/contact/view/case', + 'qs' => 'reset=1&id=%%caseid%%&cid=%%cid%%&action=view&context=&selectedChild=case', + "title" => ts('Manage Case %1', [1 => $caseId]), + 'class' => 'no-popup', + ]; + + $caseLinkValues = [ + 'aid' => $activityId, + 'caseid' => $caseId, + 'cid' => current(CRM_Case_BAO_Case::getCaseClients($caseId) ?? []), + // Unlike other 'context' params, this 'ctx' param is appended raw to the URL. + 'cxt' => '', + ]; + + $caseActivityPermissions = CRM_Core_Action::VIEW | CRM_Core_Action::ADVANCED; + // Allow Edit link if: + // 1. Activity type is NOT view-only type. CRM-5871 + // 2. User has edit permission. + if (!isset($viewOnlyCaseActivityTypeIDs[$values['activity_type_id']]) + && CRM_Case_BAO_Case::checkPermission($activityId, 'edit', $values['activity_type_id'], $userID)) { + // We're allowed to edit. + $caseActivityPermissions |= CRM_Core_Action::UPDATE; + } - $actionMask = array_sum(array_keys($actionLinks)) & $mask; - - $activity['links'] = CRM_Core_Action::formLink($actionLinks, - $actionMask, - [ - 'id' => $values['activity_id'], - 'cid' => $params['contact_id'], - 'cxt' => $context, - 'caseid' => CRM_Utils_Array::value('case_id', $values), - ], - ts('more'), - FALSE, - 'activity.tab.row', - 'Activity', - $values['activity_id'] - ); + $activity['links'] = CRM_Core_Action::formLink($actionLinks, + $caseActivityPermissions, + $caseLinkValues, + ts('more'), + FALSE, + 'activity.tab.row', + 'Activity', + $values['activity_id'] + ); + } + else { + // Non-case activity + $actionLinks = CRM_Activity_Selector_Activity::actionLinks( + CRM_Utils_Array::value('activity_type_id', $values), + CRM_Utils_Array::value('source_record_id', $values), + $accessMailingReport, + CRM_Utils_Array::value('activity_id', $values) + ); + $actionMask = array_sum(array_keys($actionLinks)) & $mask; + + $activity['links'] = CRM_Core_Action::formLink($actionLinks, + $actionMask, + [ + 'id' => $values['activity_id'], + 'cid' => $params['contact_id'], + 'cxt' => $context, + 'caseid' => NULL, + ], + ts('more'), + FALSE, + 'activity.tab.row', + 'Activity', + $values['activity_id'] + ); + } if ($values['is_recurring_activity']) { $activity['is_recurring_activity'] = CRM_Core_BAO_RecurringEntity::getPositionAndCount($values['activity_id'], 'civicrm_activity'); diff --git a/CRM/Activity/Selector/Activity.php b/CRM/Activity/Selector/Activity.php index 744ee580bd..724119a110 100644 --- a/CRM/Activity/Selector/Activity.php +++ b/CRM/Activity/Selector/Activity.php @@ -459,7 +459,7 @@ class CRM_Activity_Selector_Activity extends CRM_Core_Selector_Base implements C 'id' => $row['activity_id'], 'cid' => $this->_contactId, 'cxt' => $this->_context, - 'caseid' => CRM_Utils_Array::value('case_id', $row), + 'caseid' => isset($row['case_id']) ? current($row['case_id']) : NULL, ], ts('more'), FALSE, diff --git a/CRM/Admin/Form/Setting/Case.php b/CRM/Admin/Form/Setting/Case.php index 3833160921..58331a8b2f 100644 --- a/CRM/Admin/Form/Setting/Case.php +++ b/CRM/Admin/Form/Setting/Case.php @@ -25,6 +25,7 @@ class CRM_Admin_Form_Setting_Case extends CRM_Admin_Form_Setting { 'civicaseAllowMultipleClients' => CRM_Core_BAO_Setting::SYSTEM_PREFERENCES_NAME, 'civicaseNaturalActivityTypeSort' => CRM_Core_BAO_Setting::SYSTEM_PREFERENCES_NAME, 'civicaseActivityRevisions' => CRM_Core_BAO_Setting::SYSTEM_PREFERENCES_NAME, + 'civicaseShowCaseActivities' => CRM_Core_BAO_Setting::SYSTEM_PREFERENCES_NAME, ]; /** diff --git a/CRM/Case/BAO/Case.php b/CRM/Case/BAO/Case.php index f481c016f0..f24066c6fb 100644 --- a/CRM/Case/BAO/Case.php +++ b/CRM/Case/BAO/Case.php @@ -15,6 +15,7 @@ * @copyright CiviCRM LLC https://civicrm.org/licensing */ + /** * This class contains the functions for Case Management. */ @@ -284,7 +285,7 @@ WHERE civicrm_case.id = %1"; * * @param int $activityId * - * @return int, case ID + * @return int|null, case ID */ public static function getCaseIdByActivityId($activityId) { $originalId = CRM_Core_DAO::singleValueQuery( diff --git a/CRM/Case/Info.php b/CRM/Case/Info.php index c25af90a00..0f996ee0c8 100644 --- a/CRM/Case/Info.php +++ b/CRM/Case/Info.php @@ -36,7 +36,7 @@ class CRM_Case_Info extends CRM_Core_Component_Info { 'translatedName' => ts('CiviCase'), 'title' => ts('CiviCase Engine'), 'search' => 1, - 'showActivitiesInCore' => 0, + 'showActivitiesInCore' => Civi::settings()->get('civicaseShowCaseActivities') ?? 0, ]; } diff --git a/settings/Case.setting.php b/settings/Case.setting.php index 23c793bdd3..44dc605569 100644 --- a/settings/Case.setting.php +++ b/settings/Case.setting.php @@ -99,4 +99,19 @@ return [ 'description' => ts('Enable tracking of activity revisions embedded within the "civicrm_activity" table. Alternatively, see "Administer => System Settings => Misc => Logging".'), 'help_text' => '', ], + 'civicaseShowCaseActivities' => [ + 'group_name' => 'CiviCRM Preferences', + 'group' => 'core', + 'name' => 'civicaseShowCaseActivities', + 'type' => 'Boolean', + 'quick_form_type' => 'YesNo', + 'default' => FALSE, + 'html_type' => 'radio', + 'add' => '5.24', + 'title' => ts('Include case activities in general activity views.'), + 'is_domain' => 1, + 'is_contact' => 0, + 'description' => ts('e.g. the Contact form\'s Activity tab listing. Without this ticked, activities that belong to a case are hidden (default behavior). Warning: enabling this option means that all case activities relating to a contact will be listed which could result in users without "access all cases and activities" permission being able to see see the summarized details (date, subject, assignees, status etc.). Such users will still be prevented from managing the case and viewing/editing the activity.'), + 'help_text' => '', + ], ]; diff --git a/templates/CRM/Admin/Form/Setting/Case.tpl b/templates/CRM/Admin/Form/Setting/Case.tpl index 5df543574f..825826c6aa 100644 --- a/templates/CRM/Admin/Form/Setting/Case.tpl +++ b/templates/CRM/Admin/Form/Setting/Case.tpl @@ -37,6 +37,12 @@ {ts}Enable embedded tracking to activity revisions within the "civicrm_activity" table. Alternatively, see "Administer => System Settings => Misc => Logging".{/ts} + + {$form.civicaseShowCaseActivities.label} + {$form.civicaseShowCaseActivities.html}
+ {$civicaseShowCaseActivities_description} + +
{include file="CRM/common/formButtons.tpl" location="bottom"}
diff --git a/tests/phpunit/CRM/Activity/BAO/ActivityTest.php b/tests/phpunit/CRM/Activity/BAO/ActivityTest.php index 890a2854a8..236be3c440 100644 --- a/tests/phpunit/CRM/Activity/BAO/ActivityTest.php +++ b/tests/phpunit/CRM/Activity/BAO/ActivityTest.php @@ -331,18 +331,36 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { /** * Test getActivities BAO method for getting count. + * */ public function testGetActivitiesCountForAdminDashboard() { + // Reset to default + $this->setShowCaseActivitiesInCore(FALSE); $this->setUpForActivityDashboardTests(); + $this->addCaseWithActivity(); + CRM_Core_Config::singleton()->userPermissionClass->permissions[] = 'access all cases and activities'; + $activityCount = CRM_Activity_BAO_Activity::getActivitiesCount($this->_params); $this->assertEquals(8, $activityCount); + + // If we're showing case activities, we exepct to see one more (the scheduled meeting)... + $this->setShowCaseActivitiesInCore(TRUE); + $activityCount = CRM_Activity_BAO_Activity::getActivitiesCount($this->_params); + $this->assertEquals(9, $activityCount); + // Reset to default + $this->setShowCaseActivitiesInCore(FALSE); } /** * Test getActivities BAO method for getting count + * */ public function testGetActivitiesCountforNonAdminDashboard() { + // Reset to default + $this->setShowCaseActivitiesInCore(FALSE); $this->createTestActivities(); + $this->addCaseWithActivity(); + CRM_Core_Config::singleton()->userPermissionClass->permissions[] = 'access all cases and activities'; $params = [ 'contact_id' => 9, @@ -358,15 +376,27 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { ]; //since we are loading activities from dataset, we know total number of activities for this contact - // 5 activities ( 2 scheduled, 3 Completed ), note that dashboard shows only scheduled activities + // 5 activities ( 2 scheduled, 3 Completed, 1 Scheduled Case activity ), + // note that dashboard shows only scheduled activities $this->assertEquals(2, CRM_Activity_BAO_Activity::getActivitiesCount($params)); + + // If we're showing case activities, we exepct to see one more (the scheduled meeting)... + $this->setShowCaseActivitiesInCore(TRUE); + $this->assertEquals(3, CRM_Activity_BAO_Activity::getActivitiesCount($params)); + // Reset to default + $this->setShowCaseActivitiesInCore(FALSE); } /** * Test getActivities BAO method for getting count + * */ public function testGetActivitiesCountforContactSummary() { + // Reset to default + $this->setShowCaseActivitiesInCore(FALSE); $this->createTestActivities(); + $this->addCaseWithActivity(); + CRM_Core_Config::singleton()->userPermissionClass->permissions[] = 'access all cases and activities'; $params = [ 'contact_id' => 9, @@ -380,8 +410,14 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { ]; //since we are loading activities from dataset, we know total number of activities for this contact - // 5 activities, Contact Summary should show all activities + // 5 activities $this->assertEquals(5, CRM_Activity_BAO_Activity::getActivitiesCount($params)); + + // If we're showing case activities, we exepct to see one more (the scheduled meeting)... + $this->setShowCaseActivitiesInCore(TRUE); + $this->assertEquals(6, CRM_Activity_BAO_Activity::getActivitiesCount($params)); + // Reset to default + $this->setShowCaseActivitiesInCore(FALSE); } /** @@ -447,7 +483,11 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { * Test getActivities BAO method. */ public function testGetActivitiesForAdminDashboard() { + $this->setShowCaseActivitiesInCore(FALSE); $this->setUpForActivityDashboardTests(); + $this->addCaseWithActivity(); + CRM_Core_Config::singleton()->userPermissionClass->permissions[] = 'access all cases and activities'; + $activitiesNew = CRM_Activity_BAO_Activity::getActivities($this->_params); // $this->assertEquals($activities, $activitiesDeprecatedFn); @@ -463,6 +503,16 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { $this->assertEquals($value['status_id'], 1, 'Verify all activities are scheduled.'); } } + + // Now check that we get the scheduled meeting, if civicaseShowCaseActivities is set. + $this->setShowCaseActivitiesInCore(TRUE); + $activitiesNew = CRM_Activity_BAO_Activity::getActivities($this->_params); + $this->assertEquals(9, count($activitiesNew)); + // Scan through to find the meeting. + $this->assertTrue(in_array('test meeting activity', array_column($activitiesNew, 'subject')), + "failed to find scheduled case Meeting activity"); + // Reset to default + $this->setShowCaseActivitiesInCore(FALSE); } /** @@ -492,7 +542,10 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { * Test getActivities BAO method. */ public function testGetActivitiesforNonAdminDashboard() { + $this->setShowCaseActivitiesInCore(FALSE); $this->createTestActivities(); + $this->addCaseWithActivity(); + CRM_Core_Config::singleton()->userPermissionClass->permissions[] = 'access all cases and activities'; $contactID = 9; $params = [ @@ -527,6 +580,17 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { } } } + + // Now check that we get the scheduled meeting, if civicaseShowCaseActivities is set. + $this->setShowCaseActivitiesInCore(TRUE); + $activities = CRM_Activity_BAO_Activity::getActivities($params); + $this->assertEquals(3, count($activities)); + // Scan through to find the meeting. + $this->assertTrue(in_array('test meeting activity', array_column($activities, 'subject')), + "failed to find scheduled case Meeting activity"); + + // Reset to default + $this->setShowCaseActivitiesInCore(FALSE); } /** @@ -586,7 +650,11 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { * Test getActivities BAO method. */ public function testGetActivitiesForContactSummary() { + // Reset to default + $this->setShowCaseActivitiesInCore(FALSE); $this->createTestActivities(); + $this->addCaseWithActivity(); + CRM_Core_Config::singleton()->userPermissionClass->permissions[] = 'access all cases and activities'; $contactID = 9; $params = [ @@ -635,12 +703,24 @@ class CRM_Activity_BAO_ActivityTest extends CiviUnitTestCase { $this->assertArrayHasKey($contactID, $value['assignee_contact_name']); } } + + // Now check that we get the scheduled meeting, if civicaseShowCaseActivities is set. + $this->setShowCaseActivitiesInCore(TRUE); + $activities = CRM_Activity_BAO_Activity::getActivities($params); + $this->assertEquals(6, count($activities)); + // Scan through to find the meeting. + $this->assertTrue(in_array('test meeting activity', array_column($activities, 'subject')), + "failed to find scheduled case Meeting activity"); + // Reset to default + $this->setShowCaseActivitiesInCore(FALSE); } /** * Test getActivities BAO method. */ public function testGetActivitiesforContactSummaryWithActivities() { + // Reset to default + $this->setShowCaseActivitiesInCore(FALSE); $this->createTestActivities(); // parameters for different test cases, check each array key for the specific test-case @@ -1417,4 +1497,95 @@ $text $mut->stop(); } + /** + * Adds a case with one activity. + * + */ + protected function addCaseWithActivity() { + // case is not enabled by default do that now. + $enableResult = CRM_Core_BAO_ConfigSetting::enableComponent('CiviCase'); + $this->assertTrue($enableResult, 'Cannot enable CiviCase in line ' . __LINE__); + + // We need a minimal case setup. + $case_type_id = civicrm_api3('CaseType', 'get', ['return' => 'id', 'name' => 'test_case_type'])['id'] ?? NULL; + if (!$case_type_id) { + $params = [ + 'name' => 'test_case_type', + 'title' => 'test_case_type', + "is_active" => "1", + "definition" => [ + "activityTypes" => [ + ["name" => "Open Case", "max_instances" => "1"], + ["name" => "Meeting"], + ], + "activitySets" => [ + [ + "name" => "standard_timeline", + "label" => "Standard Timeline", + "timeline" => "1", + "activityTypes" => [ + [ + "name" => "Open Case", + "status" => "Completed", + "label" => "Open Case", + "default_assignee_type" => "1", + ], + ], + ], + ], + "timelineActivityTypes" => [ + [ + "name" => "Open Case", + "status" => "Completed", + "label" => "Open Case", + "default_assignee_type" => "1", + ], + ], + "caseRoles" => [ + [ + "name" => "Case Coordinator", + "creator" => "1", + "manager" => "1", + ], + ], + ], + ]; + $case_type_id = $this->callAPISuccess('CaseType', 'create', $params)['id']; + } + + // Create a case with Contact #3 as the client. + $case_id = civicrm_api3('case', 'get', ['subject' => 'test case 1'])['id'] ?? NULL; + if (!$case_id) { + // Create case + $params = [ + 'subject' => 'test case 1', + 'contact_id' => 3, + 'status_id' => 'Open', + 'case_type_id' => $case_type_id, + 'creator_id' => 3, + ]; + $case_id = $this->callAPISuccess('case', 'create', $params)['id']; + } + + // Create a scheduled 'Meeting' activity that belongs to this case, but is + // assigned to contact #9 + $activity_id = $this->callAPISuccess('Activity', 'create', [ + 'activity_type_id' => 'Meeting', + 'status_id' => 'Scheduled', + 'case_id' => $case_id, + 'source_contact_id' => 3, + 'assignee_id' => [9], + 'subject' => 'test meeting activity', + ])['id'] ?? NULL; + } + + /** + * Change setting, and the cache of it. + */ + protected function setShowCaseActivitiesInCore(bool $val) { + Civi::settings()->set('civicaseShowCaseActivities', $val ? 1 : 0); + CRM_Core_Component::getEnabledComponents(); + Civi::$statics['CRM_Core_Component']['info']['CiviCase']->info['showActivitiesInCore'] = $val; + } + }