From 67fe43bea97e7ccabe7db1d3ea478bac332c1520 Mon Sep 17 00:00:00 2001 From: Frédéric Buclin Date: Fri, 22 Oct 2010 13:43:20 +0200 Subject: Bug 523205: editflagtypes.cgi should use Bugzilla::FlagType methods to create and edit flag types a=LpSolit --- Bugzilla/FlagType.pm | 257 +++++-- editflagtypes.cgi | 748 +++++++-------------- template/en/default/admin/flag-type/edit.html.tmpl | 56 +- template/en/default/filterexceptions.pl | 4 - template/en/default/global/user-error.html.tmpl | 7 +- 5 files changed, 480 insertions(+), 592 deletions(-) diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm index 0cc392ed2..b48c7023c 100644 --- a/Bugzilla/FlagType.pm +++ b/Bugzilla/FlagType.pm @@ -48,6 +48,7 @@ whose names start with _ or are specifically noted as being private. =cut +use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::Util; use Bugzilla::Group; @@ -58,57 +59,124 @@ use base qw(Bugzilla::Object); #### Initialization #### ############################### -=begin private - -=head1 PRIVATE VARIABLES/CONSTANTS - -=over - -=item C - -basic sets of columns and tables for getting flag types from the -database. - -=back - -=cut +use constant DB_TABLE => 'flagtypes'; +use constant LIST_ORDER => 'sortkey, name'; use constant DB_COLUMNS => qw( - flagtypes.id - flagtypes.name - flagtypes.description - flagtypes.cc_list - flagtypes.target_type - flagtypes.sortkey - flagtypes.is_active - flagtypes.is_requestable - flagtypes.is_requesteeble - flagtypes.is_multiplicable - flagtypes.grant_group_id - flagtypes.request_group_id + id + name + description + cc_list + target_type + sortkey + is_active + is_requestable + is_requesteeble + is_multiplicable + grant_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 +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 -(i.e. words like "LEFT OUTER JOIN") before the table name, since tables take -multiple separators based on the join type, and therefore it is not possible -to join them later using a single known separator. + # Extract everything which is not a valid column name. + $params->{grant_group_id} = delete $params->{grant_group}; + $params->{request_group_id} = delete $params->{request_group}; + 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'; -use constant LIST_ORDER => 'flagtypes.sortkey, flagtypes.name'; + $dbh->bz_start_transaction(); + 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 ###### @@ -179,10 +247,121 @@ sub sortkey { return $_[0]->{'sortkey'}; } sub request_group_id { return $_[0]->{'request_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 #### ############################### +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 =over diff --git a/editflagtypes.cgi b/editflagtypes.cgi index c09d0edb0..20a4fc0f6 100755 --- a/editflagtypes.cgi +++ b/editflagtypes.cgi @@ -39,72 +39,104 @@ use Bugzilla::Util; use Bugzilla::Error; use Bugzilla::Product; use Bugzilla::Component; -use Bugzilla::Bug; -use Bugzilla::Attachment; use Bugzilla::Token; -local our $cgi = Bugzilla->cgi; -local our $template = Bugzilla->template; -local our $vars = {}; +# Make sure the user is logged in and has the right privileges. +my $user = Bugzilla->login(LOGIN_REQUIRED); +my $cgi = Bugzilla->cgi; +my $template = Bugzilla->template; # We need this everywhere. -$vars = get_products_and_components($vars); +my $vars = get_products_and_components(); + +print $cgi->header(); -# Make sure the user is logged in and is an administrator. -my $user = Bugzilla->login(LOGIN_REQUIRED); $user->in_group('editcomponents') || ThrowUserError("auth_failure", {group => "editcomponents", action => "edit", object => "flagtypes"}); -################################################################################ -# Main Body Execution -################################################################################ - -# All calls to this script should contain an "action" variable whose value -# determines what the user wants to do. The code below checks the value of -# that variable and runs the appropriate code. - -# Determine whether to use the action specified by the user or the default. my $action = $cgi->param('action') || 'list'; my $token = $cgi->param('token'); -my @categoryActions; +my $product = $cgi->param('product'); +my $component = $cgi->param('component'); +my $flag_id = $cgi->param('id'); -if (@categoryActions = grep(/^categoryAction-.+/, $cgi->param())) { - $categoryActions[0] =~ s/^categoryAction-//; - processCategoryChange($categoryActions[0], $token); - exit; +if ($product) { + $product = Bugzilla::Product->check({ name => $product, allow_inaccessible => 1 }); } -if ($action eq 'list') { list(); } -elsif ($action eq 'enter') { edit($action); } -elsif ($action eq 'copy') { edit($action); } -elsif ($action eq 'edit') { edit($action); } -elsif ($action eq 'insert') { insert($token); } -elsif ($action eq 'update') { update($token); } -elsif ($action eq 'confirmdelete') { confirmDelete(); } -elsif ($action eq 'delete') { deleteType($token); } -elsif ($action eq 'deactivate') { deactivate($token); } -else { - ThrowUserError('unknown_action', {action => $action}); +if ($component) { + ($product && $product->id) + || ThrowUserError('flag_type_component_without_product'); + $component = Bugzilla::Component->check({ product => $product, name => $component }); } -exit; +# If 'categoryAction' is set, it has priority over 'action'. +if (my ($category_action) = grep { $_ =~ /^categoryAction-(?:\w+)$/ } $cgi->param()) { + $category_action =~ s/^categoryAction-//; -################################################################################ -# Functions -################################################################################ + my @inclusions = $cgi->param('inclusions'); + my @exclusions = $cgi->param('exclusions'); + if ($category_action eq 'include') { + my $category = ($product ? $product->id : 0) . ":" . + ($component ? $component->id : 0); + push(@inclusions, $category) unless grep($_ eq $category, @inclusions); + } + elsif ($category_action eq 'exclude') { + my $category = ($product ? $product->id : 0) . ":" . + ($component ? $component->id : 0); + push(@exclusions, $category) unless grep($_ eq $category, @exclusions); + } + elsif ($category_action eq 'removeInclusion') { + my @inclusion_to_remove = $cgi->param('inclusion_to_remove'); + foreach my $remove (@inclusion_to_remove) { + @inclusions = grep { $_ ne $remove } @inclusions; + } + } + elsif ($category_action eq 'removeExclusion') { + my @exclusion_to_remove = $cgi->param('exclusion_to_remove'); + foreach my $remove (@exclusion_to_remove) { + @exclusions = grep { $_ ne $remove } @exclusions; + } + } + + # Convert the array @clusions('prod_ID:comp_ID') back to a hash of + # the form %clusions{'prod_name:comp_name'} = 'prod_ID:comp_ID' + my %inclusions = clusion_array_to_hash(\@inclusions); + my %exclusions = clusion_array_to_hash(\@exclusions); + + $vars->{'groups'} = [Bugzilla::Group->get_all]; + $vars->{'action'} = $action; -sub list { - my $product = validateProduct(scalar $cgi->param('product')); - my $component = validateComponent($product, scalar $cgi->param('component')); + my $type = {}; + $type->{$_} = $cgi->param($_) foreach $cgi->param(); + # Make sure boolean fields are defined, else they fall back to 1. + foreach my $boolean qw(is_active is_requestable is_requesteeble is_multiplicable) { + $type->{$boolean} ||= 0; + } + + # That's what I call a big hack. The template expects to see a group object. + $type->{'grant_group'} = {}; + $type->{'grant_group'}->{'name'} = $cgi->param('grant_group'); + $type->{'request_group'} = {}; + $type->{'request_group'}->{'name'} = $cgi->param('request_group'); + + $type->{'inclusions'} = \%inclusions; + $type->{'exclusions'} = \%exclusions; + $vars->{'type'} = $type; + $vars->{'token'} = $token; + + $template->process("admin/flag-type/edit.html.tmpl", $vars) + || ThrowTemplateError($template->error()); + exit; +} + +if ($action eq 'list') { my $product_id = $product ? $product->id : 0; my $component_id = $component ? $component->id : 0; - my $show_flag_counts = (defined $cgi->param('show_flag_counts')) ? 1 : 0; - - # Define the variables and functions that will be passed to the UI template. - $vars->{'selected_product'} = $cgi->param('product'); - $vars->{'selected_component'} = $cgi->param('component'); + my $show_flag_counts = $cgi->param('show_flag_counts') ? 1 : 0; + my $group_id = $cgi->param('group'); my $bug_flagtypes; my $attach_flagtypes; @@ -116,8 +148,8 @@ sub list { $attach_flagtypes = $component->flag_types->{'attachment'}; # Filter flag types if a group ID is given. - $bug_flagtypes = filter_group($bug_flagtypes); - $attach_flagtypes = filter_group($attach_flagtypes); + $bug_flagtypes = filter_group($bug_flagtypes, $group_id); + $attach_flagtypes = filter_group($attach_flagtypes, $group_id); } # If only a product is specified but no component, then restrict the list @@ -127,18 +159,15 @@ sub list { $attach_flagtypes = $product->flag_types->{'attachment'}; # Filter flag types if a group ID is given. - $bug_flagtypes = filter_group($bug_flagtypes); - $attach_flagtypes = filter_group($attach_flagtypes); + $bug_flagtypes = filter_group($bug_flagtypes, $group_id); + $attach_flagtypes = filter_group($attach_flagtypes, $group_id); } # If no product is given, then show all flag types available. else { - $bug_flagtypes = - Bugzilla::FlagType::match({'target_type' => 'bug', - 'group' => scalar $cgi->param('group')}); + my $flagtypes = Bugzilla::FlagType::match({ group => $group_id }); - $attach_flagtypes = - Bugzilla::FlagType::match({'target_type' => 'attachment', - 'group' => scalar $cgi->param('group')}); + $bug_flagtypes = [grep { $_->target_type eq 'bug' } @$flagtypes]; + $attach_flagtypes = [grep { $_->target_type eq 'attachment' } @$flagtypes]; } if ($show_flag_counts) { @@ -149,37 +178,43 @@ sub list { $bug_lists{$flagtype->id} = {}; my $flags = Bugzilla::Flag->match({type_id => $flagtype->id}); # Build lists of bugs, triaged by flag status. - map { push(@{$bug_lists{$flagtype->id}->{$map{$_->status}}}, $_->bug_id) } @$flags; + push(@{$bug_lists{$flagtype->id}->{$map{$_->status}}}, $_->bug_id) foreach @$flags; } $vars->{'bug_lists'} = \%bug_lists; $vars->{'show_flag_counts'} = 1; } + $vars->{'selected_product'} = $product ? $product->name : ''; + $vars->{'selected_component'} = $component ? $component->name : ''; $vars->{'bug_types'} = $bug_flagtypes; $vars->{'attachment_types'} = $attach_flagtypes; - # Return the appropriate HTTP response headers. - print $cgi->header(); - - # Generate and return the UI (HTML page) from the appropriate template. $template->process("admin/flag-type/list.html.tmpl", $vars) || ThrowTemplateError($template->error()); + exit; } +if ($action eq 'enter') { + my $type = $cgi->param('target_type'); + ($type eq 'bug' || $type eq 'attachment') + || ThrowCodeError('flag_type_target_type_invalid', { target_type => $type }); -sub edit { - my ($action) = @_; + $vars->{'action'} = 'insert'; + $vars->{'token'} = issue_session_token('add_flagtype'); + $vars->{'type'} = { 'target_type' => $type, + 'inclusions' => { '__Any__:__Any__' => '0:0' } }; + # Get a list of groups available to restrict this flag type against. + $vars->{'groups'} = [Bugzilla::Group->get_all]; - my $flag_type; - if ($action eq 'enter') { - validateTargetType(); - } - else { - $flag_type = validateID(); - } + $template->process("admin/flag-type/edit.html.tmpl", $vars) + || ThrowTemplateError($template->error()); + exit; +} + +if ($action eq 'edit' || $action eq 'copy') { + $vars->{'type'} = Bugzilla::FlagType->check({ id => $flag_id }); - $vars->{'last_action'} = $cgi->param('action'); - if ($cgi->param('action') eq 'enter' || $cgi->param('action') eq 'copy') { + if ($action eq 'copy') { $vars->{'action'} = "insert"; $vars->{'token'} = issue_session_token('add_flagtype'); } @@ -188,343 +223,163 @@ sub edit { $vars->{'token'} = issue_session_token('edit_flagtype'); } - # If copying or editing an existing flag type, retrieve it. - if ($cgi->param('action') eq 'copy' || $cgi->param('action') eq 'edit') { - $vars->{'type'} = $flag_type; - } - # Otherwise set the target type (the minimal information about the type - # that the template needs to know) from the URL parameter and default - # the list of inclusions to all categories. - else { - my %inclusions; - $inclusions{"__Any__:__Any__"} = "0:0"; - $vars->{'type'} = { 'target_type' => scalar $cgi->param('target_type'), - 'inclusions' => \%inclusions }; - } # Get a list of groups available to restrict this flag type against. - my @groups = Bugzilla::Group->get_all; - $vars->{'groups'} = \@groups; - # Return the appropriate HTTP response headers. - print $cgi->header(); + $vars->{'groups'} = [Bugzilla::Group->get_all]; - # Generate and return the UI (HTML page) from the appropriate template. $template->process("admin/flag-type/edit.html.tmpl", $vars) || ThrowTemplateError($template->error()); + exit; } -sub processCategoryChange { - my ($categoryAction, $token) = @_; - validateIsActive(); - validateIsRequestable(); - validateIsRequesteeble(); - validateAllowMultiple(); - - my @inclusions = $cgi->param('inclusions'); - my @exclusions = $cgi->param('exclusions'); - if ($categoryAction eq 'include') { - my $product = validateProduct(scalar $cgi->param('product')); - my $component = validateComponent($product, scalar $cgi->param('component')); - my $category = ($product ? $product->id : 0) . ":" . - ($component ? $component->id : 0); - push(@inclusions, $category) unless grep($_ eq $category, @inclusions); - } - elsif ($categoryAction eq 'exclude') { - my $product = validateProduct(scalar $cgi->param('product')); - my $component = validateComponent($product, scalar $cgi->param('component')); - my $category = ($product ? $product->id : 0) . ":" . - ($component ? $component->id : 0); - push(@exclusions, $category) unless grep($_ eq $category, @exclusions); - } - elsif ($categoryAction eq 'removeInclusion') { - my @inclusion_to_remove = $cgi->param('inclusion_to_remove'); - foreach my $remove (@inclusion_to_remove) { - @inclusions = grep { $_ ne $remove } @inclusions; - } - } - elsif ($categoryAction eq 'removeExclusion') { - my @exclusion_to_remove = $cgi->param('exclusion_to_remove'); - foreach my $remove (@exclusion_to_remove) { - @exclusions = grep { $_ ne $remove } @exclusions; - } - } - - # Convert the array @clusions('prod_ID:comp_ID') back to a hash of - # the form %clusions{'prod_name:comp_name'} = 'prod_ID:comp_ID' - my %inclusions = clusion_array_to_hash(\@inclusions); - my %exclusions = clusion_array_to_hash(\@exclusions); - - my @groups = Bugzilla::Group->get_all; - $vars->{'groups'} = \@groups; - $vars->{'action'} = $cgi->param('action'); - - my $type = {}; - foreach my $key ($cgi->param()) { $type->{$key} = $cgi->param($key) } - # That's what I call a big hack. The template expects to see a group object. - # This script needs some rewrite anyway. - $type->{'grant_group'} = {}; - $type->{'grant_group'}->{'name'} = $cgi->param('grant_group'); - $type->{'request_group'} = {}; - $type->{'request_group'}->{'name'} = $cgi->param('request_group'); - - $type->{'inclusions'} = \%inclusions; - $type->{'exclusions'} = \%exclusions; - $vars->{'type'} = $type; - $vars->{'token'} = $token; +if ($action eq 'insert') { + check_token_data($token, 'add_flagtype'); - # Return the appropriate HTTP response headers. - print $cgi->header(); + my $name = $cgi->param('name'); + my $description = $cgi->param('description'); + my $target_type = $cgi->param('target_type'); + my $cc_list = $cgi->param('cc_list'); + my $sortkey = $cgi->param('sortkey'); + my $is_active = $cgi->param('is_active'); + my $is_requestable = $cgi->param('is_requestable'); + my $is_specifically = $cgi->param('is_requesteeble'); + my $is_multiplicable = $cgi->param('is_multiplicable'); + my $grant_group = $cgi->param('grant_group'); + my $request_group = $cgi->param('request_group'); + my @inclusions = $cgi->param('inclusions'); + my @exclusions = $cgi->param('exclusions'); + + my $flagtype = Bugzilla::FlagType->create({ + name => $name, + description => $description, + target_type => $target_type, + cc_list => $cc_list, + sortkey => $sortkey, + is_active => $is_active, + is_requestable => $is_requestable, + is_requesteeble => $is_specifically, + is_multiplicable => $is_multiplicable, + grant_group => $grant_group, + request_group => $request_group, + inclusions => \@inclusions, + exclusions => \@exclusions + }); - # Generate and return the UI (HTML page) from the appropriate template. - $template->process("admin/flag-type/edit.html.tmpl", $vars) - || ThrowTemplateError($template->error()); -} - -# Convert the array @clusions('prod_ID:comp_ID') back to a hash of -# the form %clusions{'prod_name:comp_name'} = 'prod_ID:comp_ID' -sub clusion_array_to_hash { - my $array = shift; - my %hash; - my %products; - my %components; - foreach my $ids (@$array) { - trick_taint($ids); - my ($product_id, $component_id) = split(":", $ids); - my $product_name = "__Any__"; - if ($product_id) { - $products{$product_id} ||= new Bugzilla::Product($product_id); - $product_name = $products{$product_id}->name if $products{$product_id}; - } - my $component_name = "__Any__"; - if ($component_id) { - $components{$component_id} ||= new Bugzilla::Component($component_id); - $component_name = $components{$component_id}->name if $components{$component_id}; - } - $hash{"$product_name:$component_name"} = $ids; - } - return %hash; -} - -sub insert { - my $token = shift; - check_token_data($token, 'add_flagtype'); - my $name = validateName(); - my $description = validateDescription(); - my $cc_list = validateCCList(); - validateTargetType(); - validateSortKey(); - validateIsActive(); - validateIsRequestable(); - validateIsRequesteeble(); - validateAllowMultiple(); - validateGroups(); - - my $dbh = Bugzilla->dbh; - - my $target_type = $cgi->param('target_type') eq "bug" ? "b" : "a"; - - $dbh->bz_start_transaction(); - - # Insert a record for the new flag type into the database. - $dbh->do('INSERT INTO flagtypes - (name, description, cc_list, target_type, - sortkey, is_active, is_requestable, - is_requesteeble, is_multiplicable, - grant_group_id, request_group_id) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)', - undef, ($name, $description, $cc_list, $target_type, - $cgi->param('sortkey'), $cgi->param('is_active'), - $cgi->param('is_requestable'), $cgi->param('is_requesteeble'), - $cgi->param('is_multiplicable'), scalar($cgi->param('grant_gid')), - scalar($cgi->param('request_gid')))); - - # Get the ID of the new flag type. - my $id = $dbh->bz_last_key('flagtypes', 'id'); - - # Populate the list of inclusions/exclusions for this flag type. - validateAndSubmit($id); - - $dbh->bz_commit_transaction(); - - $vars->{'name'} = $name; - $vars->{'message'} = "flag_type_created"; delete_token($token); - $vars->{'bug_types'} = Bugzilla::FlagType::match({'target_type' => 'bug'}); - $vars->{'attachment_types'} = Bugzilla::FlagType::match({'target_type' => 'attachment'}); + $vars->{'name'} = $flagtype->name; + $vars->{'message'} = "flag_type_created"; - # Return the appropriate HTTP response headers. - print $cgi->header(); + my @flagtypes = Bugzilla::FlagType->get_all; + $vars->{'bug_types'} = [grep { $_->target_type eq 'bug' } @flagtypes]; + $vars->{'attachment_types'} = [grep { $_->target_type eq 'attachment' } @flagtypes]; $template->process("admin/flag-type/list.html.tmpl", $vars) || ThrowTemplateError($template->error()); + exit; } - -sub update { - my $token = shift; +if ($action eq 'update') { check_token_data($token, 'edit_flagtype'); - my $flag_type = validateID(); - my $id = $flag_type->id; - my $name = validateName(); - my $description = validateDescription(); - my $cc_list = validateCCList(); - validateTargetType(); - validateSortKey(); - validateIsActive(); - validateIsRequestable(); - validateIsRequesteeble(); - validateAllowMultiple(); - validateGroups(); - - my $dbh = Bugzilla->dbh; - my $user = Bugzilla->user; - $dbh->bz_start_transaction(); - $dbh->do('UPDATE flagtypes - SET name = ?, description = ?, cc_list = ?, - sortkey = ?, is_active = ?, is_requestable = ?, - is_requesteeble = ?, is_multiplicable = ?, - grant_group_id = ?, request_group_id = ? - WHERE id = ?', - undef, ($name, $description, $cc_list, $cgi->param('sortkey'), - $cgi->param('is_active'), $cgi->param('is_requestable'), - $cgi->param('is_requesteeble'), $cgi->param('is_multiplicable'), - scalar($cgi->param('grant_gid')), scalar($cgi->param('request_gid')), - $id)); - - # Update the list of inclusions/exclusions for this flag type. - validateAndSubmit($id); - - $dbh->bz_commit_transaction(); - - # 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, $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, $id); - Bugzilla::Flag->force_retarget($flag_ids); - - # Now silently remove requestees from flags which are no longer - # specifically requestable. - if (!$cgi->param('is_requesteeble')) { - $dbh->do('UPDATE flags SET requestee_id = NULL WHERE type_id = ?', - undef, $id); - } - $vars->{'name'} = $name; - $vars->{'message'} = "flag_type_changes_saved"; + my $name = $cgi->param('name'); + my $description = $cgi->param('description'); + my $cc_list = $cgi->param('cc_list'); + my $sortkey = $cgi->param('sortkey'); + my $is_active = $cgi->param('is_active'); + my $is_requestable = $cgi->param('is_requestable'); + my $is_specifically = $cgi->param('is_requesteeble'); + my $is_multiplicable = $cgi->param('is_multiplicable'); + my $grant_group = $cgi->param('grant_group'); + my $request_group = $cgi->param('request_group'); + my @inclusions = $cgi->param('inclusions'); + my @exclusions = $cgi->param('exclusions'); + + my $flagtype = Bugzilla::FlagType->check({ id => $flag_id }); + $flagtype->set_name($name); + $flagtype->set_description($description); + $flagtype->set_cc_list($cc_list); + $flagtype->set_sortkey($sortkey); + $flagtype->set_is_active($is_active); + $flagtype->set_is_requestable($is_requestable); + $flagtype->set_is_specifically_requestable($is_specifically); + $flagtype->set_is_multiplicable($is_multiplicable); + $flagtype->set_grant_group($grant_group); + $flagtype->set_request_group($request_group); + $flagtype->set_clusions({ inclusions => \@inclusions, exclusions => \@exclusions}); + $flagtype->update(); + delete_token($token); - $vars->{'bug_types'} = Bugzilla::FlagType::match({'target_type' => 'bug'}); - $vars->{'attachment_types'} = Bugzilla::FlagType::match({'target_type' => 'attachment'}); + $vars->{'name'} = $flagtype->name; + $vars->{'message'} = "flag_type_changes_saved"; - # Return the appropriate HTTP response headers. - print $cgi->header(); + my @flagtypes = Bugzilla::FlagType->get_all; + $vars->{'bug_types'} = [grep { $_->target_type eq 'bug' } @flagtypes]; + $vars->{'attachment_types'} = [grep { $_->target_type eq 'attachment' } @flagtypes]; $template->process("admin/flag-type/list.html.tmpl", $vars) || ThrowTemplateError($template->error()); + exit; } - -sub confirmDelete { - my $flag_type = validateID(); - - $vars->{'flag_type'} = $flag_type; +if ($action eq 'confirmdelete') { + $vars->{'flag_type'} = Bugzilla::FlagType->check({ id => $flag_id }); $vars->{'token'} = issue_session_token('delete_flagtype'); - # Return the appropriate HTTP response headers. - print $cgi->header(); - # Generate and return the UI (HTML page) from the appropriate template. $template->process("admin/flag-type/confirm-delete.html.tmpl", $vars) || ThrowTemplateError($template->error()); + exit; } - -sub deleteType { - my $token = shift; +if ($action eq 'delete') { check_token_data($token, 'delete_flagtype'); - my $flag_type = validateID(); - my $id = $flag_type->id; - my $dbh = Bugzilla->dbh; - $dbh->bz_start_transaction(); + my $flagtype = Bugzilla::FlagType->check({ id => $flag_id }); + $flagtype->remove_from_db(); - # Get the name of the flag type so we can tell users - # what was deleted. - $vars->{'name'} = $flag_type->name; - - $dbh->do('DELETE FROM flags WHERE type_id = ?', undef, $id); - $dbh->do('DELETE FROM flaginclusions WHERE type_id = ?', undef, $id); - $dbh->do('DELETE FROM flagexclusions WHERE type_id = ?', undef, $id); - $dbh->do('DELETE FROM flagtypes WHERE id = ?', undef, $id); - $dbh->bz_commit_transaction(); - - $vars->{'message'} = "flag_type_deleted"; delete_token($token); - $vars->{'bug_types'} = Bugzilla::FlagType::match({'target_type' => 'bug'}); - $vars->{'attachment_types'} = Bugzilla::FlagType::match({'target_type' => 'attachment'}); + $vars->{'name'} = $flagtype->name; + $vars->{'message'} = "flag_type_deleted"; - # Return the appropriate HTTP response headers. - print $cgi->header(); + my @flagtypes = Bugzilla::FlagType->get_all; + $vars->{'bug_types'} = [grep { $_->target_type eq 'bug' } @flagtypes]; + $vars->{'attachment_types'} = [grep { $_->target_type eq 'attachment' } @flagtypes]; $template->process("admin/flag-type/list.html.tmpl", $vars) || ThrowTemplateError($template->error()); + exit; } - -sub deactivate { - my $token = shift; +if ($action eq 'deactivate') { check_token_data($token, 'delete_flagtype'); - my $flag_type = validateID(); - validateIsActive(); - - my $dbh = Bugzilla->dbh; - $dbh->bz_start_transaction(); - $dbh->do('UPDATE flagtypes SET is_active = 0 WHERE id = ?', undef, $flag_type->id); - $dbh->bz_commit_transaction(); + my $flagtype = Bugzilla::FlagType->check({ id => $flag_id }); + $flagtype->set_is_active(0); + $flagtype->update(); - $vars->{'message'} = "flag_type_deactivated"; - $vars->{'flag_type'} = $flag_type; delete_token($token); - $vars->{'bug_types'} = Bugzilla::FlagType::match({'target_type' => 'bug'}); - $vars->{'attachment_types'} = Bugzilla::FlagType::match({'target_type' => 'attachment'}); + $vars->{'message'} = "flag_type_deactivated"; + $vars->{'flag_type'} = $flagtype; - # Return the appropriate HTTP response headers. - print $cgi->header(); + my @flagtypes = Bugzilla::FlagType->get_all; + $vars->{'bug_types'} = [grep { $_->target_type eq 'bug' } @flagtypes]; + $vars->{'attachment_types'} = [grep { $_->target_type eq 'attachment' } @flagtypes]; - # Generate and return the UI (HTML page) from the appropriate template. $template->process("admin/flag-type/list.html.tmpl", $vars) || ThrowTemplateError($template->error()); + exit; } +ThrowUserError('unknown_action', {action => $action}); + +##################### +# Helper subroutines +##################### + sub get_products_and_components { - my $vars = shift; + my $vars = {}; my @products = Bugzilla::Product->get_all; # We require all unique component names. @@ -539,172 +394,37 @@ sub get_products_and_components { return $vars; } -################################################################################ -# Data Validation / Security Authorization -################################################################################ - -sub validateID { - my $id = $cgi->param('id'); - my $flag_type = new Bugzilla::FlagType($id) - || ThrowCodeError('flag_type_nonexistent', { id => $id }); - - return $flag_type; -} - -sub validateName { - my $name = $cgi->param('name'); - ($name && $name !~ /[ ,]/ && length($name) <= 50) - || ThrowUserError("flag_type_name_invalid", - { name => $name }); - trick_taint($name); - return $name; -} - -sub validateDescription { - my $description = $cgi->param('description'); - length($description) < 2**16-1 - || ThrowUserError("flag_type_description_invalid"); - trick_taint($description); - return $description; -} - -sub validateCCList { - my $cc_list = $cgi->param('cc_list'); - length($cc_list) <= 200 - || ThrowUserError("flag_type_cc_list_invalid", - { cc_list => $cc_list }); - - my @addresses = split(/[, ]+/, $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}); - } - trick_taint($cc_list); - return $cc_list; -} - -sub validateProduct { - my $product_name = shift; - return unless $product_name; - - my $product = Bugzilla::Product->check({ name => $product_name, - allow_inaccessible => 1 }); - return $product; -} - -sub validateComponent { - my ($product, $component_name) = @_; - return unless $component_name; - - ($product && $product->id) - || ThrowUserError("flag_type_component_without_product"); - - my $component = Bugzilla::Component->check({ product => $product, - name => $component_name }); - return $component; -} - -sub validateSortKey { - # $sortkey is destroyed if detaint_natural fails. - my $sortkey = $cgi->param('sortkey'); - detaint_natural($sortkey) - && $sortkey < 32768 - || ThrowUserError("flag_type_sortkey_invalid", - { sortkey => scalar $cgi->param('sortkey') }); - $cgi->param('sortkey', $sortkey); -} - -sub validateTargetType { - grep($cgi->param('target_type') eq $_, ("bug", "attachment")) - || ThrowCodeError("flag_type_target_type_invalid", - { target_type => scalar $cgi->param('target_type') }); -} - -sub validateIsActive { - $cgi->param('is_active', $cgi->param('is_active') ? 1 : 0); -} - -sub validateIsRequestable { - $cgi->param('is_requestable', $cgi->param('is_requestable') ? 1 : 0); -} - -sub validateIsRequesteeble { - $cgi->param('is_requesteeble', $cgi->param('is_requesteeble') ? 1 : 0); -} +sub filter_group { + my ($flag_types, $gid) = @_; + return $flag_types unless $gid; -sub validateAllowMultiple { - $cgi->param('is_multiplicable', $cgi->param('is_multiplicable') ? 1 : 0); -} + my @flag_types = grep {($_->grant_group && $_->grant_group->id == $gid) + || ($_->request_group && $_->request_group->id == $gid)} @$flag_types; -sub validateGroups { - my $dbh = Bugzilla->dbh; - # Convert group names to group IDs - foreach my $col ('grant', 'request') { - my $name = $cgi->param($col . '_group'); - if ($name) { - trick_taint($name); - my $gid = $dbh->selectrow_array('SELECT id FROM groups - WHERE name = ?', undef, $name); - $gid || ThrowUserError("group_unknown", { name => $name }); - $cgi->param($col . '_gid', $gid); - } - } + return \@flag_types; } -# At this point, values either come the DB itself or have been recently -# added by the user and have passed all validation tests. -# The only way to have invalid product/component combinations is to -# hack the URL. So we silently ignore them, if any. -sub validateAndSubmit { - my ($id) = @_; - my $dbh = Bugzilla->dbh; - - # Cache product objects. +# Convert the array @clusions('prod_ID:comp_ID') back to a hash of +# the form %clusions{'prod_name:comp_name'} = 'prod_ID:comp_ID' +sub clusion_array_to_hash { + my $array = shift; + my %hash; my %products; - foreach my $category_type ("inclusions", "exclusions") { - # Will be used several times below. - my $sth = $dbh->prepare("INSERT INTO flag$category_type " . - "(type_id, product_id, component_id) " . - "VALUES (?, ?, ?)"); - - $dbh->do("DELETE FROM flag$category_type WHERE type_id = ?", undef, $id); - foreach my $category ($cgi->param($category_type)) { - trick_taint($category); - my ($product_id, $component_id) = split(":", $category); - # Does the product exist? - if ($product_id) { - $products{$product_id} ||= new Bugzilla::Product($product_id); - next unless defined $products{$product_id}; - } - # A component was selected without a product being selected. - next if (!$product_id && $component_id); - # Does the component belong to this product? - if ($component_id) { - my @match = grep {$_->id == $component_id} @{$products{$product_id}->components}; - next unless scalar(@match); - } - $product_id ||= undef; - $component_id ||= undef; - $sth->execute($id, $product_id, $component_id); + my %components; + foreach my $ids (@$array) { + trick_taint($ids); + my ($product_id, $component_id) = split(":", $ids); + my $product_name = "__Any__"; + if ($product_id) { + $products{$product_id} ||= new Bugzilla::Product($product_id); + $product_name = $products{$product_id}->name if $products{$product_id}; + } + my $component_name = "__Any__"; + if ($component_id) { + $components{$component_id} ||= new Bugzilla::Component($component_id); + $component_name = $components{$component_id}->name if $components{$component_id}; } + $hash{"$product_name:$component_name"} = $ids; } -} - -sub filter_group { - my $flag_types = shift; - return $flag_types unless Bugzilla->cgi->param('group'); - - my $gid = scalar $cgi->param('group'); - my @flag_types = grep {($_->grant_group && $_->grant_group->id == $gid) - || ($_->request_group && $_->request_group->id == $gid)} @$flag_types; - - return \@flag_types; + return %hash; } diff --git a/template/en/default/admin/flag-type/edit.html.tmpl b/template/en/default/admin/flag-type/edit.html.tmpl index ebebf5082..88c92b4e3 100644 --- a/template/en/default/admin/flag-type/edit.html.tmpl +++ b/template/en/default/admin/flag-type/edit.html.tmpl @@ -23,20 +23,15 @@ [% PROCESS "global/js-products.html.tmpl" %] -[% IF type.target_type == "bug" %] - [% title = BLOCK %]Create Flag Type for [% terms.Bugs %][% END %] - [% typeLabelLowerPlural = BLOCK %][% terms.bugs %][% END %] - [% typeLabelLowerSingular = BLOCK %][% terms.bug %][% END %] +[% IF action == "insert" %] + [% title = BLOCK %] + Create Flag Type for [% type.target_type == "bug" ? terms.Bugs : "Attachments" %] + [% IF type.id %] + Based on [% type.name FILTER html %] + [% END %] + [% END %] + [% doc_section = "flags-overview.html#flags-create" %] [% 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 %] [% doc_section = "flags-overview.html#flags-edit" %] [% END %] @@ -53,10 +48,10 @@ %]
- + - + [% FOREACH category = type.inclusions %] [% END %] @@ -72,7 +67,7 @@ Name: - a short name identifying this type
+ a short name identifying this type.
@@ -81,7 +76,7 @@ Description: - a comprehensive description of this type
+ a comprehensive description of this type.
[% INCLUDE global/textarea.html.tmpl name = 'description' minrows = 4 @@ -95,9 +90,9 @@ Category: - the products/components to which [% typeLabelLowerPlural %] must - (inclusions) or must not (exclusions) belong in order for users - to be able to set flags of this type for them + the products/components to which [% type.target_type == "bug" ? terms.bugs : "attachments" %] + must (inclusions) or must not (exclusions) belong in order for users + to be able to set flags of this type for them. @@ -196,7 +191,7 @@ + the same [% type.target_type == "bug" ? terms.bug : "attachment" %]) @@ -204,7 +199,7 @@ @@ -213,19 +208,16 @@ - + diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl index 099748122..b85bb7acd 100644 --- a/template/en/default/filterexceptions.pl +++ b/template/en/default/filterexceptions.pl @@ -426,12 +426,8 @@ ], 'admin/flag-type/edit.html.tmpl' => [ - 'action', 'type.id', - 'type.target_type', 'type.sortkey || 1', - 'typeLabelLowerPlural', - 'typeLabelLowerSingular', 'selname', ], diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index 4e3ffae50..7d816082d 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -653,7 +653,7 @@ [% ELSIF error == "flag_type_description_invalid" %] [% title = "Flag Type Description Invalid" %] [% 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" %] [% title = "Flag Type Name Invalid" %] @@ -687,8 +687,8 @@ [% ELSIF error == "flag_type_sortkey_invalid" %] [% title = "Flag Type Sort Key Invalid" %] - The sort key must be an integer between 0 and 32767 inclusive. - It cannot be [% sortkey FILTER html %]. + The sort key [% sortkey FILTER html %] must be an integer + between 0 and [% constants.MAX_SMALLINT FILTER none %]. [% ELSIF error == "freetext_too_long" %] [% title = "Text Too Long" %] @@ -756,6 +756,7 @@ [% title = "System Groups not deletable" %] [% name FILTER html %] is a system group. This group cannot be deleted. + [% ELSIF error == "group_unknown" %] [% title = "Unknown Group" %] The group [% name FILTER html %] does not exist. Please specify -- cgit v1.2.3-24-g4f1b
@@ -139,10 +134,10 @@
Sort Key: - a number between 1 and 32767 by which this type will be sorted - when displayed to users in a list; ignore if you don't care - what order the types appear in or if you want them to appear - in alphabetical order
+ a number between 1 and [% constants.MAX_SMALLINT FILTER none %] by which + this type will be sorted when displayed to users in a list; ignore if you + don't care what order the types appear in or if you want them to appear + in alphabetical order.
Grant Group: the group allowed to grant/deny flags of this type - (to allow all users to grant/deny these flags, select no group)
+ (to allow all users to grant/deny these flags, select no group).
[% PROCESS select selname = "grant_group" %]
Request Group: if flags of this type are requestable, the group allowed to request them - (to allow all users to request these flags, select no group)
+ (to allow all users to request these flags, select no group).
Note that the request group alone has no effect if the grant group is not defined!
[% PROCESS select selname = "request_group" %]
  - +