Block titles not copied on switching to a new theme

The linked patch is not quite correct. It inserts the title as an integer instead of as a string, so all the titles get set to zero (because php tries to read the titles as numbers and fails to parse anything useful out of them).

To fix this, change the last %d to '%s' (with quotes). Thus, the amended patch would be:

@@ -1063,8 +1063,8 @@
       if (!array_key_exists($block['region'], $regions)) {
         $block['region'] = system_default_region($theme);
       }
-      db_query("INSERT INTO {blocks} (module, delta, theme, status, weight, region, visibility, pages, custom, throttle) VALUES ('%s', '%s', '%s', %d, %d, '%s', %d, '%s', %d, %d)",
-          $block['module'], $block['delta'], $theme, $block['status'], $block['weight'], $block['region'], $block['visibility'], $block['pages'], $block['custom'], $block['throttle']);
+      db_query("INSERT INTO {blocks} (module, delta, theme, status, weight, region, visibility, pages, custom, throttle, title) VALUES ('%s', '%s', '%s', %d, %d, '%s', %d, '%s', %d, %d, '%s')",
+          $block['module'], $block['delta'], $theme, $block['status'], $block['weight'], $block['region'], $block['visibility'], $block['pages'], $block['custom'], $block['throttle'], $block['title']);
     }
   }
}

Holy cow. I just ran into this in D7.

Updated version of the original patch attached. The premise is sound, and the patch still works.

Setting back to block.module.

Can drupal_write_record() be used here instead?

It seems like that might avoid these kinds of bugs in the future.

Why yes, we can.

Katherine and I actually beat the testbot to figuring out this was wrong, but good that the testbot saw it too :)

We need to be writing new database records, rather than overwriting existing ones.

So something like this perhaps:

unset($block['bid']);
drupal_write_record('block', $block);

Or alternatively going back to the original approach would be fine too, but in the long run I feel like a drupal_write_record() solution might be a little cleaner.

I don't really see the need for drupal_write_record() now that we have the new DB layer, to be honest, but it makes no real difference to me. As long as it works. :) Here's a patch with unset($block['bid']) as suggested.

Looks good to me, and I verified that it works.

I think drupal_write_record() is good because it prevents having to keep track of the entire list of database columns in the code, which is kind of fragile :)

Committed to CVS HEAD. Thanks.