Compatibility with CiviCRM 3.3

@thomasf:
Ah, I haven't used 3.3 yet, thanks.

Before I commit this patch, I'd like to know about its impact on CiviCRM version compatibility. What versions of CiviCRM, if any, would no longer work with the 6.x branch of CiviCRM Subscribe if this were committed? If there are versions >=2.1 that would no longer work, I would like to have some kind of compatibility layer, since I want CiviCRM Subscribe to work "out of the box" for all 6.x versions of CiviCRM.

Also, set to "needs review" when an issue includes a patch. Thanks for your contribution!

Thanks for the revised patch. I'll try to test this one before the New Year - got some other (paying) work to wrap up first. I'll have to set up a testbed with at least 2 version of Civi to test it though.

Sorry for not looking at this sooner. Code looks like it should be reasonable. I will test it with my 2.x install soon, and this may be the last commit to this module (since I am planning on deprecating it in favor of CiviCRM Webform Integration).

$civicrm_version_string = CRM_Utils_System::version();
$civicrm_version = intval(implode('', explode('.', $civicrm_version_string, 2)));
$civicrm_version >= 33 ? $use_api_v2 = true : $use_api_v2 = false; // Starting from CiviCRM v3.3.x api v1 is not included anymore

It's kind of absurd to nest implode and explode together just to get rid of a dot.
Besides there's no reason to do so, the dot wasn't doing any harm anyway. All you need is this:

$civicrm_version = (float)CRM_Utils_System::version();
$civicrm_version >= 3.3 ? $use_api_v2 = true : $use_api_v2 = false; // Starting from CiviCRM v3.3.x api v1 is not included anymore

Not that those one-liner if statements are very drupalish. And comments go above the line, not on the same line, as the code they are referring to.

But aside from nitpicky stuff, this code does not seem to pass contact_id into the api, and I believe it is a required parameter.

  if($use_api_v2){
    $civicrm_params = array ( 
      'email'          => $form_state['values']['email'],
      'group_id'       => $group_id,
    );
    $contact_added = civicrm_mailer_event_subscribe($civicrm_params);
  }
  else {
    $contact_added = crm_mailer_event_subscribe($form_state['values']['email'], $group_id);
  }

How is either v1 or v2 api supposed to know what contact you are talking about?

@colemanw: In re: the 2.2 branch, see http://svn.civicrm.org/civicrm/branches/v2.2/api/Mailer.php.

This is how the function is defined:

/**
 * Handle a subscription event
 *
 * @param string $email     The email address to subscribe
 * @param int $group_id     The group of the subscription
 * @return boolean
 */
function crm_mailer_event_subscribe($email, $group_id) {
    $se =&
        CRM_Mailing_Event_BAO_Subscribe::subscribe($group_id, $email);

    if ($se !== null) {
        /* Ask the contact for confirmation */
        $se->send_confirm_request($email);
        return true;
    }
    return false;
}

Thus, the wrapper function only needs an email address, not a Contact ID.

I won't quote the whole civicrm_mailer_event_subscribe() function from http://svn.civicrm.org/civicrm/branches/v3.3/api/v2/Mailer.php, but suffice it to say that it has the same kind of parameters.

$errors = _civicrm_mailer_check_params( $params, array('email', 'group_id') ) ;

That said, colemanw, I like your suggestion on simplifying the syntax. I think it would be even better for me to revive the old civicrm_subscribe() function and use that as a wrapper around this logic. That way the internal details of implementation would be hidden from the caller - i.e., the form submit function, and the civicrm_subscribe() function could be called from elsewhere if needed.

I'm not sure if it is required or not, but at least the v2 API does accept contact_id as a @param. For this reason, I'm not a big fan of the v1 api (what if multiple contacts have the same email address? Too messy). So for my version of legacy support for webform_civicrm I've decided to just copy and paste the entire v2 civicrm_mailer_event_subscribe() function into one of my module's include files. Hey, it works. And I can get rid of it (and indeed all support for apis < v3) in the D7 version.

Committed this to -dev as follows: http://drupalcode.org/project/civicrm_subscribe.git/blobdiff/bd9a4f9d4c7...

I've done some basic testing, but more would be wanted before I tag for the next beta.

I can confirm that this in fact has resolved issues that I had with version of CiviCRM. Can I suggest that you actually deploy a new Beta version. I **generally** don't install dev versions on client sites just cause they are deemed unstable. But what you got here seems stable and you haven't released in a while?

Thanks for your awesome work!

Sorry for not making another release. I am no longer maintaining this module since I stopped using CiviCRM last year, but am seeking a new maintainer.