[drupal-devel] [task] Clean up block module tables

[TDobes]

 Project:      Drupal
 Version:      cvs
 Component:    block.module
 Category:     tasks
 Priority:     normal
 Assigned to:  drumm
 Reported by:  drumm
 Updated by:   TDobes
 Status:       patch

It should be noted that the patch here contains a fix for a CRITICAL bug
in block.module:  At the moment, custom blocks cannot be deleted.  A
patch containing the fix alone (without other improvements) is located
in another issue, which was marked as a duplicate [1] quite a while
ago.
Since we're now in a feature freeze, perhaps I should reopen the other
issue so the deletion fix alone can be committed?
[1] http://drupal.org/node/14625


TDobes



Previous comments:
------------------------------------------------------------------------

December 9, 2004 - 15:07 : drumm

Attachment: http://drupal.org/files/issues/block.module_2.diff (9.54 KB)

* renamed 'boxes' to 'blocks_custom'
* changed the 'bid' column of that table to 'delta' and removed auto
increment in favor of the usual db_next_id
* made associated changes in code to avoid 'box' and 'bid'
* removed unique constriant on custom box titles
(http://drupal.org/node/11724)
* put the delete link for custom blocks back in, it was apparently lost
in another update


------------------------------------------------------------------------

December 9, 2004 - 15:26 : jhriggs

I think the only thing I would question is the renaming of bid.  It
seems inconsistent with the rest of drupal.  We use nid, rid, uid,
etc., etc.  So, this should be a block ID, bid.


------------------------------------------------------------------------

December 9, 2004 - 16:05 : drumm

Delta is the best name since this is what hook_block() calls it.
Adding a bid (b as in block) to the blocks table is a good idea, but I
wouldn't want to confuse things with a patch that is renaming bid in
one context and adding bid in a related but different context.


------------------------------------------------------------------------

December 9, 2004 - 16:22 : Dries

I prefer bid.  I'd rather change delta to bid, instead of changing bid
to delta.  It is more consistent.  Otherwise the patch looks good.


------------------------------------------------------------------------

December 9, 2004 - 16:43 : drumm

Delta really isn't a good id. It isn't unique. The closest thing to a
primary key is the blocks table is (module, delta). I think bid should
be added as a real primary key on the blocks table. If there is a good
word to use in place of delta that isn't bid, then that would be good
to use.


------------------------------------------------------------------------

December 10, 2004 - 18:22 : drumm

Updated patch which adds:
* Added index to weight column in blocks table
* Added primary key bid to blocks table
* Replaced module/delta construct with bid where possible
In additon to the previous:
* renamed 'boxes' to 'blocks_custom'
* changed the 'bid' column of that table to 'delta' and removed auto
increment in favor of the usual db_next_id
* made associated changes in code to avoid 'box' and 'bid'
* removed unique constriant on custom box titles
(http://drupal.org/node/11724)
* put the delete link for custom blocks back in, it was apparently lost
in another update


------------------------------------------------------------------------

December 10, 2004 - 18:23 : drumm

Attachment: http://drupal.org/files/issues/block.module_3.diff (20.41 KB)

And the patch too


------------------------------------------------------------------------

December 12, 2004 - 02:58 : Dries

The upgrade path gave me problems.  I got a fatal error:

<?php
$ret[] = update_sql('ALTER TABLE {blocks_custom} DROP INDEX title');
?>


The index did not exist on my setup.  Weird.
I got several hundred warnings about this line:

<?php
foreach ($blocks as $delta => $block) {
?>


It's somewhat tricky, because you can't execute that update twice.  My
user and block tables are pretty unusable now.  :)
(Is the block->info field still being used?  It looks quite redundant
to me.)


------------------------------------------------------------------------

December 12, 2004 - 03:52 : Dries

I ran this update on a copy of the drupal.org database and it seems to
work.  The reason my previous attempt failed, is because of the way
error reporting was configured.  My copy of the drupal.org database
doesn't have an index on 'blocks_custom.title' either, but it did not
result in a fatal error nor did it cause the upgrade script to halt. 
Warning are suppressed so they are simply not shown.  
In short, depending on how error reporting is configured, the upgrade
path might or might not work.  If it doesn't work, you're screwed
because you won't be able to run the upgrade script again.  At least,
not without intimate knowledge of Drupal's internals.


-- 
View: http://drupal.org/node/14158
Edit: http://drupal.org/project/comments/add/14158