CiviCRM Community Forums (archive)

Hi guys,
In a recent upgrade from 3.1.3 to 3.2.3, new mailings that go out via CiviMail seem to no longer record themselves as activities.
This worked fine in 3.1.3. Aside from the upgrade the only change to the civiCRM configuration has been to setup an account for bounce handling in civimail, but this shouldn't affect the CiviMail activities because those get recorded at the time of sending right?
All other activities get recorded fine.
Running Drupal 6.16.

Is this is a deliberate change or is there something wrong with my setup?
If there is something wrong, any ideas what could be causing this, and why it worked before the upgrade and not now?

Things I have tried to no avail:
- giving bulk mailing user full permissions to everything
- running mailing queue url under administrator account

I have tried to debug the PHP to isolate the problem but I can't seem to track from bin/civimail.cronjob.php, where the activities are getting recorded. If someone could point me to the place where this happens I can try debugging?

Any help gratefully received

Ah good. It's not just me then!
Maybe the problem only occurs during an upgrade from specific point versions?
Is anyone else having this problem with a fresh 3.2.3 install?

I am happy to debug this, but I need a bit of a pointer where to start. I tried to trace back the commands from civimail.cronjob.php but I couldn't see which function/file was handling the activity creation. If someone could point me to a starting point file/function where the activities are supposed to be created during a civimail batch, I'll happily to to go through this.

This is quite a major problem for us, as it's negatively affecting our reporting.

Hello,

I tried but not able to replicate on my local install.

Could you please put some debug statements so it would give us some valuable pointers.

activity creation process start from CRM/Mailing/BAO/Job.php - line around 401

here once we have $targetParams params ready,
we are calling civicrm_activity_create() function to create an activity record. - on line 435

you could check return values of this function like

$result = civicrm_activity_create($activity, 'Email');
crm_core_error::debug( 'result', $result );
exit( );

Main points about Mailing Activity.
1. We do create single activity record for each mailing. ( present in civicrm_activity table)
2. We do create target records for each recipient. ( present in civicrm_activity_target )

1. If civicrm_activity table fails to create record related to mailing - which implies activity creation it self fails.
2. If civicrm_activity_target fails to create record for each recipients - which implies fails to create activity target records.

Could you please take a closer look at db state before and after mailing process and let us know your observation.

Also does it related to any mailing batch limit ?
( when we have recipients > 1000 -> fails to create activity/target records )

thanks

kiran

Ok Step 2, debugging api\v2\Activity.php
function civicrm_activity_create()
Wierdly, this function only takes 1 parameter, the $params array, whereas CRM\mailing\bao\job.php above seems to be passing two parameters?

Anyway, I've added lines to printout what is happening:

function &civicrm_activity_create( &$params )
{
    echo print_r($params, true);

    _civicrm_initialize( );
    
    $errors = array( );
    
    // check for various error and required conditions
    $errors = _civicrm_activity_check_params( $params, true ) ;

    echo print_r($errors, true);

    if ( !empty( $errors ) ) {
        return $errors;
    }
    
    // processing for custom data
    $values = array();
    _civicrm_custom_format_params( $params, $values, 'Activity' );
    if ( ! empty($values['custom']) ) {
        $params['custom'] = $values['custom'];
    }

    echo print_r($params, true);

    // create activity
    $activity = CRM_Activity_BAO_Activity::create( $params );

    echo print_r($activity, true);

    if ( !is_a( $activity, 'CRM_Core_Error' ) && isset( $activity->id ) ) {
        $activityArray = array( 'is_error' => 0 );
    } else {
        $activityArray = array( 'is_error' => 1 );
    }

    echo print_r($activityArray, true);

    _civicrm_object_to_array( $activity, $activityArray);

    echo print_r($activityArray, true);

    return $activityArray;
}

The results of this are:


Array
(
    [source_contact_id] => 102
    [target_contact_id] => Array
        (
            [0] => 102
            [1] => 48852
        )
 
    [activity_type_id] => 34
    [source_record_id] => 174
    [activity_date_time] => 20101102114303
    [subject] => test of activity logging which appears to be broken
    [status_id] => 2
    [deleteActivityTarget] =>
)
Array
(
    [is_error] => 1
    [error_message] => Invalid Target Contact Id
)

So, it seems it thinks there is an invalid target contact id. I have checked both contact Id's and they definitely exist and are comtacts in the system. I can get to their contact view pages using those Id's, and they are in the civicrm_contact table.
Also, another wierd thing. If this is then returning the $errors array immediately (and thus not processing the activities), why is CRM\mailing\bao\job.php not recognising these errors and returning false in the lines?

   require_once 'api/v2/Activity.php';
   if ( is_a( civicrm_activity_create($activity, 'Email'), 'CRM_Core_Error' ) ) {
       return false;
   }

This should return false, but it does not as my debugging in step 2 showed code that ran after these lines in the function.

Now, step 3. Debug why _civicrm_activity_check_params()  is giving this error...

