There were changes in D6 taxonomy.module but forum.module was not updated for them. So here's a patch fixing the all(hopefully). Also it seems some of these updates should go into Drupal 5 too.

  • Arguments order for taxonomy_node_get_terms_by_vocabulary() was wrong.
  • Taxonomy term was not saved into {forum} when adding new forum post.
  • The query in forum_get_forums() was broken. And there was usage of $forum->tid which isn't defined. And it's so since September 2004!
  • 7 Notice errors fixed(including above one).
CommentFileSizeAuthor
#26 drupal6-forum_fix-1_1.patch3.1 KBGurpartap Singh
#25 drupal6-forum_fix-1_0.patch2.86 KBGurpartap Singh
#24 drupal6-forum_fix-1.patch1.88 KBGurpartap Singh
#22 drupal6-forum_fix_8.patch5.87 KBGurpartap Singh
#18 drupal6-forum_fix_7.patch5.89 KBGurpartap Singh
#15 drupal6-forum_fix_6.patch5.92 KBGurpartap Singh
#13 drupal6-forum_fix_5.patch6.1 KBGurpartap Singh
#7 drupal6-forum_fix_4.patch5.94 KBGurpartap Singh
#5 drupal6-forum_fix_3.patch6.2 KBGurpartap Singh
#4 drupal6-forum_fix_2.patch5.86 KBGurpartap Singh
#2 drupal6-forum_fix_1.patch5.85 KBGurpartap Singh
#1 drupal6-forum_fix_0.patch5.05 KBGurpartap Singh
drupal6-forum_fix.patch5.02 KBGurpartap Singh

Comments

Gurpartap Singh’s picture

Gurpartap Singh commented
StatusFileSize
new drupal6-forum_fix_0.patch5.05 KB

Missed a variable in the fix in hook_insert().

Gurpartap Singh’s picture

Gurpartap Singh commented
StatusFileSize
new drupal6-forum_fix_1.patch5.85 KB

This one fixes the code, for errors seen when no forums have been defined.

Gurpartap Singh’s picture

Gurpartap Singh commented

Sorry there was a variable typo.

Gurpartap Singh’s picture

Gurpartap Singh commented
StatusFileSize
new drupal6-forum_fix_2.patch5.86 KB

lol and here's the patch.

Gurpartap Singh’s picture

Gurpartap Singh commented
StatusFileSize
new drupal6-forum_fix_3.patch6.2 KB

Fixed container problems(a fapi3 and form defaults bug).

dries’s picture

dries commented

Care to explain the following blurbs?

1. 

-    $sql = "SELECT r.tid, COUNT(n.nid) AS topic_count, SUM(l.comment_count) AS comment_count FROM {node} n INNER JOIN {node_comment_statistics} l ON n.nid = l.nid INNER JOIN {term_node} r ON n.nid = r.nid WHERE n.status = 1 AND n.type = 'forum' GROUP BY r.tid";
+    $sql = "SELECT r.tid, COUNT(n.nid) AS topic_count, SUM(l.comment_count) AS comment_count FROM {term_node} r INNER JOIN {node} n ON r.nid = n.nid INNER JOIN {node_comment_statistics} l ON n.nid = l.nid WHERE n.status = 1 AND n.type = 'forum' GROUP BY r.tid";

2. 

-  db_query('INSERT INTO {forum} (nid, vid, tid) VALUES (%d, %d, %d)', $node->nid, $node->vid, $node->tid);
+  $vocab_id = _forum_get_vid();
+  db_query('INSERT INTO {forum} (nid, vid, tid) VALUES (%d, %d, %d)', $node->nid, $node->vid, $node->taxonomy[$vocab_id]);

Some explanation would help me, and others, review this patch. Thanks.

Gurpartap Singh’s picture

Gurpartap Singh commented
Title: Forum module heavily broken. » Broken forum.module
StatusFileSize
new drupal6-forum_fix_4.patch5.94 KB

That query was just rearranged, nothing great in that. For the tid, I'm not sure why in Drupal 6.x-dev, it doesn't assign $node->tid(it does so in Drupal 5.x). In other words, I don't know what else to do.

Gurpartap Singh’s picture

Gurpartap Singh commented
Status: Needs review » Needs work

Patch needs update for the latest forum.module in HEAD.

Gurpartap Singh’s picture

Gurpartap Singh commented

