From d0b581a88b12ff3c4a03057e0ec5c341217a970d Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Sat, 13 Aug 2022 18:51:25 -0400 Subject: [PATCH] CiviCase - Remove the civicaseActivityRevisions setting and related code This setting has been deprecated for years, and core CiviCase doesn't respect it anyway. Only the API was really paying attention to it. Time to get rid of that stuff as a preamble to ripping out all of the case activity revisioning. --- CRM/Activity/DAO/Activity.php | 18 +-- CRM/Admin/Form/Setting/Case.php | 1 - api/v3/Activity.php | 63 +--------- api/v3/examples/Setting/GetFields.ex.php | 15 --- settings/Case.setting.php | 15 --- tests/phpunit/api/v3/CaseTest.php | 140 +---------------------- xml/schema/Activity/Activity.xml | 11 +- 7 files changed, 20 insertions(+), 243 deletions(-) diff --git a/CRM/Activity/DAO/Activity.php b/CRM/Activity/DAO/Activity.php index 006deffe81..8d6164047e 100644 --- a/CRM/Activity/DAO/Activity.php +++ b/CRM/Activity/DAO/Activity.php @@ -6,7 +6,7 @@ * * Generated from xml/schema/CRM/Activity/Activity.xml * DO NOT EDIT. Generated by CRM_Core_CodeGen - * (GenCodeChecksum:96205b68fd8160c65b7f66a11c169078) + * (GenCodeChecksum:016e3be7b0f4a706b96d0c1e8523b25f) */ /** @@ -206,18 +206,22 @@ class CRM_Activity_DAO_Activity extends CRM_Core_DAO { public $relationship_id; /** + * Unused deprecated column. + * * @var bool|string * (SQL type: tinyint) * Note that values will be retrieved from the database as a string. + * @deprecated */ public $is_current_revision; /** - * Activity ID of the first activity record in versioning chain. + * Unused deprecated column. * * @var int|string|null * (SQL type: int unsigned) * Note that values will be retrieved from the database as a string. + * @deprecated */ public $original_id; @@ -663,12 +667,10 @@ class CRM_Activity_DAO_Activity extends CRM_Core_DAO { 'is_current_revision' => [ 'name' => 'is_current_revision', 'type' => CRM_Utils_Type::T_BOOLEAN, - 'title' => ts('Is this activity a current revision in versioning chain?'), + 'title' => ts('Is current (unused)'), + 'description' => ts('Unused deprecated column.'), 'required' => TRUE, - 'import' => TRUE, 'where' => 'civicrm_activity.is_current_revision', - 'headerPattern' => '/(is.)?(current.)?(revision|version(ing)?)/i', - 'export' => TRUE, 'default' => '1', 'table_name' => 'civicrm_activity', 'entity' => 'Activity', @@ -679,8 +681,8 @@ class CRM_Activity_DAO_Activity extends CRM_Core_DAO { 'original_id' => [ 'name' => 'original_id', 'type' => CRM_Utils_Type::T_INT, - 'title' => ts('Original Activity ID'), - 'description' => ts('Activity ID of the first activity record in versioning chain.'), + 'title' => ts('Original ID (unused)'), + 'description' => ts('Unused deprecated column.'), 'where' => 'civicrm_activity.original_id', 'table_name' => 'civicrm_activity', 'entity' => 'Activity', diff --git a/CRM/Admin/Form/Setting/Case.php b/CRM/Admin/Form/Setting/Case.php index 6ab3a59dd8..c952ac24d8 100644 --- a/CRM/Admin/Form/Setting/Case.php +++ b/CRM/Admin/Form/Setting/Case.php @@ -24,7 +24,6 @@ class CRM_Admin_Form_Setting_Case extends CRM_Admin_Form_Setting { 'civicaseRedactActivityEmail' => CRM_Core_BAO_Setting::SYSTEM_PREFERENCES_NAME, '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/api/v3/Activity.php b/api/v3/Activity.php index 275105a503..9d0a86bce4 100644 --- a/api/v3/Activity.php +++ b/api/v3/Activity.php @@ -63,71 +63,14 @@ function civicrm_api3_activity_create($params) { // needs testing $params['skipRecentView'] = TRUE; - // If this is a case activity, see if there is an existing activity - // and set it as an old revision. Also retrieve details we'll need. - // this handling should all be moved to the BAO layer - $case_id = ''; - $createRevision = FALSE; - $oldActivityValues = []; - // Lookup case id if not supplied - if (!isset($params['case_id']) && !empty($params['id'])) { - $params['case_id'] = CRM_Core_DAO::singleValueQuery("SELECT case_id FROM civicrm_case_activity WHERE activity_id = " . (int) $params['id']); - } - if (!empty($params['case_id'])) { - $case_id = $params['case_id']; - if (!empty($params['id']) && Civi::settings()->get('civicaseActivityRevisions')) { - $oldActivityParams = ['id' => $params['id']]; - if (!$oldActivityValues) { - CRM_Activity_BAO_Activity::retrieve($oldActivityParams, $oldActivityValues); - } - if (empty($oldActivityValues)) { - throw new API_Exception(ts("Unable to locate existing activity.")); - } - else { - $activityDAO = new CRM_Activity_DAO_Activity(); - $activityDAO->id = $params['id']; - $activityDAO->is_current_revision = 0; - if (!$activityDAO->save()) { - throw new API_Exception(ts("Unable to revision existing case activity.")); - } - $createRevision = TRUE; - } - } - } - - if ($case_id && $createRevision) { - // This is very similar to the copy-to-case action. - if (!CRM_Utils_Array::crmIsEmptyArray($oldActivityValues['target_contact'])) { - $oldActivityValues['targetContactIds'] = implode(',', array_unique($oldActivityValues['target_contact'])); - } - if (!CRM_Utils_Array::crmIsEmptyArray($oldActivityValues['assignee_contact'])) { - $oldActivityValues['assigneeContactIds'] = implode(',', array_unique($oldActivityValues['assignee_contact'])); - } - $oldActivityValues['mode'] = 'copy'; - $oldActivityValues['caseID'] = $case_id; - $oldActivityValues['activityID'] = $oldActivityValues['id']; - $oldActivityValues['contactID'] = $oldActivityValues['source_contact_id']; - - $copyToCase = CRM_Activity_Page_AJAX::_convertToCaseActivity($oldActivityValues); - if (empty($copyToCase['error_msg'])) { - // now fix some things that are different from copy-to-case - // then fall through to the create below to update with the passed in params - $params['id'] = $copyToCase['newId']; - $params['is_auto'] = 0; - $params['original_id'] = empty($oldActivityValues['original_id']) ? $oldActivityValues['id'] : $oldActivityValues['original_id']; - } - else { - throw new API_Exception(ts("Unable to create new revision of case activity.")); - } - } - // create activity $activityBAO = CRM_Activity_BAO_Activity::create($params); if (isset($activityBAO->id)) { - if ($case_id && $isNew && !$createRevision) { + // Fixme - Move business logic out of API + if (!empty($params['case_id']) && $isNew) { // If this is a brand new case activity, add to case(s) - foreach ((array) $case_id as $singleCaseId) { + foreach ((array) $params['case_id'] as $singleCaseId) { $caseActivityParams = ['activity_id' => $activityBAO->id, 'case_id' => $singleCaseId]; CRM_Case_BAO_Case::processCaseActivity($caseActivityParams); } diff --git a/api/v3/examples/Setting/GetFields.ex.php b/api/v3/examples/Setting/GetFields.ex.php index e48cac75aa..2765ad7ac7 100644 --- a/api/v3/examples/Setting/GetFields.ex.php +++ b/api/v3/examples/Setting/GetFields.ex.php @@ -199,21 +199,6 @@ function setting_getfields_expectedresult() { 'description' => 'How to sort activity-types on the \"Manage Case\" screen? (Set \"Default\" to load setting from the legacy \"Settings.xml\" file.)', 'help_text' => '', ], - 'civicaseActivityRevisions' => [ - 'group_name' => 'CiviCRM Preferences', - 'group' => 'core', - 'name' => 'civicaseActivityRevisions', - 'type' => 'Boolean', - 'quick_form_type' => 'YesNo', - 'default' => '', - 'html_type' => 'radio', - 'add' => '4.7', - 'title' => 'Enable deprecated Embedded Activity Revisions', - 'is_domain' => 1, - 'is_contact' => 0, - 'description' => 'Enable tracking of activity revisions embedded within the \"civicrm_activity\" table. This should not be enabled on new installs and will be unsupported in the future. You should enable \"Administer => System Settings => Misc => Logging\" instead.', - 'help_text' => '', - ], 'civicaseShowCaseActivities' => [ 'group_name' => 'CiviCRM Preferences', 'group' => 'core', diff --git a/settings/Case.setting.php b/settings/Case.setting.php index 0b19250e61..692a00ec7f 100644 --- a/settings/Case.setting.php +++ b/settings/Case.setting.php @@ -82,21 +82,6 @@ return [ 'description' => ts('How to sort activity-types on the "Manage Case" screen? (Set "Default" to load setting from the legacy "Settings.xml" file.)'), 'help_text' => '', ], - 'civicaseActivityRevisions' => [ - 'group_name' => 'CiviCRM Preferences', - 'group' => 'core', - 'name' => 'civicaseActivityRevisions', - 'type' => 'Boolean', - 'quick_form_type' => 'YesNo', - 'default' => FALSE, - 'html_type' => 'radio', - 'add' => '4.7', - 'title' => ts('Enable deprecated Embedded Activity Revisions'), - 'is_domain' => 1, - 'is_contact' => 0, - 'description' => ts('Enable tracking of activity revisions embedded within the "civicrm_activity" table. This should not be enabled on new installs and will be unsupported in the future. You should enable "Administer => System Settings => Misc => Logging" instead.'), - 'help_text' => '', - ], 'civicaseShowCaseActivities' => [ 'group_name' => 'CiviCRM Preferences', 'group' => 'core', diff --git a/tests/phpunit/api/v3/CaseTest.php b/tests/phpunit/api/v3/CaseTest.php index a172c07bed..0af14d90f3 100644 --- a/tests/phpunit/api/v3/CaseTest.php +++ b/tests/phpunit/api/v3/CaseTest.php @@ -495,126 +495,6 @@ class api_v3_CaseTest extends CiviCaseTestCase { $this->assertContains($case['id'], $result['case_id']); } - /** - * Test activity api update for case activities. - */ - public function testCaseActivityUpdate_Tracked() { - $this->settingsStack->push('civicaseActivityRevisions', TRUE); - - // Need to create the case and activity before we can update it - $this->testCaseActivityCreate(); - - $params = [ - 'activity_id' => $this->_caseActivityId, - 'case_id' => 1, - 'activity_type_id' => 14, - 'source_contact_id' => $this->_loggedInUser, - 'subject' => 'New subject', - ]; - $result = $this->callAPISuccess('activity', 'create', $params); - - $this->assertEquals($result['values'][$result['id']]['subject'], $params['subject']); - - // id should be one greater, since this is a new revision - $this->assertEquals($result['values'][$result['id']]['id'], $this->_caseActivityId + 1); - $this->assertEquals($result['values'][$result['id']]['original_id'], $this->_caseActivityId); - - // Check revision is as expected - $revParams = [ - 'activity_id' => $this->_caseActivityId, - ]; - $revActivity = $this->callAPISuccess('activity', 'get', $revParams); - $this->assertEquals($revActivity['values'][$this->_caseActivityId]['is_current_revision'], - 0); - $this->assertEquals($revActivity['values'][$this->_caseActivityId]['is_deleted'], - 0 - ); - } - - /** - * If you disable `civicaseActivityRevisions`, then editing an activity - * will *not* create or change IDs. - */ - public function testCaseActivityUpdate_Untracked() { - $this->settingsStack->push('civicaseActivityRevisions', FALSE); - - // Need to create the case and activity before we can update it - $this->testCaseActivityCreate(); - - $oldIDs = CRM_Utils_SQL_Select::from('civicrm_activity') - ->select('id, original_id, is_current_revision') - ->orderBy('id') - ->execute()->fetchAll(); - - $params = [ - 'activity_id' => $this->_caseActivityId, - 'case_id' => 1, - 'activity_type_id' => 14, - 'source_contact_id' => $this->_loggedInUser, - 'subject' => 'New subject', - ]; - $result = $this->callAPISuccess('activity', 'create', $params); - $this->assertEquals($result['values'][$result['id']]['subject'], $params['subject']); - - // id should not change because we've opted out. - $this->assertEquals($this->_caseActivityId, $result['values'][$result['id']]['id']); - $this->assertEmpty($result['values'][$result['id']]['original_id']); - - $newIDs = CRM_Utils_SQL_Select::from('civicrm_activity') - ->select('id, original_id, is_current_revision') - ->orderBy('id') - ->execute()->fetchAll(); - $this->assertEquals($oldIDs, $newIDs); - } - - public function testCaseActivityUpdateCustom() { - $this->settingsStack->push('civicaseActivityRevisions', TRUE); - - // Create a case first - $result = $this->callAPISuccess('case', 'create', $this->_params); - - // Create custom field group - // Note the second parameter is Activity on purpose, not Case. - $custom_ids = $this->entityCustomGroupWithSingleFieldCreate(__FUNCTION__, 'ActivityTest.php'); - - // create activity - $params = [ - 'case_id' => $result['id'], - // follow up - 'activity_type_id' => 14, - 'subject' => 'Test followup', - 'source_contact_id' => $this->_loggedInUser, - 'target_contact_id' => $this->_params['contact_id'], - 'custom_' . $custom_ids['custom_field_id'] => "custom string", - ]; - $result = $this->callAPISuccess('activity', 'create', $params); - - $aid = $result['values'][$result['id']]['id']; - - // Update activity - $params = [ - 'activity_id' => $aid, - 'case_id' => 1, - 'activity_type_id' => 14, - 'source_contact_id' => $this->_loggedInUser, - 'subject' => 'New subject', - ]; - $this->callAPISuccess('activity', 'create', $params); - - // Retrieve revision and check custom fields got copied. - $revParams = [ - 'activity_id' => $aid + 1, - 'return.custom_' . $custom_ids['custom_field_id'] => 1, - ]; - $revAct = $this->callAPISuccess('activity', 'get', $revParams); - - $this->assertEquals($revAct['values'][$aid + 1]['custom_' . $custom_ids['custom_field_id']], "custom string", - "Error message: " . CRM_Utils_Array::value('error_message', $revAct)); - - $this->customFieldDelete($custom_ids['custom_field_id']); - $this->customGroupDelete($custom_ids['custom_group_id']); - } - public function testCaseGetByStatus() { // Create 2 cases with different status ids. $case1 = $this->callAPISuccess('Case', 'create', [ @@ -933,15 +813,9 @@ class api_v3_CaseTest extends CiviCaseTestCase { * * See the case.addtimeline api. * - * @param bool $enableRevisions - * - * @dataProvider caseActivityRevisionExamples - * * @throws \Exception */ - public function testCaseAddtimeline($enableRevisions) { - $this->settingsStack->push('civicaseActivityRevisions', $enableRevisions); - + public function testCaseAddtimeline() { $caseSpec = [ 'title' => 'Application with Definition', 'name' => 'Application_with_Definition', @@ -1025,18 +899,6 @@ class api_v3_CaseTest extends CiviCaseTestCase { $this->assertEquals(1, $result['is_deleted']); } - /** - * Get case activity revision sample data. - * - * @return array - */ - public function caseActivityRevisionExamples() { - $examples = []; - $examples[] = [FALSE]; - $examples[] = [TRUE]; - return $examples; - } - public function testTimestamps() { $params = $this->_params; $case_created = $this->callAPISuccess('case', 'create', $params); diff --git a/xml/schema/Activity/Activity.xml b/xml/schema/Activity/Activity.xml index c6f33a9846..09a55c4514 100644 --- a/xml/schema/Activity/Activity.xml +++ b/xml/schema/Activity/Activity.xml @@ -293,12 +293,12 @@ is_current_revision - Is this activity a current revision in versioning chain? + Is current (unused) boolean 1 + Unused deprecated column. true - true - /(is.)?(current.)?(revision|version(ing)?)/i + true 2.2 @@ -309,8 +309,9 @@ original_id int unsigned - Original Activity ID - Activity ID of the first activity record in versioning chain. + Original ID (unused) + Unused deprecated column. + true true -- 2.25.1