Mailhandler cannot assign emailed content to group
struesda - April 16, 2007 - 03:18
| Project: | Organic groups |
| Version: | 6.x-1.1 |
| Component: | og.module |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Description
When I add the og_groups: [2545] command to my mailhander mailbox - it still does not assign the created node to the group.
After some debugging it seems to be in the og_submit_group() function where it has code to 'Normalize the groups' in this property.
Since the actual id (2545 above) comes in as a value in the array - not as a key - the array_keys() function call strips away these values and leaves only the keys - which do not correspond with any groups.
If I remove this array_keys() call - things seem to work fine.
I will be submitting this as a patch shortly - but I wanted to do a quick check to make sure I wasn't missing anything.
#1
where's the patch :)
#2
ok - I'm very much a slacker who got too busy with other things! Here's the patch!
#3
nevermind - apparently there is a problem with file attachments - I will try again later.
#4
trying to attach the patch again
#5
there is some (interesting) extraneous code in og_nodeapi() in the patch. what are you up to :).
i would actually like to attack this differently. i want to make it possible to use associative arrays in mailhandler commands. the syntax for that should be the identical as our .info files. for example, see the $region example at http://drupal.org/node/171205.
so, at the beginning of mailhandler_process_message() we should actually write the contents of the commands variable along with any commands in the email to the temp dir and then pass that filename to drupal_parse_info_file() which will hand back the $data array we need. then we merge with $node and proceed.
AFAICT, the .info syntax is backwards compatible with what we have now so no update is required. nice.
#6
Now that I look back at it I see that extra stuff in og_nodeapi() - but I don't remember putting it there! (of course I've slept a lot since then). Perhaps it was something that the og2list README said to put in there - I remember that module required too much manual installation.
The associative arrays ideas sounds big improvement. Would there be a way to add a drupal_parse_info_string() function - rather than incur the overhead of file IO for this kind of thing? I don't know much about the .info code - but I would think that the logic would be the same - just the input source would be different.
UPDATE: I just took a quick peek at the drupal_parse_info_file() code - and it would be pretty easy to:
- move most of the code to a drupal_parse_info_string() function
- change drupal_parse_info_file() to
$data = file_get_contents($filename);return drupal_parse_info_string($data);
Then you get both - plus you can easily build other inputs as well (parse_xml..parse_array..parse_db_select ...)
#7
i think that would be a good improvement for core, but it should not slow down my patch. retrieving messages is a seldom operation and we only have to write the commands once per retrieve so performance is a non issue.
#8
So is this .info feature in drupal 6 only?
If so - then for 4.7/5 do we keep with this version of the patch?
#9
no, we have .info in 5 as well. i don't expect any changes for 4.7
#10
watching
#11
Is this patch applied to the latest release of OG?
#12
OK. Been at this for more than a few hours now. Trying to get mailhandler to work in D6. Using the og_groups: [x] command was not working until I commented out the line as per the patch above. I am trying to understand what is happening in this function from the latest D6 version of og.module:
function og_presave_group(&$node) {
if (!empty($node->og_groups_inaccessible)) {
// Add the inaccessible groups which did not show in Audience selector
$node->og_groups = (array)$node->og_groups + $node->og_groups_inaccessible;//note 1
}
/**
* Change $node->theme to $node->og_theme so it matches node_load(). The node form uses $theme, not $og_theme.
* If author chose the default theme, then '' is written to DB and group follows changes made by site admin.
*/
if (isset($node->theme)) {
$node->og_theme = $node->theme;
}
else {
$node->og_theme = NULL;
}
// Normalize og_groups array
if (isset($node->og_groups)) {
$node->og_groups = array_filter($node->og_groups);//note 2
//$node->og_groups = array_keys($node->og_groups);//note 3
}
// Support devel module's bulk node generation.
// Affiliate group posts with group(s). Also populate special group fields.
if (isset($node->devel_generate)) {
og_devel_generate($node);
}
}
Note 1: It seems we are using short hand to cast an objects properties to an array and merge at same time?
Note 2: Then filter the array against what we set earlier, which appears to be designed to remove null values?
Note 3: This is the line that gets commented out so we return values and get it to work with mailhandler.
Sorry if these are such stupid questions but it's late and I am really trying to grok this function.
I know Moshe has plans to work this another way but if this fix works and does not cause unintended consequences it could be a good way for a bunch of us to get by in the short term.
Thoughts please.
Thanks,
Coby
#13
changing it to ANMI since I also changed the version to 6. I will create a patch if I can get some confirmation on the implications of the above fix. I have already tested it and know it works in the mailhandler use case.
#14
Here is what I have so far:
presave is the new submit. It allows a better workflow for modules to perform action(s) prior to save.
if (!empty($node->og_groups_inaccessible)) {// Add the inaccessible groups which did not show in Audience selector
$node->og_groups = (array)$node->og_groups + $node->og_groups_inaccessible;
}
This is for a node that may have multiple groups (audiences); some of which may not be available to the current user. This should not be the case for Mailhandler as you will only be allowed to save a node if you have permissions to the group declared in the email command. This is more of an issue on update, so perhaps we have issues if someone is trying to use the subject line to edit the node.
// Normalize og_groups arrayif (isset($node->og_groups)) {
$node->og_groups = array_filter($node->og_groups);
Is useful to get rid of null values. Should not have any if we are using mailhandler. But I guess it's possible.
$node->og_groups = array_keys($node->og_groups);}
This is the problem line. As mentioned by @struesda above the keys in this array are not going to be helpful - we want the values. So, was this a way to make sure we have the correct number of dimensions for the object properties? Is this a problem in the way Mailhandler is passing og_groups? Or is this a bug in OG that needs to be patched?
Thanks,
Coby
#15
line 1447 in og.module
//$node->og_groups = array_keys($node->og_groups);still needs to be commented out to get mailhandler to work with og
#16
Mailhandler to og is working for me in 6.9 with the commented out array_keys line, described above. However, all new messages processed by mailhandler are being flagged as Public. Is there a mailhandler command I can use to change this?
#17
Yep, just did it myself and it seems to be working. Has anyone figured out why that line needs to be in there?
#18
subscribe
#19
Adding a mailhandler command
og_public: 0seems to work for me.
#20
I'm using the following code to set the group IDs as both the key and value of the array, which is how OG likes it to be.
The code I'm using so far is this - you could put it in mymodule_mailhandler() if that makes more sense to you.
<?php
/**
* mailhandler assigns the values set with og_groups: [1,12,123] as
*
* $node->og_groups = array( 1, 12, 123 ) ;
*
* however, og wants these values to have the gid as the key and value, eg
*
* $node->og_groups = array(
* 1 => 1,
* 12 => 12,
* 123 => 123
* ) ;
*
*/
function og_mailhandler($node, $result, $i, $header, $mailbox) {
if ( isset($node->og_groups) && is_array($node->og_groups) ) {
$node->og_groups = array_combine($node->og_groups, $node->og_groups));
}
return $node ;
}
?>
#21
EDIT: do not use the patch attached to this comment - it contains a typo, re-posting fixed version in next comment ...
#22
fixed typo, pls review