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 ++- 5 files changed, 40 insertions(+), 16 deletions(-) (limited to 'Bugzilla') 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"); -- cgit v1.2.3-24-g4f1b