Bug 264601 - Bugzilla will now tell a user which field contained an "invalid bug

 number or alias."
Patch by Frédéric Buclin <LpSolit@netscape.net>
r=myk, a=justdave

diff --git a/CGI.pl b/CGI.pl
index 311d349ce8f8b7337c304a55c74092d199ab106e..af6b492add911d64b3aee7c203c3a21b54d48a89 100644 (file)
--- a/CGI.pl
+++ b/CGI.pl
@@ -148,7 +148,7 @@ sub ValidateBugID {
     # database, and that the user is authorized to access that bug.
     # We detaint the number here, too
 
-    my ($id, $skip_authorization) = @_;
+    my ($id, $field) = @_;
     
     # Get rid of white-space around the ID.
     $id = trim($id);
@@ -157,7 +157,9 @@ sub ValidateBugID {
     my $alias = $id;
     if (!detaint_natural($id)) {
         $id = BugAliasToID($alias);
-        $id || ThrowUserError("invalid_bug_id_or_alias", {'bug_id' => $alias});
+        $id || ThrowUserError("invalid_bug_id_or_alias",
+                              {'bug_id' => $alias,
+                               'field'  => $field });
     }
     
     # Modify the calling code's original variable to contain the trimmed,
@@ -170,7 +172,7 @@ sub ValidateBugID {
     FetchOneColumn()
       || ThrowUserError("invalid_bug_id_non_existent", {'bug_id' => $id});
 
-    return if $skip_authorization;
+    return if ($field eq "dependson" || $field eq "blocked");
     
     return if CanSeeBug($id, $::userid);
 

diff --git a/attachment.cgi b/attachment.cgi
index 06a04291e70de66e674e8be5c5472190bd09e578..c3b6307a1739ef845b37778b277aa4ae64dd74ba 100755 (executable)
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -1028,11 +1028,6 @@ sub update
   # Get the bug ID for the bug to which this attachment is attached.
   SendSQL("SELECT bug_id FROM attachments WHERE attach_id = $::FORM{'id'}");
   my $bugid = FetchSQLData();
-  unless ($bugid) 
-  {
-    ThrowUserError("invalid_bug_id",
-                   { bug_id => $bugid });
-  }
   
   # Lock database tables in preparation for updating the attachment.
   SendSQL("LOCK TABLES attachments WRITE , flags WRITE , " . 

diff --git a/post_bug.cgi b/post_bug.cgi
index 473a1563a273e270d17d647066cf5b84e60d790c..f5f6f340d9469a208bead7d462189bfdbca8b08a 100755 (executable)
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -257,7 +257,7 @@ foreach my $field ("dependson", "blocked") {
         my @validvalues;
         foreach my $id (split(/[\s,]+/, $::FORM{$field})) {
             next unless $id;
-            ValidateBugID($id, 1);
+            ValidateBugID($id, $field);
             push(@validvalues, $id);
         }
         $::FORM{$field} = join(",", @validvalues);

diff --git a/process_bug.cgi b/process_bug.cgi
index 2c4e96dd38a5571c1d7976a0f83bda1d438783f0..69586cf7901b9a0959743cba947c1da086d9490c 100755 (executable)
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -121,23 +121,13 @@ foreach my $field ("dependson", "blocked") {
         my @validvalues;
         foreach my $id (split(/[\s,]+/, $::FORM{$field})) {
             next unless $id;
-            ValidateBugID($id, 1);
+            ValidateBugID($id, $field);
             push(@validvalues, $id);
         }
         $::FORM{$field} = join(",", @validvalues);
     }
 }
 
-# If we are duping bugs, let's also make sure that we can change 
-# the original.  This takes care of issue A on bug 96085.
-if (defined $::FORM{'dup_id'} && $::FORM{'knob'} eq "duplicate") {
-    ValidateBugID($::FORM{'dup_id'});
-
-    # Also, let's see if the reporter has authorization to see the bug
-    # to which we are duping.  If not we need to prompt.
-    DuplicateUserConfirm();
-}
-
 # do a match on the fields if applicable
 
 # The order of these function calls is important, as both Flag::validate
@@ -491,8 +481,8 @@ sub DuplicateUserConfirm {
         return;
     }
 
-    my $dupe = trim($::FORM{'id'});
-    my $original = trim($::FORM{'dup_id'});
+    my $dupe = $::FORM{'id'};
+    my $original = $::FORM{'dup_id'};
     
     SendSQL("SELECT reporter FROM bugs WHERE bug_id = " . SqlQuote($dupe));
     my $reporter = FetchOneColumn();
@@ -520,7 +510,7 @@ sub DuplicateUserConfirm {
     $template->process("bug/process/confirm-duplicate.html.tmpl", $vars)
       || ThrowTemplateError($template->error());
     exit;
-} # end DuplicateUserConfirm()
+}
 
 if (defined $::FORM{'id'}) {
     # since this means that we were called from show_bug.cgi, now is a good
@@ -976,28 +966,22 @@ SWITCH: for ($::FORM{'knob'}) {
         last SWITCH;
     };
     /^duplicate$/ && CheckonComment( "duplicate" ) && do {
-        ChangeStatus('RESOLVED');
-        ChangeResolution('DUPLICATE');
-        CheckFormFieldDefined(\%::FORM,'dup_id');
-        my $num = trim($::FORM{'dup_id'});
-        SendSQL("SELECT bug_id FROM bugs WHERE bug_id = " . SqlQuote($num));
-        $num = FetchOneColumn();
-        if (!$num) {
-            ThrowUserError("dupe_invalid_bug_id")
-        }
-        if (!defined($::FORM{'id'}) || $num == $::FORM{'id'}) {
+        # Make sure we can change the original bug (issue A on bug 96085)
+        CheckFormFieldDefined(\%::FORM, 'dup_id');
+        ValidateBugID($::FORM{'dup_id'}, 'dup_id');
+
+        # Also, let's see if the reporter has authorization to see
+        # the bug to which we are duping. If not we need to prompt.
+        DuplicateUserConfirm();
+
+        $duplicate = $::FORM{'dup_id'};
+        if (!defined($::FORM{'id'}) || $duplicate == $::FORM{'id'}) {
             ThrowUserError("dupe_of_self_disallowed");
         }
-        my $checkid = trim($::FORM{'id'});
-        SendSQL("SELECT bug_id FROM bugs where bug_id = " .  SqlQuote($checkid));
-        $checkid = FetchOneColumn();
-        if (!$checkid) {
-            ThrowUserError("invalid_bug_id",
-                           { bug_id => $checkid });
-        }
-        $::FORM{'comment'} .= "\n\n*** This bug has been marked as a duplicate of $num ***";
-        $duplicate = $num;
-
+        ChangeStatus('RESOLVED');
+        ChangeResolution('DUPLICATE');
+        $::FORM{'comment'} .= "\n\n*** This bug has been marked " .
+                              "as a duplicate of $duplicate ***";
         last SWITCH;
     };
 

diff --git a/template/en/default/global/field-descs.none.tmpl b/template/en/default/global/field-descs.none.tmpl
index 7a21ca056a6825988037913234e9d6e3d69fed7e..da804ca87bd14e457d715392b744ff86ecc6152b 100644 (file)
--- a/template/en/default/global/field-descs.none.tmpl
+++ b/template/en/default/global/field-descs.none.tmpl
@@ -26,6 +26,7 @@
 [% field_descs = { "[Bug creation]"       => "[$terms.Bug creation]",
                    "alias"                => "Alias",
                    "assigned_to"          => "Assignee",
+                   "blocked"              => "Blocks",
                    "bug_file_loc"         => "URL",
                    "bug_id"               => "$terms.Bug ID",
                    "bug_severity"         => "Severity",
@@ -37,6 +38,8 @@
                    "component"            => "Component",
                    "creation_ts"          => "$terms.Bug Creation time",
                    "delta_ts"             => "Last Changed time",
+                   "dependson"            => "Depends on",
+                   "dup_id"               => "Duplicate",
                    "estimated_time"       => "Orig. Est.",
                    "everconfirmed"        => "Ever confirmed?",
                    "groupset"             => "Groupset",

diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 81a96885d2b813c5adfeb7d339d336694a25c079..d5d7b6b36788ecb2e2ccb5e4ac900cc502fd2b13 100644 (file)
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -152,11 +152,6 @@
     [% title = "Description Required" %]
     You must provide a description of the [% terms.bug %].
 
-  [% ELSIF error == "dupe_invalid_bug_id" %]
-    [% title = BLOCK %]Valid [% terms.Bug %] Number Required[% END %]
-    You must specify a valid [% terms.bug %] number of which this 
-    [%+ terms.bug %] is a duplicate.  The [% terms.bug %] has not been changed.
-
   [% ELSIF error == "dupe_of_self_disallowed" %]
     [% title = "Cannot mark $terms.abug as a duplicate of itself" %]
     You can't mark [% terms.abug %] as a duplicate of itself.
@@ -326,19 +321,24 @@
     [% title = "Invalid Attachment ID" %]
     The attachment id [% attach_id FILTER html %] is invalid.
 
-  [% ELSIF error == "invalid_bug_id" %]
-    [% title = BLOCK %]Invalid [% terms.Bug %] ID[% END %]
-    The [% terms.bug %] id [% bug_id FILTER html %] is invalid.
-
   [% ELSIF error == "invalid_bug_id_non_existent" %]
     [% title = BLOCK %]Invalid [% terms.Bug %] ID[% END %]
     [% terms.Bug %] #[% bug_id FILTER html %] does not exist.
     
   [% ELSIF error == "invalid_bug_id_or_alias" %]
     [% title = BLOCK %]Invalid [% terms.Bug %] ID[% END %]
-    '[% bug_id FILTER html %]' is not a valid [% terms.bug %] number
-    [% IF Param("usebugaliases") %] nor an alias
-    to [% terms.abug %] number[% END %].
+    [% IF bug_id %]
+      '[% bug_id FILTER html %]' is not a valid [% terms.bug %] number
+      [% IF Param("usebugaliases") %]
+        nor an alias to [% terms.abug %] number
+      [% END %].
+    [% ELSE %]
+      [% IF field %]
+        The '[% field_descs.$field FILTER html %]' field
+        cannot be empty.
+      [% END %]
+      You must enter a valid [% terms.bug %] number!
+    [% END %]
     <noscript>
       If you are trying to use QuickSearch, you need to enable
       JavaScript in your browser.