Posted by spiderman on June 23, 2008 at 5:32pm
| Project: | Discuss This! |
| Version: | 5.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | spiderman |
| Status: | needs work |
Issue Summary
as pointed out by toemaz, the hard-coded text with which the module auto-creates forum topics should be configurable. my plan is to allow the admin to choose a prefix for the subject (to be followed by the original node's title), and some kind of "template" text (using token.module, perhaps?) for the subject. suggestions and patches welcome :)
Comments
#1
Suggestion: don't the text in the node->body, but simply use hook_view or nodeapi to add a field to the $content array with the text inside. That way the text can be localized as well.
#2
I'm not sure what you mean here- I think you missed a verb in the phrase "don't the text in the node->body", but I'm also not convinced you understand what i'm talking about (no disrespect, I think we might just have a miscommunication).
At present, when the user clicks the Discuss This! link for the first time, it triggers the discussthis_new function, which in turn calls _discussthis_new_topic. This function uses drupal_execute to create a new forum node, and fills in the title/body of this node on behalf of the author the admin selected. How would hook_view or nodeapi help me here?
Thanks,
Derek
#3
As of commit #123181 I've added token.module support and added admin-configurable custom templates for the title and body of the auto-created forum topics.
#4
<?php$default_body = 'Following is a discussion on the [node-type-name] item titled: [node-link]'.
'.<br /> Below is the discussion so far. Feel free to add your own comments!<br />';
$values['body'] = token_replace(variable_get('discussthis_newtemplate',$default_body), 'discussthis', $node);
?>
My proposal: leave the body empty ($values['body'] = '') and use hook_nodeapi instead in this module to add an extra $content['discuss_this'] to the $node when $op == 'view' and when $node->type == 'forum'.
Advantages:
#5
ah, ok- i think i see what you mean now.. will try to incorporate this today, but i would be most happy if you wanted to roll a patch to this effect :)
#6
Well, to be honest, I wont be using the module because it doesn't fit my needs. I was just having fun reviewing it ;-)
#7
Hey, no worries- I appreciate the feedback nonetheless :)
#8
Automatically closed -- issue fixed for two weeks with no activity.
#9
I'm attaching a patch that implements #4, in case you're still interested in the idea. It seemed like a good way to make the default easy to override, and the point about avoiding the search index is well taken.
#10
Um, duh. It would be nice if the patch didn't mess with the node body on forum topics that discussthis didn't create. New patch attached.
#11
@ksenzee: appreciate the patch, but on applying it, I've found it seems to create more bugs :(
First, the $values['body'] = '' produced an error complaining about the body field being required. When I put in a instead, it ended up adding extra whitespace to the display of the forum topic itself.
Second, I found I was getting an error about duplicate term_node entries being inserted, but didn't track down where this was coming from.
All of this highlights for me the desperate need to re-work the node creation mechanism, as it is clearly very fragile and inelegant for the moment. I'm going to give this some love in the next few days, but if you have any ideas, please feel welcome to throw them out!
Thanks!
#12
Correction: the duplicate term_node insert warning was due to another problem, which is now fixed. Unfortunately, this problem also highlights the fragility of the node creation process, which is now at the top of my hitlist ;)