I have been working for the last six months on og2list - both a rudimentary 5.x port and an advanced custom version (which I will eventually be backporting the features from or hopefully releasing as a new version). Anyway, I am finally almost rolling it out to the largest site, which has hundreds of groups with thousands of users. I've started testing using this production environment with existing groups and a few things have come up regarding notifications.

First off og2list didn't support og_uid_global and was overriding the og_manage form, hijacking the submit button if your notifications were set to always or never. I fixed all that up, copying og's behaviour and got it all working nicely (there are 3 patches to og2list head I made that clean all this stuff up).

Now I am looking at notification problems... I turned on og2list and all the existing group notifications stopped. Looking at og, it checks with a module_exist( ) to see if og2list is present, and if so none of the notifications happen any more . I guess in 4.7 or before og2list was doing an equivalent job of these notifications but compared to what og 3.x is doing it is terrible. (maybe old versions of og did all those nice links at the bottom etc too? I suspect not and that is why og2list is still handling it).

Anyway - I've come to the conclusion that a patch to og is also going to be required for me to get this working right. The plan I have is to modify those checks for og2list, so that og is only not doing the notifications if ALL of the following are true:
- og2list module enabled
- current node (or node being commented on if a comment) is type=forum
- current group for current node/comemnt has an og2list list (i.e. $og_node->og2list_recipient not empty)

I am going to do the same thing on the og2list side, it will only generate emails for content that is an og2list forum message or reply.

Currently what is happening is this:
- og notifications stop for all existing groups with subscribed users.
- og2list sends notifications for my og2list enabled groups but not existing groups and subscribed users (I still haven't figured out why, more debugging needed)
- og2list notifications don't have any of those nice links at the bottom that og is adding.

I added some links as a feature to my custom og2list, but again I really only want these showing up for list content.

I'm creating this issue to see what you think (Moshe) and also to eventually hold the patch if I'm correct in thinking we have to let og have back most of the notifications.

CommentFileSizeAuthor
#2 og_hook_og_notify.patch2.22 KBdamien_vancouver

Comments

moshe weitzman’s picture

moshe weitzman commented
Category: bug » feature

makes sense. for the og patch, i want to simply call a hook and if any module returns TRUE, og skips notification. Here i some pseudocode:


foreach(module_implements(og_notify) as $module) {
  if module_invoke($module, 'og_notify') {
    $skip = TRUE;
    break;
  }
}

so all your logic will reside in a new og2list_og_notify()

damien_vancouver’s picture

damien_vancouver commented
Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new og_hook_og_notify.patch2.22 KB

OK, this is at last ready and tested on my end... works beautifully.

Moshe, I implemented it exactly as you suggested.

the attached patch replaces the two spots with a module_enabled('og2list') to instead call the new hook_og_notify(). The hook just sends the $node as an argument, note that this must be the original node not the comment's edit form (reason being is that og2list looks at the parent node to figure out whether to accept notifications).

There is a sister issue for the og2list patch here: http://drupal.org/node/165299

It contains a LOT more detail on replicating the problem, testing the patches, etc etc.

Hopefully this og side of things is straightforward enough that you may not have to go through all that, merely test that nothing is blown up. I have tested it in both of my current versions of og2list and it is now working properly!

attached patch was created and then tested against og v1.367.

If we could get this committed sometime soon (say in the next week or so), it would allow us to finally release a production ready 5.x og2list.. the og2list_hook_og_notify.patch from the sister issue and one other outstanding patch need testing and to make it to RTBC and then we can finally release 5.x-1.0.

thanks! Let me know if there are any changes I need to make, if I can help with further testing or the setup of a test environment if you want to test it alongside og2list.

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Fixed

committed. thx.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)