Bug 238878: Make hidden-fields template, User Matching and Flags use direct CGI instead of [% form.foo %] - Patch by Teemu Mannermaa <wicked@etlicon.fi> r=LpSolit a=justdave

This commit is contained in:
lpsolit%gmail.com 2005-04-07 23:37:56 +00:00
parent 87901ae5f5
commit 93d4b2c6af
9 changed files with 93 additions and 89 deletions

View File

@ -228,7 +228,7 @@ sub count {
=over
=item C<validate($data, $bug_id)>
=item C<validate($cgi, $bug_id)>
Validates fields containing flag modifications.
@ -238,17 +238,17 @@ Validates fields containing flag modifications.
sub validate {
my $user = Bugzilla->user;
my ($data, $bug_id) = @_;
my ($cgi, $bug_id) = @_;
# Get a list of flags to validate. Uses the "map" function
# to extract flag IDs from form field names by matching fields
# whose name looks like "flag-nnn", where "nnn" is the ID,
# and returning just the ID portion of matching field names.
my @ids = map(/^flag-(\d+)$/ ? $1 : (), keys %$data);
my @ids = map(/^flag-(\d+)$/ ? $1 : (), $cgi->param());
foreach my $id (@ids)
{
my $status = $data->{"flag-$id"};
my $status = $cgi->param("flag-$id");
# Make sure the flag exists.
my $flag = get($id);
@ -277,9 +277,9 @@ sub validate {
# feature and the attachment is marked private).
if ($status eq '?'
&& $flag->{type}->{is_requesteeble}
&& trim($data->{"requestee-$id"}))
&& trim($cgi->param("requestee-$id")))
{
my $requestee_email = trim($data->{"requestee-$id"});
my $requestee_email = trim($cgi->param("requestee-$id"));
if ($requestee_email ne $flag->{'requestee'}->{'email'}) {
# We know the requestee exists because we ran
# Bugzilla::User::match_field before getting here.
@ -299,7 +299,7 @@ sub validate {
# Throw an error if the target is a private attachment and
# the requestee isn't in the group of insiders who can see it.
if ($flag->{target}->{attachment}->{exists}
&& $data->{'isprivate'}
&& $cgi->param('isprivate')
&& Param("insidergroup")
&& !$requestee->in_group(Param("insidergroup")))
{
@ -353,21 +353,21 @@ sub snapshot {
=over
=item C<process($target, $timestamp, $data)>
=item C<process($target, $timestamp, $cgi)>
Processes changes to flags.
The target is the bug or attachment this flag is about, the timestamp
is the date/time the bug was last touched (so that changes to the flag
can be stamped with the same date/time), the data is the form data
with flag fields that the user submitted.
can be stamped with the same date/time), the cgi is the CGI object
used to obtain the flag fields that the user submitted.
=back
=cut
sub process {
my ($target, $timestamp, $data) = @_;
my ($target, $timestamp, $cgi) = @_;
my $dbh = Bugzilla->dbh;
my $bug_id = $target->{'bug'}->{'id'};
@ -381,14 +381,14 @@ sub process {
my @old_summaries = snapshot($bug_id, $attach_id);
# Cancel pending requests if we are obsoleting an attachment.
if ($attach_id && $data->{'isobsolete'}) {
if ($attach_id && $cgi->param('isobsolete')) {
CancelRequests($bug_id, $attach_id);
}
# Create new flags and update existing flags.
my $new_flags = FormToNewFlags($target, $data);
my $new_flags = FormToNewFlags($target, $cgi);
foreach my $flag (@$new_flags) { create($flag, $timestamp) }
modify($data, $timestamp);
modify($cgi, $timestamp);
# In case the bug's product/component has changed, clear flags that are
# no longer valid.
@ -521,7 +521,7 @@ sub migrate {
=over
=item C<modify($data, $timestamp)>
=item C<modify($cgi, $timestamp)>
Modifies flags in the database when a user changes them.
Note that modified flags are always set active (is_active = 1) -
@ -533,14 +533,14 @@ attachment.cgi midairs. See bug 223878 for details.
=cut
sub modify {
my ($data, $timestamp) = @_;
my ($cgi, $timestamp) = @_;
# Use the date/time we were given if possible (allowing calling code
# to synchronize the comment's timestamp with those of other records).
$timestamp = ($timestamp ? &::SqlQuote($timestamp) : "NOW()");
# Extract a list of flags from the form data.
my @ids = map(/^flag-(\d+)$/ ? $1 : (), keys %$data);
my @ids = map(/^flag-(\d+)$/ ? $1 : (), $cgi->param());
# Loop over flags and update their record in the database if necessary.
# Two kinds of changes can happen to a flag: it can be set to a different
@ -550,8 +550,8 @@ sub modify {
foreach my $id (@ids) {
my $flag = get($id);
my $status = $data->{"flag-$id"};
my $requestee_email = trim($data->{"requestee-$id"});
my $status = $cgi->param("flag-$id");
my $requestee_email = trim($cgi->param("requestee-$id"));
# Ignore flags the user didn't change. There are two components here:
@ -672,7 +672,7 @@ sub clear {
=over
=item C<FormToNewFlags($target, $data)
=item C<FormToNewFlags($target, $cgi)
Someone pleasedocument this function.
@ -681,20 +681,20 @@ Someone pleasedocument this function.
=cut
sub FormToNewFlags {
my ($target, $data) = @_;
my ($target, $cgi) = @_;
# Get information about the setter to add to each flag.
# Uses a conditional to suppress Perl's "used only once" warnings.
my $setter = new Bugzilla::User($::userid);
# Extract a list of flag type IDs from field names.
my @type_ids = map(/^flag_type-(\d+)$/ ? $1 : (), keys %$data);
@type_ids = grep($data->{"flag_type-$_"} ne 'X', @type_ids);
my @type_ids = map(/^flag_type-(\d+)$/ ? $1 : (), $cgi->param());
@type_ids = grep($cgi->param("flag_type-$_") ne 'X', @type_ids);
# Process the form data and create an array of flag objects.
my @flags;
foreach my $type_id (@type_ids) {
my $status = $data->{"flag_type-$type_id"};
my $status = $cgi->param("flag_type-$type_id");
&::trick_taint($status);
# Create the flag record and populate it with data from the form.
@ -706,7 +706,7 @@ sub FormToNewFlags {
};
if ($status eq "?") {
my $requestee = $data->{"requestee_type-$type_id"};
my $requestee = $cgi->param("requestee_type-$type_id");
if ($requestee) {
my $requestee_id = login_to_id($requestee);
$flag->{'requestee'} = new Bugzilla::User($requestee_id);

View File

@ -303,7 +303,7 @@ sub count {
=over
=item C<validate($data, $bug_id, $attach_id)>
=item C<validate($cgi, $bug_id, $attach_id)>
Get a list of flag types to validate. Uses the "map" function
to extract flag type IDs from form field names by matching columns
@ -316,13 +316,13 @@ and returning just the ID portion of matching field names.
sub validate {
my $user = Bugzilla->user;
my ($data, $bug_id, $attach_id) = @_;
my ($cgi, $bug_id, $attach_id) = @_;
my @ids = map(/^flag_type-(\d+)$/ ? $1 : (), keys %$data);
my @ids = map(/^flag_type-(\d+)$/ ? $1 : (), $cgi->param());
foreach my $id (@ids)
{
my $status = $data->{"flag_type-$id"};
my $status = $cgi->param("flag_type-$id");
# Don't bother validating types the user didn't touch.
next if $status eq "X";
@ -348,9 +348,9 @@ sub validate {
# feature and the attachment is marked private).
if ($status eq '?'
&& $flag_type->{is_requesteeble}
&& trim($data->{"requestee_type-$id"}))
&& trim($cgi->param("requestee_type-$id")))
{
my $requestee_email = trim($data->{"requestee_type-$id"});
my $requestee_email = trim($cgi->param("requestee_type-$id"));
# We know the requestee exists because we ran
# Bugzilla::User::match_field before getting here.
@ -370,7 +370,7 @@ sub validate {
# the requestee isn't in the group of insiders who can see it.
if ($attach_id
&& Param("insidergroup")
&& $data->{'isprivate'}
&& $cgi->param('isprivate')
&& !$requestee->in_group(Param("insidergroup")))
{
ThrowUserError("flag_requestee_unauthorized_attachment",

View File

@ -687,7 +687,8 @@ sub match {
# 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 $::FORM and passes them to match()
# 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.
#
@ -710,7 +711,7 @@ sub match {
# How to call it:
#
# Bugzilla::User::match_field ({
# Bugzilla::User::match_field($cgi, {
# 'field_name' => { 'type' => fieldtype },
# 'field_name2' => { 'type' => fieldtype },
# [...]
@ -720,8 +721,8 @@ sub match {
#
sub match_field {
my $fields = shift; # arguments as a hash
my $cgi = shift; # CGI object to look up fields in
my $fields = shift; # arguments as a hash
my $matches = {}; # the values sent to the template
my $matchsuccess = 1; # did the match fail?
my $need_confirm = 0; # whether to display confirmation screen
@ -729,11 +730,9 @@ sub match_field {
# prepare default form values
my $vars = $::vars;
$vars->{'form'} = \%::FORM;
$vars->{'mform'} = \%::MFORM;
# What does a "--do_not_change--" field look like (if any)?
my $dontchange = $vars->{'form'}->{'dontchange'};
my $dontchange = $cgi->param('dontchange');
# Fields can be regular expressions matching multiple form fields
# (f.e. "requestee-(\d+)"), so expand each non-literal field
@ -747,7 +746,7 @@ sub match_field {
$expanded_fields->{$field_pattern} = $fields->{$field_pattern};
}
else {
my @field_names = grep(/$field_pattern/, keys %{$vars->{'form'}});
my @field_names = grep(/$field_pattern/, $cgi->param());
foreach my $field_name (@field_names) {
$expanded_fields->{$field_name} =
{ type => $fields->{$field_pattern}->{'type'} };
@ -774,23 +773,27 @@ sub match_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 $::MFORM does not define them.
# 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($vars->{'mform'}->{$field});
next if !defined $cgi->param($field);
# Skip it if this is a --do_not_change-- field
next if $dontchange && $dontchange eq $vars->{'form'}->{$field};
next if $dontchange && $dontchange eq $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 $::FORM and $::MFORM
# modified by the search, and put back into the CGI environment
# incrementally.
my $raw_field = join(" ", @{$vars->{'mform'}->{$field}});
$vars->{'form'}->{$field} = '';
$vars->{'mform'}->{$field} = [];
my $raw_field = join(" ", $cgi->param($field));
# When we add back in values later, it matters that we delete
# the param 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
$cgi->delete($field);
my @queries = ();
@ -835,12 +838,12 @@ sub match_field {
if ((scalar(@{$users}) == 1)
&& (@{$users}[0]->{'login'} eq $query))
{
# delimit with spaces if necessary
if ($vars->{'form'}->{$field}) {
$vars->{'form'}->{$field} .= " ";
}
$vars->{'form'}->{$field} .= @{$users}[0]->{'login'};
push @{$vars->{'mform'}->{$field}}, @{$users}[0]->{'login'};
$cgi->append(-name=>$field,
-values=>[@{$users}[0]->{'login'}]);
# XXX FORM compatilibity code, will be removed in bug 225818
$::FORM{$field} = join(" ", $cgi->param($field));
next;
}
@ -850,12 +853,13 @@ sub match_field {
# here is where it checks for multiple matches
if (scalar(@{$users}) == 1) { # exactly one match
# delimit with spaces if necessary
if ($vars->{'form'}->{$field}) {
$vars->{'form'}->{$field} .= " ";
}
$vars->{'form'}->{$field} .= @{$users}[0]->{'login'};
push @{$vars->{'mform'}->{$field}}, @{$users}[0]->{'login'};
$cgi->append(-name=>$field,
-values=>[@{$users}[0]->{'login'}]);
# XXX FORM compatilibity code, will be removed in bug 225818
$::FORM{$field} = join(" ", $cgi->param($field));
$need_confirm = 1 if &::Param('confirmuniqueusermatch');
}

View File

@ -126,10 +126,11 @@ elsif ($action eq "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({ '^requestee(_type)?-(\d+)$' =>
{ 'type' => 'single' } });
Bugzilla::Flag::validate(\%::FORM, $bugid);
Bugzilla::FlagType::validate(\%::FORM, $bugid, $::FORM{'id'});
Bugzilla::User::match_field($cgi, {
'^requestee(_type)?-(\d+)$' => { 'type' => 'single' }
});
Bugzilla::Flag::validate($cgi, $bugid);
Bugzilla::FlagType::validate($cgi, $bugid, $::FORM{'id'});
insert($data);
}
@ -155,10 +156,11 @@ elsif ($action eq "update")
# 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({ '^requestee(_type)?-(\d+)$' =>
{ 'type' => 'single' } });
Bugzilla::Flag::validate(\%::FORM, $bugid);
Bugzilla::FlagType::validate(\%::FORM, $bugid, $::FORM{'id'});
Bugzilla::User::match_field($cgi, {
'^requestee(_type)?-(\d+)$' => { 'type' => 'single' }
});
Bugzilla::Flag::validate($cgi, $bugid);
Bugzilla::FlagType::validate($cgi, $bugid, $::FORM{'id'});
update();
}
@ -1033,7 +1035,7 @@ sub insert
# Create flags.
my $target = Bugzilla::Flag::GetTarget(undef, $attachid);
Bugzilla::Flag::process($target, $timestamp, \%::FORM);
Bugzilla::Flag::process($target, $timestamp, $cgi);
# Define the variables and functions that will be passed to the UI template.
$vars->{'mailrecipients'} = { 'changer' => Bugzilla->user->login,
@ -1168,7 +1170,7 @@ sub update
# is obsoleting this attachment without deleting any requests
# the user submits at the same time.
my $target = Bugzilla::Flag::GetTarget(undef, $::FORM{'id'});
Bugzilla::Flag::process($target, $timestamp, \%::FORM);
Bugzilla::Flag::process($target, $timestamp, $cgi);
# Update the attachment record in the database.
SendSQL("UPDATE attachments

View File

@ -198,7 +198,7 @@ if ($cgi->param('update')) {
}
}
if (scalar %{$arglist}) {
&Bugzilla::User::match_field($arglist);
&Bugzilla::User::match_field($cgi, $arglist);
}
for my $sid (@scheduleids) {

View File

@ -60,7 +60,7 @@ my $dbh = Bugzilla->dbh;
# do a match on the fields if applicable
&Bugzilla::User::match_field ({
&Bugzilla::User::match_field ($cgi, {
'cc' => { 'type' => 'multi' },
'assigned_to' => { 'type' => 'single' },
'qa_contact' => { 'type' => 'single' },

View File

@ -44,6 +44,7 @@ use Bugzilla::Util;
# Use the Flag module to modify flag data if the user set flags.
use Bugzilla::Flag;
use Bugzilla::FlagType;
# Shut up misguided -w warnings about "used only once":
@ -138,7 +139,7 @@ foreach my $field ("dependson", "blocked") {
# 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({
&Bugzilla::User::match_field($cgi, {
'qa_contact' => { 'type' => 'single' },
'newcc' => { 'type' => 'multi' },
'masscc' => { 'type' => 'multi' },
@ -148,8 +149,8 @@ foreach my $field ("dependson", "blocked") {
# Validate flags, but only if the user is changing a single bug,
# since the multi-change form doesn't include flag changes.
if (defined $::FORM{'id'}) {
Bugzilla::Flag::validate(\%::FORM, $::FORM{'id'});
Bugzilla::FlagType::validate(\%::FORM, $::FORM{'id'});
Bugzilla::Flag::validate($cgi, $::FORM{'id'});
Bugzilla::FlagType::validate($cgi, $::FORM{'id'});
}
######################################################################
@ -1814,7 +1815,7 @@ foreach my $id (@idlist) {
# Set and update flags.
if ($UserInEditGroupSet) {
my $target = Bugzilla::Flag::GetTarget($id);
Bugzilla::Flag::process($target, $timestamp, \%::FORM);
Bugzilla::Flag::process($target, $timestamp, $cgi);
}
if ($bug_changed) {
SendSQL("UPDATE bugs SET delta_ts = $sql_timestamp WHERE bug_id = $id");

View File

@ -21,8 +21,6 @@
#%]
[%# INTERFACE:
# form: hash; the form values submitted to the script
# mform: hash; the form multi-values submitted to the script
# fields: hash/record; the fields being matched, each of which has:
# type: single|multi: whether or not the user can select multiple matches
# flag_type: for flag requestee fields, the type of flag being requested

View File

@ -20,23 +20,22 @@
#%]
[%# INTERFACE:
# form: hash; the form fields/values for which to generate hidden fields.
# mform: hash; the form fields/values with multiple values for which to
# generate hidden fields.
# exclude: string; a regular expression matching fields to exclude
# from the list of hidden fields generated by this template
#%]
[%# The global Bugzilla->cgi object is used to obtain form variable values. %]
[% USE Bugzilla %]
[% cgi = Bugzilla.cgi %]
[%# Generate hidden form fields for non-excluded fields. %]
[% FOREACH field = form %]
[% NEXT IF exclude && field.key.search(exclude) %]
[% IF mform.${field.key}.size > 1 %]
[% FOREACH mvalue = mform.${field.key} %]
<input type="hidden" name="[% field.key FILTER html %]"
[% FOREACH field = cgi.param() %]
[% NEXT IF exclude && field.search(exclude) %]
[%# The '.slice(0)' bit is here to force the 'param(field)' to be evaluated
in a list context, so we can avoid extra code checking for single valued or
empty fields %]
[% FOREACH mvalue = cgi.param(field).slice(0) %]
<input type="hidden" name="[% field FILTER html %]"
value="[% mvalue FILTER html FILTER html_linebreak %]">
[% END %]
[% ELSE %]
<input type="hidden" name="[% field.key FILTER html %]"
value="[% field.value FILTER html FILTER html_linebreak %]">
[% END %]
[% END %]