Module dependencies error in bookexpand.info?

agentrickard - June 27, 2007 - 20:33
Project:Book Expand
Version:5.x-1.0.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:dwees
Status:closed
Description

The current (bookexpand-5.x-1.0.x-dev.tar.gz) download includes this line in bookexpand.info:

dependencies = og views views_rss book

But 'og' is not required to run this module. It is simply an option for access control (not a requirement). It might be suggested, but you should be able to use the module without og. AFAIK, Views isn't required either.

In my installation, I changed the line to:

dependencies = book

Which I believe is more correct, unless there is a reason why both Views and Views RSS must be installed? I can't see any reason in the code.

After doing so, the $options array in settings:   $options = array ('None', 'Role', 'Group', 'User', 'Private'); should check for the existence of the OG module (and other suggested dependencies).

Similarly, bookexpand.install should only install the og_book table if og is installed.

<?php
function bookexpand_install() {
if (
module_exists('og')) {
    switch (
$GLOBALS['db_type']) {
      case
'mysql':
      case
'mysqli':
       
db_query("CREATE TABLE IF NOT EXISTS {og_book} (
          `og_nid` int(11) NOT NULL default '0',
          `book_nid` int(11) default NULL,
      PRIMARY KEY  (`og_nid`)
          ) /*!40100 DEFAULT CHARACTER SET utf8 */;"
);
    }
  }
}
?>

#1

dwees - July 4, 2007 - 15:46
Assigned to:Anonymous» dwees

Updated the dependencies as requested. Am installing og_book anyway whether or not OG is installed because 1 empty unused table won't cause any problems, However, I did make sure that it won't be populated unless Og is installed. I am doing this because one might enable my module first, then Og, and this way no one will have to build og_book by themselves.

#2

dwees - July 4, 2007 - 15:47
Status:active» closed

#3

agentrickard - July 4, 2007 - 17:38

Good point. Thanks.

 
 

Drupal is a registered trademark of Dries Buytaert.