322285 - Cancelling a flag should remove it completely from the DB

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, enhancement)

Description

[Frédéric Buclin]

Actually, when a flag is cancelled, its is_active bit is set to 0, but the flag itself is kept in the DB. Per discussion with myk on IRC, cancelled flags should be removed completely and the is_active bit should go away.

Comment 1

[Frédéric Buclin]

For the record, and before someone is going to complain: yes, I know is_active has been introduced in bug 223878. And I say that this solution is a big hack!

The right fix is to change flags.id to an auto-increment field. And that's the goal of Flag.pm to catch deleted flags.

When we delete a product or a bug, we don't mark them as inactive!

Comment 2

[Frédéric Buclin]

Attachment: patch, v1

Tested successfully with MySQL.

PostgreSQL is complaining:

checksetup.pl: DBD::Pg::db do failed: ERROR: syntax error near «unique» at character 26
checksetup.pl:  [for Statement "SELECT CAST(id AS serial unique) FROM flags LIMIT 1"] at Bugzilla/DB.pm line 443
checksetup.pl:       Bugzilla::DB::bz_alter_column_raw('Bugzilla::DB::Pg=HASH(0xa038e24)', 'flags', 'id', 'HASH(0xa168eb4)', 'HASH(0xa0e5c08)', 'undef') called at Bugzilla/DB.pm line 402
checksetup.pl:       Bugzilla::DB::bz_alter_column('Bugzilla::DB::Pg=HASH(0xa038e24)', 'flags', 'id', 'HASH(0xa168eb4)') called at ./checksetup.pl line 4286

Comment 3

[Frédéric Buclin]

mkanat, as expected, PostgreSQL is complaining. A workaround could be to create a new column tmp_id with the auto_increment format, then delete the column id and then rename tmp_id to id. But as MySQL is working fine with my current patch, I think Pg.pm should be fixed instead. Especially because I plan to do the same change in the flagtypes table in another patch. ;)

Comment 4

[Frédéric Buclin]

Comment on attachment 213761 [details] [diff] [review]
patch, v1

mkanat, if you are faster than myk, could you review this one?

Comment 5

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 213761 [details] [diff] [review]
patch, v1

>-    # Get a list of active flag types available for this target.
>+    # Get a list of flag types available for this target.
>     my $flag_types = Bugzilla::FlagType::match(
>         { 'target_type'  => $target->{'type'},
>           'product_id'   => $target->{'product_id'},
>-          'component_id' => $target->{'component_id'},
>-          'is_active'    => 1 });
>+          'component_id' => $target->{'component_id'} });

This is FlagType.is_active, which is different from flags.is_active and has a valid use.

Comment 6

[Frédéric Buclin]

Attachment: patch, v1.1

fix myk's comment.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 217495 [details] [diff] [review]
patch, v1.1

Looks good, seems to work. r=myk

Comment 8

[Frédéric Buclin]

Attachment: unbitrotten patch, v1.2

There was a small bitrot in attachment.cgi due to the removal of deprecated DB routines in this file.

Comment 9

[Frédéric Buclin]

Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.107; previous revision: 1.106
done
Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.476; previous revision: 1.475
done
Checking in editflagtypes.cgi;
/cvsroot/mozilla/webtools/bugzilla/editflagtypes.cgi,v  <--  editflagtypes.cgi
new revision: 1.34; previous revision: 1.33
done
Checking in editusers.cgi;
/cvsroot/mozilla/webtools/bugzilla/editusers.cgi,v  <--  editusers.cgi
new revision: 1.118; previous revision: 1.117
done
Checking in importxml.pl;
/cvsroot/mozilla/webtools/bugzilla/importxml.pl,v  <--  importxml.pl
new revision: 1.51; previous revision: 1.50
done
Checking in request.cgi;
/cvsroot/mozilla/webtools/bugzilla/request.cgi,v  <--  request.cgi
new revision: 1.31; previous revision: 1.30
done
Checking in Bugzilla/Attachment.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Attachment.pm,v  <--  Attachment.pm
new revision: 1.31; previous revision: 1.30
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.117; previous revision: 1.116
done
Checking in Bugzilla/Flag.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v  <--  Flag.pm
new revision: 1.61; previous revision: 1.60
done
Checking in Bugzilla/FlagType.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/FlagType.pm,v  <--  FlagType.pm
new revision: 1.24; previous revision: 1.23
done
Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.126; previous revision: 1.125
done
Checking in Bugzilla/DB/Schema.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Schema.pm,v  <--  Schema.pm
new revision: 1.49; previous revision: 1.48
done