I was recently contacted by users of my module, webform_civicm inquiring about why it wasn't working with your module, so I did a bit of investigating. It seems that the way you're using the pre and post hooks is specific to profile edits and admin edits of contacts. So I worked on it a bit to see if it could be made to work in any other circumstances as well, such as API calls to add/edit a contact. The key seemed to be where you target the hook to act on 'Individual' or 'Household' or 'Organization'. But those aren't actually that relevant -- the only object your module really cares about is 'Address'. I think the address object may be a somewhat recent addition to these hooks, which may be why you didn't originally use it. So I have updated the hook implementations to only fire on the address object and ignore all the others. This gives us much more consistent results and lets us avoid a lot of unnecessary conditionals -- we no longer have to check if the contact has a cid or an address, for instance, since an address always has an associated cid.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | hook_updates-1333006-4.patch | 11.76 KB | colemanw |
| #1 | hook_updates-1333006-1.patch | 12.81 KB | colemanw |
Comments
Comment #1
colemanw commentedHere is the patch. Also updated a few other things while I was at it, like setting 'alias'=>TRUE for links to civicrm contacts, and simplifying contact<->uf lookups.
Comment #2
colemanw commentedPS. Reading your simpletest file, I see that you did intend this module to work with API-created contacts. However it would have only worked with addresses created from the contact api. But in many cases using the separate address api is preferable, and this module was not responding to those events.
Comment #3
dalin"I think the address object may be a somewhat recent addition to these hooks, which may be why you didn't originally use it."
Yes. This module has been around since CiviCRM 1.7ish, so things have changed a lot.
"Also updated a few other things while I was at it, like setting 'alias'=>TRUE for links to civicrm contacts, and simplifying contact<->uf lookups."
The alias and trailing space cleanup is fine. But I'm not a big fan of the other changes that you made outside of the pre/post hooks. Specifically this:
function _cd_civicrm_user_cid($id, $type = 'uf')CiviCRM needs to use this sort of confusing syntax because it works with multiple CMSes. But we're just Drupal here. Nor is this consistent with "The Drupal Way" of not combining multiple hooks/API-functions into one. Nor is it consistent with the naming scheme for the rest of the module's functions which is
[module name]_[noun]_[verb][_[noun]]
Before I try to apply the patch, did you run the simpletests to see if they all still pass?
Comment #4
colemanw commentedWe could get totally sidetracked debating about the uf-lookup function and "the Drupal way" but I don't think that's necessary, especially because I just did some grepping and found out that the reverse function (_cd_civicrm_contact_id_get_uid) isn't actually being called anywhere.
So in this new patch I have restored the single purpose nature of the cid lookup, and removed the unused reverse function.
Sorry, I don't have a sunlight account so probably can't run the simpletests locally. I don't actually use sunlight, I was just submitting this patch to help a friend. But according to her everything is working well with the patch applied.
Comment #4.0
colemanw commentedsince an address always has an associated cid