Going to the main Drupal admin page at /admin/config throws the following error:

Fatal error: Call to undefined function _bible_webimport_check() in /home/drupalpro/websites/bible.dev/sites/all/modules/contrib/bible/bible.install on line 10 Call Stack: 0.0000 329208 1. {main}() /home/drupalpro/websites/bible.dev/index.php:0 0.0329 2844300 2. menu_execute_active_handler() /home/drupalpro/websites/bible.dev/index.php:21 0.0334 2892900 3. call_user_func_array() /home/drupalpro/websites/bible.dev/includes/menu.inc:519 0.0334 2893092 4. system_admin_config_page() /home/drupalpro/websites/bible.dev/includes/menu.inc:0 0.0334 2893136 5. system_status() /home/drupalpro/websites/bible.dev/modules/system/system.admin.inc:13 0.0383 2935348 6. module_invoke_all() /home/drupalpro/websites/bible.dev/modules/system/system.admin.inc:2334 0.0392 2940560 7. call_user_func_array() /home/drupalpro/websites/bible.dev/includes/module.inc:895 0.0392 2940812 8. bible_requirements() /home/drupalpro/websites/bible.dev/includes/module.inc:0

The problem appears to be that with the _bible_webimport_check() function moved to an include, it is not in scope when that code executes.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rhuffstedtler’s picture

Here's a quick patch to move it back, but I don't know that this is what we really want to do.

To me, the notion of checking the installed bible versions in hook_requirements() introduces unnecessary load and an external dependency that could break the main admin page. I think we should reconsider this and only check for updates when the admin is looking at the list of installed bibles. Once we expand webimport to sources beyond drupalbible.org, I'm not sure it will be a valuable check in any case. It also seems inconsistent with the philosophy of what hook_requirements is for, since the bible versions are not really a requirement of the module - they are data displayed by the module.

dieuwe’s picture

Priority: Major » Critical

@rhuffstedtler I agree, the check should probably be done when looking at the list of installed Bibles.

Perhaps we should look at how other modules notify you of updates to their resources? Like translations, etc?

berenddeboer’s picture

Priority: Critical » Normal
Status: Active » Fixed

@rhuffstedtler, fixed that it doesn't break, but agree with your comments. But load isn't a big deal I think, it's only checked when you go to the status page AFAIK.

berenddeboer’s picture

BTW, load may indeed be more significant, forgot that admin/config checks status as well, see #2478231: Update module Bible 7.x-1.5 I has the following bugs:.

Status: Fixed » Closed (fixed)

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