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 :)

CommentFileSizeAuthor
#10 273886_forum_body_nodeapi.patch2.13 KBksenzee
#9 273886_forum_body_nodeapi.patch1.69 KBksenzee

Comments

toemaz’s picture

toemaz commented

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.

spiderman’s picture

spiderman
he/they
Primary language English
Location Halifax, NS
commented

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

spiderman’s picture

spiderman
he/they
Primary language English
Location Halifax, NS
commented
Status: Active » Fixed

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.

toemaz’s picture

toemaz commented 
  $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:

  1. the discussthis_newtemplate can be localized (translated)
  2. it's doesn't fill up the search index
  3. another developer can still decide to change the theming or even discard the $content['discuss_this'] field
spiderman’s picture

spiderman
he/they
Primary language English
Location Halifax, NS
commented

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 :)

toemaz’s picture

toemaz commented

Well, to be honest, I wont be using the module because it doesn't fit my needs. I was just having fun reviewing it ;-)

spiderman’s picture

spiderman
he/they
Primary language English
Location Halifax, NS
commented

Hey, no worries- I appreciate the feedback nonetheless :)

Anonymous’s picture

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

Automatically closed -- issue fixed for two weeks with no activity.

ksenzee’s picture

ksenzee
she/her
commented
Status: Closed (fixed) » Needs review
StatusFileSize
new 273886_forum_body_nodeapi.patch1.69 KB

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.

ksenzee’s picture

ksenzee
she/her
commented
StatusFileSize
new 273886_forum_body_nodeapi.patch2.13 KB

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.

spiderman’s picture

spiderman
he/they
Primary language English
Location Halifax, NS
commented
Status: Needs review » Needs work

@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!

spiderman’s picture

spiderman
he/they
Primary language English
Location Halifax, NS
commented

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 ;)

dom.’s picture

dom. commented
Issue summary: View changes
Status: Needs work » Closed (won't fix)

Module maintainer has changed. I'm closing this issue after years of inactivity, please open a new issue if needed.