Skip to content

bugzilla
bugzilla
Commit 67fe43be authored by Frédéric Buclin's avatar Frédéric Buclin
Browse files
Options

Bug 523205: editflagtypes.cgi should use Bugzilla::FlagType methods to create and edit flag types

a=LpSolit
parent f770095a
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 246 additions and 78 deletions
Bugzilla/FlagType.pm
View file @ 67fe43be
...@@ -48,6 +48,7 @@ whose names start with _ or are specifically noted as being private. ...@@ -48,6 +48,7 @@ whose names start with _ or are specifically noted as being private.
=cut =cut
use Bugzilla::Constants;
use Bugzilla::Error; use Bugzilla::Error;
use Bugzilla::Util; use Bugzilla::Util;
use Bugzilla::Group; use Bugzilla::Group;
...@@ -58,57 +59,124 @@ use base qw(Bugzilla::Object); ...@@ -58,57 +59,124 @@ use base qw(Bugzilla::Object);
#### Initialization #### #### Initialization ####
############################### ###############################
=begin private use constant DB_TABLE => 'flagtypes';
use constant LIST_ORDER => 'sortkey, name';
=head1 PRIVATE VARIABLES/CONSTANTS
=over
=item C<DB_COLUMNS>
basic sets of columns and tables for getting flag types from the
database.
=back
=cut
use constant DB_COLUMNS => qw( use constant DB_COLUMNS => qw(
flagtypes.id id
flagtypes.name name
flagtypes.description description
flagtypes.cc_list cc_list
flagtypes.target_type target_type
flagtypes.sortkey sortkey
flagtypes.is_active is_active
flagtypes.is_requestable is_requestable
flagtypes.is_requesteeble is_requesteeble
flagtypes.is_multiplicable is_multiplicable
flagtypes.grant_group_id grant_group_id
flagtypes.request_group_id request_group_id
); );
=pod use constant UPDATE_COLUMNS => qw(
name
description
cc_list
sortkey
is_active
is_requestable
is_requesteeble
is_multiplicable
grant_group_id
request_group_id
);
=over use constant VALIDATORS => {
name => \&_check_name,
description => \&_check_description,
cc_list => \&_check_cc_list,
target_type => \&_check_target_type,
sortkey => \&_check_sortey,
is_active => \&Bugzilla::Object::check_boolean,
is_requestable => \&Bugzilla::Object::check_boolean,
is_requesteeble => \&Bugzilla::Object::check_boolean,
is_multiplicable => \&Bugzilla::Object::check_boolean,
grant_group => \&_check_group,
request_group => \&_check_group,
};
use constant UPDATE_VALIDATORS => {
grant_group_id => \&_check_group,
request_group_id => \&_check_group,
};
###############################
=item C<DB_TABLE> sub create {
my $class = shift;
Which database(s) is the data coming from? $class->check_required_create_fields(@_);
my $params = $class->run_create_validators(@_);
Note: when adding tables to DB_TABLE, make sure to include the separator # Extract everything which is not a valid column name.
(i.e. words like "LEFT OUTER JOIN") before the table name, since tables take $params->{grant_group_id} = delete $params->{grant_group};
multiple separators based on the join type, and therefore it is not possible $params->{request_group_id} = delete $params->{request_group};
to join them later using a single known separator. my $inclusions = delete $params->{inclusions};
my $exclusions = delete $params->{exclusions};
=back my $flagtype = $class->insert_create_data($params);
=end private $flagtype->set_clusions({ inclusions => $inclusions,
exclusions => $exclusions });
return $flagtype;
}
=cut sub update {
my $self = shift;
my $dbh = Bugzilla->dbh;
use constant DB_TABLE => 'flagtypes'; $dbh->bz_start_transaction();
use constant LIST_ORDER => 'flagtypes.sortkey, flagtypes.name'; my $changes = $self->SUPER::update(@_);
# Clear existing flags for bugs/attachments in categories no longer on
# the list of inclusions or that have been added to the list of exclusions.
my $flag_ids = $dbh->selectcol_arrayref('SELECT DISTINCT flags.id
FROM flags
INNER JOIN bugs
ON flags.bug_id = bugs.bug_id
LEFT OUTER JOIN flaginclusions AS i
ON (flags.type_id = i.type_id
AND (bugs.product_id = i.product_id
OR i.product_id IS NULL)
AND (bugs.component_id = i.component_id
OR i.component_id IS NULL))
WHERE flags.type_id = ?
AND i.type_id IS NULL',
undef, $self->id);
Bugzilla::Flag->force_retarget($flag_ids);
$flag_ids = $dbh->selectcol_arrayref('SELECT DISTINCT flags.id
FROM flags
INNER JOIN bugs
ON flags.bug_id = bugs.bug_id
INNER JOIN flagexclusions AS e
ON flags.type_id = e.type_id
WHERE flags.type_id = ?
AND (bugs.product_id = e.product_id
OR e.product_id IS NULL)
AND (bugs.component_id = e.component_id
OR e.component_id IS NULL)',
undef, $self->id);
Bugzilla::Flag->force_retarget($flag_ids);
# Silently remove requestees from flags which are no longer
# specifically requestable.
if (!$self->is_requesteeble) {
$dbh->do('UPDATE flags SET requestee_id = NULL WHERE type_id = ?',
undef, $self->id);
}
$dbh->bz_commit_transaction();
return $changes;
}
############################### ###############################
#### Accessors ###### #### Accessors ######
...@@ -179,10 +247,121 @@ sub sortkey { return $_[0]->{'sortkey'}; } ...@@ -179,10 +247,121 @@ sub sortkey { return $_[0]->{'sortkey'}; }
sub request_group_id { return $_[0]->{'request_group_id'}; } sub request_group_id { return $_[0]->{'request_group_id'}; }
sub grant_group_id { return $_[0]->{'grant_group_id'}; } sub grant_group_id { return $_[0]->{'grant_group_id'}; }
################################
# Validators
################################
sub _check_name {
my ($invocant, $name) = @_;
$name = trim($name);
($name && $name !~ /[\s,]/ && length($name) <= 50)
|| ThrowUserError('flag_type_name_invalid', { name => $name });
return $name;
}
sub _check_description {
my ($invocant, $desc) = @_;
$desc = trim($desc);
$desc || ThrowUserError('flag_type_description_invalid');
return $desc;
}
sub _check_cc_list {
my ($invocant, $cc_list) = @_;
length($cc_list) <= 200
|| ThrowUserError('flag_type_cc_list_invalid', { cc_list => $cc_list });
my @addresses = split(/[,\s]+/, $cc_list);
# We do not call Util::validate_email_syntax because these
# addresses do not require to match 'emailregexp' and do not
# depend on 'emailsuffix'. So we limit ourselves to a simple
# sanity check:
# - match the syntax of a fully qualified email address;
# - do not contain any illegal character.
foreach my $address (@addresses) {
($address =~ /^[\w\.\+\-=]+@[\w\.\-]+\.[\w\-]+$/
&& $address !~ /[\\\(\)<>&,;:"\[\] \t\r\n]/)
|| ThrowUserError('illegal_email_address',
{addr => $address, default => 1});
}
return $cc_list;
}
sub _check_target_type {
my ($invocant, $target_type) = @_;
($target_type eq 'bug' || $target_type eq 'attachment')
|| ThrowCodeError('flag_type_target_type_invalid', { target_type => $target_type });
return $target_type;
}
sub _check_sortey {
my ($invocant, $sortkey) = @_;
(detaint_natural($sortkey) && $sortkey <= MAX_SMALLINT)
|| ThrowUserError('flag_type_sortkey_invalid', { sortkey => $sortkey });
return $sortkey;
}
sub _check_group {
my ($invocant, $group) = @_;
return unless $group;
trick_taint($group);
$group = Bugzilla::Group->check($group);
return $group->id;
}
############################### ###############################
#### Methods #### #### Methods ####
############################### ###############################
sub set_name { $_[0]->set('name', $_[1]); }
sub set_description { $_[0]->set('description', $_[1]); }
sub set_cc_list { $_[0]->set('cc_list', $_[1]); }
sub set_sortkey { $_[0]->set('sortkey', $_[1]); }
sub set_is_active { $_[0]->set('is_active', $_[1]); }
sub set_is_requestable { $_[0]->set('is_requestable', $_[1]); }
sub set_is_specifically_requestable { $_[0]->set('is_requesteeble', $_[1]); }
sub set_is_multiplicable { $_[0]->set('is_multiplicable', $_[1]); }
sub set_grant_group { $_[0]->set('grant_group_id', $_[1]); }
sub set_request_group { $_[0]->set('request_group_id', $_[1]); }
sub set_clusions {
my ($self, $list) = @_;
my $dbh = Bugzilla->dbh;
my $flag_id = $self->id;
my %products;
foreach my $category (keys %$list) {
my $sth = $dbh->prepare("INSERT INTO flag$category
(type_id, product_id, component_id) VALUES (?, ?, ?)");
$dbh->do("DELETE FROM flag$category WHERE type_id = ?", undef, $flag_id);
foreach my $prod_comp (@{$list->{$category} || []}) {
my ($prod_id, $comp_id) = split(':', $prod_comp);
# Does the product exist?
if ($prod_id && detaint_natural($prod_id)) {
$products{$prod_id} ||= new Bugzilla::Product($prod_id);
next unless defined $products{$prod_id};
# Does the component belong to this product?
if ($comp_id && detaint_natural($comp_id)) {
my $found = grep { $_->id == $comp_id } @{$products{$prod_id}->components};
next unless $found;
}
}
$prod_id ||= undef;
$comp_id ||= undef;
$sth->execute($flag_id, $prod_id, $comp_id);
}
}
}
=pod =pod
=over =over
......
template/en/default/admin/flag-type/edit.html.tmpl
View file @ 67fe43be
...@@ -23,20 +23,15 @@ ...@@ -23,20 +23,15 @@
[% PROCESS "global/js-products.html.tmpl" %] [% PROCESS "global/js-products.html.tmpl" %]
[% IF type.target_type == "bug" %] [% IF action == "insert" %]
[% title = BLOCK %]Create Flag Type for [% terms.Bugs %][% END %] [% title = BLOCK %]
[% typeLabelLowerPlural = BLOCK %][% terms.bugs %][% END %] Create Flag Type for [% type.target_type == "bug" ? terms.Bugs : "Attachments" %]
[% typeLabelLowerSingular = BLOCK %][% terms.bug %][% END %] [% IF type.id %]
Based on [% type.name FILTER html %]
[% END %]
[% END %]
[% doc_section = "flags-overview.html#flags-create" %]
[% ELSE %] [% ELSE %]
[% title = "Create Flag Type for Attachments" %]
[% typeLabelLowerPlural = BLOCK %]attachments[% END %]
[% typeLabelLowerSingular = BLOCK %]attachment[% END %]
[% END %]
[% doc_section = "flags-overview.html#flags-create" %]
[% IF last_action == "copy" %]
[% title = BLOCK %]Create Flag Type Based on [% type.name FILTER html %][% END %]
[% ELSIF last_action == "edit" %]
[% title = BLOCK %]Edit Flag Type [% type.name FILTER html %][% END %] [% title = BLOCK %]Edit Flag Type [% type.name FILTER html %][% END %]
[% doc_section = "flags-overview.html#flags-edit" %] [% doc_section = "flags-overview.html#flags-edit" %]
[% END %] [% END %]
...@@ -53,10 +48,10 @@ ...@@ -53,10 +48,10 @@
%] %]
<form method="post" action="editflagtypes.cgi"> <form method="post" action="editflagtypes.cgi">
<input type="hidden" name="action" value="[% action %]"> <input type="hidden" name="action" value="[% action FILTER html %]">
<input type="hidden" name="id" value="[% type.id %]"> <input type="hidden" name="id" value="[% type.id %]">
<input type="hidden" name="token" value="[% token FILTER html %]"> <input type="hidden" name="token" value="[% token FILTER html %]">
<input type="hidden" name="target_type" value="[% type.target_type %]"> <input type="hidden" name="target_type" value="[% type.target_type FILTER html %]">
[% FOREACH category = type.inclusions %] [% FOREACH category = type.inclusions %]
<input type="hidden" name="inclusions" value="[% category.value FILTER html %]"> <input type="hidden" name="inclusions" value="[% category.value FILTER html %]">
[% END %] [% END %]
...@@ -72,7 +67,7 @@ ...@@ -72,7 +67,7 @@
<tr> <tr>
<th>Name:</th> <th>Name:</th>
<td> <td>
a short name identifying this type<br> a short name identifying this type.<br>
<input type="text" name="name" value="[% type.name FILTER html %]" <input type="text" name="name" value="[% type.name FILTER html %]"
size="50" maxlength="50"> size="50" maxlength="50">
</td> </td>
...@@ -81,7 +76,7 @@ ...@@ -81,7 +76,7 @@
<tr> <tr>
<th>Description:</th> <th>Description:</th>
<td> <td>
a comprehensive description of this type<br> a comprehensive description of this type.<br>
[% INCLUDE global/textarea.html.tmpl [% INCLUDE global/textarea.html.tmpl
name = 'description' name = 'description'
minrows = 4 minrows = 4
...@@ -95,9 +90,9 @@ ...@@ -95,9 +90,9 @@
<th>Category:</th> <th>Category:</th>
<td> <td>
the products/components to which [% typeLabelLowerPlural %] must the products/components to which [% type.target_type == "bug" ? terms.bugs : "attachments" %]
(inclusions) or must not (exclusions) belong in order for users must (inclusions) or must not (exclusions) belong in order for users
to be able to set flags of this type for them to be able to set flags of this type for them.
<table> <table>
<tr> <tr>
<td style="vertical-align: top;"> <td style="vertical-align: top;">
...@@ -139,10 +134,10 @@ ...@@ -139,10 +134,10 @@
<tr> <tr>
<th>Sort Key:</th> <th>Sort Key:</th>
<td> <td>
a number between 1 and 32767 by which this type will be sorted a number between 1 and [% constants.MAX_SMALLINT FILTER none %] by which
when displayed to users in a list; ignore if you don't care this type will be sorted when displayed to users in a list; ignore if you
what order the types appear in or if you want them to appear don't care what order the types appear in or if you want them to appear
in alphabetical order<br> in alphabetical order.<br>
<input type="text" name="sortkey" value="[% type.sortkey || 1 %]" size="5" maxlength="5"> <input type="text" name="sortkey" value="[% type.sortkey || 1 %]" size="5" maxlength="5">
</td> </td>
</tr> </tr>
...@@ -196,7 +191,7 @@ ...@@ -196,7 +191,7 @@
<input type="checkbox" id="is_multiplicable" name="is_multiplicable" <input type="checkbox" id="is_multiplicable" name="is_multiplicable"
[% " checked" IF type.is_multiplicable || !type.is_multiplicable.defined %]> [% " checked" IF type.is_multiplicable || !type.is_multiplicable.defined %]>
<label for="is_multiplicable">multiplicable (multiple flags of this type can be set on <label for="is_multiplicable">multiplicable (multiple flags of this type can be set on
the same [% typeLabelLowerSingular %])</label> the same [% type.target_type == "bug" ? terms.bug : "attachment" %])</label>
</td> </td>
</tr> </tr>
...@@ -204,7 +199,7 @@ ...@@ -204,7 +199,7 @@
<th>Grant Group:</th> <th>Grant Group:</th>
<td> <td>
the group allowed to grant/deny flags of this type the group allowed to grant/deny flags of this type
(to allow all users to grant/deny these flags, select no group)<br> (to allow all users to grant/deny these flags, select no group).<br>
[% PROCESS select selname = "grant_group" %] [% PROCESS select selname = "grant_group" %]
</td> </td>
</tr> </tr>
...@@ -213,19 +208,16 @@ ...@@ -213,19 +208,16 @@
<th>Request Group:</th> <th>Request Group:</th>
<td> <td>
if flags of this type are requestable, the group allowed to request them if flags of this type are requestable, the group allowed to request them
(to allow all users to request these flags, select no group)<br> (to allow all users to request these flags, select no group).<br>
Note that the request group alone has no effect if the grant group is not defined!<br> Note that the request group alone has no effect if the grant group is not defined!<br>
[% PROCESS select selname = "request_group" %] [% PROCESS select selname = "request_group" %]
</td> </td>
</tr> </tr>
<tr> <tr>
<th></th> <th>&nbsp;</th>
<td> <td>
<input type="submit" id="save" value=" <input type="submit" id="save" value="[% action == "insert" ? "Create" : "Save Changes" %]">
[%- IF (last_action == "enter" || last_action == "copy") %]Create
[%- ELSE %]Save Changes
[%- END %]">
</td> </td>
</tr> </tr>
......
template/en/default/filterexceptions.pl
View file @ 67fe43be
...@@ -426,12 +426,8 @@ ...@@ -426,12 +426,8 @@
], ],
'admin/flag-type/edit.html.tmpl' => [ 'admin/flag-type/edit.html.tmpl' => [
'action',
'type.id', 'type.id',
'type.target_type',
'type.sortkey || 1', 'type.sortkey || 1',
'typeLabelLowerPlural',
'typeLabelLowerSingular',
'selname', 'selname',
], ],
......
template/en/default/global/user-error.html.tmpl
View file @ 67fe43be
...@@ -653,7 +653,7 @@ ...@@ -653,7 +653,7 @@
[% ELSIF error == "flag_type_description_invalid" %] [% ELSIF error == "flag_type_description_invalid" %]
[% title = "Flag Type Description Invalid" %] [% title = "Flag Type Description Invalid" %]
[% admindocslinks = {'flags-overview.html#flags-admin' => 'Administering Flags'} %] [% admindocslinks = {'flags-overview.html#flags-admin' => 'Administering Flags'} %]
The description must be less than 32K. You must enter a description for this flag type.
[% ELSIF error == "flag_type_name_invalid" %] [% ELSIF error == "flag_type_name_invalid" %]
[% title = "Flag Type Name Invalid" %] [% title = "Flag Type Name Invalid" %]
...@@ -687,8 +687,8 @@ ...@@ -687,8 +687,8 @@
[% ELSIF error == "flag_type_sortkey_invalid" %] [% ELSIF error == "flag_type_sortkey_invalid" %]
[% title = "Flag Type Sort Key Invalid" %] [% title = "Flag Type Sort Key Invalid" %]
The sort key must be an integer between 0 and 32767 inclusive. The sort key <em>[% sortkey FILTER html %]</em> must be an integer
It cannot be <em>[% sortkey FILTER html %]</em>. between 0 and [% constants.MAX_SMALLINT FILTER none %].
[% ELSIF error == "freetext_too_long" %] [% ELSIF error == "freetext_too_long" %]
[% title = "Text Too Long" %] [% title = "Text Too Long" %]
...@@ -756,6 +756,7 @@ ...@@ -756,6 +756,7 @@
[% title = "System Groups not deletable" %] [% title = "System Groups not deletable" %]
<em>[% name FILTER html %]</em> is a system group. <em>[% name FILTER html %]</em> is a system group.
This group cannot be deleted. This group cannot be deleted.
[% ELSIF error == "group_unknown" %] [% ELSIF error == "group_unknown" %]
[% title = "Unknown Group" %] [% title = "Unknown Group" %]
The group [% name FILTER html %] does not exist. Please specify The group [% name FILTER html %] does not exist. Please specify
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment