Allow forum topics to be custom node types

a pat ch which givces this functionality has a pretty good chance at landing in core. an analogous patch for blog has been submitted and well received .... i haven't looked at this one in particular.

the UI issues here are just as important as the geek issues (i.e. any node can be part of the forum)

Similar work was started in 2004 as 'akvaforum' - separate module, where forum structure may include many terms and many vocabularies. Also any nodetypes may be forum topics. Module works faster than standard forum module with less queries to db.
Now it some uncomplete (navigation links and breadcrumbs missed) and works for 4.5. Anyone interested for such forum for 4.6?

Contribs CVS: modules/akvaforum
http://www.drupal.ru/node/447 - some screenshots

Similar work was started in 2004 as 'akvaforum' - separate module, where forum structure may include many terms and many vocabularies. Also any nodetypes may be forum topics. Module works faster than standard forum module with less queries to db.
Now it some uncomplete (navigation links and breadcrumbs missed) and works for 4.5. Anyone interested for such forum for 4.6?

Contribs CVS: modules/akvaforum
http://www.drupal.ru/node/447 - some screenshots

Looks interesting.

1. We should look into the UI issues. The UI affects all users, the code just affects developers. For each developer, there are at least a thousand users so let's make sure it is 100% intuitive.

2. I don't like the fact non-forum nodes are still treated as a special case -- the forum_nodeapi() has special treatment for non-forum nodes. Can't we tidy up that some more?

3. Use proper PHPDoc comments to document forum_nodeapi().

4. Empty functions can be removed.

5. Did you measure how this affects the performance of (large) forums?

One of the things i realized during DrupalCon , is that we can generalize a lot of functionality into providing different 'views' of nodes.
ie :
an event calendar, is a listing of nodes that have atleast a start date.
a blog page, is a listing of nodes that have the blog bit set (or are assigned to a specific blog, for true multi-author blogs).
a forum is a listing of nodes that are assigned to a forum term (of course this is the most complex of the examples).

I don't know if this helps any, but perhaps we need to look at providing a node listing API that can handle different sources, and displays.

Patch does not apply anymore:
3 out of 14 hunks failed

Moving this to 6.x-dev queue. The patch is old and outdated, but the issue remains, and a new patch for 6.x would still be welcome.

Marked http://drupal.org/node/30980 as duplicate.

Going to try and work on this today...

forum.schema can be just deleted, as forum table is no more used up. the patch should include and update function as well, but in the next patch :)

taxonomy module in head seems really suffering from break-a-boo bug, so, first goto example.com/drupal/?q=forum once so that forum vocabulary is created, only then:run the following once only (in a module or wherever you can run php in drupal) to tell taxonomy that we have a term

db_query("INSERT INTO {vocabulary_node_types} (vid, type) VALUES (1, 'story')"); here vid is the vocabulary ID, it's going to be 1 if you are running nothing other than forum module after clean installation. And we are making 'story' node type to be forum vocabulary's one of the node type.

now create a new node, without selecting any category(taxonomy module is broken at that part), and run the following once only when you have created your node:

db_query('INSERT INTO {term_node} (nid, vid, tid) VALUES (1, 1, 1)'); remember to change nid tid and vid values, here vid is usually equals to nid, unless you have revisions.

First, thanks so much for picking this up!!!

What about moving the code to create the Forum vocabulary into hook_install? Would that help alleviate some of this weirdness?

OK I think my patch at http://drupal.org/node/149327 is a duplicate, and this seems more developed. Can someone confirm this before I mark as such?

Also this should bear in mind: http://drupal.org/node/145353

either way, subscribing.

Vocabulary now created during hook_install() is processed, and it's deleted in hook_uninstall(). Now you don't need to run those weird commands as in #11, a workaround fixing taxonomy.module is included in patch for easy testing, and it'll removed from patch whenever code goes RTBC, or that issue is fixed up.

Review!

There's currently some brokenness in HEAD which makes this a bit difficult to test... for instance, you can't edit/delete any forum or container. And poll (the content type that most people are likely going to want to put in forums) is completely broken atm.

However, aside from that, I did some cursory testing and this looks really good.

