Thank you for this useful module!

We have a small but important issue. Quiz is not compatible with spaces 6.x-2.0-beta7. This is because spaces have a weight of -1 in the system table, and in spaces_init menu_get_item is called. When you are viewing a question node in quiz menu_get_item will probably result in a node_load. Question nodes are implemented as classes. We use autoload to make sure the classes gets loaded.

To make a long store short. Since spaces_init gets called before autoload_init we get a class not found error. Patch for fixing this is coming up...

CommentFileSizeAuthor
#13 autoload-777408.patch638 bytesmikeryan
#1 autoload-777408-1.patch662 bytesfalcon

Comments

falcon’s picture

falcon commented
StatusFileSize
new autoload-777408-1.patch662 bytes

Patch has been attached. I set the weight to a quite low number. I can't imagine any module needing to have a lower weight than this one. Might be something wrong with my imagination though...

falcon’s picture

falcon commented
Status: Active » Needs review

I hope you'll find the time to review this change!

vegardjo’s picture

vegardjo commented

Subscribe

kars-t’s picture

kars-t commented
Status: Needs review » Needs work

Hi

I'd say that the number is too high and that you should use hook_install with an update instead of hook_enable.

falcon’s picture

falcon commented
Status: Needs work » Needs review

Regarding the number: I guess the one who commits the patch can change the number to anything he likes.
Regarding hook_enable: In the spaces module hook_enable is used instead of hook install. The hook_enable function is commited by yhahn of development seed. I followed his example. By using hook_enable you can be sure that the module already exists in the systems table.

kars-t’s picture

kars-t commented
Status: Needs review » Needs work

I see that for example the dev module uses hook_install for D6 and hook_enable for D7
Maybe one should update this page about setting a modules weight. http://drupal.org/node/110238
Learned something new :)

But still you should add an hook_update to the patch so people currently using this module will get the weight set by running update.php
Look at the dev module .install for an easy example.

falcon’s picture

falcon commented

I agree, it should be an update function there. Creating the update function is copy-paste, and I'm sure the project maintainer can copy paste

db_query("UPDATE {system} SET weight = -282828 WHERE name = 'autoload' AND type = 'module'");

into whatever hooks he prefer to paste it into, if he wants to add it.

falcon’s picture

falcon commented
Status: Needs work » Active

We've got several bug reports on the quiz module that relates to this issue. I will wait a few more days and if nothing happens with this issue I will temporarily add the fix to quiz_update_N and quiz_enable.

halcyonCorsair’s picture

halcyonCorsair commented

Subscribing.

halcyonCorsair’s picture

halcyonCorsair commented

I see this has already been fixed in CVS, would you consider releasing that change?

Crell’s picture

Crell commented
Status: Active » Fixed

I don't know what you're talking about, halcyon. The code in the 6.x-1.x branch doesn't even have a .install file.

That said, I went ahead and added one. I went with hook_install() for no particular reason. :-)

Crell’s picture

Crell commented

Correction, I went with hook_enable(). Even I can't keep them straight.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Fixed » Needs review
StatusFileSize
new autoload-777408.patch638 bytes

Looks like that patch added hook_enable to autoload.install but didn't remove it from autoload.module, running drush updatedb bit me...

dkruglyak’s picture

dkruglyak commented
Category: task » bug
Priority: Normal » Critical
Status: Needs review » Reviewed & tested by the community

Yes it breaks my install too. Just commit mikeryan's fix!

falcon’s picture

falcon commented

Yeah, it seems this is kind of my fault. Sorry. I didn't notice that a fix was already commited, but not released when I created this issue...

dkruglyak’s picture

dkruglyak commented

So can we commit the fix ASAP? In the latest CVS I still see both autoload.install and autoload.module redeclare autoload_enable function.

Crell’s picture

Crell commented
Status: Reviewed & tested by the community » Fixed

And bah on me for not noticing it either. Fixed in dev.

Status: Fixed » Closed (fixed)

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

derEremit’s picture

derEremit commented
Status: Closed (fixed) » Active

Just experienced that problem while testing quiz for an old d6 site.
Setup a new empty site just with admin_menu quiz and autoload. Had to set the weight higher than -100 ( -10000).
So out of the box it doesn't work

Crell’s picture

Crell commented
Status: Active » Closed (fixed)

Module weights are not a be-all-end-all solution. They cannot work with every situation out of the box. In the long run they should go away. As should Drupal 6, with Drupal 8 coming out next year. :-)