Merge pull request #19741 from demeritcowboy/case-custom-nochange

#2019 - Don't create an empty activity if nothing changed when changing case custom data

CRM/Case/Form/CustomData.php

@@ -112,24 +112,27 @@ class CRM_Case_Form_CustomData extends CRM_Core_Form {
     $session = CRM_Core_Session::singleton();
     $session->pushUserContext(CRM_Utils_System::url('civicrm/contact/view/case', "reset=1&id={$this->_entityID}&cid={$this->_contactID}&action=view"));
 
-    $activityTypeID = CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Change Custom Data');
-    $activityParams = [
-      'activity_type_id' => $activityTypeID,
-      'source_contact_id' => $session->get('userID'),
-      'is_auto' => TRUE,
-      'subject' => $this->_customTitle . " : change data",
-      'status_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_status_id', 'Completed'),
-      'target_contact_id' => $this->_contactID,
-      'details' => $this->formatCustomDataChangesForDetail($params),
-      'activity_date_time' => date('YmdHis'),
-    ];
-    $activity = CRM_Activity_BAO_Activity::create($activityParams);
-
-    $caseParams = [
-      'activity_id' => $activity->id,
-      'case_id' => $this->_entityID,
-    ];
-    CRM_Case_BAO_Case::processCaseActivity($caseParams);
+    $formattedDetails = $this->formatCustomDataChangesForDetail($params);
+    if (!empty($formattedDetails)) {
+      $activityTypeID = CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Change Custom Data');
+      $activityParams = [
+        'activity_type_id' => $activityTypeID,
+        'source_contact_id' => $session->get('userID'),
+        'is_auto' => TRUE,
+        'subject' => $this->_customTitle . " : change data",
+        'status_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_status_id', 'Completed'),
+        'target_contact_id' => $this->_contactID,
+        'details' => $formattedDetails,
+        'activity_date_time' => date('YmdHis'),
+      ];
+      $activity = CRM_Activity_BAO_Activity::create($activityParams);
+
+      $caseParams = [
+        'activity_id' => $activity->id,
+        'case_id' => $this->_entityID,
+      ];
+      CRM_Case_BAO_Case::processCaseActivity($caseParams);
+    }
 
     $transaction->commit();
   }

tests/phpunit/CRM/Case/Form/CustomDataTest.php

@@ -330,4 +330,66 @@ class CRM_Case_Form_CustomDataTest extends CiviCaseTestCase {
     return strtr($input, $conversion_table);
   }
 
+  /**
+   * Test that when custom case data is edited but not changed that it doesn't
+   * create a meaningless empty activity.
+   */
+  public function testCustomDataNoChangeNoActivity() {
+    // Create a custom group and field
+    $customField = $this->callAPISuccess('custom_field', 'create', [
+      'custom_group_id' => $this->custom_group['id'],
+      'label' => 'What?',
+      'data_type' => 'String',
+      'html_type' => 'Text',
+      'is_active' => 1,
+    ]);
+    $customField = $customField['values'][$customField['id']];
+
+    // Create a case and set the custom field to something
+    $individual = $this->individualCreate();
+    $caseObj = $this->createCase($individual, $this->_loggedInUser);
+    $caseId = $caseObj->id;
+    $this->callAPISuccess('CustomValue', 'create', [
+      "custom_{$customField['id']}" => 'immutable text',
+      'entity_id' => $caseId,
+    ]);
+
+    // run the form
+    $form = new CRM_Case_Form_CustomData();
+    $form->controller = new CRM_Core_Controller_Simple('CRM_Case_Form_CustomData', 'Case Data');
+    $form->set('groupID', $this->custom_group['id']);
+    $form->set('entityID', $caseId);
+    // this is case type
+    $form->set('subType', 1);
+    $form->set('cid', $individual);
+    ob_start();
+    $form->controller->_actions['display']->perform($form, 'display');
+    ob_end_clean();
+
+    // Don't change any field values and now call postProcess.
+    // Because the form does a redirect it triggers an exception during unit
+    // tests but that's ok.
+    $hadException = FALSE;
+    try {
+      $form->controller->_actions['upload']->perform($form, 'upload');
+    }
+    catch (CRM_Core_Exception_PrematureExitException $e) {
+      $hadException = TRUE;
+      $this->assertEquals('redirect', $e->errorData['context']);
+      // compare parts of query string separately to avoid any clean url format mismatches
+      $this->assertStringContainsString('civicrm/contact/view/case', $e->errorData['url']);
+      $this->assertStringContainsString("reset=1&id={$caseId}&cid={$individual}&action=view", $e->errorData['url']);
+    }
+    // Make sure we did have an exception since otherwise we might get a
+    // false pass for the asserts above since they'd never run.
+    $this->assertTrue($hadException);
+
+    // there shouldn't be an activity of type Change Custom Data on the case
+    $result = $this->callAPISuccess('Activity', 'get', [
+      'activity_type_id' => 'Change Custom Data',
+      'case_id' => $caseId,
+    ]);
+    $this->assertEquals(0, $result['count']);
+  }
+
 }