From db2e681e87ad1d15bc0c8f8ef08e222da7ea97c1 Mon Sep 17 00:00:00 2001 From: Frédéric Buclin Date: Mon, 1 Feb 2010 13:11:53 -0800 Subject: Bug 534057: Auto-completion no longer works in email_in.pl Patch by Frédéric Buclin r/a=mkanat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Bugzilla/Flag.pm | 4 +- Bugzilla/User.pm | 194 +++++++++++------------- editcomponents.cgi | 4 +- editwhines.cgi | 2 +- email_in.pl | 11 ++ post_bug.cgi | 2 +- process_bug.cgi | 2 +- request.cgi | 2 +- template/en/default/global/user-error.html.tmpl | 6 + userprefs.cgi | 2 +- 10 files changed, 114 insertions(+), 115 deletions(-) diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index 2fa4c8ded..d33b14d31 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -790,9 +790,9 @@ sub extract_flags_from_cgi { my ($class, $bug, $attachment, $vars, $skip) = @_; my $cgi = Bugzilla->cgi; - my $match_status = Bugzilla::User::match_field($cgi, { + my $match_status = Bugzilla::User::match_field({ '^requestee(_type)?-(\d+)$' => { 'type' => 'multi' }, - }, $skip); + }, undef, $skip); $vars->{'match_field'} = 'requestee'; if ($match_status == USER_MATCH_FAILED) { diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 75a4fcf1d..59324383f 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -1124,57 +1124,15 @@ sub match { return \@users; } -# match_field() is a CGI wrapper for the match() function. -# -# Here's what it does: -# -# 1. Accepts a list of fields along with whether they may take multiple values -# 2. Takes the values of those fields from the first parameter, a $cgi object -# and passes them to match() -# 3. Checks the results of the match and displays confirmation or failure -# messages as appropriate. -# -# The confirmation screen functions the same way as verify-new-product and -# confirm-duplicate, by rolling all of the state information into a -# form which is passed back, but in this case the searched fields are -# replaced with the search results. -# -# The act of displaying the confirmation or failure messages means it must -# throw a template and terminate. When confirmation is sent, all of the -# 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. -# -# In order to do a simple match without dealing with templates, confirmation, -# or globals, simply calling Bugzilla::User::match instead will be -# sufficient. - -# How to call it: -# -# Bugzilla::User::match_field($cgi, { -# 'field_name' => { 'type' => fieldtype }, -# 'field_name2' => { 'type' => fieldtype }, -# [...] -# }); -# -# fieldtype can be either 'single' or 'multi'. -# - sub match_field { - my $cgi = shift; # CGI object to look up fields in my $fields = shift; # arguments as a hash + my $data = shift || Bugzilla->input_params; # hash to look up fields in 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 + my @non_conclusive_fields; # fields which don't have a unique user. my $params = Bugzilla->params; @@ -1192,7 +1150,8 @@ sub match_field { $expanded_fields->{$field_pattern} = $fields->{$field_pattern}; } else { - my @field_names = grep(/$field_pattern/, $cgi->param()); + my @field_names = grep(/$field_pattern/, keys %$data); + foreach my $field_name (@field_names) { $expanded_fields->{$field_name} = { type => $fields->{$field_pattern}->{'type'} }; @@ -1218,7 +1177,7 @@ sub match_field { # No need to look for a valid requestee if the flag(type) # has been deleted (may occur in race conditions). delete $expanded_fields->{$field_name}; - $cgi->delete($field_name); + delete $data->{$field_name}; } } } @@ -1227,35 +1186,19 @@ sub match_field { $fields = $expanded_fields; for my $field (keys %{$fields}) { + #Concatenate login names, so that we have a common way to handle them. + my $raw_field; + if (ref $data->{$field}) { + $raw_field = join(" ", @{$data->{$field}}); + } + else { + $raw_field = $data->{$field}; + } + $raw_field = clean_text($raw_field || ''); - # Tolerate fields that do not exist. - # - # This is so that fields like qa_contact can be specified in the code - # and it won't break if the CGI object does not know about them. - # - # It has the side-effect that if a bad field name is passed it will be - # quietly ignored rather than raising a code error. - - next if !defined $cgi->param($field); - - # We need to move the query to $raw_field, where it will be split up, - # modified by the search, and put back into the CGI environment - # incrementally. - - my $raw_field = join(" ", $cgi->param($field)); - - # When we add back in values later, it matters that we delete - # the field here, and not set it to '', so that we will add - # things to an empty list, and not to a list containing one - # empty string. - # If the field accepts only one match (type eq "single") and - # no match or more than one match is found for this field, - # we will set it back to '' so that the field remains defined - # outside this function (it was if we came here; else we would - # have returned earlier above). - # If the field accepts several matches (type eq "multi") and no match - # is found, we leave this field undefined (= empty array). - $cgi->delete($field); + # Tolerate fields that do not exist (in case you specify + # e.g. the QA contact, and it's currently not in use). + next unless ($raw_field && $raw_field ne ''); my @queries = (); @@ -1263,11 +1206,9 @@ sub match_field { # into @queries, or in the case of fields which only accept single # entries, we simply use the verbatim text. - $raw_field =~ s/^\s+|\s+$//sg; # trim leading/trailing space - # single field if ($fields->{$field}->{'type'} eq 'single') { - @queries = ($raw_field) unless $raw_field =~ /^\s*$/; + @queries = ($raw_field); # multi-field } @@ -1288,35 +1229,22 @@ sub match_field { $limit = $params->{'maxusermatches'} + 1; } + my @logins; for my $query (@queries) { - my $users = match( $query, # match string $limit, # match limit 1 # exclude_disabled ); - # skip confirmation for exact matches - if ((scalar(@{$users}) == 1) - && (lc(@{$users}[0]->login) eq lc($query))) - - { - $cgi->append(-name=>$field, - -values=>[@{$users}[0]->login]); - - next; - } - - $matches->{$field}->{$query}->{'users'} = $users; - $matches->{$field}->{$query}->{'status'} = 'success'; - # here is where it checks for multiple matches - if (scalar(@{$users}) == 1) { # exactly one match + push(@logins, @{$users}[0]->login); - $cgi->append(-name=>$field, - -values=>[@{$users}[0]->login]); + # skip confirmation for exact matches + next if (lc(@{$users}[0]->login) eq lc($query)); + $matches->{$field}->{$query}->{'status'} = 'success'; $need_confirm = 1 if $params->{'confirmuniqueusermatch'}; } @@ -1324,6 +1252,7 @@ sub match_field { && ($params->{'maxusermatches'} != 1)) { $need_confirm = 1; $match_multiple = 1; + push(@non_conclusive_fields, $field); if (($params->{'maxusermatches'}) && (scalar(@{$users}) > $params->{'maxusermatches'})) @@ -1331,23 +1260,31 @@ sub match_field { $matches->{$field}->{$query}->{'status'} = 'trunc'; pop @{$users}; # take the last one out } + else { + $matches->{$field}->{$query}->{'status'} = 'success'; + } } else { # everything else fails $matchsuccess = 0; # fail + push(@non_conclusive_fields, $field); $matches->{$field}->{$query}->{'status'} = 'fail'; $need_confirm = 1; # confirmation screen shows failures } + + $matches->{$field}->{$query}->{'users'} = $users; } - # Above, we deleted the field before adding matches. If no match - # or more than one match has been found for a field expecting only - # one match (type eq "single"), we set it back to '' so - # that the caller of this function can still check whether this + + # If no match or more than one match has been found for a field + # expecting only one match (type eq "single"), we set it back to '' + # so that the caller of this function can still check whether this # field was defined or not (and it was if we came here). - if (!defined $cgi->param($field) - && $fields->{$field}->{'type'} eq 'single') { - $cgi->param($field, ''); + if ($fields->{$field}->{'type'} eq 'single') { + $data->{$field} = $logins[0] || ''; + } + else { + $data->{$field} = \@logins; } } @@ -1363,22 +1300,24 @@ sub match_field { } # Skip confirmation if we were told to, or if we don't need to confirm. - return $retval if ($behavior == MATCH_SKIP_CONFIRM || !$need_confirm); + if ($behavior == MATCH_SKIP_CONFIRM || !$need_confirm) { + return wantarray ? ($retval, \@non_conclusive_fields) : $retval; + } my $template = Bugzilla->template; + my $cgi = Bugzilla->cgi; my $vars = {}; - $vars->{'script'} = Bugzilla->cgi->url(-relative => 1); # for self-referencing URLs + $vars->{'script'} = $cgi->url(-relative => 1); # for self-referencing URLs $vars->{'fields'} = $fields; # fields being matched $vars->{'matches'} = $matches; # matches that were made $vars->{'matchsuccess'} = $matchsuccess; # continue or fail $vars->{'matchmultiple'} = $match_multiple; - print Bugzilla->cgi->header(); + print $cgi->header(); $template->process("global/confirm-user-match.html.tmpl", $vars) || ThrowTemplateError($template->error()); - exit; } @@ -2296,6 +2235,49 @@ Untaints C<$passwd1> if successful. If a second password is passed in, this function also verifies that the two passwords match. +=item C + +=over + +=item B + +Wrapper for the C function. + +=item B + +=over + +=item C<$fields> - A hashref with field names as keys and a hash as values. +Each hash is of the form { 'type' => 'single|multi' }, which specifies +whether the field can take a single login name only or several. + +=item C<$data> (optional) - A hashref with field names as keys and field values +as values. If undefined, Cinput_params> is used. + +=item C<$behavior> (optional) - If set to C, no confirmation +screen is displayed. In that case, the fields which don't match a unique user +are left undefined. If not set, a confirmation screen is displayed if at +least one field doesn't match any login name or match more than one. + +=back + +=item B + +If the third parameter is set to C, the function returns +either C if all fields can be set unambiguously, +C if at least one field doesn't match any user account, +or C if some fields match more than one user account. + +If the third parameter is not set, then if all fields could be set +unambiguously, nothing is returned, else a confirmation page is displayed. + +=item B + +This function must be called early in a script, before anything external +is done with the data. + +=back + =back =head1 SEE ALSO diff --git a/editcomponents.cgi b/editcomponents.cgi index c550f0d8c..70afd9c9f 100755 --- a/editcomponents.cgi +++ b/editcomponents.cgi @@ -118,7 +118,7 @@ if ($action eq 'add') { if ($action eq 'new') { check_token_data($token, 'add_component'); # Do the user matching - Bugzilla::User::match_field ($cgi, { + Bugzilla::User::match_field ({ 'initialowner' => { 'type' => 'single' }, 'initialqacontact' => { 'type' => 'single' }, 'initialcc' => { 'type' => 'multi' }, @@ -219,7 +219,7 @@ if ($action eq 'edit') { if ($action eq 'update') { check_token_data($token, 'edit_component'); # Do the user matching - Bugzilla::User::match_field ($cgi, { + Bugzilla::User::match_field ({ 'initialowner' => { 'type' => 'single' }, 'initialqacontact' => { 'type' => 'single' }, 'initialcc' => { 'type' => 'multi' }, diff --git a/editwhines.cgi b/editwhines.cgi index 671774ef7..c59cf8be5 100755 --- a/editwhines.cgi +++ b/editwhines.cgi @@ -193,7 +193,7 @@ if ($cgi->param('update')) { } } if (scalar %{$arglist}) { - &Bugzilla::User::match_field($cgi, $arglist); + Bugzilla::User::match_field($arglist); } for my $sid (@scheduleids) { diff --git a/email_in.pl b/email_in.pl index 044c13040..c7b11d8a7 100755 --- a/email_in.pl +++ b/email_in.pl @@ -152,6 +152,17 @@ sub post_bug { $fields->{$field} = undef if !exists $fields->{$field}; } + my ($retval, $non_conclusive_fields) = + Bugzilla::User::match_field({ + 'assigned_to' => { 'type' => 'single' }, + 'qa_contact' => { 'type' => 'single' }, + 'cc' => { 'type' => 'multi' } + }, $fields, MATCH_SKIP_CONFIRM); + + if ($retval != USER_MATCH_SUCCESS) { + ThrowUserError('user_match_too_many', {fields => $non_conclusive_fields}); + } + my $bug = Bugzilla::Bug->create($fields); debug_print("Created bug " . $bug->id); return ($bug, $bug->comments->[0]); diff --git a/post_bug.cgi b/post_bug.cgi index 749acb492..e8f9acc01 100755 --- a/post_bug.cgi +++ b/post_bug.cgi @@ -85,7 +85,7 @@ if ($token) { } # do a match on the fields if applicable -Bugzilla::User::match_field ($cgi, { +Bugzilla::User::match_field ({ 'cc' => { 'type' => 'multi' }, 'assigned_to' => { 'type' => 'single' }, 'qa_contact' => { 'type' => 'single' }, diff --git a/process_bug.cgi b/process_bug.cgi index 21cf94fc7..6c9baa3d3 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -142,7 +142,7 @@ if (defined $cgi->param('dontchange')) { } # do a match on the fields if applicable -Bugzilla::User::match_field($cgi, { +Bugzilla::User::match_field({ 'qa_contact' => { 'type' => 'single' }, 'newcc' => { 'type' => 'multi' }, 'masscc' => { 'type' => 'multi' }, diff --git a/request.cgi b/request.cgi index 594ed1833..b54477cb1 100755 --- a/request.cgi +++ b/request.cgi @@ -63,7 +63,7 @@ unless (defined $cgi->param('requestee') $fields->{'requestee'}->{'type'} = 'single'; } -Bugzilla::User::match_field($cgi, $fields); +Bugzilla::User::match_field($fields); if ($action eq 'queue') { queue(); diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index 80172bb03..467d4a174 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -1628,6 +1628,12 @@ [% name FILTER html %] does not exist or you are not allowed to see that user. + [% ELSIF error == "user_match_too_many" %] + [% title = "No Conclusive Match" %] + [% terms.Bugzilla %] cannot make a conclusive match for one or more + of the names and/or email addresses you entered for + the [% fields.join(', ') FILTER html %] field(s). + [% ELSIF error == "user_not_insider" %] [% title = "User Not In Insidergroup" %] Sorry, but you are not allowed to (un)mark comments or attachments diff --git a/userprefs.cgi b/userprefs.cgi index cffae38cc..e6ee8fb8a 100755 --- a/userprefs.cgi +++ b/userprefs.cgi @@ -247,7 +247,7 @@ sub SaveEmail { my $cgi = Bugzilla->cgi; my $user = Bugzilla->user; - Bugzilla::User::match_field($cgi, { 'new_watchedusers' => {'type' => 'multi'} }); + Bugzilla::User::match_field({ 'new_watchedusers' => {'type' => 'multi'} }); ########################################################################### # Role-based preferences -- cgit v1.2.3-24-g4f1b