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

Better handle Case Manager and roles on closed cases #13510

Closed
wants to merge 12 commits into from

Conversation

ray-wright
Copy link
Contributor

Overview

In Civi 4.7 case roles were refactored via #6806 which added a query to only load active relationship from getCaseRole function. This is problematic for closed cases - since theoretically, they should only contain inactive relationships. I'm proposing removing a better way to handle inactive roles rather than just not showing them.

Before

On closed cases, the case manager did not display on case listings or inside the manage screen. In addition, roles other than the case manager also appear blank on closed cases. (This is because they are inactive, i.e. end date in the past and disabled.)
nocasemanager
nocasemanager2

After

On case listings:

  • Default to the active case manager, if no active case manager found, show the name of the most recently expired case manager.

On the manage case screen:

  • Show ALL roles whether or not they are inactive.
  • Add a column for end date and gray out inactive roles for visual clarity.
  • Add a checkbox for 'show inactive relationships'
    -- Check by default on closed cases (and show inactive relationships)
    -- Uncheck by default on open cases (and hide inactive relationships)

propercasemanager
propercasemanager2
propercasemanager3

Comments

This is the first improvement I've suggested that affects multiple areas, please feel free to clean up or suggest more efficient methods! (FYI, I do still need a little a little help in the Activity>Page>Ajax.php file.)

In the getCaseRoles function, added is_active and end_date to the query and to the values array (for use in the case roles table on the manage case screen controlled in the caseview.tpl)

In the getCaseManagerContact > managerRoleQuery function, I removed the portion of the where clause that was only returning active case managers. I added the is_active and end_date to the query here as well. Then, I added logic that first searched for an active case manager, if none found then it would show the most recently 'ended' manager. (This controls what name displays on case listings, like on a contact card or case search. This way for closed cases a case manager is still listed.)
This builds on @jitendrapurohit pull civicrm@a56aac7

First, set activeOnly to false in the getCaseRoles call - to ensure we get both open and inactive roles.

Add the is_active and end_date as rows to the $caseRoles array.

Then using those rows - add to the data table. Use is_active to add the 'disabled' class where appropriate. Add a column to display the end date. (If an end date is present, do not show the actions.)

To help avoid confusion, swap the trash can icon for the ban icon and change label text to 'expire'. (As that action isn't 'trashing' anything, it just adds an end date to the relationship - so the relationship still shows in the list.)
In the case roles data table, add a column for end_date. 

Add a smarty checkbox called 'show inactive relationships.' JS to show/hide rows with the 'disabled' class based on clicking the checkbox. Additional JS logic checks the box by default on closed cases.

Add to the ajax call, an 'on complete' to hide 'disabled' rows if the case is open. If there are no disabled rows to hide, gray out the 'show inactive relationships' checkbox and text.

Adjust DeleteCaseRoleDialog text to be less consistent (change to 'expire').
Add the class of case status to the smarty array caseDetails. (This is then used in the CaseView.tpl file)

(This code provided by @colemanw, it is a part of my overall plan but I didn't write it, huge thanks to Coleman for the help.)
code rearranged for clarity
@civibot
Copy link

civibot bot commented Jan 29, 2019

(Standard links)

@civibot civibot bot added the master label Jan 29, 2019
@aydun
Copy link
Contributor

aydun commented Jan 30, 2019

test this please

@ray-wright
Copy link
Contributor Author

I believe this will replace the earlier work started here #13144. @jitendrapurohit please feel free to weigh in.

@ray-wright
Copy link
Contributor Author

One current caveat is that the start and end dates for a relationship are only dates, rather that date times. So if two relationships share an end date, there's no way to tell which one is the most recently 'expired' relationship. Would it be problematic to change the relationship start_date and end_date in the xml schema from date to datetime? If it is not, can anyone help me change that and generate the Contact>Relationship>DAO file?

@colemanw
Copy link
Member

colemanw commented Feb 4, 2019

@jitendrapurohit could you please review this?

@jitendrapurohit
Copy link
Contributor

jitendrapurohit commented Feb 5, 2019

This approach sounds like a good solution. Thanks for raising this @ray-wright.

I tried to review this but soon after applying the PR, I got an error on the Roles tab, i.e, when I open the manage screen of a closed case, I see -

image

seems due to the changes made in https://github.com/civicrm/civicrm-core/pull/13510/files#diff-33fb19b4d7d2903e556b3c4a2ed1ffd5R210 which adds an extra function for the datatable. Not sure but maybe, we should only load the required rows instead of hiding it later after it is fetched from DB?

i.e, redraw the table values when the inactive checkbox is enabled/disabled?

@ray-wright
Copy link
Contributor Author

@jitendrapurohit would that then be done in the BAO file? At this point, perhaps it needs a form similar to how the activities filter works on cases? I just wasn't sure if that was a lot of work for a small 'checkbox'.

This allows the reporter role selector to pull case roles that are no longer active - such as for a closed case. Soon closed cases you can still filter activities by role.
@colemanw
Copy link
Member

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

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

Successfully merging this pull request may close these issues.

4 participants