Bug 87404: Attachments don't work if you need to use user matching

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=justdave

3 files changed, 79 insertions, 4 deletions

diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 3e68647c6..ff3d38721 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -48,8 +48,20 @@ use base qw(Exporter);
 @Bugzilla::User::EXPORT = qw(insert_new_user is_available_username
     login_to_id
     UserInGroup
+    USER_MATCH_MULTIPLE USER_MATCH_FAILED USER_MATCH_SUCCESS
+    MATCH_SKIP_CONFIRM
 );
 
+#####################################################################
+# Constants
+#####################################################################
+
+use constant USER_MATCH_MULTIPLE => -1;
+use constant USER_MATCH_FAILED   => 0;
+use constant USER_MATCH_SUCCESS  => 1;
+
+use constant MATCH_SKIP_CONFIRM  => 1;
+
 ################################################################################
 # Functions
 ################################################################################
@@ -723,6 +735,11 @@ sub match {
 # searchable fields have been replaced by exact fields and the calling script
 # is executed as normal.
 #
+# You also have the choice of *never* displaying the confirmation screen.
+# In this case, match_field will return one of the three USER_MATCH 
+# constants described in the POD docs. To make match_field behave this
+# way, pass in MATCH_SKIP_CONFIRM as the third argument.
+#
 # match_field must be called early in a script, before anything external is
 # done with the form data.
 #
@@ -744,9 +761,11 @@ sub match {
 sub match_field {
     my $cgi          = shift;   # CGI object to look up fields in
     my $fields       = shift;   # arguments as a hash
+    my $behavior     = shift || 0; # A constant that tells us how to act
     my $matches      = {};      # the values sent to the template
     my $matchsuccess = 1;       # did the match fail?
     my $need_confirm = 0;       # whether to display confirmation screen
+    my $match_multiple = 0;     # whether we ever matched more than one user
 
     # prepare default form values
 
@@ -881,6 +900,7 @@ sub match_field {
             elsif ((scalar(@{$users}) > 1)
                     && (&::Param('maxusermatches') != 1)) {
                 $need_confirm = 1;
+                $match_multiple = 1;
 
                 if ((&::Param('maxusermatches'))
                    && (scalar(@{$users}) > &::Param('maxusermatches')))
@@ -899,7 +919,19 @@ sub match_field {
         }
     }
 
-    return 1 unless $need_confirm; # skip confirmation if not needed.
+    my $retval;
+    if (!$matchsuccess) {
+        $retval = USER_MATCH_FAILED;
+    }
+    elsif ($match_multiple) {
+        $retval = USER_MATCH_MULTIPLE;
+    }
+    else {
+        $retval = USER_MATCH_SUCCESS;
+    }
+
+    # Skip confirmation if we were told to, or if we don't need to confirm.
+    return $retval if ($behavior == MATCH_SKIP_CONFIRM || !$need_confirm);
 
     $vars->{'script'}        = $ENV{'SCRIPT_NAME'}; # for self-referencing URLs
     $vars->{'fields'}        = $fields; # fields being matched
@@ -1219,6 +1251,28 @@ there is currently no way to modify a user from this package.
 Note that the currently logged in user (if any) is available via
 L<Bugzilla-E<gt>user|Bugzilla/"user">.
 
+=head1 CONSTANTS
+
+=item C<USER_MATCH_MULTIPLE>
+
+Returned by C<match_field()> when at least one field matched more than 
+one user, but no matches failed.
+
+=item C<USER_MATCH_FAILED>
+
+Returned by C<match_field()> when at least one field failed to match 
+anything.
+
+=item C<USER_MATCH_SUCCESS>
+
+Returned by C<match_field()> when all fields successfully matched only one
+user.
+
+=item C<MATCH_SKIP_CONFIRM>
+
+Passed in to match_field to tell match_field to never display a 
+confirmation screen.
+
 =head1 METHODS
 
 =over 4
diff --git a/attachment.cgi b/attachment.cgi
index 1971f93a3..0c010a061 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -23,6 +23,7 @@
 #                 Daniel Raichle <draichle@gmx.net>
 #                 Dave Miller <justdave@syndicomm.com>
 #                 Alexander J. Vincent <ajvincent@juno.com>
+#                 Max Kanat-Alexander <mkanat@bugzilla.org>
 
 ################################################################################
 # Script Initialization
@@ -900,9 +901,18 @@ sub insert
     # The order of these function calls is important, as both Flag::validate
     # and FlagType::validate assume User::match_field has ensured that the
     # values in the requestee fields are legitimate user email addresses.
-    Bugzilla::User::match_field($cgi, {
-        '^requestee(_type)?-(\d+)$' => { 'type' => 'single' }
-    });
+    my $match_status = Bugzilla::User::match_field($cgi, {
+        '^requestee(_type)?-(\d+)$' => { 'type' => 'single' },
+    }, MATCH_SKIP_CONFIRM);
+
+    $vars->{'match_field'} = 'requestee';
+    if ($match_status == USER_MATCH_FAILED) {
+        $vars->{'message'} = 'user_match_failed';
+    }
+    elsif ($match_status == USER_MATCH_MULTIPLE) {
+        $vars->{'message'} = 'user_match_multiple';
+    }
+
     Bugzilla::Flag::validate($cgi, $bugid);
     Bugzilla::FlagType::validate($cgi, $bugid, $cgi->param('id'));
 
diff --git a/template/en/default/global/messages.html.tmpl b/template/en/default/global/messages.html.tmpl
index f9087a21f..30855ed37 100644
--- a/template/en/default/global/messages.html.tmpl
+++ b/template/en/default/global/messages.html.tmpl
@@ -17,6 +17,7 @@
   # Rights Reserved.
   #
   # Contributor(s): Gervase Markham <gerv@gerv.net>
+  #                 Max Kanat-Alexander <mkanat@bugzilla.org>
   #%]
 
 [%# This is a list of all the possible messages. Please keep them in
@@ -236,6 +237,16 @@
   [% ELSIF message_tag == "shutdown" %]
     [% title = "$terms.Bugzilla is Down" %]
     [% Param("shutdownhtml") %]
+
+  [% ELSIF message_tag == "user_match_failed" %]
+    You entered a username that did not match any known 
+    [% terms.Bugzilla %] users, so we have instead left
+    the [% match_field FILTER html %] field blank.
+
+  [% ELSIF message_tag == "user_match_multiple" %]
+    You entered a username that matched more than one
+    user, so we have instead left the [% match_field FILTER html %]
+    field blank.
     
   [% ELSE %]
     [%# Give sensible error if error functions are used incorrectly.