Let me start by saying excellent module. This is great to use. I am far from an expert on modules. When activating the module I couldn't get it to create the necessary database table. I modified the quicktabs.install file to be the following and all seems to work well (install and uninstall):

// $Id: quicktabs.install,v 1.3 2008/04/17 22:59:23 katbailey Exp $

function quicktabs_schema() {
  $schema['quicktabs'] = array(
    'description' => t('The quicktabs table.'),
    'fields' => array(
      'qtid' => array(
        'description' => t('The primary identifier for a qt block.'),
        'type' => 'serial', 
        'unsigned' => TRUE, 
        'size' => 'big', 
        'not null' => TRUE, 
        'disp-width' => '20'),
      'title' => array(
        'description' => t('The title of this quicktabs block.'),
        'type' => 'varchar', 
        'length' => '255', 
        'not null' => TRUE),
      'tabs' => array(
        'description' => t('A serialized array of the contents of this qt block.'),
        'type' => 'text', 
        'size' => 'medium', 
        'not null' => TRUE)
			),
   'primary key' => array('qtid'),
	);
  return $schema;
}

function quicktabs_install() {
  drupal_install_schema('quicktabs');
}

function quicktabs_uninstall() {
  drupal_uninstall_schema('quicktabs');
  // Delete any variables that have been set.
  $result = db_query("SELECT name FROM {variable} WHERE name LIKE 'quicktabs_%'");
  while ($row = db_fetch_object($result)) {
    variable_del($row->name);
  }
}

Hope this helps someone else and maybe it can make it into the release.

CommentFileSizeAuthor
#14 quicktabs_install.patch1.36 KBkatbailey
#9 quicktabs_install.patch1.25 KBkatbailey
quicktabs.install.txt1.2 KBcforrest
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

katbailey’s picture

katbailey CreditAttribution: katbailey commented

ok, I've made changes to the install file more or less in line with the above. Thanks, cforrest!
The changes are in the latest tarball.

Katherine

katbailey’s picture

katbailey CreditAttribution: katbailey commented
Status: Active » Fixed
jaydub’s picture

jaydub CreditAttribution: jaydub commented
Status: Fixed » Active

The change made here is due to the fact that drupal 6 no longer
uses a sequences table which you had been using to store the
qtid. For Drupal 6 you really should also add in a module update
to convert existing Drupal 5 versions of quicktabs and the
{quicktabs} table to use a new serial column rather than
the regular int column + sequences that you've used in
drupal 5. People upgrading from Drupal 5 to Drupal 6
of this module will not automatically get the new table
definition, you will have to provide that.

I can help :) but first I'm checking out the Drupal 5
version. Since this is not resolved I am reopening
hope that's ok.

katbailey’s picture

katbailey CreditAttribution: katbailey commented

Hi jaydub,
it would be great if you could help with this. I'm currently trying to do an overhaul of the D5 version to allow for dynamic addition and removal of tabs which is proving quite a task so it's hard for me to get time to look after these other issues (and you obviously have far more expertise in this particular area than I do ;-)

thanks,
Katherine

Pasqualle’s picture

Pasqualle
Location 🇭🇺 Budapest
CreditAttribution: Pasqualle commented
Title: had to modify quicktabs.install to get the module to create the {quicktabs} table during module activation » missing upgrade from D5 to D6
Version: 6.x-1.x-dev » 6.x-2.x-dev
Priority: Normal » Critical

I did not tried, but from source code the qtid column seems incorrect after upgrading from D5 to D6.
And I see more and more D5 sites used with the QT module, so this could be a huge problem.

I would appreciate if someone would make an upgrade test from D5 to D6, and check the table structure. The qtid column needs to be a serial (auto increment) column in QT for D6.

Pasqualle’s picture

Pasqualle
Location 🇭🇺 Budapest
CreditAttribution: Pasqualle commented
Issue tags: +Release blocker
katbailey’s picture

katbailey CreditAttribution: katbailey commented

We need to use db_change_field, as per the following example from http://api.drupal.org/api/function/db_change_field/6:

