Midair with product change while adding an attachment silently clears flags - Patch by Tiago R. Mello <timello@gmail.com> r/a=LpSolit

6 files changed, 36 insertions, 18 deletions

diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm
index 736959b2f..cc3e16893 100644
--- a/Bugzilla/Attachment.pm
+++ b/Bugzilla/Attachment.pm
@@ -484,7 +484,7 @@ sub _validate_data {
         my $imgdata = $img->ImageToBlob();
         $data = $imgdata;
         $cgi->param('contenttype', 'image/png');
-        $$hr_vars->{'convertedbmp'} = 1;
+        $hr_vars->{'convertedbmp'} = 1;
     }
 
     # Make sure the attachment does not exceed the maximum permitted size
@@ -810,12 +810,12 @@ sub insert_attachment_for_bug {
         '^requestee(_type)?-(\d+)$' => { 'type' => 'multi' },
     }, MATCH_SKIP_CONFIRM);
 
-    $$hr_vars->{'match_field'} = 'requestee';
+    $hr_vars->{'match_field'} = 'requestee';
     if ($match_status == USER_MATCH_FAILED) {
-        $$hr_vars->{'message'} = 'user_match_failed';
+        $hr_vars->{'message'} = 'user_match_failed';
     }
     elsif ($match_status == USER_MATCH_MULTIPLE) {
-        $$hr_vars->{'message'} = 'user_match_multiple';
+        $hr_vars->{'message'} = 'user_match_multiple';
     }
 
     # Escape characters in strings that will be used in SQL statements.
