I think only two changes are needed:

- Remove the call to db_column_exists() (I think it's only needed in D6, for the scenario where a user is upgrading from D5).

- Replace any db_drop_field($ret, 'flags', '//field_name//'); with $ret[] = update_sql('ALTER TABLE {flags} DROP COLUMN //field_name//').

(The documentation for both mysql and pgsql says that the "COLUMN" keyword is optional. (D6's db_drop_field() for some reason uses this keyword when talking with pgsql, but not when talking with mysql.)

Comments

arhak’s picture

arhak commented

subscribing

mooffie’s picture

mooffie commented

I see that Nathan has fixed this issue in 'dev'. I guess what's posponing the release of beta5 is that nobody has checked the upgrade path(?). Could somebody please do this? That is, install beta4, upgrade to 'dev', and report that everything went fine.

sirkitree’s picture

sirkitree commented

I installed a new copy of drupal and flag beta4. created two node, bookmarked one with default flag.
Updated flag to cvs DRUPAL-5, ran update.php - http://img.skitch.com/20081104-micd2dstr33jwbf1jqm1wxm2km.png
Flags still worked as expected, (un)bookmark.

quicksketch’s picture

quicksketch
he/him
commented

Thanks sirkitree. I hadn't rolled a new release since I wasn't sure if I liked the code I'd written in flag.install. It does work regardless though, so maybe I should go ahead and make another release and we can worry about making it more elegant later.

mooffie’s picture

mooffie commented

Nathan, I think we should release it as is. The code is ok.

We want to get user feedback about the new features, so it's important to release asap.

BTW, why are you doing _flag_column_exists() in the D5 version? I know why it's needed in the D6 one (in case the user is upgrading from D5), but I don't know why it's needed in D5.

quicksketch’s picture

quicksketch
he/him
commented

Yeah, that's one of the parts that wasn't really necessary. I realized it after putting it in. We can safely remove it without consequence I believe.

mooffie’s picture

mooffie commented

We can remove it later. I don't think it's that important. (Whatever, please keep _flag_column_exists(), maybe we will need it later.)

This issue blocks three other patches...

quicksketch’s picture

quicksketch
he/him
commented
Status: Active » Fixed

I've made new Drupal 5 and 6 releases. Since I already created the beta 5 release for Drupal 5 and I'm unable to delete releases, I created a beta 6 for both the Drupal 5 and Drupal 6 versions.

mooffie’s picture

mooffie commented

Thanks!

Status: Fixed » Closed (fixed)

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