Provide upgrade path for module-based default flags
Amitaibu - October 28, 2009 - 07:28
| Project: | Flag |
| Version: | 6.x-2.x-dev |
| Component: | Flag core |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
This patch is a fix of the symptom, I haven't digged enough to see the cause.
| Attachment | Size |
|---|---|
| flag-user-access.patch | 1.02 KB |

#1
This actually causes a WSOD on admin/build/flags
$roles = array_flip(array_intersect(array_flip(user_roles()), $flag->roles['flag']));Need to check some more. hmm, I think we should consider accepting new features only if they come with simpletest.
#2
Ok, the issue is that there's no backward compatibility. Before the $flag was:
'roles' => array (0 => 2);and now it's'roles' => array ('flag' => array (0 => 2,), 'unflag' => array (0 => 2,),),We need an upgrade path for DB flags, and to deal with flags in code as-well.
#3
There already is an upgrade path for DB flags through update.php. Code-flags can't be updated by Flag since (well), they're in code. Right now our only big difference between 1.x and 2.x is the $flag->roles property is now two keys instead of one, but there may be others that will prevent Flag from working properly with 1.x-version flag definitions.
Perhaps we should follow Views here and add a version to our import/exports?
$flag->api_version = '2';And then all 1.x flags would be ignored (or displayed as disabled) until they were upgraded manually by the developer? We could probably use our new export interface to provide the developer with the needed new export.
#4
Here's a simple approach to the problem, all default flags that are not specifically marked as
$flag->api_version == 2will be disabled and a message shown that they need to be upgraded when visiting "admin/build/flags". A code export is given to the administrator so they can copy/paste the new code into their module.Moving to a feature request, since backwards-compatibility or an upgrade-assistant is not typical in Drupal development.
#5
- Add hook_update_N() to update existing flags.
- Add api_versiob to $flag->options().
- Add api_version to default 'bookmarks' flag.
- Add flag_update_export(&$flag) that is making sure every flag is updated before export.
#6
The hook_update_N isn't necessary, the roles are already converted in flag_update_6200(). The API version isn't stored in the database, so there's no need to update it or put it on the 'bookmarks' flag (all flags in the database are assumed to be the latest API version).
#7
Ok, that makes sense :). One thing that might be changed is
+ if ($flag->api_version < FLAG_API_VERSION) {to
+ if (empty($flag->api_version) || $flag->api_version < FLAG_API_VERSION) {#8
actually, you already took care of initializing $flag->api_version, so disregard #7. So I think this patch can go in.
#9
Thanks Amitaibu, I updated your patch in #5 to remove the unnecessary parts, but I kept the upgrade function flag_update_export() as a separate function so that it wasn't bundled in the page callback. I've committed the attached version, let me know if any further changes seem necessary.
#10
Automatically closed -- issue fixed for 2 weeks with no activity.