@@ -902,12 +902,12 @@ sub insert_attachment_for_bug {
     Bugzilla->error_mode(ERROR_MODE_DIE);
     eval {
         Bugzilla::Flag::validate($cgi, $bug->bug_id, -1, SKIP_REQUESTEE_ON_ERROR);
-        Bugzilla::Flag::process($bug, $attachment, $timestamp, $cgi);
+        Bugzilla::Flag::process($bug, $attachment, $timestamp, $cgi, $hr_vars);
     };
     Bugzilla->error_mode($error_mode_cache);
     if ($@) {
-        $$hr_vars->{'message'} = 'flag_creation_failed';
-        $$hr_vars->{'flag_creation_error'} = $@;
+        $hr_vars->{'message'} = 'flag_creation_failed';
+        $hr_vars->{'flag_creation_error'} = $@;
     }
 
     # Return the new attachment object.
diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index a6914a661..3a4d03df6 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -502,7 +502,7 @@ sub snapshot {
 
 =over
 
-=item C<process($bug, $attachment, $timestamp, $cgi)>
+=item C<process($bug, $attachment, $timestamp, $cgi, $hr_vars)>
 
 Processes changes to flags.
 
@@ -516,7 +516,7 @@ object used to obtain the flag fields that the user submitted.
 =cut
 
 sub process {
-    my ($bug, $attachment, $timestamp, $cgi) = @_;
+    my ($bug, $attachment, $timestamp, $cgi, $hr_vars) = @_;
     my $dbh = Bugzilla->dbh;
 
     # Make sure the bug (and attachment, if given) exists and is accessible
@@ -540,7 +540,7 @@ sub process {
     }
 
     # Create new flags and update existing flags.
-    my $new_flags = FormToNewFlags($bug, $attachment, $cgi);
+    my $new_flags = FormToNewFlags($bug, $attachment, $cgi, $hr_vars);
     foreach my $flag (@$new_flags) { create($flag, $bug, $attachment, $timestamp) }
     modify($bug, $attachment, $cgi, $timestamp);
 
@@ -562,7 +562,10 @@ sub process {
     my $flags = Bugzilla::Flag->new_from_list($flag_ids);
     foreach my $flag (@$flags) {
         my $is_retargetted = retarget($flag, $bug);
-        clear($flag, $bug, $flag->attachment) unless $is_retargetted;
+        unless ($is_retargetted) {
+            clear($flag, $bug, $flag->attachment);
+            $hr_vars->{'message'} = 'flag_cleared';
+        }
     }
 
     $flag_ids = $dbh->selectcol_arrayref(
@@ -939,7 +942,7 @@ sub clear {
 
 =over
 
-=item C<FormToNewFlags($bug, $attachment, $cgi)>
+=item C<FormToNewFlags($bug, $attachment, $cgi, $hr_vars)>
 
 Checks whether or not there are new flags to create and returns an
 array of flag objects. This array is then passed to Flag::create().
@@ -949,7 +952,7 @@ array of flag objects. This array is then passed to Flag::create().
 =cut
 
 sub FormToNewFlags {
-    my ($bug, $attachment, $cgi) = @_;
+    my ($bug, $attachment, $cgi, $hr_vars) = @_;
     my $dbh = Bugzilla->dbh;
     my $setter = Bugzilla->user;
     
@@ -966,6 +969,14 @@ sub FormToNewFlags {
           'component_id' => $bug->{'component_id'},
           'is_active'    => 1 });
 
+    foreach my $type_id (@type_ids) {
+        # Checks if there are unexpected flags for the product/component.
+        if (!scalar(grep { $_->id == $type_id } @$flag_types)) {
+            $hr_vars->{'message'} = 'unexpected_flag_types';
+            last;
+        }
+    }
+
     my @flags;
     foreach my $flag_type (@$flag_types) {
         my $type_id = $flag_type->id;
diff --git a/attachment.cgi b/attachment.cgi
index d9521b591..82f650f3c 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -348,7 +348,7 @@ sub insert {
     my $bug = new Bugzilla::Bug($bugid);
     my $attachment =
         Bugzilla::Attachment->insert_attachment_for_bug(THROW_ERROR, $bug, $user,
-                                                        $timestamp, \$vars);
+                                                        $timestamp, $vars);
 
   # Insert a comment about the new attachment into the database.
   my $comment = "Created an attachment (id=" . $attachment->id . ")\n" .
@@ -508,7 +508,7 @@ sub update {
   # to attachments so that we can delete pending requests if the user
   # is obsoleting this attachment without deleting any requests
   # the user submits at the same time.
-  Bugzilla::Flag::process($bug, $attachment, $timestamp, $cgi);
+  Bugzilla::Flag::process($bug, $attachment, $timestamp, $cgi, $vars);
 
   # Update the attachment record in the database.
   $dbh->do("UPDATE  attachments 
diff --git a/post_bug.cgi b/post_bug.cgi
index 759c9c250..8ce7f0723 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -191,7 +191,7 @@ if (defined $cgi->param('version')) {
 if (defined($cgi->upload('data')) || $cgi->param('attachurl')) {
     $cgi->param('isprivate', $cgi->param('commentprivacy'));
     my $attachment = Bugzilla::Attachment->insert_attachment_for_bug(!THROW_ERROR,
-                                                  $bug, $user, $timestamp, \$vars);
+                                                  $bug, $user, $timestamp, $vars);
 
     if ($attachment) {
         # Update the comment to include the new attachment ID.
@@ -227,7 +227,7 @@ my $error_mode_cache = Bugzilla->error_mode;
 Bugzilla->error_mode(ERROR_MODE_DIE);
 eval {
     Bugzilla::Flag::validate($cgi, $id, undef, SKIP_REQUESTEE_ON_ERROR);
-    Bugzilla::Flag::process($bug, undef, $timestamp, $cgi);
+    Bugzilla::Flag::process($bug, undef, $timestamp, $cgi, $vars);
 };
 Bugzilla->error_mode($error_mode_cache);
 if ($@) {
diff --git a/process_bug.cgi b/process_bug.cgi
index 3e1246e75..56931c293 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -1140,7 +1140,7 @@ foreach my $id (@idlist) {
         }
     }
     # Set and update flags.
-    Bugzilla::Flag::process($new_bug_obj, undef, $timestamp, $cgi);
+    Bugzilla::Flag::process($new_bug_obj, undef, $timestamp, $cgi, $vars);
 
     if ($bug_changed) {
         $dbh->do(q{UPDATE bugs SET delta_ts = ? WHERE bug_id = ?},
diff --git a/template/en/default/global/messages.html.tmpl b/template/en/default/global/messages.html.tmpl
index 6db207dd4..b9b423ca6 100644
--- a/template/en/default/global/messages.html.tmpl
+++ b/template/en/default/global/messages.html.tmpl
@@ -201,6 +201,10 @@
     [%+ new_email FILTER html %] has been canceled.
    Your old account settings have been reinstated.
 
+  [% ELSIF message_tag == "flag_cleared" %]
+    Some flags didn't apply in the new product/component
+    and have been cleared.
+
   [% ELSIF message_tag == "flag_creation_failed" %]
     [% title = "Flag Creation Failure" %]
     An error occured while validating flags:
@@ -481,6 +485,9 @@
   [% ELSIF message_tag == "term" %]
     [% terms.$term FILTER html %]
 
+  [% ELSIF message_tag == "unexpected_flag_types" %]
+    Some flags could not be set. Please check your changes.
+
   [% ELSIF message_tag == "user_match_failed" %]
     You entered a username that did not match any known 
     [% terms.Bugzilla %] users, so we have instead left