Adding a 'user load' event to hook_og for dynamic subscriptions

Here's a patch to implement this. This uses module_invoke_all to merge new organic group nids into the list og produces for a user by default.

It's been tested under Drupal 4.7 and a modified og.module from HEAD (rev. 1.148)

Mosh,

Thanks for looking the patch over.

Cool. I'll keep tracking the na_arbitrator stuff. When do you think you'll have something useable in HEAD?

It's easy enough for me to keep using a private version of og until you're ready with your new approach.

Rob

Moshe,

I understand you've gotten through most of the major upgrade of og.module you referred to some months ago. Webchick suggests that I revive this issue so that it can get some discussion, since in order to get Organic Groups to work well with CiviCRM smart groups (memberships defined by canned queries, basically), it needs to be possible for a Drupal module to reliably modify what og_get_subcriptions() returns. I've just studied the current 4.7 sources, and this is still not possible.

If you are still unconvinced that there is demand for this, it may make sense to take this discussion to the CiviCRM dev list, where the folks there can explain in more detail what various groups would like to do by integrating OG with CiviCRM.

Thanks,
Rob

yes, this is a good time to revive this discussion ... the last patch is nice and simple. but does it really suffice? for example, each group has a subscribers page which lists all members. doesn't that have to change as well? and other modules use the subscriber list for things like assigning issues.

I'd need to think about this, and most likely, some other folks who want OG support should probably chime in.

The big problem here is that it could be expensive. A typical use case here would be a political site where you want to limit people by where they live. You might have thousands of "hits" in the CiviCRM data store, of which only a handful actually have Drupal accounts.

I'll need to check if CiviCRM can do this sort of search efficiently, because otherwise the proposed Drupal hook will take quite a long time to execute.

Mosh,

Giving your points a bit more thought, it sounds like a "external membership source" like CiviCRM might need to handle several types of call backs:

• a "load" op, where for user X you want to load an array of subscriptions
• a "list" op, where for group Y you want a list of Drupal UID for that group.

One of the issues I'm less clear on is how this needs to interact with the Node access system. It's possible that there would need to be some support to make sure that if user X loads a subscription to group Y via the hook, that OG's node access queries include these groups. You'd know a lot more about that I would, although I have spent a fair bit of time lately in hook_db_rewrite_sql. Do you have a sense what if anything a patch would need to do for this to work right?

this gets messy though. for instance, there is a count of subscribers shown on the list of groups page. are we goig to have to ask civicrm for those counts?

I went back and forth on this one with Lobo last night.

Here's what I recommend: the hook needs to ask for 3 things, AFAICT:

1. Given a UID, return a list of dynamic subscriptions
2. Given a GID, return a count of Drupal users with qualified contact records in the external membership source
3. Given a GID, return a list of Drupal users with qualified contact records in the external membership source

In current builds of CiviCRM 1.5, (1) is easy, and (2) and (3) can be done, but not with great efficiency when the number of contacts in CiviCRM is very large.

From the perspective of OG, here's what I suggest:

• I define an exension of hook_og's behavior to cover these 3 cases, and I implement a patch that lets client code handle these requests.
• I'll implement a Drupal module that does a trivial implementation of all 3 new call backs, so that OG can be easily tested, and to make it easy for other implementors of external membership sources to figure out how to handle the new behavior.
• I'll contribute some patches to CiviCRM to handle (2) and (3) efficiently (Lobo has already told me that in principle, he's willing to accept these), and I'll also implement the call backs in my CiviCRM related Drupal modules. It's likely that some early implementations will not handle (2) or (3), since a CiviCRM-based membership source has good ways of counting and listing eligible members, and may not need OG's code to handle this for them. So I'll likely make it a module option whether to respond to these requests from OG or not.

Tell me what you think of this. I'll procede on the OG code, since this will remove any Drupal side impediment to doing these kinds of external membership source modules.

Mosh,

I did a quick look at DRUPAL-4-7 and HEAD. In both cases, the following call backs are implemented:

module_invoke_all('og', 'user insert', $gid, $uid, $args);
module_invoke_all('og', 'user update', $gid, $uid, $args);
module_invoke_all('og', 'user delete', $gid, $uid);
$links = module_invoke_all('og_create_links', $node);

I propose adding the following 3:

// call back for subs.  function should return an array of subscriptions.
$subs_array = module_invoke_all('og', 'user load', NULL, $uid, $args);

//call back for users.  function should return an array of UID.
$uid_array = module_invoke_all('og', 'list', $gid, NULL, $args);

//call back for count of subscribers.  function returns a count.
//Note that this may be problematic, since it's possible that two different handlers could be counting the same user.
//I don't see a good work around for this, and it may make sense to simply pull the uid array with 'list' and count that, 
//instead of implementing this callback.
$counts = module_invoke_all('og', 'count', $gid, NULL, $args);

Does that work for you?

I don't see this as a problem. Most use cases I've seen make the most sense if the organic group is created on the Drupal side. A module that implements dynamic membership can then be required to have such a group owned by a real Drupal user.

In addition: OG should only have to deal with real Drupal users. A module implementing dynamic membership should only be telling OG about real users. If a module needs to "presubscribe" user in Drupal, it needs to do this transparently to OG.

In practice, this isn't a big problem, since it's relatively easy to write code that hooks civicrm into creating Drupal users on the fly as needed. I suspect it wouldn't be hard for other external membership sources either (e.g., a LDAP user store).

OK, here's a patch to the DRUPAL_4_7 branch. HEAD is probably pretty similar, but since I don't have an install handy, it makes sense to get this working, and I'll then investigate what's required for HEAD.

