Quicktools exportables

Hey Young! thanks for the patch

question:
Does it make sense to use a different column for the export id? Like views also have two columns "vid" (serial) and "name" (varchar).. I am afraid that changing the primary key of the db table is not a good idea in -rc state.

the views::load() function can work with vid and name also

$where = (is_numeric($arg) ? "vid =  %d" : "name = '%s'"); 

The numeric column is faster when we need to join tables; like views_display is joined with views table through the vid column. I still think that the 2 id columns are intentional in views module.
And I am thinking about the same for QT-7.x as I would like to create a separate table for tabs, joined on qtid column.

quick question: is it a problem for you if this feature would be part of 6.x-3.x, but not part of 6.x-2.x? as I do not know when 6.x-3.x version will have stable releases. Are you comfortable using 6.x-3.x-alpha on sites where you need this feature?

I think it would be a good idea to separate the tabs column into another table as with different extensible style and content plugins that serialized array could become a large mess. And if I join the tables on qtid I can probably allow changing the name (the string identifier), it could be handy (I always rename my views and display id-s through the export/import function).
but I planned this only for the D7 version..

So, originally I was really for the idea of getting this into the 2.x version - however testing it on some existing sites has shown it would cause too many problems for existing users trying to update. Normally, they would expect to just have to run update.php and be done but with the changed qtids, they'd have to re-enable all of their QT blocks. They'd also likely have to re-theme them all. 3.x it'll have to be... :-(

Would it be possible to tag a 3.x-dev branch and commit this then? I will try to test this patch soon.

Edit: Just noticed that there is a 3.x-dev branch already. It's too early in the morning here ;)

Tagging.

This doesn't apply cleanly anymore to 6.x-2.x-dev and 6.x-3.x-dev only have the LICENSE.txt and .info file in it...

Going to re-roll this by hand for 6.x-2.x as I could really use it on a new project.

Here's the re-roll against the latest 6.x-2.x-dev

Yes, the 3.x branch simply never happened. I would like to get this in but what I don't understand is why we can't hold on to the serial qtid field as well as a machine name, at least for internal look-ups. That way we could add code in our update hook that changes existing QT block placements to use their machine name instead of the numeric id (i.e. at least it can still look them up because the numeric id hasn't been blown away). Otherwise people with existing QT block placements will have all these disappear off their pages after updating. Thoughts?

I'm not sure I follow. I was able to create 2 QTs, and insert them another QT with their machine name and export them all to code. I had to make s slight modification to the QT form, removing the maxlength => 10 - but other then that it all worked...

@sirkitree, we seem to be talking along crossed wires here :-/ My concern is just regarding people updating their existing Quicktabs if we make this change, but maybe I'm missing something. With the patch as it stands it seems to me that it won't just be a question of running update.php and everything being fine. If a site has existing QT blocks placed in various regions, these will surely disappear after the update because the ids will have changed, right? So I'm wondering why we can't hold onto the numeric ids, which would be better to use for internal look-ups anyway, and add a machine_name column. Then in the update function we would make a note of the block placements and update them to reference the QT by machine name instead of id.

@katbailey, while numeric lookups seemed fine at one point, the problem is that if anything else is exporting block info, it's far better to have a machine name there, than a numeric. Example: I'm exporting a context and I want to include quicktab info... if it exports that I want quicktab id=3, that context isn't portable. If it exports I want quicktab id=hometabs, then that is portable, allows moving the context, the views it uses, and the quicktab itself all into code, and running in them in different environments.

And while you can make the argument that those cases should use machine name ids, and leave numeric ones for local use like now, that will get confusing quickly... it means that _sometime_ you can use machine ids, and sometimes you can refer to a local #? Why introduce that complexity? What does the number give you?
Uniform urls? How is /admin/build/quicktabs/5/edit better than /admin/build/quicktabs/hometabs/edit ?
Programmingwise, PHP doesn't care if you loop thru an array of machinenames versus an array of numeric qids...

If we can help get Drupal at large to stop using uncontrollably incremented numbers to identify things, we all win.

Features/CTools/Views/etc have really help define the right way to handle this stuff... allowing developers to make portable and (re)encodable methods of moving structure from one site to another, ie development to staging to production, and not forcing you to hack in hardcoded numbers in places just because you'd like to use things you can't control until they get assigned an 'official' production incremental key number.

Comment on the patch itself in next post...

This patch isn't working for me... I implemented mymodule_quicktabs_default_quicktabs()
but the export code (using ctools 1.4) is looking at the api in the quicktabs_schema, and so looking for api info, and not the simple hook implementation. I messed around with things debugging, but didn't solve it. i think removing the api info is a step, or requires adding it to modules that wish to implement quicktabs...

Not quite ready yet (I thought so above, but the proof was did it work for me or not, and the answer was not.)

@sethcohn, thanks for helping to hash this out. As I said, I'm really motivated to move to machine names, but I still haven't heard a solution to the problem of existing QT block placements disappearing when sites update to the new QT with machine names.

well, there is a solution:

Make it optional to migrate existing blockids from the number to the title as machine name.
It's a bad idea, and it's likely to break...

Make sure the machine name is exposed, so that someone can fix it later if desired. (ie "If you change this, your block placements will disappear")

In other words, let people keep the existing (bad) #s if they _need_ them.

Honestly, I think it's a upgrade path issue. Which means you need to warn people: "This will change your block placements!!" which is way less work, and better in the long run.

I am using this code as a base for another project and I thing there is a missing line in the update_6027, $used array is declared but never updated, so if there are more than one quicktab with the same name, update will fail.
So, somewhere in the while loop, something like $used = $row['id']; need to be used.

I have committed a version of this to the 3.x branch - the main change I made was to rename the qtid field to machine_name because I felt that was more appropriate.

Re #16, @sethcohn I had a bit of a hard time figuring this out myself but basically what it boils down to is that quicktabs is requiring modules to explicitly support a particular version of the api. Therefore, it won't suffice for the module to just implement hook_quicktabs_default_quicktabs, but they'll have to implement hook_ctools_plugin_api() as well. So, for example:

/**
* Implementation of hook_ctools_plugin_api().
*
* Tell Ctools that we support the quicktabs API.
*/
function mymodule_ctools_plugin_api($owner, $api) {
  if ($owner == 'quicktabs' && $api == 'quicktabs') {
    return array('version' => 1, 'path' => drupal_get_path('module', 'mymodule') . '/includes/quicktabs');
  }
}

Then in your mymodule.quicktabs.inc file at the above path, you'll implement hook_quicktabs_default_quicktabs.