civicrm / org.civicrm.civicase

CiviCase Extension
Other
9 stars 35 forks source link

error when moving contact activity to a case #131

Open lcdservices opened 4 years ago

lcdservices commented 4 years ago

From the contact's activity tab, locate an activity, click the hamburger menu and click move to case. Select a case and save. There are no error messages on screen, but the activity is not moved. The console reports an error with $scope.item.id undefined.

There are two issues. The first traces here: https://github.com/civicrm/org.civicrm.civicase/blob/master/ang/civicase/ActivityFeed.js#L130 $scope.item is undefined, triggering the error and preventing the Activity.create api call. I gather from the context that the intent is to only trigger the API call if the case_id passed by the activity params is different from the context. So that was probably intended to apply when moving/copying a case activity to another case -- not when filing a contact activity to a case. But in my testing, $scope.item does not exist in either context, so I think that condition is inherently flawed. We either need to only condition on the existence of model.activity.case_id or find another way to compare against the contextual case_id (when appropriate -- i.e. not when moving a contact activity).

As a side note -- the civicase extension handles filing/moving/copying to case differently from core, and I'm not sure why. The core behavior works fine and offers more functionality. Why wouldn't we just reuse it?

The second issue is that even if we trigger the Activity.create API call, there is a flaw in the Activity API logic, found here: https://github.com/civicrm/civicrm-core/blob/master/api/v3/Activity.php#L142 That condition will only attached an activity to a case if the activity is new. This probably is due to a false assumption that only case activities with revision history enabled will be attached to a case. That assumption doesn't account for contact activities or CiviCase usage with revision history disabled (in deference to system logging). Removing the && $isNew condition restores functionality. Note that this isn't an issue in the core File on Case tool as it doesn't use the API to attach the activity to a case.

I can file PRs for the above, but would like others to review the above and see if they have concerns.