I'm doing more error checking than the old patch, to make sure that the subscription arrays look like real subscription arrays.

Also, I'm only doing the 'user load' portion here. Looking through the og.module source, user lists are getting implemented two ways I know of (you may of course know of others):

1. Indirectly, via og_list_users_sql
2. Directly, by munging the {og_uid} table.

To hook this safely, it's probably necessary to factor code that is doing either of these two things, so that we can make sure the notion of a "list of subscribers" is abstracted away a bit from the the {og_uid} table.

The enclosed code loads without error; I'll write a small test module to test it, and will upload it when ready.

Mosh,

Here's a simple test module (4.7 compatible, might be 5.0 compatible as well, depending upon what hook_settings does in 5.0).

It's working on my 4.7 install, and makes it clear how to call the proposed hook.

As before, I'm not touching the subscriber listing code yet, since it's a bigger job, and may not be needed for a CiviCRM use of the feature. It might make sense to put the "factoring" of the listing code as a separate issue, but that's up to you.

thanks,
Rob

Just bumping this a bit to get feedback on my patch.

Just apply the patch to your test system; you can use the test module og_ul_test.module to see an example of a module that calls the modified hook.

Thanks,
Rob

dalin,

thanks for testing the patch, BTW.

Just to make sure: which patch did you use?

My latest patch did not actually access CiviNode or CiviCRM directly, since I was simply testing the 'user load' mechanism itself. But if I remember right, CiviNode may still have a version of the 'user load' hook that is incompatible with the current one.

I'll see if I can whip you up an example module that uses the current calling conventions. I'm also working on some of the stuff that Moshe suggested, and that will hopefully get up here soon.

Moshe asked that I look into handling the subscriber listing features of og.

I have a "candidate". The trick here is that you pretty much have to turn a query into an object (since it simply won't work to use Drupal query result objects as "handlers" for a query: some sources aren't doing SQL queries at all). So as a first step, I've taken the current code and wrapped the database queries. This will allow me to support CiviCRM (or even LDAP) as a second pass, when I turn the creations of objects into a "factory method". Then each organic group can have its own private notion of membership, and it will no longer be hard-coded into og.module as it currently is.

I'm putting this up in two parts. The first part is a new .inc file that defines a "membership source object". the second is the patch to og that uses it.

Oops.... minor glitch with og_membership.inc. Here's the fixed version.

Here is the patch that uses the *revised* og_membership.inc

It does indeed use an OO approach. But it's worth remembering that there's more to OO programming than simply classes; the "handle" based approach that database libraries use are also basically OO.

I'm not married to using classes, but what *really* is needed here is abstraction. Your current code is very low level, which is also not good practice. How would you suggest removing your low level references to the og_uid table, which is the root cause of why adding an external membership source is so difficult.

You don't have to like my approach, but I think you are obligated to suggest a reasonable alternative if you reject it. So step up to the plate and tell me what you *will* accept.

This issue has been floating around for around 6 months, and past a certain point, I do think you have an obligation to help get it resolved, one way or another.

Sounds like my goals are incompatible with og.module.

But this functionality is needed within Drupal; and og has a lot more features than this class of application really needs. So another module, I guess.

Since the core need is relatively simple, I should have something in a couple of weeks.

I am starting a bit of work that may be relevant to this.

The main issue is that OG really does not (and as it is currently going, will not) support integrating lists of users culled from a non-Drupal source. The reason for this is the very tight integration between the node access system and og, which is based upon rewriting queries. If your member list can easily be added to Drupal side query -- which can require constructions of queries with IN () clauses with thousands of items -- you are hosed.

I'm doing a module now that requires tight integration with CiviCRM. The approach I'm taking will require two hooks, both implemented via CiviCRM's callback mechanism:

1. A hook that will let CiviCRM learn a bit more about the current user (currently, it only really knows the user's Drupal UID). This could be used to get a list of Drupal side og subscriptions, or in my case, the ID of a political region like a Canadian riding or US Congressional district.
2. A hook that would act a little like Drupal's hook_db_rewrite_sql, except it will execute on the CiviCRM side. This hook could use the "user info" cached from the first hook to construct modified WHERE clauses, letting you do things like restrict which contacts are visible in a CiviCRM query.

Doing this in two steps will make it easier to support integration with Joomla as well, since modules that need to know a lot about Drupal don't have to know much about CiviCRM, and visa versa. It also means that we can share code more easily, since the query-rewriter can be shared between the Drupal and Joomla user frameworks, and the "user query" function can be very simple and very framework specific.

Note this scheme might allow you get the effect of some of the other integration schemes that we've tried in the past, but without either doing really scary mirroring, real-time manupulation of node access tables, or G'd forbid patching of og.module, which is just a maintenance nightmare.

If this works well, we can start thinking about how to generalize this. But for now, I consider patching og for this purpose a dead issue.

Moshe,

If you truly want the feature, facilitate its implementation.

The issue here is architectural, and I do not believe anyone other than yourself can make it happen. Since this issue has been hanging out here for more than a year (this is not the first thread on the topic), some other solution can and will be found. I do not believe it will make large use of OG.

I don't know your intentions. But as a practical measure, a solution for this will likely be found for the CiviCRM community, and fairly soon. I would love to have your help. But I am not expecting it, based upon several previous attempts to solve this problem within the OG framework. I can't maintain patches on a moving target, and I can't ask my clients to do so either.

If you think that this can be resolved within the framework of your module, then prove it to me. But I will not continue to submit patches to OG for this purpose.