hook_submit() is not being run when a forum topic node is submitted, for some fapi3 changes reason. That is why $node->tid(assigbed in forum_submit()) is not available in forum_insert().

eaton’s picture

eaton commented

Am I mistaken, or could this be done in a #submit handler on the form itself?

In looking over the FAPI3 changes, it appears that we did in fact drop hook_submit. Er. Oops. :-)

In looking closer, though, the refactoring of node.module's form handling means that it would be totally possible to put #submit = 'form_submit' into the form definition and have it work fine... unless I'm missing something. If that is not workable, we can go back in and look at reinstating the 'submit' hook and op.

Any thoughts?

eaton’s picture

eaton commented

This also flushed out http://drupal.org/node/145390, a hitherto unnoticed FAPI3 bug in the node module. Whoops, and thanks! ;-)

Gurpartap Singh’s picture

Gurpartap Singh commented

Form submit handler would be better if it does all what hook_submit() has to provide.
And with that, submit handlers for node types are also broken, a patch for it's fix is here, by eaton: http://drupal.org/node/145390

Gurpartap Singh’s picture

Gurpartap Singh commented
Status: Needs work » Needs review
StatusFileSize
new drupal6-forum_fix_5.patch6.1 KB

This is the latest patch for forum.module. And it looks like fapi needs another patch, of which eaton is taking care. Array data(altered) in the submit handler was not being passed into hook_insert(). after that this patch should be running the module smoothly.. in case there is nothing else left out.

Highlighted patch here: http://pastebin.mozilla.org/66839

Gurpartap Singh’s picture

Gurpartap Singh commented

http://drupal.org/node/145390#comment-246474 After this patch to node.module, the above patch works well. Please review :-)

Gurpartap Singh’s picture

Gurpartap Singh commented
Title: Broken forum.module » Notice errors: forum.module
StatusFileSize
new drupal6-forum_fix_6.patch5.92 KB

Wrong variable was put to check in forum_get_forums(). Fixed in this patch. Please review..

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Priority: Normal » Critical
Status: Needs review » Reviewed & tested by the community

Since forum module atm is largely completely useless with this patch, and since it seemed to solve all the bugs I could find without introducing new ones, marking RTBC (also raising severity level, we need forum module working to squeeze more features in this weekend :)).

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: Notice errors: forum.module » forum.module is broken (notices, container issues, and other badness)
Gurpartap Singh’s picture

Gurpartap Singh commented
StatusFileSize
new drupal6-forum_fix_7.patch5.89 KB

This one with very slight change. Removes title = ''; from theme_forum_display().

pwolanin’s picture

pwolanin commented

Patch code looks fine and forums work- +1

morphir’s picture

morphir commented

+1

removes all errors I get.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Needs work

Patch no longer applies.

Gurpartap Singh’s picture

Gurpartap Singh commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new drupal6-forum_fix_8.patch5.87 KB
dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks Gurpartap. Keep rocking.

Gurpartap Singh’s picture

Gurpartap Singh commented
Title: forum.module is broken (notices, container issues, and other badness) » Use _forum_get_vid() for vocab. ID
Status: Fixed » Needs review
StatusFileSize
new drupal6-forum_fix-1.patch1.88 KB

There exists a function _forum_get_vid() which returns vocabulary ID from a variable, which if not set, then it finds directly from db and returns it. But forum module is currently directly using the vocabulary variable, than the proper function for this. This patch fixes it and hence closing the taxonomy module errors caused in forums.

Gurpartap Singh’s picture

Gurpartap Singh commented
StatusFileSize
new drupal6-forum_fix-1_0.patch2.86 KB

There was some more badness in forms, like validation handler was assigned which doesn't exist(and there's no need of it in that case). And fixed arguments in a submit handler for node form.

Gurpartap Singh’s picture

Gurpartap Singh commented
Title: Use _forum_get_vid() for vocab. ID » Use _forum_get_vid() for vocab. ID and other issues
StatusFileSize
new drupal6-forum_fix-1_1.patch3.1 KB

Also in forum_submit, we needed to set form_state['values'] so that changed/assigned form values can be passed to hook_insert.

Gurpartap Singh’s picture

Gurpartap Singh commented
Status: Needs review » Fixed

Marking back to fixed, as the some of the required changes have been done in this related patch: http://drupal.org/node/20295

Anonymous’s picture

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