Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Case Manager is not shown for closed cases (Gitlab issue 542) #13831

Closed
wants to merge 4 commits into from

Conversation

kainuk
Copy link
Contributor

@kainuk kainuk commented Mar 15, 2019

Overview

When a case is closed the case manager is not shown anymore in the case view. So when a returning client arrives at the desk, and his old cases are looked into, the name of the case manager is not shown. So she (the manager of the old case) cannot be contacted for context.

Before

before

After

after

Some User Story Background

When a case is closed all the roles (including the case manager) get the enddate of the case. This is done in CRM_Case_Form_Activity_ChangeCaseStatus. It makes sense, a closed case means, no more actions, and the relationship comes in some also to an end.

However, the relationship is still active (means the field is_active=1). That makes also makes sense. The case still shows up in your case dashboard, so you can browse in your historical cases. It even shows the future planned activities for the case. These should not be there and present a choice. The user must cancel the activity or reopen the case.

Technical Details

However, the code CRM_Case_BAO_Case::getCaseRoles that retrieves the active roles, also selects on end_date. But I think and end_date in the past does not mean an inactive relationship

if ($activeOnly) {
      $query .= ' AND rel.is_active = 1 AND (rel.end_date IS NULL OR rel.end_date > NOW())';
    }

Comments

This issue is reported on gitlab at https://lab.civicrm.org/dev/core/issues/542
This issue has another PR #13510 . This PR gives the user the choice to see al the inactive relationships with a checkbox. This complicates the screen and also shows former (in_active) managers, that contact much less relevant information. If the user is interested in the former case roles he can look into the Assign Case Role activities.

@civibot
Copy link

civibot bot commented Mar 15, 2019

(Standard links)

@civibot civibot bot added the master label Mar 15, 2019
@eileenmcnaughton
Copy link
Contributor

the test failure is related

api_v3_CaseTest::testCaseCreateWithResolvedStatus
Checking for empty array
Failed asserting that 0 matches expected 1.

/home/jenkins/bknix-dfl/build/core-13831-7qzii/sites/all/modules/civicrm/tests/phpunit/api/v3/CaseTest.php:187
/home/jenkins/bknix-dfl/build/core-13831-7qzii/sites/all/modules/civicrm/tests/phpunit/CiviTest/CiviUnitTestCase.php:196
/home/jenkins/bknix-dfl/civicrm-buildkit/extern/phpunit5/phpunit5.phar:598

@jitendrapurohit can you review this in conjunction with #13510

kainuk added 2 commits March 17, 2019 14:30
…is OK, but because the is_active relation ships are changed both tests should return the same results.
@kainuk
Copy link
Contributor Author

kainuk commented Mar 17, 2019

I changed the test to get it running with this patch.

@eileenmcnaughton
Copy link
Contributor

@kainuk OK but I think the test was added as part of adding the criteria you removed & you adjusted the test to facilitate removing the criteria so I'm keen to see input from @jitendrapurohit

@kainuk
Copy link
Contributor Author

kainuk commented Mar 17, 2019

Maybe @ray-wright can check if this patch also works for him.

@ray-wright
Copy link
Contributor

This patch does not work for me. There are two controlling 'states' to relationships - enabled/disabled and end date (as they are called in the interface). So you can have an enabled role that has expired or vice versa, a disabled role that has not expired. (So to be active a relationship has to meet both criteria).

Enabled/disabled is controlled by the 'is_active' column in the relationship table and expired is controlled by the 'end_date' (or lack thereof ) in the same table.

There is a cron job that typically runs once a day and disables expired relationships for that day - keeping your active relationships clean. Only removing the end date criteria in the activeOnly parameter won't help if a role is both expired and disabled (which usually happens after that cron job has run.)

In the PR I made (which I totally accept is not perfect) I removed both criteria by setting activeOnly to False in the ajax.php file that calls the getCaseRoles function for this section.

While my PR does gives the user the choice to see all the inactive relationships with a checkbox - I don't believe it complicates the screen as by default, on an open case the box is unchecked. On a closed case it is checked so that you can see the case managers as technically they are both expired and disabled.

@kainuk
Copy link
Contributor Author

kainuk commented Mar 18, 2019

Hi @ray-wright, thanks for your feedback, I did not know about the cron job. Ik like the word expired, it gives a clear description of the state of the relationship. I think my customer is able to live with your solution - meaning a closed case means you see the history of all the case managers, instead of the active manager. We will not enable the cron job, because too much information will be lost in the case dashboard.

@ray-wright
Copy link
Contributor

@kainuk I'm curious about the information that would be lost in the case dashboard. Would you like to connect on mattermost at some point?

@eileenmcnaughton
Copy link
Contributor

@mattwire @jitendrapurohit @colemanw is someone able to work through this & figure out how to deal with it. This PR adds a test so it would be valuable to resolve it

@jitendrapurohit
Copy link
Contributor

Hi All,

Still thinking back to fix done at #13144 which contained fixes for the closed cases and kept the working of opened cases as it is after #13144 (comment) update. The PR is closed as of now but I think it was done because we had some recent submission for the fixes.

The problem here is to show disabled role for closed case so don't think we need to make any changes to the working of non-closed cases?

I've slightly modified the patch at 5e8192b which now displays the latest disabled role for the closed case. For eg, consider the below scenario -

  • Case created with manager A.
  • Case manager updated to B.
  • Case status set to Closed.
  • Now the manage case screen only displays the B contact as a manager which seems to be the expected behavior.

@kainuk @ray-wright Can you test the patch and let us know how you feel about it? We can then paste the fix into this PR and possibly merge it?

@kainuk
Copy link
Contributor Author

kainuk commented Apr 8, 2019

Hello @jitendrapurohit, this certainly is a creative solution for the concern of my customer that he only needs to see the last active client manager. I shall ask him if he wants to test this.

@ray-wright
Copy link
Contributor

@jitendrapurohit @kainuk your modified patch does work for the Manage Case Screen. (It shows the most recent contact assigned to a role no matter if it is disabled and/or expired.)

The portion that is then missing for me is that on case listings - no case manager is shown for closed cases. I'm wondering if on line 1940 in the getCaseManagerContact function it is possible to drop the "is_active"? (Then I'd want to add in your order by clause.) If that's not possible I also tested adding a very similar "activeOnly" variable to this call - and got that to work.

I got both methods to work but am not sure which option to propose. (I only see two files that reference the getCaseManagerContact function, this BAO Case file and the Case > Selector > Search.php)

@colemanw
Copy link
Member

Please see proposal at https://lab.civicrm.org/dev/core/issues/542

@colemanw colemanw closed this Jun 14, 2019
@kainuk
Copy link
Contributor Author

kainuk commented Jun 14, 2019

Hi @colemanw, I think the proposal will work form my client. If you have a PR I can test it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants