Forums not useable out-of-the-box: disabled comments, no forums

I agree, Configuring the forum module could be made easier by having a default 'General Discussion' forum created to start.

We should make them easier to use; especially by configuring common settings by default.

I also think it's confusing you can add a forum topic, when no forum exists. See related issue: #221527: Don't show Create Forum topic (node/add/forum) when there isn't a forum

However, I didn't see your number 3: that comments were disabled by default?
Do you mean that permissions don't allow forum creation by authenticated users by default? That probably won't be changed.

Not sure I understand 3.

Add usability tag so other Usability team people can find this.

I just tested this out on a fresh install and was able to replicate #2. After enabling forum and attempting to add a new forum post, I received the validation error message that "Forums field is required."

Next, I added a term to my Forums vocabulary and was able to post without an error message.

After this, I tried to reproduce the error message "Forums field is required" by deleting the term I had added to my Forums vocabulary. However, when I clicked "edit" under Operations (screenshot attached), I am sent to the page listing all content tagged with that term. There doesn't appear to be any way to delete the forum taxonomy terms after you add them.

Finally, in reference to #3, my first forum topic had comments automatically enabled.

Sure, patch please.

A related issue, but it'd be really great if there was a system-wide setting to control whether modules were expected to create sample data when installed so that beginners had less of an "ok, now what" reaction upon installing something? This could be used to decide whether a single "General Chat" forum was added, maybe even an initial topic & comment?

A dsm() on install with a link to the forum creation admin would be a partway solution. Or if we create a single term to start the user off, tell them about that, eg 'An initial forum board has been created for you. Customize the boards on the [link]admin page'.

From a functional point of view this seems to work just fine and it makes complete sense from a usability point of view.

As the bot likes it mark RTBC

Sensible defaults, this is a good improvement.

+++ modules/forum/forum.module	3 Jan 2010 22:22:27 -0000
@@ -281,12 +281,17 @@ function forum_node_validate($node, $for
+        if (!$term) {
+          form_set_error('taxonomy_forums', t('You must select a forum.'));
+        }

Why was this added? Also, it seems to be the only error message starting with 'You'.

+++ modules/forum/forum.admin.inc	3 Jan 2010 22:22:27 -0000
@@ -243,10 +243,12 @@ function forum_overview($form, &$form_st
-        $form[$key]['edit']['#markup'] = l(t('edit container'), 'admin/structure/forum/edit/container/' . $term['tid']);
+        $form[$key]['edit']['#title'] = t('edit container');
+        $form[$key]['edit']['#href'] = 'admin/structure/forum/edit/container/' . $term['tid'];
...
-        $form[$key]['edit']['#markup'] = l(t('edit forum'), 'admin/structure/forum/edit/forum/' . $term['tid']);
+        $form[$key]['edit']['#title'] = t('edit forum');
+        $form[$key]['edit']['#href'] = 'admin/structure/forum/edit/forum/' . $term['tid'];

Cause for the re-roll, probably.

+++ modules/forum/forum.install	3 Jan 2010 22:22:27 -0000
@@ -56,6 +59,16 @@ function forum_enable() {
+    // Create a default forum so forum posts can be created.
+    $edit = array(
+      'name' => t('General discussion'),
+      'description' => '',
+      'parent' => array(0),
+      'vid' => $vocabulary->vid,
+    );
+    $term = (object) $edit;
+    taxonomy_term_save($term);

This belongs into hook_install(), not _enable().

6 days to code freeze. Better review yourself.

But, yeah, this would be nice, but not strictly critical.

'You must' is a copy writing no-go, so changing
"+ form_set_error('taxonomy_forums', t('You must select a forum.'));

to

+ form_set_error('taxonomy_forums', t('Select a forum.'));

(don't tell me what I should do, just tell me what needs doing)

#18 passes too.

Sorry, but I'm going to have to -1 creating sample content in hook_enable(). That's what install profiles are for, since it's safe to make assumptions about said content there, but not at the module level. It's too bad our core forum module is not up to snuff enough to enable it in default.profile, but maybe we can try and organize some effort around this in Drupal 8.

I also don't see anything in the patch that enables comments by default on forums, even though that's in the original bug report. Additionally, it smells like we could use some test coverage here.

forum_enable() already creates a vocabulary for the forum, so I don't see that also creating a term makes it worse.

Ugh. I didn't see that other patch go in. I guess there's precedence for this then, sadly... :(

I do fundamentally believe that modules are *functional* pieces and should not make assumptions about how they will be used in the field. The creation of the vocabulary is something that is functionally required for this module to actually work, the creation of terms is not. It also makes assumptions that they will not want to group their forums under categories, etc. etc. Ick.

But sigh. I guess it's no worse than what Contact module does now. Still needs tests, though.

> Ugh. I didn't see that other patch go in. I guess there's precedence for this then, sadly... :(

LOL... I'm pretty sure that's been in since 4.7 or earlier -- image_galleries module cribs it for its own vocab!

> the creation of terms is not.

Hmm... I think it would be enough to have a drupal_set_message on installation that says 'Now go and create some forum topics!'.

Here is a patch that includes that from #18 above and also adds a test to the forum.test file which tests for the creation of the default forum. It works for me.

Setting status for the testbot to have a go at the patch

The .install file patch above causes an exception to get thrown because of a duplicate key error.

Steps to replicate:
1. apply the patch
2. enable the module
3. disable/uninstall the module
4. re-enable the module.

This patch catches that exception with a message that the vocabulary already exists.

NOTE: the simpletest patch in #27 is not included here.

Test fails (test #27)

We needed to add that validation because the forum module makes it's 'Forum selector' taxonomy term option required, but fieldapi doesn't validate when the user selects '- None -', so we have to do it manually. I'll re-roll without the 'You'.

What?

This patch only updates the forum.test. The patch includes the additional tests written in comment #27 without the changes to other forum files.

Since the addition of the default forum vocabulary, the orphan test was failing. The test has been updated to validate the testing for orphan items.

This patch works with the patch from comment #30.

Ok, here's a simpler version of #30. The variable "forum_nav_vocabulary" was being deleted as part os #27. #30 handled an exception that was thrown as a result. After talking with Dave Reid briefly, it made more sense to just /not/ delete the variable :)

So, the current status of this issue is:
Patch from comment #33 adds tests
Patch from this comment fixes handling of default vocabulary and avoids errors in hook_uninstall.

It seems to me that the patch in #30 is on the right track, but instead of just catching the exception and putting out an error, maybe it should at least check the exception for that one case and only handle it there if it is the exception we expect ?

Probably better would be to check for the vocabulary existing, and not create it if it is there.

The patch in #34 I am not sure of, on one hand it seems better to delete the variable behind us to clean up, on the other hand we are not deleting the Forum vocabulary itself, it would seem like a bad idea to do delete the vocabulary, and leaving behind the variable as a descriptor of what it is seems OK. I think on the balance it is the smartest thing to do.

Conferring with pcarman we decided to roll the test patch in with the default vocabulary patch as the automated testing was failing.

Here it is in one bundle

Here's a slightly revised version with proper comment formatting.

Reviewed in context of #30+, RTBC. Looks great, works well; thanks for fixing up those comments in *.test.

Great, here's the revised patch that addresses those concerns.

I can confirm that the patch in #40 passes all the forum tests and works, I also red through it and all makes sense.

Heya @rgristroph do you mean RTBC?

By your comments it seems so.

Yes, I think this is RTBC.

Committed to CVS HEAD. Thanks.

Any solution/backport for 6.x?