Ok, time for step 3!
Debugging api\v2\Activity.php
function _civicrm_activity_check_params ()

Again, I have added debug lines to observe the array status as it works through.

function _civicrm_activity_check_params ( &$params, $addMode = false )
{
    // return error if we do not get any params
    if ( empty( $params ) ) {
        return civicrm_create_error( ts( 'Input Parameters empty' ) );
    }

    $contactIds = array( 'source'   => CRM_Utils_Array::value( 'source_contact_id', $params ),
                         'assignee' => CRM_Utils_Array::value( 'assignee_contact_id', $params ),
                         'target'   => CRM_Utils_Array::value( 'target_contact_id', $params )
                         );

    echo print_r($contactIds, true);

    foreach ( $contactIds as $key => $value ) {
        echo print_r($key, true).", ";
        echo print_r($value, true)."\r\n";
        //if ($debug) echo CRM_Core_DAO::getFieldValue( 'CRM_Contact_DAO_Contact', $value, 'id' );

        if ( $value &&
             !CRM_Core_DAO::getFieldValue( 'CRM_Contact_DAO_Contact', $value, 'id' ) ) {
            return civicrm_create_error( ts( 'Invalid %1 Contact Id', array( 1 => ucfirst( $key ) ) ) );
        }
    }

.....

this results in the following output:

Array
(
    [source] => 102
    [assignee] =>
    [target] => Array
        (
            [0] => 102
            [1] => 48852
        )
 
)
source, 102
assignee,
target, Array
(
    [0] => 102
    [1] => 48852
)
Array
(
    [is_error] => 1
    [error_message] => Invalid Target Contact Id
)

So....

Assignee contact ids are empty. This should be correct as there is no assignee for bulk mailing. Source id is correct as the contact who created the mailing. Target(s) is still an array, but has existing and correct contact IDs. Another possible bug I noticed here, is that if $value is empty (such as in the case of assignee here), $value takes on the


The offending line here is:

if ( $value && !CRM_Core_DAO::getFieldValue( 'CRM_Contact_DAO_Contact', $value, 'id' ) ) {

$value is set, but CRM_Core_DAO::getFieldValue( 'CRM_Contact_DAO_Contact', $value, 'id' ) returns a CRM error when passed the target contact ids array as $value.
You can see in the code above I commented out a line:
echo CRM_Core_DAO::getFieldValue( 'CRM_Contact_DAO_Contact', $value, 'id' );

For $key = 'target', this returned blank (i.e. false.). Also wierdly, trying to echo the value for $key = 'assignee' resulted in printing out the html page for a CRM fatal error (see below) and ending execution, but as long as I didn't try to echo the value it was fine...


So, step 4, why does CRM\core\dao.php generate an error when passed this array. Does it expect a single value?
It seems to have problems when passed an array, or when passed nothing (as in when $key = 'assignee')

I'll report back when I've worked the next stage out!

Code: [Select]

Array
(
    [source] => 102
    [assignee] =>
    [target] => Array
        (
            [0] => 102
            [1] => 48852
        )
 
)

...Target(s) is still an array, but has existing and correct contact IDs.

Certainly looks as though civicrm_activity_create is being called incorrectly, on two counts:
- Passed 2 arguments instead of one.
- AFAICS civicrm_activity_create /  _civicrm_activity_check_params expect to be passed single ids rather than arrays for *_contact_id .

Dave J

Hello grahamgilchrist,

Great job w/ debug

Could u please fire this query w/ your db and let us know result,

query = select id from civicrm_contact where id IN ( 102, 48852 );

davej : If I'm not wrong api create should respect both single *_contact_id as well as an array of ids.

yes, we could safely drop second parameter to civicrm_create_activity( $activity, 'Email' );

thanks

kiran

Could u please fire this query w/ your db and let us know result,

query = select id from civicrm_contact where id IN ( 102, 48852 );

Yes indeed. I can confirm those two contacts exist. Results of that query gives two rows with
102
48852

Incidentally, I've just looked through the code for 3.1.3, and found out why we did not have this error before.
What has changed somewhere between 3.1.3 and 3.2.3 (sorry not sure exactly which version) is the _civicrm_activity_check_params() function in API\v2\Activity.php.

In 3.1.3, this function did only a check on valid source id's. However, the activities were still registered, as the call to CRM_Activity_BAO_Activity::create( $params ) did support an array of target contact ids.

Now in 3.2.3 it checks valid source, assignee, and target contact ids, then returns an error (because the targets are sent as an array). Presumably, if it got past this error checking, the activity create BAO call would still work (I've not checked this). This change is probably a result of unit testing and error trapping in the APIs (which is great), but it looks like we need to either:

1) update the API so that it can accept arrays of contact id's and check them appropriately (via a loop or something?)
2) update the API to have a flag for ignoring activities where assignee or target are not valid (are there any others apart from civimail?)
3) update the civimail BAO code to loop through each contact with an API call. This seems like it might produce a lot of extra overhead, so the prior options seem preferable

What do you guys think?