Here's a patch to allow the 4.6 commands for taxonomy and og (http://drupal.org/node/40777#comment-123925) to work with cvs/4.7.

The bug arises because in 4.7 the $node->taxonomy replaces $node->tid (I think it's more complicated than this, but more or less...), and the $node->og_groups is no longer a simple array (the nids are the keys instead of the values).

It's all a bit hackish primarily due to the lack of support in drupal for programmatic creation of nodes, and I understand that the new and better way to link lists and organic groups doesn't require mailhandler, but I'm trying to do it anyway....

CommentFileSizeAuthor
mailhandlerfix.diff.txt1.09 KBadixon

Comments

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Needs work

too hackish

adixon’s picture

adixon commented

Can you be more specific? To me, the hackishness (or at least the danger) is in the concept itself of converting commands to node properties that are then fed to the node_save function (i.e., it depends on the implementation of those properties by the corresponding module, which can and does change the structure of those properties). I think my code (which converts an if to a switch and adds required changes for taxonomy and og) is a nice clear way to keep the code working within it's current limitations. If you're just referring to my array manipulations for the og case - let me know, i could probably improve that.

moshe weitzman’s picture

moshe weitzman commented

i refer to og and taxonomy specific code in the mailhandler module. what other modules will need special treatment are next? is there a pattern here that we can abstract?

adixon’s picture

adixon commented

Yes, that's exactly the core problem here. The 'command' concept takes key -> value pairs and converts them into node properties, with fingers crossed, and then passes them to the node_save function which invokes all the corresponding module hooks. So either there needs to be a standard for such key value pairs (which would allow programatic creation of nodes in this fashion), or we need to allow the modules to fix themselves.

In fact, mailhandler actually has a hook that's only used afaik by listhandler, to avoid loops. We could use that to allow the modules to convert the properties set by the command to the required (internal) module format that would be used in the hook_insert later on, then we wouldn't need that switch. So for example, we could throw out the og_groups part of this patch if we add this function to og:

function og_mailhandler($node){
     $tmp = array_flip($node->og_groups);
      foreach(array_keys($tmp) as $key) $tmp[$key] = TRUE;
     $node->og_groups = $tmp;
     return $node;
}

Does that sound like a better approach?

Gerhard - does that sound compatible with what you had intended with the _mailhandler hook?

killes@www.drop.org’s picture

killes@www.drop.org commented

the mailhandler hook is so old, I really don't recall my intentions for it. I guess I mainly wanted to get listhandler to work.

"The bug arises because in 4.7 the $node->taxonomy replaces $node->tid (I think it's more complicated than this, but more or less...),"

That's not quite true, taxonomy is used by the taxonomy module, whereas tid is used by forum module.

moshe weitzman’s picture

moshe weitzman commented

hey, i invented hook_mailhandler() :)

your og function is a better solution than this patch. alternatively, we could invent enhance the command syntax for brackets. i even have a comment suggesting this in the code. search for "//TODO: allow for nested arrays in commands". the syntax would be something like [key:value,key:value]

would be nicer to enhance bracket syntax so no new functions need be written.

zvizvi’s picture

zvizvi commented

hi,
Thanks!

I've used a version of the first patch on 5.7:

      if (!empty($data[0])) {
        if($data[0] == 'og_groups') {
          $tmp = array_flip($data[1]);
          foreach(array_keys($tmp) as $key) $tmp[$key] = TRUE;
          $data[1] = $tmp;
        }
        $node->$data[0] = $data[1];
      }

(replacing lines 311-313, sorry for not being able to patch)

probably "too hacky" too, but it works.
the group hook idea sounds fine, but not as a patch, meaning:
I'd rather loose transparent upgrade on mailhandler module than on og.
best would be, of course, that mailhandler would natively the og's new representation via expression language. but that wouldn't happen, would it?

cheers,
Zvika

z.stolar’s picture

z.stolar commented
Status: Needs work » Closed (fixed)

I think the better way is to implement hook_mailhandler in og, if it isn't already done.

The nested array idea is nice, though I'd be careful with /too complex/ options for the user, as it may lead to errors.

Closing the issue for now. Feel free to reopen if necessary.