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.

Sign up for GitHub

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

CRM-21598 - Case Activity issues with custom Completed Status Type #11456

Merged
merged 5 commits into from
May 15, 2018
Merged

CRM-21598 - Case Activity issues with custom Completed Status Type #11456

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
de33391
CRM-21598 - Case Activity issues with custom Completed Status Type
Dec 26, 2017
e3a72b3
update unit test
Dec 27, 2017
846154a
minor fix
Jan 12, 2018
0dba8ce
additional changes
Feb 19, 2018
5f7f881
add doc for case activity sequence
Feb 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions CRM/Case/BAO/Case.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1039,12 +1039,10 @@ public static function getCaseActivity($caseID, &$params, $contactID, $context =

$caseDeleted = CRM_Core_DAO::getFieldValue('CRM_Case_DAO_Case', $caseID, 'is_deleted');

// define statuses which are handled like Completed status (others are assumed to be handled like Scheduled status)
$compStatusValues = array();
$compStatusNames = array('Completed', 'Left Message', 'Cancelled', 'Unreachable', 'Not Required');
foreach ($compStatusNames as $name) {
$compStatusValues[] = CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_status_id', $name);
}
$compStatusValues = array_keys(
CRM_Activity_BAO_Activity::getStatusesByType(CRM_Activity_BAO_Activity::COMPLETED) +
CRM_Activity_BAO_Activity::getStatusesByType(CRM_Activity_BAO_Activity::CANCELLED)
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jitendrapurohit why are you merging these 2 arrays? I think just the statusesByType should provide the needed information, correct?

Copy link
Contributor Author

@jitendrapurohit jitendrapurohit Jan 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't seem so. 'Left Message', 'Unreachable' and 'Not Required' belongs to activity statuses of type INCOMPLETE and 'Cancelled' is of type CANCELLED.

But the current flow doesn't want to display them in red color if they have an overdue date(see comment in L1042). statusesByType will only return the completed activity status.

We can use the following API if we intend to use only one variable -

$compStatusValues = civicrm_api3('OptionValue', 'get', array(
  'sequential' => 1,
  'return' => array("value"),
  'option_group_id' => "activity_status",
  'name' => array('IN' => array("Left Message", "Cancelled", "Unreachable", "Not Required")),
  'filter' => CRM_Activity_BAO_Activity::COMPLETED,
  'options' => array('or' => array(array("name", "filter"))),
));
$compStatusValues = CRM_Utils_Array::collect('value', $compStatusValues['values']);

But seems it'll not be a good change w.r.t readability than the current one?


$contactViewUrl = CRM_Utils_System::url("civicrm/contact/view", "reset=1&cid=", FALSE, NULL, FALSE);
$hasViewContact = CRM_Core_Permission::giveMeAllACLs();
Expand Down
6 changes: 3 additions & 3 deletions Civi/CCase/SequenceListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public function onCaseChange(\Civi\CCase\Event\CaseChangeEvent $event) {
}

$actTypes = array_flip(\CRM_Core_PseudoConstant::activityType(TRUE, TRUE, FALSE, 'name'));
$actStatuses = array_flip(\CRM_Core_PseudoConstant::activityStatus('name'));
$actStatuses = array_flip(\CRM_Activity_BAO_Activity::getStatusesByType(\CRM_Activity_BAO_Activity::COMPLETED));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this function do and does it also need to respond to CANCELLED activities?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function calls the next activity in sequence when the current activity is set to Completed. It was only checking for the single activity status before(Completed - line 62 below). Now, after the above changes, it triggers the creation of next activity when the current activity is updated to any of the statuses of TYPE = COMPLETED.

I'm not really sure if it makes sense to create next activity when the present activity is CANCELLED. But, this was not the case before this PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, can you add what you just said about what the function does into the docblock for future reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👍


$actIndex = $analyzer->getActivityIndex(array('activity_type_id', 'status_id'));

Expand All @@ -59,7 +59,7 @@ public function onCaseChange(\Civi\CCase\Event\CaseChangeEvent $event) {
$this->createActivity($analyzer, $actTypeXML);
return;
}
elseif (empty($actIndex[$actTypeId][$actStatuses['Completed']])) {
elseif (!in_array(key($actIndex[$actTypeId]), $actStatuses)) {
// Haven't gotten past this step yet!
return;
}
Expand All @@ -68,7 +68,7 @@ public function onCaseChange(\Civi\CCase\Event\CaseChangeEvent $event) {
//CRM-17452 - Close the case only if all the activities are complete
$activities = $analyzer->getActivities();
foreach ($activities as $activity) {
if ($activity['status_id'] != $actStatuses['Completed']) {
if (!in_array($activity['status_id'], $actStatuses)) {
return;
}
}
Expand Down
18 changes: 12 additions & 6 deletions tests/phpunit/Civi/CCase/SequenceListenerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ public function setUp() {
'subject' => 'Test case',
'contact_id' => 17,
);
//Add an activity status with Type = Completed
$this->callAPISuccess('OptionValue', 'create', array(
'option_group_id' => "activity_status",
'filter' => \CRM_Activity_BAO_Activity::COMPLETED,
'label' => "Skip Activity",
));
}

public function testSequence() {
Expand Down Expand Up @@ -60,18 +66,18 @@ public function testSequence() {
$this->assertEquals($actStatuses['Scheduled'], self::ag($analyzer->getSingleActivity('Mental health evaluation'), 'status_id'));
$this->assertFalse($analyzer->hasActivity('Secure temporary housing'));

// Complete second activity; schedule third
//Complete second activity using "Skip Activity"(Completed); schedule third
\CRM_Utils_Time::setTime('2013-11-30 03:00:00');
$this->callApiSuccess('Activity', 'create', array(
'id' => self::ag($analyzer->getSingleActivity('Mental health evaluation'), 'id'),
'status_id' => $actStatuses['Completed'],
'status_id' => $actStatuses['Skip Activity'],
));
$analyzer->flush();
$this->assertEquals($caseStatuses['Open'], self::ag($analyzer->getCase(), 'status_id'));
$this->assertApproxTime('2013-11-30 01:00:00', self::ag($analyzer->getSingleActivity('Medical evaluation'), 'activity_date_time'));
$this->assertEquals($actStatuses['Completed'], self::ag($analyzer->getSingleActivity('Medical evaluation'), 'status_id'));
$this->assertApproxTime('2013-11-30 02:00:00', self::ag($analyzer->getSingleActivity('Mental health evaluation'), 'activity_date_time'));
$this->assertEquals($actStatuses['Completed'], self::ag($analyzer->getSingleActivity('Mental health evaluation'), 'status_id'));
$this->assertEquals($actStatuses['Skip Activity'], self::ag($analyzer->getSingleActivity('Mental health evaluation'), 'status_id'));
$this->assertApproxTime('2013-11-30 03:00:00', self::ag($analyzer->getSingleActivity('Secure temporary housing'), 'activity_date_time'));
$this->assertEquals($actStatuses['Scheduled'], self::ag($analyzer->getSingleActivity('Secure temporary housing'), 'status_id'));

Expand All @@ -88,7 +94,7 @@ public function testSequence() {
$this->assertApproxTime('2013-11-30 01:00:00', self::ag($analyzer->getSingleActivity('Medical evaluation'), 'activity_date_time'));
$this->assertEquals($actStatuses['Completed'], self::ag($analyzer->getSingleActivity('Medical evaluation'), 'status_id'));
$this->assertApproxTime('2013-11-30 02:00:00', self::ag($analyzer->getSingleActivity('Mental health evaluation'), 'activity_date_time'));
$this->assertEquals($actStatuses['Completed'], self::ag($analyzer->getSingleActivity('Mental health evaluation'), 'status_id'));
$this->assertEquals($actStatuses['Skip Activity'], self::ag($analyzer->getSingleActivity('Mental health evaluation'), 'status_id'));
$this->assertApproxTime('2013-11-30 03:00:00', self::ag($analyzer->getSingleActivity('Secure temporary housing'), 'activity_date_time'));
$this->assertEquals($actStatuses['Scheduled'], self::ag($analyzer->getSingleActivity('Secure temporary housing'), 'status_id'));
$this->assertApproxTime('2013-11-30 04:00:00', self::ag($analyzer->getSingleActivity('Follow up'), 'activity_date_time'));
Expand All @@ -104,7 +110,7 @@ public function testSequence() {
$this->assertApproxTime('2013-11-30 01:00:00', self::ag($analyzer->getSingleActivity('Medical evaluation'), 'activity_date_time'));
$this->assertEquals($actStatuses['Completed'], self::ag($analyzer->getSingleActivity('Medical evaluation'), 'status_id'));
$this->assertApproxTime('2013-11-30 02:00:00', self::ag($analyzer->getSingleActivity('Mental health evaluation'), 'activity_date_time'));
$this->assertEquals($actStatuses['Completed'], self::ag($analyzer->getSingleActivity('Mental health evaluation'), 'status_id'));
$this->assertEquals($actStatuses['Skip Activity'], self::ag($analyzer->getSingleActivity('Mental health evaluation'), 'status_id'));
$this->assertApproxTime('2013-11-30 03:00:00', self::ag($analyzer->getSingleActivity('Secure temporary housing'), 'activity_date_time'));
$this->assertEquals($actStatuses['Completed'], self::ag($analyzer->getSingleActivity('Secure temporary housing'), 'status_id'));
$this->assertApproxTime('2013-11-30 04:00:00', self::ag($analyzer->getSingleActivity('Follow up'), 'activity_date_time'));
Expand All @@ -121,7 +127,7 @@ public function testSequence() {
$this->assertApproxTime('2013-11-30 01:00:00', self::ag($analyzer->getSingleActivity('Medical evaluation'), 'activity_date_time'));
$this->assertEquals($actStatuses['Completed'], self::ag($analyzer->getSingleActivity('Medical evaluation'), 'status_id'));
$this->assertApproxTime('2013-11-30 02:00:00', self::ag($analyzer->getSingleActivity('Mental health evaluation'), 'activity_date_time'));
$this->assertEquals($actStatuses['Completed'], self::ag($analyzer->getSingleActivity('Mental health evaluation'), 'status_id'));
$this->assertEquals($actStatuses['Skip Activity'], self::ag($analyzer->getSingleActivity('Mental health evaluation'), 'status_id'));
$this->assertApproxTime('2013-11-30 03:00:00', self::ag($analyzer->getSingleActivity('Secure temporary housing'), 'activity_date_time'));
$this->assertEquals($actStatuses['Completed'], self::ag($analyzer->getSingleActivity('Secure temporary housing'), 'status_id'));
$this->assertApproxTime('2013-11-30 04:00:00', self::ag($analyzer->getSingleActivity('Follow up'), 'activity_date_time'));
Expand Down