<?php

db_drop_primary_key($ret, 'foo');
db_change_field($ret, 'foo', 'bar', 'bar',
array('type' => 'serial', 'not null' => TRUE),
array('primary key' => array('bar')));

?>
katbailey’s picture

katbailey CreditAttribution: katbailey commented
Assigned: Unassigned » katbailey
katbailey’s picture

katbailey CreditAttribution: katbailey commented
FileSize
quicktabs_install.patch1.25 KB
katbailey’s picture

katbailey CreditAttribution: katbailey commented
Status: Active » Needs review
katbailey’s picture

katbailey CreditAttribution: katbailey commented

Hmm, somehow the actual comment that I wrote got lost (d.o is acting a bit weird since the upgrade), here it is again:

Here's a patch with the proper update function. I'd like others (Pasqualle?) to review this as I'm a bit unsure what the ramifications are of removing quicktabs_update_1() and replacing it with quicktabs_update_6000(). Is it ok to do this or do we need to keep hook_update_1() as is? I tested it on a purely test install of D5, upgrading to D6 and it worked perfectly.

Pasqualle’s picture

Pasqualle
Location 🇭🇺 Budapest
CreditAttribution: Pasqualle commented

renaming the update number would affect users who uses that update version (or earlier version). quicktabs_update_1() was created early with 6.x-2.x version. So if someone is using a really old 2.x-dev release then running the update 6000 would try to add that ajax column again.. I don't see any real problem with this rename of the update function..

Katherine if you have the time please also test the update from 6.x-1.0-beta1 release. That should be fine also, but just to be sure..

and the issue about the lost comment: #379470: Change in the issue description is discarded after clicking on the attach button

katbailey’s picture

katbailey CreditAttribution: katbailey commented
Status: Needs review » Needs work

No, this doesn't work - we need a separate update function for moving from 6.x-1.x (which just adds the ajax column) and for moving from 5.x to 6.x (which changes the qtid and adds the ajax column).

katbailey’s picture

katbailey CreditAttribution: katbailey commented
Status: Needs work » Needs review
FileSize
quicktabs_install.patch1.36 KB

OK, here's another shot at it - I tried it both upgrading from 5.x to 6.x-2x and from 6.x-1.0-beta1 to 6.x-2.x and it worked fine, though you have to select the 6001 update when doing the latter so that it doesn't do the 6000. We can put a note on the project page pointing this out. Thoughts?

Pasqualle’s picture

Pasqualle
Location 🇭🇺 Budapest
CreditAttribution: Pasqualle commented

what is the error when running update 6000 on 6.x-1.0-beta1? Is it makes the table unusable? Probably everyone updating from beta1 will run that update, the note on project page will not change that..

katbailey’s picture

katbailey CreditAttribution: katbailey commented

It doesn't make the table unusable - the query just fails because it can't drop the primary key.

Pasqualle’s picture

Pasqualle
Location 🇭🇺 Budapest
CreditAttribution: Pasqualle commented
Status: Needs review » Reviewed & tested by the community

I was thinking about what could be checked before running the update_6000, and I do not have any better solution.

so the update_6000 will fail for 6.x-1.0-beta1 users, but it is not a problem..
maybe a short message on the project page (after the rc3 release), that the upgrade from 5.x-1.x to 6.x-2.0 works now, and update 6000 from 6.x-1.0-beta will fail, or users can skip that update step.. And also that users should upgrade to 2.0 release as support for 6.x-1.0 will be dropped after the stable 2.0 is released..

katbailey’s picture

katbailey CreditAttribution: katbailey commented
Status: Reviewed & tested by the community » Fixed

OK, so I was having a bad CVS day on Thursday and actually committed this unintentionally with http://drupal.org/cvs?commit=182710, figured there was no point changing it back if it would end up being committed anyway :-P.
I've added a task regarding the note on the project page: #404024: Note on project page re updates.

Status: Fixed » Closed (fixed)
Issue tags: -Release blocker

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

Issue tags: +Release blocker

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.