Unexpected error after disabling - unistalling - re-enabling forum module

Marking this as a 7.x-dev version to make sure it shows up on the "Critical issues" page as defined in the "Contributor links" block. Can you please check if this problem persists in the latest HEAD version?

Untested patch.

Didn't mean to change module.

Checked the patch - fixes the problem by checking whether the appropriate field exists not through a hard coded name but actually the right machine name based on what vocabulary the install process just created and does all the right things in the uninstall phase too

Sorry, had another look and changed it back to needs review purely because ideally the comment on line 43 should be

//Create the appropriate field for forums if it doesn't already exist
rather than
// Create the 'taxonomy_forums' field if it doesn't already exist.

which is probably what caused the problem to start with...

and here is the patch with the comment adjusted as well (everything else remains the same)

Looks fine, my patch so I won't re-RTBC.

Looking at this, I think forum module should create a single 'forum' field regardless of vocabulary, but that's a bigger change for a followup, if at all for D7.

Tested this with the steps described above.
Before patch: error
After patch: works

==> RTBC

+++ modules/forum/forum.install	9 Jun 2010 01:03:12 -0000
@@ -103,6 +103,9 @@ function forum_uninstall() {
+  ¶

extra space

Powered by Dreditor.

Damn, missed that one...
Luckily YesCT has better eyes ;)

I rerolled, still rtbc till bot says no.

dreditor has better eyes. :) And I felt a bit silly for marking it needs work...

Great job :). It needed work.

