it's recommended that contrib modules follow core's example in transforming their block deltas from integers to strings. that means that contrib modules, in their update from 6.x -> 7.x, will be operating against a core database table.

IMO this warrants a helper function, both for upgrade safety reasons, and to make it more likely that contrib authors will actually participate in this change.

attached patch is a first effort to provide this function. it's pretty simple -- i think the only things to really debate are:

  1. should this go in?
  2. the exact function name
  3. is the code doc solid

Comments

hunmonk’s picture

just a note that the patch doesn't currently leverage the new function for the core block updates, but it easily could -- and i'm happy to add that if people think this function is a good idea... :)

hunmonk’s picture

StatusFileSize
new7.22 KB

the first patch wasn't quite complete, and fixing it convinces me even more that a helper function is needed here, since properly doing this update is a bit involved -- if you examine the existing system_update_7004() you'll see what i'm talking about.

the new approach basically steals the relevant code from system_update_7004() and moves it to update.inc, thus making it available as a helper function to any modules that need it.

with the helper function, a contrib module's process for updating it's blocks would look like this:

function mymodule_update_7XXX(&$sandbox) {
  $renamed_deltas = array(
    'mymodule' => array(
      '0' => 'mymodule-block-1',
      '1' => 'mymodule-block-2',
    ),
  );
  update_fix_block_deltas($sandbox, $renamed_deltas);

  // This conditional would be optional.
  if (!isset($sandbox['progress'])) {
    // Anything they may want to do just once during the update...
  }
}
moshe weitzman’s picture

Seems like a very good idea. There are a lot of numeric deltas. Please don't make every contrib author figure this out.

roychri’s picture

Assigned: hunmonk » roychri

I will test this extensively and report back.

hunmonk’s picture

StatusFileSize
new7.23 KB

there was a small logic error in my example function in #2, and my adjustments to system_update_7004() to leverage the helper function. any one-time updates need to be run *before* update_fix_block_deltas() is run, as it sets $sandbox['progress'] the first time it's run.

attached patch fixes system_update_7004() to account for this -- it's a simple copy/paste of the code from one side of the helper function to the other, no other changes have been made.

and here's an updated example function:

function mymodule_update_7XXX(&$sandbox) {
  $renamed_deltas = array(
    'mymodule' => array(
      '0' => 'mymodule-block-1',
      '1' => 'mymodule-block-2',
    ),
  );

  // This conditional would be optional.
  if (!isset($sandbox['progress'])) {
    // Anything they may want to do just once during the update...
  }

  update_fix_block_deltas($sandbox, $renamed_deltas);
}
roychri’s picture

My test plan is:

  1. Install Drupal 6
  2. Download and enable DEVEL module
  3. Move the devel block to the left region and configure custom title and visibility settings.
  4. Disable the module devel
  5. Upgrade to Drupal 7
  6. Download devel-7.x-dev and enable it.
  7. Run the update.php again to upgrade devel
  8. Make sure the devel block is gone and my custom config are gone.
  9. Modify devel-7.x-dev's install file to use hummonk's function to upgrading the blocks
  10. Start over from step 1 but this time make sure the devel block is on the left region and contains my custom config after the upgrade.

I tried these steps but Drupal7 upgrade is broken at the moment. I cannot test this properly.
I will try again later.

roychri’s picture

Status: Needs review » Postponed

This issue depends on #563106 to be fixed.

hunmonk’s picture

Priority: Normal » Critical
Status: Postponed » Active

added this to #563106: Cannot upgrade from Drupal 6 to Drupal 7 - meta issue -- this is really a no brainer to ease the upgrade process, especially given all the D7UX promises.

webchick’s picture

Status: Active » Needs review

Marking as a patch.

webchick’s picture

Priority: Critical » Normal
Status: Needs review » Needs work

A couple of things:

1. Let's name this in some way so that it's obvious this is a temporary helper function for 6 => 7 upgrades only. This'll need to be removed in Drupal 8, because there's no point.

2. Make the fact that it should be removed in Drupal 8 explicit, with a @todo in the PHPDoc.

sun’s picture

roychri’s picture

Assigned: roychri » Unassigned

I cannot work on it right now. If someone wants to work on it, feel free to take it. Otherwise I will check it out later when I have more time.

hunmonk’s picture

Status: Needs work » Needs review
StatusFileSize
new7.26 KB

re-rolled and updated based on webchick's requests in #10

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

OK, lets do it.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Ok, great. Committed to HEAD. Thanks!

Status: Fixed » Closed (fixed)

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

hunmonk’s picture

Priority: Normal » Minor
Status: Closed (fixed) » Needs review
Issue tags: -D7 upgrade path
StatusFileSize
new592 bytes

in what might be the most insignificant patch of the D7 release cycle, this fixes a bad semicolon in the example code in the code comments for update_fix_d7_block_deltas().

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community
dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

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