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.

AttachmentSize
flag-user-access.patch1.02 KB

#1

Amitaibu - October 28, 2009 - 07:33
Title:user_access() uses wrong variables» $flag->roles['flag'] is missing, causing WSOD
Priority:normal» critical
Status:active» needs work

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

Amitaibu - October 28, 2009 - 07:42
Status:needs work» active

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

quicksketch - October 28, 2009 - 15:14
Priority:critical» normal

We need an upgrade path for DB flags, and to deal with flags in code as-well.

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

quicksketch - November 2, 2009 - 08:21
Title:$flag->roles['flag'] is missing, causing WSOD» Provide upgrade path for module-based default flags
Category:bug report» feature request
Status:active» needs review

Here's a simple approach to the problem, all default flags that are not specifically marked as $flag->api_version == 2 will 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.

AttachmentSize
flag_default_update.patch 5.52 KB

#5

Amitaibu - November 2, 2009 - 10:36

- 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.

AttachmentSize
616646-flag_default_update-5.patch 7.84 KB

#6

quicksketch - November 2, 2009 - 16:18

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

Amitaibu - November 2, 2009 - 17:39

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

Amitaibu - November 2, 2009 - 19:14

actually, you already took care of initializing $flag->api_version, so disregard #7. So I think this patch can go in.

#9

quicksketch - November 3, 2009 - 02:10
Status:needs review» fixed

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.

AttachmentSize
flag_default_update.patch 5.97 KB

#10

System Message - November 17, 2009 - 02:20
Status:fixed» closed

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

 
 

Drupal is a registered trademark of Dries Buytaert.