Posted by hunmonk on October 16, 2009 at 7:42pm
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | update system |
| Category: | task |
| Priority: | minor |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
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:
- should this go in?
- the exact function name
- is the code doc solid
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| block_update_helper.patch | 1.47 KB | Idle | Passed: 14484 passes, 0 fails, 0 exceptions | View details |
Comments
#1
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... :)
#2
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:
<?php
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...
}
}
?>
#3
Seems like a very good idea. There are a lot of numeric deltas. Please don't make every contrib author figure this out.
#4
I will test this extensively and report back.
#5
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:
<?php
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);
}
?>
#6
My test plan is:
I tried these steps but Drupal7 upgrade is broken at the moment. I cannot test this properly.
I will try again later.
#7
This issue depends on #563106 to be fixed.
#8
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.
#9
Marking as a patch.
#10
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.
#11
subscribing
#12
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.
#13
re-rolled and updated based on webchick's requests in #10
#14
OK, lets do it.
#15
Ok, great. Committed to HEAD. Thanks!
#16
Automatically closed -- issue fixed for 2 weeks with no activity.
#17
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().
#18
#19
Committed to CVS HEAD. Thanks.
#20
Automatically closed -- issue fixed for 2 weeks with no activity.