Not my day :(

why is this taxonomy and forums specific dependent?
will there be other combinations that will have a similar problem?

Yes, but those modules also need to add a field_delete_field() on uninstall.

Not sure if it related to this issue, but while testing this patch i get this.

Fatal error: Class name must be a valid object or a string in /var/www/drupal/includes/common.inc on line 6529 Call Stack: 0.0000 648256 1. {main}() /var/www/drupal/index.php:0 0.0171 4174848 2. menu_execute_active_handler() /var/www/drupal/index.php:22 0.0173 4247624 3. call_user_func_array() /var/www/drupal/includes/menu.inc:469 0.0173 4248112 4. drupal_get_form() /var/www/drupal/includes/menu.inc:0 0.0174 4249312 5. drupal_build_form() /var/www/drupal/includes/form.inc:78 0.0180 4278888 6. drupal_process_form() /var/www/drupal/includes/form.inc:225 0.0201 4475784 7. form_execute_handlers() /var/www/drupal/includes/form.inc:640 0.0201 4480912 8. system_modules_uninstall_submit() /var/www/drupal/includes/form.inc:1054 0.0203 4583672 9. drupal_uninstall_modules() /var/www/drupal/modules/system/system.admin.inc:1377 0.0206 4615368 10. module_invoke() /var/www/drupal/includes/install.inc:585 0.0206 4616184 11. call_user_func_array() /var/www/drupal/includes/module.inc:699 0.0206 4616552 12. forum_uninstall() /var/www/drupal/includes/module.inc:0 0.0208 4678128 13. taxonomy_vocabulary_load() /var/www/drupal/modules/forum/forum.install:106 0.0208 4678560 14. taxonomy_vocabulary_load_multiple() /var/www/drupal/modules/taxonomy/taxonomy.module:981 0.0208 4679024 15. entity_load() /var/www/drupal/modules/taxonomy/taxonomy.module:967 0.0208 4679104 16. entity_get_controller() /var/www/drupal/includes/common.inc:6518

Step to reproduce.
Install fresh D7. All settings by default.
Disable and then uninstall taxonomy.
Enable forum module and step forward confirmation about taxonomy. Feel good that there is no error and patch seems to be working.
Disable forum and taxonomy again. Uninstall it. After you confirm last step of uninstall form you get error as above.

Confirmed

Yes, I had the same error.

The problem comes from the line $vocabulary = taxonomy_vocabulary_load(variable_get('forum_nav_vocabulary'));
Taxonomy_vocabulary_load call the function "entity_get_info", which look for the taxonomy entity. But if the taxonomy module is already disable, then the entity doesn't exists in cache. A rapid solution would be find the vocabulary id in the database with db_query:

$query = "SELECT machine_name FROM {taxonomy_vocabulary} WHERE vid = :vid";
$result = db_query($query, array(':vid' => variable_get('forum_nav_vocabulary')));
$vocabulary = $result->fetchObject();

Patch should include a test

I've had a lot of trouble reproducing either error (either following the steps in the original issue or in #20). Is it possible that this error has been fixed in the past 10 days or so?

Following the steps here http://drupal.org/node/821290#comment-3062766 I can recreate the problem on current head.

Sorry, an edge case such as this doesn't get critical status. Keep working on a fix, though!

Vocabularies using the entity loader/hook_entity_info() is turning out to be more trouble than it was worth. #22 is the right fix until we have some way to reduce the interdependencies.

The problem is being caused is 2 fold
* The field creation code is being called because _field_info_collate_fields is not caching inactive fields although it is caching deleting fields this is generating the original error message
* However even when the above problem is solved we have a second problem as modules_enabled is being called we have the value forum whereas the field is associated with taxonomy hence the field is left inactive.

Have taken some code from #17 and and tweaked it a little to sort out the first part of issue and have also attempted to resolve the second problem and ensure we always have a working instance bundle on forums.

Reworking of patch to prevent errors when modules are enabled in certain order and incorrect signature for function call.

Putting back to needs work as the more I think about it the need to explicitly enable is wrong. Will dig some more later.

The issue is caused by the following process occuring
* Taxonomy module is install first and enabled correctly but its field isn't enabled as part of the enabling process
* Forum then tries to perform operations on a field that is still inactive
* The field would then be made active when modules_enabled is invoked later in the install process of course by then it is too late.

There seems to be 4 options
* Move when modules_enabled is invoked however this seems to be a finalise process so doesn't seem to be option
* Add a call to associate fields as part of the individual install process and just clear the cache modules_enabled()
* Add a hook_enable to the taxonomy module
* Call field_associate_fields in forum_enable and add comments to explain why on earth it is happening.

Patch attached for option 4, although option 2 looks like it might be a more robust solution but I don't know enough about the impact such a change might have on the field_api.

Moving this to field system, I like the look of option #2 but I'm also not sure what the impact would be.

#31: forum_enable_errors.patch queued for re-testing.

#882364: re-enabling Forum module after disabling Taxonomy module causes an exception because Forum thinks taxonomy_forum is inactive was filed as a dupe of this issue. Over there I figured out the same thing that marcingy points out here: The root cause of this is that hook_modules_enabled() doesn't work as you might expect it to: #727876: Enabling modules one at a time works differently than enabling them all at once

And webchick, I'm moving this back to critical pending your reading this paragraph. You don't have to uninstall anything to cause this. All you have to do is disable taxonomy and enable forum (edit: re-enable forum), and whoops! you get an exception. And who knows what state your site is in at that point, with forum module half-installed. I don't think it's an edge case if just enabling and disabling core modules throws up exceptions all over the place. If my impassioned plea falls on deaf ears, then by all means bump it back down. I won't cry. Much.

ksenzee's right, no need to uninstall any module to trigger this bug on a fresh Drupal 7 install:
- enable forum
- disable forum
- disable taxonomy
- enable forum again and hit continue to the question "You must enable the Taxonomy module to install Forum."

#727876: Enabling modules one at a time works differently than enabling them all at once might be the "right" way to prevent this which seems like the right way to solve any similar issue but this issue might not get solved quicly, and we have a working patch here. I wrote a test for it. I would have RTBC'ed this but I'd like someone to review the test I added.

i reviewed this in the GUI and ran the tests locally--looks good to me.

+++ modules/forum/forum.install
@@ -24,6 +24,10 @@ function forum_install() {
+  // If we enable forum at the same time as taxonomy we need to call
+  // field_associate_field as otherwise the field won't be enabled until
+  // hook modules_enabled is called which takes place after hook_enable events.
+  field_associate_fields('taxonomy');

- Typo in documentation: should be field_associate_fields instead of field_associate_field.

- Read the PHPdoc of field_associate_fields() and couldn't really tell what it was about (see http://api.drupal.org/api/function/field_associate_fields/7). We can fix that in another issue though.

Powered by Dreditor.

Typo fixed!

Committed to CVS HEAD. Thanks. Another one down.