CATL-1114: Support Cases In Scheduled Reminders #31
Conversation
|
@vinuvarshith , Can you add GIFs/screenshots to the PR to demonstrate how it works? |
| * | ||
| * Note: This value is chosen to match legacy DB IDs. | ||
| */ | ||
| const CASE_MAPPING_ID = 99; |
There was a problem hiding this comment.
How did you arrive at this particular value for the ID? Will it clash with some other value?
There was a problem hiding this comment.
This is just a random unique id in this case.
| /** | ||
| * @inheritdoc | ||
| */ | ||
| public function getRecipientListing($type) { |
There was a problem hiding this comment.
Can we use short array syntax in this function.
CRM/Case/ActionMapping.php
Outdated
| $selectedValues = (array) \CRM_Utils_Array::explodePadded($schedule->entity_value); | ||
| $selectedStatuses = (array) \CRM_Utils_Array::explodePadded($schedule->entity_status); | ||
|
|
||
| $query = \CRM_Utils_SQL_Select::from("civicrm_case c")->param($defaultParams); |
There was a problem hiding this comment.
Won't something like CRM_Utils_SQL_Select::from("{$this->entity} e") this work as I see in many core Action Mapping classes rather than hardcoding the table name?
At worst if the above does not work, we can get the table name from the BAO/DAO.
There was a problem hiding this comment.
I was following CRM/Contribute/ActionMapping/ByPage.php
I have updated it to {$this->entity} now
CRM/Case/ActionMapping.php
Outdated
| $query->join('ce', "INNER JOIN civicrm_email ce ON ce.contact_id = ct.id"); | ||
|
|
||
| if (!empty($selectedValues)) { | ||
| $query->where("c.case_type_id IN (#caseId)") |
There was a problem hiding this comment.
Should the param placeholder name not be '#caseTypeId' ? Since we are searching in the case_type_id column
CRM/Case/ActionMapping.php
Outdated
| ->param('selectedStatuses', $selectedStatuses); | ||
| } | ||
|
|
||
| if (!empty($caseRoles)) { |
There was a problem hiding this comment.
Can't seem to find where the $caseRoles parameter is set in this function?
There was a problem hiding this comment.
this was not required here and is handled later in the function. I have removed this now.
CRM/Case/ActionMapping.php
Outdated
| $query['casContactIdField'] = 'ct.id'; | ||
| $query['casEntityIdField'] = 'r.case_id'; | ||
| $query['casContactTableAlias'] = 'ct'; | ||
| $query->where('r.is_active = 1 AND c.is_deleted = 0'); |
There was a problem hiding this comment.
An active relationship is more than checking if the relationship is active alone, We will also need to check the start and end date of the relationship to verify that start date is <= today OR NULL and end date >= today OR NULL.
There was a problem hiding this comment.
you are right. Fixed this now.
CRM/Case/ActionMapping.php
Outdated
| return [ | ||
| 'start_date' => ts('Case Start Date'), | ||
| 'end_date' => ts('Case End Date'), | ||
| 'case_status' => ts('Case Status Change'), |
There was a problem hiding this comment.
Case status is not a date field, how come it is returned alongside other date fields?
There was a problem hiding this comment.
case_status is not directly a date field but it is. It is the date on which that case status was updated.
So we dont have a direct way currently to find this date (its not stored).
Jamie said we would need to have discussions with core team to maybe add a new field or improve how case status changes are tracked.
But for now (before my last day), I was asked to see if there is a way to support this and using case activity was the only way I could do this.
I have explained this in the ticket CATL-1114.
CRM/Case/ActionMapping.php
Outdated
| @@ -126,7 +137,40 @@ public function createQuery($schedule, $phase, $defaultParams) { | |||
| throw new CRM_Core_Exception("Invalid date field"); | |||
| } | |||
|
|
|||
| $query['casDateField'] = $schedule->start_action_date; | |||
| if ($schedule->start_action_date == 'case_status') { | |||
There was a problem hiding this comment.
Don't think this looks right, for a date field to be returning the case status. Can you explain why it is like this? IT is a PR going to core and if it's not proper, we might not be able to get this in. Looks more like a hack.
There was a problem hiding this comment.
This is not the case_status that is used here. I have changed the name 'case_status' to 'case_status_change_date' to be a little better.
This might look a little hacky because we dont have a better way to get when a case status has changed in civicrm core currently.
Pls also see this comment #31 (comment)
| $subQuery->join('opg2', 'INNER JOIN civicrm_option_group opg2 ON opg2.id = opt2.option_group_id'); | ||
| $subQuery->where("opt2.name = @activityStatus")->param('activityStatus', $activityStatus); | ||
| $subQuery->where("opg2.name = 'activity_status'"); | ||
| $query->where("act.status_id = (" . $subQuery->toSQL() . ")"); |
There was a problem hiding this comment.
Can't we build the activity statuses from this CRM_Activity_BAO_Activity::buildOptions('status_id') and use that in the query for the IN rather than using a subquery
There was a problem hiding this comment.
I tried using that but I cant really get the query result that I am looking. Maybe you can explain a bit more on that subquery part..
I have tried writing tests for this but its not straightforward. The core team said they would help us write tests for this. So its better to get this QAed locally by our team, meanwhile discuss with core team about the approach used for 'case status change date' and after that get help from then to add tests. |
Overview
This is an improved and rebased version of this core PR
civicrm#13648
This improvement is aimed at including cases in the list of entities that can be configured for scheduled reminders. This will enable us to send scheduled emails to case roles (configurable) after a predefined interval of time. This will also have the ability filter the list of these cases and only pick certain cases based on case status (configurable).
Details
I have worked more on this and made some fixes and support for 'Case status change'.
So now we will have one more option in the 'When' dropdown for this. This works based on the case activity we add when we change a case status. So when we use this option with a case status 'foo' (set in scheduled reminder), then when any case is moved from any status to 'foo', then those cases become our match.
The actual reminder, however, is on triggered as configured (once daily by default). One more issue/point is that in the scheduled reminder, 'when', setting, we only have a minimum of 1 hour 'before' or 'after'. So even if the status is changed, the reminder wont be triggered after that set time has elapsed.