core#1623: My Case dashlet doesn't sort by name but contact_id instead

[monishdeb]

Overview

My Case dashlet doesn't sort by name but contact_id instead

Steps to replicate:

1. Enable CiviCase component
2. Create multiple case for different contact
3. Go to dashboard and add 'My Cases' dashlet.
4. Try to sort by 'Contact'

Before

After

Expected behaviour : As you can sort by 'Contact' doesn't consider the name. Internally it is sorted by contact ID which is not correct, which should be by sort name.

Technical Details

The fix is in core BAO fn and its been called in various places in codebase:

Monishs-MacBook-Pro:civicrm-core monish$ grep -irn "CRM_Case_BAO_Case::getCases(" CRM/
CRM//Contact/BAO/Relationship.php:1459:              $userCases = CRM_Case_BAO_Case::getCases(FALSE);
CRM//Dashlet/Page/MyCases.php:49:    if (CRM_Case_BAO_Case::getCases(FALSE, ['type' => 'any'], $context, TRUE)) {
CRM//Dashlet/Page/AllCases.php:49:    if (CRM_Case_BAO_Case::getCases(TRUE, ['type' => 'any'], 'dashboard', TRUE)) {
CRM//Case/Page/DashBoard.php:71:    $upcoming = CRM_Case_BAO_Case::getCases($allCases, [], 'dashboard', TRUE);
CRM//Case/Page/DashBoard.php:72:    $recent = CRM_Case_BAO_Case::getCases($allCases, ['type' => 'recent'], 'dashboard', TRUE);
CRM//Case/Page/AJAX.php:188:    $cases = CRM_Case_BAO_Case::getCases($allCases, $params);
CRM//Case/Page/Tab.php:54:          $userCases = CRM_Case_BAO_Case::getCases(FALSE, ['type' => 'any']);
CRM//Case/Form/Activity.php:96:      $allCases = CRM_Case_BAO_Case::getCases(TRUE, $params);
CRM//Case/BAO/Case.php:1852:      $filterCases = CRM_Case_BAO_Case::getCases(FALSE);

Checked the impact of changing the key from contact_id to sort_name on each case and its safe.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[monishdeb]

ping @demeritcowboy @lcdservices @eileenmcnaughton

[demeritcowboy]

Thanks @monishdeb looks good. I see some unrelated errors for other columns but it's separate.

• General standards
  • [r-explain] PASS
  • [r-user] PASS
  • [r-doc] PASS
  • [r-run] PASS
    • When I click on status or type I get a datatables error but I see it on dmaster.demo and local too so is separate.
    • Checked the various places it's used.
• Developer standards
  • [r-tech] PASS
  • [r-code] PASS
  • [r-maint] PASS Nice test.
  • [r-test] PASS

[eileenmcnaughton]

@monishdeb the actual fix is fine but check the ExportTest for our new (cool) way of handling civiexit in unit tests - we aren't doing stuff like

 if ($params['is_unittest']) {
      return $casesDT;
    } 

anymore

see in CiviExit

    if (CIVICRM_UF === 'UnitTests') {
      throw new CRM_Core_Exception_PrematureExitException('civiExit called', $testParameters);
    }

[monishdeb]

Thanks @eileenmcnaughton for pointing that out. It helped me to avoid tweaking the core AJAX fn. But to still capture the output I need to bring some modification. I have assigned the output as an error data of CRM_Core_Exception_PrematureExitException so that it can be later used by UTs to compare with the expected result. Please have look now.

[eileenmcnaughton]

Do you use an IDE - phpstorm highlights the need to declare the exception here - * @throws \CRM_Core_Exception

[eileenmcnaughton]

@monishdeb I think you missed something off your commit - maybe

+++ b/CRM/Utils/JSON.php
@@ -27,7 +27,7 @@ class CRM_Utils_JSON {
   public static function output($input) {
     CRM_Utils_System::setHttpHeader('Content-Type', 'application/json');
     echo json_encode($input);
-    CRM_Utils_System::civiExit();
+    CRM_Utils_System::civiExit(0, $input);
   }

[eileenmcnaughton]

or perhaps

--- a/CRM/Utils/JSON.php
+++ b/CRM/Utils/JSON.php
@@ -25,9 +25,13 @@ class CRM_Utils_JSON {
    * @param mixed $input
    */
   public static function output($input) {
+    if (CIVICRM_UF === 'UnitTests') {
+      throw new CRM_Core_Exception_PrematureExitException('civiExit called', $input);
+    }
+

[monishdeb]

argh.. It was in my unsaved changes :(

Seems like I need to change my IDE to phpstorm from Atom

[eileenmcnaughton]

:-)

[eileenmcnaughton]

@monishdeb my first thought was the change you made but I wonder if

#16647 (comment)

is better - because i happens before messing with headers & echo

[eileenmcnaughton]

lol snap

[monishdeb]

Yup I agree, already updated :) Few more lines is always better than avoiding unnecessary process(es).

[eileenmcnaughton]

@monishdeb something still wrong with the test