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

a=LpSolit

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<DB_COLUMNS>
-
-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<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
-(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 @@
 %]
 
 <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="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 %]
     <input type="hidden" name="inclusions" value="[% category.value FILTER html %]">
   [% END %]
@@ -72,7 +67,7 @@
     <tr>
       <th>Name:</th>
       <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 %]"
                size="50" maxlength="50">
       </td>
@@ -81,7 +76,7 @@
     <tr>
       <th>Description:</th>
       <td>
-        a comprehensive description of this type<br>
+        a comprehensive description of this type.<br>
         [% INCLUDE global/textarea.html.tmpl
           name           = 'description'
           minrows        = 4
@@ -95,9 +90,9 @@
       <th>Category:</th>
 
       <td>
-        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.
         <table>
           <tr>
             <td style="vertical-align: top;">
@@ -139,10 +134,10 @@
     <tr>
       <th>Sort Key:</th>
       <td>
-        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<br>
+        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.<br>
         <input type="text" name="sortkey" value="[% type.sortkey || 1 %]" size="5" maxlength="5">
       </td>
     </tr>
@@ -196,7 +191,7 @@
         <input type="checkbox" id="is_multiplicable" name="is_multiplicable"
                [% " checked" IF type.is_multiplicable || !type.is_multiplicable.defined %]>
         <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>
     </tr>
 
@@ -204,7 +199,7 @@
       <th>Grant Group:</th>
       <td>
         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" %]
       </td>
     </tr>
@@ -213,19 +208,16 @@
       <th>Request Group:</th>
       <td>
         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>
         [% PROCESS select selname = "request_group" %]
       </td>
     </tr>
 
     <tr>
-      <th></th>
+      <th>&nbsp;</th>
       <td>
-        <input type="submit" id="save" value="
-          [%- IF (last_action == "enter" || last_action == "copy") %]Create
-          [%- ELSE %]Save Changes
-          [%- END %]">
+        <input type="submit" id="save" value="[% action == "insert" ? "Create" : "Save Changes" %]">
       </td>
     </tr>
 
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 <em>[% sortkey FILTER html %]</em>.
+    The sort key <em>[% sortkey FILTER html %]</em> 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" %]
     <em>[% name FILTER html %]</em> 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