Some code-wise things:

 /**
  * Implementation of hook_nodeapi().
+ *
+ * Operations
+ * -validate Check in particular that only a "leaf" term in the associated taxonomy
+ * vocabulary is selected, not a "container" term.
+ * -submit Assign forum taxonomy when adding a topic from within a forum.
+ Operations
+ * -validate Check in particular that only a "leaf" term in the associated taxonomy
+ * vocabulary is selected, not a "container" term.
+ * -submit Assign forum taxonomy when adding a topic from within a forum.

This looks odd... there are many more ops than just submit and validate being covered here. So I'd stick with just "Implementation of hook_nodeapi()." and move the comments here in-line to the validate and submit ops.

   $form['#submit'][] = 'forum_form_submit';
-  $form['#validate'][] = 'forum_form_validate';

Why is validate removed and not submit?

-    $node->taxonomy[arg(3)]->vid = _forum_get_vid();
+    $node->taxonomy[arg(3)]->vid = variable_get('forum_nav_vocabulary', '');

Why get rid of a handy API function in favour of more typing?

+        // NOTE: Just workaround code(for the $form_state argument) to test this patch.
+        // This needs work, in a separate issue.

Is this actually fixing anything apart from remove some notices? If not, don't worry about it. HEAD has notices all over the place, including editing taxonomy vocabularies, etc. :P

Thanks for the review!!

#1 ok.. will put them as comments near to the code.

#2 validate hooks were removed because they don't exist and we don't need to. submit hook is needed becasue forms for container and forum are different.

#3 _forum_get_vid() has been removed, as it's purpose was just to return vid from variable, if it's not present, create a new vocabulary for forum module. but we moved that part to hook_install().

#4 // NOTE: This($temp_array) is a workaround to get taxonomy module to work for forums. This is to pass the form state first argument, empty array. otherwise it won't let you choose a category when adding/editing node. that needs separate work as discussed on irc if you remember.

(there was another function which was removed, it was just some dead code.)

Thanks for the review again!, will submit updated one once flat forum patch is submitted ;)

Re-roll. Now with system_update_N(). forum.schema needs to be deleted when committing this patch. Also added node type check in hook_nodeapi().

Manually apply this diff to taxonomy.module to get taxonomy module working (Workaround code for the $form_state argument to test this patch) for term selection on node add/edit forms:

-        $form['taxonomy'][$vocabulary->vid] = taxonomy_form($vocabulary->vid, array_keys($default_terms), $vocabulary->help);
+        $temp_array = array();
+        $form['taxonomy'][$vocabulary->vid] = taxonomy_form($temp_array, $vocabulary->vid, array_keys($default_terms), $vocabulary->help);

Please review....

I spent 15 minutes looking at the code and it looks sound to me. It's amazing how few changes were required to make this work, and it might actually be a tad faster as well. Good job! :) This looks promising, and worthy a CHANGELOG.txt entry.

I've no comments regarding coding style. I haven't tested the patch yet, but I'll do that soon. I'll wait for webchick to confirm that you made the changes she requested. Webchick can mark this RTBC when see things this is ready to go in.

Sorry the last patch included the manual diff part(#17).

1 doubt: search for in forum_form_alter (the condition's else part):

  if ($form_id == 'taxonomy_form_vocabulary') {
    if (isset($form['vid']) && $form['vid']['#value'] == $vid) {
      // .... Code to disable certain fields for forum vocabulary....
    }
    else {
      unset($form['nodes']['forum']);
    }
  }

Is the else part meant to disallow other vocabularies to assign forum node type to themselves? Don't know what it is meant for, but we have other vocabularies having forum node type on drupal.org itself. Moreover, that unset() doesn't act in any way, although $form[....] = array(); works, and disallows selecting forum node type in other vocabulary add/edit forms. Patch for this was from http://drupal.org/node/28625

So, does this needs to be removed, or..?

In reading the old issue, what that code appears to be doing is preventing a very strange condition where the forum vocabulary gets set to "free tagging" or some other kind, which breaks the hierarchy forum requires and thus causes the forum to break.

With the forum vocabulary being created at install now, and with the forum table being obliterated, we probably can indeed ditch this condition.

This is a re-roll with that condition removed and the CHANGELOG.txt entry added (I made a section for "Forum improvements" in the hope that we can get more of your great patches in ;)). I also had to rename system_update_6024 to system_update_6025.

Two remaining TODOs in my mind, although these could probably also be pushed to post-code freeze usability improvements if they're a huge pain in the butt:

1) It might make more sense for people if the "Forum content types" selection was mirrored on admin/content/forum/settings. It's not very intuitive for people to go under "Categories" to change forum configuration.
2) This code:

    if (user_access('create forum topics')) {
      $output .= '<li>'. l(t('Post new forum topic.'), "node/add/forum/$tid") .'</li>';
    }

should be expanded to something like:

$vocabulary = taxonomy_get_vocabulary($forum_vid);
$forum_types = array();
foreach ($vocabulary->types as $type) {
  if (node_access('create', $type)) {
     $forum_types[] = l(t('Post new $type_name.'), "node/add/$type/$tid");
  }
}
$output .= theme('list', $forum_types);

Btw, http://drupal.org/node/150299 is a fix for the critical taxonomy bug that prevents you from choosing a forum.

Oops. Forgot the patch. :P

1. Issue #150299 has been committed. That should make it easier to test this patch. :)

2. I'd suggest that -- as part of this patch -- we re-vise the forum module's help text. It's probably a good idea to add a few sentences to the help text on the administration pages. These sentences could link to the node type configuration page. Second, we should also document in the context-sensitive help that, for example, polls can be added to forums as well.

3. The main gripe I have with this patch is that it is not clear for _visitors_ (not administrators) that they can add a poll to the forums. On ?q=forum, maybe we need to add "Add new poll to forums" if the poll type has been bind to the forum vocabulary. This seems like a must have.

To make a long story short:
1. Communicate to the administrators that they can allow any node type to be posted as part of a forum!
2. Communicate to the visitor that they can post other node types as forum topics.

I think both are fairly straightforward to implement. Other than that, this patch looks RTBC.

I haven't read this entire issue... Just peek in now and then, so this is just my UI thoughts on Dries' #3:

On every forum I've been on that allows polls in the forums, you have two links/buttons: add topic and add poll. Since poll is a really common thing to add to a forum, I think it would be a good idea to have the link there to be turned on either automatically or via a setting. Personally, I lean towards a setting, so you can define exactly what is being considered as adding a poll. (That way you're not limited to the core poll module or to the content type being named poll or whatever mechanism would be there for auto detection)

Another step beyond that, though probably too much feature creep for right now, would be to add something into the content type config where you can set not only that this goes in forums but also the text of the "add link". Then any content type that had text filled in for the "add link" would automatically get a link at the top of the forum to add that type.

Michelle

More UI thoughts... I hope I'm not overstepping by posting this when I don't know the underlying code, but sometimes UI input can be helpful. :)

Anyway, one thing I was thinking about is that an admin may want role X to be able to add content type Y to a forum but not role Z. This could be feature creep or could be an alternate way of configging it. What if the forum module allowed any existing content type by default? And then each content type would get an additional permission: "use X in forums". So rather than having an option on the content type to use it in forums, it would all be permission based. This would be separate from the permission to create the content type in general. So if a person didn't have the "use X in forums" permission but did have the "create X" permission, they could create it in the normal way. If they have the "use X in forums" permission, they get the forum selection dropdown (with an option for "none" to create it the normal way) on their form.

Back to the previous post, the links at the top could loop through all content types that a person has permission to put in the forum and put a link to it at the top.

Michelle

This could be feature creep or could be an alternate way of configging it. What if the forum module allowed any existing content type by default? And then each content type would get an additional permission: "use X in forums". So rather than having an option on the content type to use it in forums, it would all be permission based.

For a while I've been thinking it'd be lovely to have taxonomy vocabularies by role (so free tagging is restricted to an editors role etc.). I have no idea what the UI for "vocabulary by role by content type" would look like, but if I'm reading you right, it'd allow for "use X in forums" type permissions alongside some other handy uses. Reckon that should go in a second issue though.

I think adding access control around content types in forums is scope creep at this point. An easy way to handle that is just to create two content types: poll and forum poll or something (either copy/paste poll.module or use poll CCK field). Use the normal access permissions like you would any other node type. And adding *all* nodes to forums by default would be totally confusing... what does it mean to post a "page" to a forum (or an "Album" or a "Customer"? ;)).

I also think it's odd to enable content types to be posted in forums and NOT have labels for them on the "Post new X" listing. So I'd say let's keep it simple. Not sure how to handle the "people who have forum and poll modules enabled probably want the ability to post polls in forums" thing. It seems weird to make an exception for poll, although I agree that most people are going to want to do that.

Marking CNW for the text and link changes.

This is probably out of scope, but yet another approach to the permissions-by-content type problem is this one: have a CCK poll field in the forum content type and use it whenever desired... No multiple content types required then. All you would need then to allow/disallow the ability to create polls, is to use CCK Field Permissions.

Of course, this would need 1) a poll CCK field, which there isn't yet, let alone that this would be in core and 2) the CCK Field Permissions module would have to be moved to core.
Or alternatively, you could say that polls in forums are not possible unless you install these two modules (but the poll CCK field doesn't exist yet of course).

In the end, this approach (for core) would only be possible if more CCK modules are included in Drupal core. And that wouldn't make it before the code freeze.

This one accomplishes the two points by Dries(using the code by webchick):

1. Communicate to the administrators that they can allow any node type to be posted as part of a forum!
-- Screen shot: http://img224.imageshack.us/img224/4766/forumsdrupal11822335145qb9.png
-- Maybe this is a basic notification of possibility, any other possible aspect? webchick?
2. Communicate to the visitor that they can post other node types as forum topics.
-- Screen shot: http://img264.imageshack.us/img264/2925/forumsdrupal11822310147jk9.png

And there's room for further improvement. "Forum topic" node type should not be affixed to forum vocabulary. Rather we can have a check to the validate handler for the vocabulary edit form, that at least 1 node type is selected. So it can serve for the cases where user prepares his own content types which can be posted in forums, say "Product" (for sale), "Poll", etc. This is how the selection form is currently: http://img187.imageshack.us/img187/4479/homedrupal1182233753230dn5.png

And the proposal is to make Forum topic optional, AND add a validation handler to check that at least 1 node type is selected.

For the permissions stuff, there's lots more scope of features that arise, like particular node types per forum/container, etc. Maybe it would be worth a look after this patch gets in.

Sorry if there's a silly question, but is there some reason why forum node types have to be so special? I don't understand why it can't simply be changed so that anything tagged with a forum vocab shows up in the forum. The UI to tag something with a forum term is already there; it just doesn't work as far as making it show up in the forum. Why not just make it work? There's been times where I've written a story and later decided I'd like it to go in a particular forum. It would be nice to simply add another tag to it.

I'll admit I don't know the code in forum.module so maybe I'm just being over simplistic... Just seems from my POV that this thread is over complicating things. :)

Michelle

I can't test the patch, but the screenshots look great. Thanks.

"And the proposal is to make Forum topic optional, AND add a validation handler to check that at least 1 node type is selected."

Sounds like a good plan to me! That would address Michelle's question about why forum types have to be so specific, and it would allow people to delete the forum topic node type entirely if they wanted to.

-      $form['nodes']['forum'] = array('#type' => 'checkbox', '#value' => 1, '#title' => t('forum topic'), '#attributes' => array('disabled' => '' ), '#description' => t('forum topic is affixed to the forum vocabulary.'));
+      $form['nodes']['#required'] = TRUE;

This is the extra change included in this patch, making the content types field "required".

To the concern about Michelle's question, to allow only permitted users to post certain content types "into forums", irrespective of the fact that they can post it out of forums. Is this right? In other words, "post X in forums" permissions makes such sense to me..

If others have a view on this, maybe this can be included? Otherwise this patch is RTBC, hopefully.

One more question which might be answered had I actually tested the code, sorry if it's a stupid one.

Forum vocabularies currently produce listings at forum/123 and taxonomy/term/123 - same for rss feeds. Since this patch removes the forum table and to an extent brings the forum vocabulary closer to a standard taxonomy vocabulary - I'm guessing it doesn't remove those duplicate feeds?

OK I just set up a brand spanking new drupal 6 installation and am reviewing now. Excuse any comments not related to the patch, will just post as I find.

* patch doesn't apply
patching file CHANGELOG.txt
Hunk #1 FAILED at 57.

* update.php gives notice: Undefined index: id in /home/mysite/public_html/drupal6/includes/form.inc on line 1929. - that's not the fault of this patch though I assume. update runs fine, just dropping forum table.

* I added a container in forum admin, then I wanted to change the name of it, I got

Fatal error: Call to undefined function forum_term_load() in /home/mysite/public_html/drupal6/includes/menu.inc on line 336

when I tried to edit
* admin/help/forum needs updating - will try to submit new text in a new issue, not relevant to this issue
* I enabled poll.module and went to admin/content/forum/settings to enable it, then followed the link to the forum vocabulary - taking me to: admin/content/taxonomy/edit/vocabulary/ - I got this

    * notice: Undefined index: edit in /home/mysite/public_html/drupal6/modules/taxonomy/taxonomy.module on line 1160.
    * notice: Trying to get property of non-object in /home/mysite/public_html/drupal6/modules/taxonomy/taxonomy.module on line 193.
    * notice: Trying to get property of non-object in /home/mysite/public_html/drupal6/modules/taxonomy/taxonomy.module on line 199.

Now I'm getting: notice: Undefined index: edit in /home/mysite/public_html/drupal6/modules/taxonomy/taxonomy.module on line 1160. on every page.

admin/content/forum and admin/content/taxonomy/edit/vocabulary/ are almost the same, except the vocabulary page doesn't give me the add forum, add container tabs.

So now I actually made a vocabulary for forums in admin >> categories.

go back to admin/content/forum and the notices have gone. But going to admin/content/forum/edit/container/7 just refreshes the page - no edit screen now.

if I go back to admin/content/taxonomy/edit/vocabulary/ I get the same notices again.

OK now I try to create a forum topic:

warning: in_array() [function.in-array]: Wrong datatype for second argument in /home/mysite/public_html/drupal6/modules/forum/forum.module on line 132.

Going to wipe db and try again doing things in a different order.

OK I started again, this time with a pre-patched drupal install, installation went smoothly.

This time I created a vocabulary before I did anything with forum containers, /admin/content/forum/edit/container/1 isn't a valid url, and nor is http://drupal6.libcom.org/admin/content/forum/edit/forum/2 - seems like the arguments are wrong?

However, although I still get undefined index errors (op, confirm, taxonomy.module 1442), I can now select node types at admin/content/taxonomy/edit/vocabulary/2 - seems like that doesn't work unless there's already another vocabulary created before you add forums maybe?

I can add nodes of type forum, poll and customforum to forums with no issues. forum_topic_navigation (which I despise, we have a blank .tpl.php file which kills it) works for all three node types, which is good.

Overall, it looks really good, and it seems most of the errors are with taxonomy and not due to this patch.

It's a shame the node-type selection isn't on the forums settings page, and I reckon

Any node type, for example Poll, can be affixed to the forum system, from the forum vocabulary edit form.

could be reworded.

Maybe

"You can choose which content types to use in forums (for example Poll), by editing options for the forum vocabulary".

This removes 'node' which doesn't seem to be used in most help texts.

Marking to CNW for the hunk error, it seems like most of the errors are down to taxonomy.module rather than this patch. So unless I'm wrong about this, a reroll for changelog.txt and I reckon this is really close to RTBC.

actually marking as code needs work for the hunk error

Reroll.

Last one was another patch, sorry.

my steps to failure:
1. download drupal cvs
2. create a db
3. apply the patch
4. install drupal
5. enable forum module

No luck. WSOD on the module screen

fwiw I'm getting:

Parse error: syntax error, unexpected T_DOUBLE_ARROW in forum.module on line 820

This looks interesting but:

- it seems not all of the SQLs include a term to limit the query to a certain forum category, so they end up querying all terms/nodes without the forum node type limit... like "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 GROUP BY r.tid"
- are the cached term lists really that performant (in the nodeapi hook)?
- why add the update to the system.install file?

I guess that means...

- Modified the two block queries so they check for vid = forum_nav_vocabulary.
- Moved the update hook to forum.install.

- are the cached term lists really that performant (in the nodeapi hook)?

Sorry, I don't know what this means. :( Marking CNR anyway, as the only remaining critical bug (the non-forum types showing up in the forum blocks) has been addressed.

Functionally, this patch is RTBC. I think it's important to note that the best application for this is NOT necessarily making existing types like 'story' or 'poll' postable in forums, unless EVERY node of those types is intended to go int other forums. Rather, I think it's most useful for creating CCK types that can go into forums, or making 'forum_announcement' content types with restricted creation permissions and different defaults (always sticky, can only be created by moderators, doesnt' allow comments, etc).

With that caveat (which is more about education than functionality), I like it. It's another step towards making the drupal forum system more useful and more flexible, like many of the other existing forum packages on the market.

I looked at the code and it looks like a good step forward.

I'll test the code later today -- doesn't this need an upgrade path?

Actually, the only upgrade path needed is the removal of the forum table, which is done in forum.install. This table is unnecessary because the relations are already stored in term_node.

Also note that there are a ton of notices and other errors, but they are there even without this patch. There's a patch somewhere back in the queue that fixes those.

Yeah I can confirm that those notices are independent of the patch (and apologies for copying them into the issue and muddying any waters) - looked at it side by side with a patched and unpatched version, and the patch doesn't add any extra.

This patch was pretty much ready before the code freeze so I'm letting it slip in. Committed to CVS HEAD.

For the permissions stuff, there's lots more scope of features that arise, like particular node types per forum/container, etc. Maybe it would be worth a look after this patch gets in.

This would certainly be an interesting feature, was looking for something that would allow one to specify one content types for one forum and another for another forum.
A search showed up a few others who have been enquiring about this.
http://drupal.org/node/368733
and
http://drupal.org/node/364921

Any updates would be welcome.

@captcha: This patch was committed and the issue closed 1.5 years ago. If you want to start up a new issue, please make a new issue for it.

Michelle