From 6c0b6e11bf1e302320bebe0ce9545eff924ab124 Mon Sep 17 00:00:00 2001 From: "jouni%heikniemi.net" <> Date: Tue, 6 Jul 2004 14:08:02 +0000 Subject: Bug 223878: Flag system dies when changing a deleted flag. r=joel, justdave a=justdave --- Bugzilla/Attachment.pm | 3 ++- Bugzilla/Bug.pm | 9 +++++---- Bugzilla/Flag.pm | 39 +++++++++++++++++++++++++++++---------- Bugzilla/FlagType.pm | 2 ++ Bugzilla/Search.pm | 3 ++- attachment.cgi | 10 +++++++--- checksetup.pl | 7 +++++++ editflagtypes.cgi | 5 ++++- request.cgi | 3 +++ 9 files changed, 61 insertions(+), 20 deletions(-) diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm index 9f0467bb7..e7b3ffe86 100644 --- a/Bugzilla/Attachment.pm +++ b/Bugzilla/Attachment.pm @@ -90,7 +90,8 @@ sub query $a{'datasize'}) = &::FetchSQLData(); # Retrieve a list of flags for this attachment. - $a{'flags'} = Bugzilla::Flag::match({ 'attach_id' => $a{'attachid'} }); + $a{'flags'} = Bugzilla::Flag::match({ 'attach_id' => $a{'attachid'}, + 'is_active' => 1 }); # We will display the edit link if the user can edit the attachment; # ie the are the submitter, or they have canedit. diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 91cd0d8c8..f1a1cf341 100755 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -226,7 +226,8 @@ sub initBug { $flag_type->{'flags'} = Bugzilla::Flag::match({ 'bug_id' => $self->{bug_id}, 'type_id' => $flag_type->{'id'}, - 'target_type' => 'bug' }); + 'target_type' => 'bug', + 'is_active' => 1 }); } $self->{'flag_types'} = $flag_types; $self->{'any_flags_requesteeble'} = grep($_->{'is_requesteeble'}, @$flag_types); @@ -238,11 +239,11 @@ sub initBug { my $num_attachment_flag_types = Bugzilla::FlagType::count({ 'target_type' => 'attachment', 'product_id' => $self->{'product_id'}, - 'component_id' => $self->{'component_id'}, - 'is_active' => 1 }); + 'component_id' => $self->{'component_id'} }); my $num_attachment_flags = Bugzilla::Flag::count({ 'target_type' => 'attachment', - 'bug_id' => $self->{bug_id} }); + 'bug_id' => $self->{bug_id}, + 'is_active' => 1 }); $self->{'show_attachment_flags'} = $num_attachment_flag_types || $num_attachment_flags; diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index 3272b8409..707316ce1 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -18,6 +18,7 @@ # Rights Reserved. # # Contributor(s): Myk Melez +# Jouni Heikniemi ################################################################################ # Module Initialization @@ -52,8 +53,8 @@ use vars qw($template $vars); # basic sets of columns and tables for getting flags from the database my @base_columns = - ("1", "id", "type_id", "bug_id", "attach_id", "requestee_id", "setter_id", - "status"); + ("is_active", "id", "type_id", "bug_id", "attach_id", "requestee_id", + "setter_id", "status"); # Note: when adding tables to @base_tables, make sure to include the separator # (i.e. a comma or words like "LEFT OUTER JOIN") before the table name, @@ -154,6 +155,11 @@ sub validate { my $flag = get($id); $flag || ThrowCodeError("flag_nonexistent", { id => $id }); + # Note that the deletedness of the flag (is_active or not) is not + # checked here; we do want to allow changes to deleted flags in + # certain cases. Flag::modify() will revive the modified flags. + # See bug 223878 for details. + # Make sure the user chose a valid status. grep($status eq $_, qw(X + - ?)) || ThrowCodeError("flag_status_invalid", @@ -226,7 +232,8 @@ sub process { # Take a snapshot of flags before any changes. my $flags = match({ 'bug_id' => $target->{'bug'}->{'id'} , - 'attach_id' => $target->{'attachment'}->{'id'} }); + 'attach_id' => $target->{'attachment'}->{'id'} , + 'is_active' => 1 }); my @old_summaries; foreach my $flag (@$flags) { my $summary = $flag->{'type'}->{'name'} . $flag->{'status'}; @@ -249,6 +256,7 @@ sub process { AND (bugs.product_id = i.product_id OR i.product_id IS NULL) AND (bugs.component_id = i.component_id OR i.component_id IS NULL)) WHERE bugs.bug_id = $target->{'bug'}->{'id'} + AND flags.is_active = 1 AND i.type_id IS NULL "); clear(&::FetchOneColumn()) while &::MoreSQLData(); @@ -257,7 +265,8 @@ sub process { FROM flags, bugs, flagexclusions e WHERE bugs.bug_id = $target->{'bug'}->{'id'} AND flags.bug_id = bugs.bug_id - AND flags.type_id = e.type_id + AND flags.type_id = e.type_id + AND flags.is_active = 1 AND (bugs.product_id = e.product_id OR e.product_id IS NULL) AND (bugs.component_id = e.component_id OR e.component_id IS NULL) "); @@ -265,7 +274,8 @@ sub process { # Take a snapshot of flags after changes. $flags = match({ 'bug_id' => $target->{'bug'}->{'id'} , - 'attach_id' => $target->{'attachment'}->{'id'} }); + 'attach_id' => $target->{'attachment'}->{'id'} , + 'is_active' => 1 }); my @new_summaries; foreach my $flag (@$flags) { my $summary = $flag->{'type'}->{'name'} . $flag->{'status'}; @@ -340,6 +350,9 @@ sub migrate { sub modify { # Modifies flags in the database when a user changes them. + # Note that modified flags are always set active (is_active = 1) - + # this will revive deleted flags that get changed through + # attachment.cgi midairs. See bug 223878 for details. my ($data, $timestamp) = @_; @@ -385,7 +398,8 @@ sub modify { SET setter_id = $::userid , requestee_id = NULL , status = '$status' , - modification_date = $timestamp + modification_date = $timestamp , + is_active = 1 WHERE id = $flag->{'id'}"); # Send an email notifying the relevant parties about the fulfillment. @@ -409,7 +423,8 @@ sub modify { SET setter_id = $::userid , requestee_id = $requestee_id , status = '$status' , - modification_date = $timestamp + modification_date = $timestamp , + is_active = 1 WHERE id = $flag->{'id'}"); # Send an email notifying the relevant parties about the request. @@ -420,7 +435,7 @@ sub modify { notify($flag, "request/email.txt.tmpl"); } } - # The user unset the flag, so delete it from the database. + # The user unset the flag; set is_active = 0 elsif ($status eq 'X') { clear($flag->{'id'}); } @@ -437,9 +452,10 @@ sub clear { my $flag = get($id); &::PushGlobalSQLState(); - &::SendSQL("DELETE FROM flags WHERE id = $id"); + &::SendSQL("UPDATE flags SET is_active = 0 WHERE id = $id"); &::PopGlobalSQLState(); - + + $flag->{'exists'} = 0; # Set the flag's status to "cleared" so the email template # knows why email is being sent about the request. $flag->{'status'} = "X"; @@ -625,6 +641,7 @@ sub sqlify_criteria { elsif ($field eq 'requestee_id') { push(@criteria, "requestee_id = $value") } elsif ($field eq 'setter_id') { push(@criteria, "setter_id = $value") } elsif ($field eq 'status') { push(@criteria, "status = '$value'") } + elsif ($field eq 'is_active') { push(@criteria, "is_active = $value") } } return @criteria; @@ -635,6 +652,8 @@ sub perlify_record { my ($exists, $id, $type_id, $bug_id, $attach_id, $requestee_id, $setter_id, $status) = @_; + return undef unless defined($exists); + my $flag = { exists => $exists , diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm index e6bfaf7ef..a428d5389 100644 --- a/Bugzilla/FlagType.pm +++ b/Bugzilla/FlagType.pm @@ -270,6 +270,7 @@ sub normalize { AND (bugs.product_id = i.product_id OR i.product_id IS NULL) AND (bugs.component_id = i.component_id OR i.component_id IS NULL)) WHERE flags.type_id IN ($ids) + AND flags.is_active = 1 AND i.type_id IS NULL "); Bugzilla::Flag::clear(&::FetchOneColumn()) while &::MoreSQLData(); @@ -280,6 +281,7 @@ sub normalize { WHERE flags.type_id IN ($ids) AND flags.bug_id = bugs.bug_id AND flags.type_id = e.type_id + AND flags.is_active = 1 AND (bugs.product_id = e.product_id OR e.product_id IS NULL) AND (bugs.component_id = e.component_id OR e.component_id IS NULL) "); diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index 2f92131fc..0ebf1e070 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -586,7 +586,8 @@ sub init { # negative conditions (f.e. "flag isn't review+"). my $flags = "flags_$chartid"; push(@supptables, "LEFT JOIN flags $flags " . - "ON bugs.bug_id = $flags.bug_id"); + "ON bugs.bug_id = $flags.bug_id " . + "AND $flags.is_active = 1"); my $flagtypes = "flagtypes_$chartid"; push(@supptables, "LEFT JOIN flagtypes $flagtypes " . "ON $flags.type_id = $flagtypes.id"); diff --git a/attachment.cgi b/attachment.cgi index c1e8f9dd0..06a04291e 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -768,7 +768,8 @@ sub viewall $a{'description'}, $a{'ispatch'}, $a{'isobsolete'}, $a{'isprivate'}, $a{'datasize'}) = FetchSQLData(); $a{'isviewable'} = isViewable($a{'contenttype'}); - $a{'flags'} = Bugzilla::Flag::match({ 'attach_id' => $a{'attachid'} }); + $a{'flags'} = Bugzilla::Flag::match({ 'attach_id' => $a{'attachid'}, + 'is_active' => 1 }); # Add the hash representing the attachment to the array of attachments. push @attachments, \%a; @@ -880,7 +881,9 @@ sub insert SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, fieldid, removed, added) VALUES ($::FORM{'bugid'}, $obsolete_id, $::userid, NOW(), $fieldid, '0', '1')"); # If the obsolete attachment has pending flags, migrate them to the new attachment. - if (Bugzilla::Flag::count({ 'attach_id' => $obsolete_id , 'status' => 'pending' })) { + if (Bugzilla::Flag::count({ 'attach_id' => $obsolete_id , + 'status' => 'pending', + 'is_active' => 1 })) { Bugzilla::Flag::migrate($obsolete_id, $attachid); } } @@ -984,7 +987,8 @@ sub edit 'component_id' => $component_id }); foreach my $flag_type (@$flag_types) { $flag_type->{'flags'} = Bugzilla::Flag::match({ 'type_id' => $flag_type->{'id'}, - 'attach_id' => $::FORM{'id'} }); + 'attach_id' => $::FORM{'id'}, + 'is_active' => 1 }); } $vars->{'flag_types'} = $flag_types; $vars->{'any_flags_requesteeble'} = grep($_->{'is_requesteeble'}, @$flag_types); diff --git a/checksetup.pl b/checksetup.pl index 684a8ca7e..3c4793407 100755 --- a/checksetup.pl +++ b/checksetup.pl @@ -1609,6 +1609,8 @@ $table{flags} = setter_id MEDIUMINT NULL , requestee_id MEDIUMINT NULL , + + is_active TINYINT NOT NULL DEFAULT 1, INDEX(bug_id, attach_id) , INDEX(setter_id) , @@ -3935,6 +3937,11 @@ if (GetFieldDef("user_group_map", "isderived")) { } } +# 2004-07-03 - Make it possible to disable flags without deleting them +# from the database. Bug 223878, jouni@heikniemi.net + +AddField('flags', 'is_active', 'tinyint not null default 1'); + # If you had to change the --TABLE-- definition in any way, then add your diff --git a/editflagtypes.cgi b/editflagtypes.cgi index 5fcabd73f..bb1b86ec0 100755 --- a/editflagtypes.cgi +++ b/editflagtypes.cgi @@ -308,6 +308,7 @@ sub update { AND (bugs.component_id = i.component_id OR i.component_id IS NULL)) WHERE flags.type_id = $::FORM{'id'} AND flags.bug_id = bugs.bug_id + AND flags.is_active = 1 AND i.type_id IS NULL "); Bugzilla::Flag::clear(FetchOneColumn()) while MoreSQLData(); @@ -318,6 +319,7 @@ sub update { WHERE flags.type_id = $::FORM{'id'} AND flags.bug_id = bugs.bug_id AND flags.type_id = e.type_id + AND flags.is_active = 1 AND (bugs.product_id = e.product_id OR e.product_id IS NULL) AND (bugs.component_id = e.component_id OR e.component_id IS NULL) "); @@ -340,7 +342,8 @@ sub confirmDelete validateID(); # check if we need confirmation to delete: - my $count = Bugzilla::Flag::count({ 'type_id' => $::FORM{'id'} }); + my $count = Bugzilla::Flag::count({ 'type_id' => $::FORM{'id'}, + 'is_active' => 1 }); if ($count > 0) { $vars->{'flag_type'} = Bugzilla::FlagType::get($::FORM{'id'}); diff --git a/request.cgi b/request.cgi index e330c2c83..047c4fa14 100755 --- a/request.cgi +++ b/request.cgi @@ -116,6 +116,9 @@ sub queue { AND flags.bug_id = bugs.bug_id "; + # Non-deleted flags only + $query .= " AND flags.is_active = 1 "; + # Limit query to pending requests. $query .= " AND flags.status = '?' " unless $cgi->param('status'); -- cgit v1.2.3-24-g4f1b