Bug 313122: Implement Product->create, $product->update and…

Bugzilla/Bug.pm

@@ -3088,7 +3088,7 @@ sub RemoveVotes {
                  undef, ($votes, $id));
     }
     # Now return the array containing emails to be sent.
-    return \@messages;
+    return @messages;
 }
 
 # If a user votes for a bug, or the number of votes required to

Bugzilla/Constants.pm

@@ -146,6 +146,7 @@ use File::Basename;
     MAX_SMALLINT
 
     MAX_LEN_QUERY_NAME
+    MAX_PRODUCT_SIZE
     MAX_MILESTONE_SIZE
     MAX_COMPONENT_SIZE
     MAX_FREETEXT_LENGTH
@@ -413,6 +414,9 @@ use constant MAX_SMALLINT => 32767;
 # The longest that a saved search name can be.
 use constant MAX_LEN_QUERY_NAME => 64;
 
+# The longest product name allowed.
+use constant MAX_PRODUCT_SIZE => 64;
+
 # The longest milestone name allowed.
 use constant MAX_MILESTONE_SIZE => 20;
 

Bugzilla/Product.pm

@@ -13,20 +13,23 @@
 # The Original Code is the Bugzilla Bug Tracking System.
 #
 # Contributor(s): Tiago R. Mello <timello@async.com.br>
+#                 Frédéric Buclin <LpSolit@gmail.com>
 
 use strict;
 
 package Bugzilla::Product;
 
-use Bugzilla::Version;
-use Bugzilla::Milestone;
-
 use Bugzilla::Constants;
 use Bugzilla::Util;
-use Bugzilla::Group;
 use Bugzilla::Error;
-
+use Bugzilla::Group;
+use Bugzilla::Version;
+use Bugzilla::Milestone;
+use Bugzilla::Field;
+use Bugzilla::Status;
 use Bugzilla::Install::Requirements;
+use Bugzilla::Mailer;
+use Bugzilla::Series;
 
 use base qw(Bugzilla::Object);
 
@@ -39,22 +42,81 @@ use constant DEFAULT_CLASSIFICATION_ID => 1;
 use constant DB_TABLE => 'products';
 
 use constant DB_COLUMNS => qw(
-   products.id
-   products.name
-   products.classification_id
-   products.description
-   products.milestoneurl
-   products.disallownew
-   products.votesperuser
-   products.maxvotesperbug
-   products.votestoconfirm
-   products.defaultmilestone
+   id
+   name
+   classification_id
+   description
+   milestoneurl
+   disallownew
+   votesperuser
+   maxvotesperbug
+   votestoconfirm
+   defaultmilestone
 );
 
+use constant REQUIRED_CREATE_FIELDS => qw(
+    name
+    description
+    version
+);
+
+use constant UPDATE_COLUMNS => qw(
+    name
+    description
+    defaultmilestone
+    milestoneurl
+    disallownew
+    votesperuser
+    maxvotesperbug
+    votestoconfirm
+);
+
+use constant VALIDATORS => {
+    classification   => \&_check_classification,
+    name             => \&_check_name,
+    description      => \&_check_description,
+    version          => \&_check_version,
+    defaultmilestone => \&_check_default_milestone,
+    milestoneurl     => \&_check_milestone_url,
+    disallownew      => \&Bugzilla::Object::check_boolean,
+    votesperuser     => \&_check_votes_per_user,
+    maxvotesperbug   => \&_check_votes_per_bug,
+    votestoconfirm   => \&_check_votes_to_confirm,
+    create_series    => \&Bugzilla::Object::check_boolean
+};
+
 ###############################
 ####     Constructors     #####
 ###############################
 
+sub create {
+    my $class = shift;
+    my $dbh = Bugzilla->dbh;
+
+    $dbh->bz_start_transaction();
+
+    $class->check_required_create_fields(@_);
+
+    my $params = $class->run_create_validators(@_);
+    # Some fields do not exist in the DB as is.
+    $params->{classification_id} = delete $params->{classification};
+    my $version = delete $params->{version};
+    my $create_series = delete $params->{create_series};
+
+    my $product = $class->insert_create_data($params);
+
+    # Add the new version and milestone into the DB as valid values.
+    Bugzilla::Version::create($version, $product);
+    Bugzilla::Milestone->create({name => $params->{defaultmilestone}, product => $product});
+
+    # Create groups and series for the new product, if requested.
+    $product->_create_bug_group() if Bugzilla->params->{'makeproductgroups'};
+    $product->_create_series() if $create_series;
+
+    $dbh->bz_commit_transaction();
+    return $product;
+}
+
 # This is considerably faster than calling new_from_list three times
 # for each product in the list, particularly with hundreds or thousands
 # of products.
@@ -78,10 +140,337 @@ sub preload {
     }
 }
 
+sub update {
+    my $self = shift;
+    my $dbh = Bugzilla->dbh;
+    my $user = Bugzilla->user;
+
+    # Don't update the DB if something goes wrong below -> transaction.
+    $dbh->bz_start_transaction();
+    my $changes = $self->SUPER::update(@_);
+
+    # We also have to fix votes.
+    my @msgs; # Will store emails to send to voters.
+    if ($changes->{maxvotesperbug} || $changes->{votesperuser} || $changes->{votestoconfirm}) {
+        # We cannot |use| these modules, due to dependency loops.
+        require Bugzilla::Bug;
+        import Bugzilla::Bug qw(RemoveVotes CheckIfVotedConfirmed);
+        require Bugzilla::User;
+        import Bugzilla::User qw(user_id_to_login);
+
+        # 1. too many votes for a single user on a single bug.
+        my @toomanyvotes_list = ();
+        if ($self->max_votes_per_bug < $self->votes_per_user) {
+            my $votes = $dbh->selectall_arrayref(
+                        'SELECT votes.who, votes.bug_id
+                           FROM votes
+                                INNER JOIN bugs
+                                ON bugs.bug_id = votes.bug_id
+                          WHERE bugs.product_id = ?
+                                AND votes.vote_count > ?',
+                         undef, ($self->id, $self->max_votes_per_bug));
+
+            foreach my $vote (@$votes) {
+                my ($who, $id) = (@$vote);
+                # If some votes are removed, RemoveVotes() returns a list
+                # of messages to send to voters.
+                push(@msgs, RemoveVotes($id, $who, 'votes_too_many_per_bug'));
+                my $name = user_id_to_login($who);
+
+                push(@toomanyvotes_list, {id => $id, name => $name});
+            }
+        }
+        $changes->{'too_many_votes'} = \@toomanyvotes_list;
+
+        # 2. too many total votes for a single user.
+        # This part doesn't work in the general case because RemoveVotes
+        # doesn't enforce votesperuser (except per-bug when it's less
+        # than maxvotesperbug).  See Bugzilla::Bug::RemoveVotes().
+
+        my $votes = $dbh->selectall_arrayref(
+                    'SELECT votes.who, votes.vote_count
+                       FROM votes
+                            INNER JOIN bugs
+                            ON bugs.bug_id = votes.bug_id
+                      WHERE bugs.product_id = ?',
+                     undef, $self->id);
+
+        my %counts;
+        foreach my $vote (@$votes) {
+            my ($who, $count) = @$vote;
+            if (!defined $counts{$who}) {
+                $counts{$who} = $count;
+            } else {
+                $counts{$who} += $count;
+            }
+        }
+        my @toomanytotalvotes_list = ();
+        foreach my $who (keys(%counts)) {
+            if ($counts{$who} > $self->votes_per_user) {
+                my $bug_ids = $dbh->selectcol_arrayref(
+                              'SELECT votes.bug_id
+                                 FROM votes
+                                      INNER JOIN bugs
+                                      ON bugs.bug_id = votes.bug_id
+                                WHERE bugs.product_id = ?
+                                      AND votes.who = ?',
+                               undef, ($self->id, $who));
+
+                foreach my $bug_id (@$bug_ids) {
+                    # RemoveVotes() returns a list of messages to send
+                    # in case some voters had too many votes.
+                    push(@msgs, RemoveVotes($bug_id, $who, 'votes_too_many_per_user'));
+                    my $name = user_id_to_login($who);
+
+                    push(@toomanytotalvotes_list, {id => $bug_id, name => $name});
+                }
+            }
+        }
+        $changes->{'too_many_total_votes'} = \@toomanytotalvotes_list;
+
+        # 3. enough votes to confirm
+        my $bug_list =
+          $dbh->selectcol_arrayref('SELECT bug_id FROM bugs WHERE product_id = ?
+                                    AND bug_status = ? AND votes >= ?',
+                      undef, ($self->id, 'UNCONFIRMED', $self->votes_to_confirm));
+
+        my @updated_bugs = ();
+        foreach my $bug_id (@$bug_list) {
+            my $confirmed = CheckIfVotedConfirmed($bug_id, $user->id);
+            push (@updated_bugs, $bug_id) if $confirmed;
+        }
+        $changes->{'confirmed_bugs'} = \@updated_bugs;
+    }
+    $dbh->bz_commit_transaction();
+
+    # Now that changes have been committed, we can send emails to voters.
+    foreach my $msg (@msgs) {
+        MessageToMTA($msg);
+    }
+
+    return $changes;
+}
+
+sub remove_from_db {
+    my $self = shift;
+    my $user = Bugzilla->user;
+    my $dbh = Bugzilla->dbh;
+
+    $dbh->bz_start_transaction();
+
+    if ($self->bug_count) {
+        if (Bugzilla->params->{'allowbugdeletion'}) {
+            foreach my $bug_id (@{$self->bug_ids}) {
+                # Note that we allow the user to delete bugs he can't see,
+                # which is okay, because he's deleting the whole Product.
+                my $bug = new Bugzilla::Bug($bug_id);
+                $bug->remove_from_db();
+            }
+        }
+        else {
+            ThrowUserError('product_has_bugs', { nb => $self->bug_count });
+        }
+    }
+
+    # XXX - This line can go away as soon as bug 427455 is fixed.
+    $dbh->do("DELETE FROM group_control_map WHERE product_id = ?", undef, $self->id);
+    $dbh->do("DELETE FROM products WHERE id = ?", undef, $self->id);
+
+    $dbh->bz_commit_transaction();
+
+    # We have to delete these internal variables, else we get
+    # the old lists of products and classifications again.
+    delete $user->{selectable_products};
+    delete $user->{selectable_classifications};
+
+}
+
+###############################
+####      Validators       ####
+###############################
+
+sub _check_classification {
+    my ($invocant, $classification_name) = @_;
+
+    my $classification_id = 1;
+    if (Bugzilla->params->{'useclassification'}) {
+        my $classification =
+            Bugzilla::Classification::check_classification($classification_name);
+        $classification_id = $classification->id;
+    }
+    return $classification_id;
+}
+
+sub _check_name {
+    my ($invocant, $name) = @_;
+
+    $name = trim($name);
+    $name || ThrowUserError('product_blank_name');
+
+    if (length($name) > MAX_PRODUCT_SIZE) {
+        ThrowUserError('product_name_too_long', {'name' => $name});
+    }
+
+    my $product = new Bugzilla::Product({name => $name});
+    if ($product && (!ref $invocant || $product->id != $invocant->id)) {
+        # Check for exact case sensitive match:
+        if ($product->name eq $name) {
+            ThrowUserError('product_name_already_in_use', {'product' => $product->name});
+        }
+        else {
+            ThrowUserError('product_name_diff_in_case', {'product'          => $name,
+                                                         'existing_product' => $product->name});
+        }
+    }
+    return $name;
+}
+
+sub _check_description {
+    my ($invocant, $description) = @_;
+
+    $description  = trim($description);
+    $description || ThrowUserError('product_must_have_description');
+    return $description;
+}
+
+sub _check_version {
+    my ($invocant, $version) = @_;
+
+    $version = trim($version);
+    $version || ThrowUserError('product_must_have_version');
+    # We will check the version length when Bugzilla::Version->create will do it.
+    return $version;
+}
+
+sub _check_default_milestone {
+    my ($invocant, $milestone) = @_;
+
+    # Do nothing if target milestones are not in use.
+    unless (Bugzilla->params->{'usetargetmilestone'}) {
+        return (ref $invocant) ? $invocant->default_milestone : undef;
+    }
+
+    $milestone = trim($milestone);
+
+    if (ref $invocant) {
+        # The default milestone must be one of the existing milestones.
+        my $mil_obj = new Bugzilla::Milestone({name => $milestone, product => $invocant});
+
+        $mil_obj || ThrowUserError('product_must_define_defaultmilestone',
+                                   {product   => $invocant->name,
+                                    milestone => $milestone});
+    }
+    else {
+        $milestone ||= '---';
+    }
+    return $milestone;
+}
+
+sub _check_milestone_url {
+    my ($invocant, $url) = @_;
+
+    # Do nothing if target milestones are not in use.
+    unless (Bugzilla->params->{'usetargetmilestone'}) {
+        return (ref $invocant) ? $invocant->milestone_url : undef;
+    }
+
+    $url = trim($url);
+    return $url;
+}
+
+sub _check_votes_per_user {
+    return _check_votes(@_, 0);
+}
+
+sub _check_votes_per_bug {
+    return _check_votes(@_, 10000);
+}
+
+sub _check_votes_to_confirm {
+    return _check_votes(@_, 0);
+}
+
+# This subroutine is only used internally by other _check_votes_* validators.
+sub _check_votes {
+    my ($invocant, $votes, $field, $default) = @_;
+
+    detaint_natural($votes);
+    # On product creation, if the number of votes is not a valid integer,
+    # we silently fall back to the given default value.
+    # If the product already exists and the change is illegal, we complain.
+    if (!defined $votes) {
+        if (ref $invocant) {
+            ThrowUserError('product_illegal_votes', {field => $field, votes => $_[1]});
+        }
+        else {
+            $votes = $default;
+        }
+    }
+    return $votes;
+}
+
 ###############################
 ####       Methods         ####
 ###############################
 
+sub _create_bug_group {
+    my $self = shift;
+    my $dbh = Bugzilla->dbh;
+
+    my $group_name = $self->name;
+    while (new Bugzilla::Group({name => $group_name})) {
+        $group_name .= '_';
+    }
+    my $group_description = get_text('bug_group_description', {product => $self});
+
+    my $group = Bugzilla::Group->create({name        => $group_name,
+                                         description => $group_description,
+                                         isbuggroup  => 1});
+
+    # Associate the new group and new product.
+    $dbh->do('INSERT INTO group_control_map
+              (group_id, product_id, entry, membercontrol, othercontrol, canedit)
+              VALUES (?, ?, ?, ?, ?, ?)',
+              undef, ($group->id, $self->id, Bugzilla->params->{'useentrygroupdefault'},
+                      CONTROLMAPDEFAULT, CONTROLMAPNA, 0));
+}
+
+sub _create_series {
+    my $self = shift;
+
+    my @series;
+    # We do every status, every resolution, and an "opened" one as well.
+    foreach my $bug_status (@{get_legal_field_values('bug_status')}) {
+        push(@series, [$bug_status, "bug_status=" . url_quote($bug_status)]);
+    }
+
+    foreach my $resolution (@{get_legal_field_values('resolution')}) {
+        next if !$resolution;
+        push(@series, [$resolution, "resolution=" . url_quote($resolution)]);
+    }
+
+    my @openedstatuses = BUG_STATE_OPEN;
+    my $query = join("&", map { "bug_status=" . url_quote($_) } @openedstatuses);
+    push(@series, [get_text('series_all_open'), $query]);
+
+    foreach my $sdata (@series) {
+        my $series = new Bugzilla::Series(undef, $self->name,
+                        get_text('series_subcategory'),
+                        $sdata->[0], Bugzilla->user->id, 1,
+                        $sdata->[1] . "&product=" . url_quote($self->name), 1);
+        $series->writeToDatabase();
+    }
+}
+
+sub set_name { $_[0]->set('name', $_[1]); }
+sub set_description { $_[0]->set('description', $_[1]); }
+sub set_default_milestone { $_[0]->set('defaultmilestone', $_[1]); }
+sub set_milestone_url { $_[0]->set('milestoneurl', $_[1]); }
+sub set_disallow_new { $_[0]->set('disallownew', $_[1]); }
+sub set_votes_per_user { $_[0]->set('votesperuser', $_[1]); }
+sub set_votes_per_bug { $_[0]->set('maxvotesperbug', $_[1]); }
+sub set_votes_to_confirm { $_[0]->set('votestoconfirm', $_[1]); }
+
 sub components {
     my $self = shift;
     my $dbh = Bugzilla->dbh;

Bugzilla/Series.pm

@@ -33,7 +33,6 @@ package Bugzilla::Series;
 
 use Bugzilla::Error;
 use Bugzilla::Util;
-use Bugzilla::User;
 
 sub new {
     my $invocant = shift;

editproducts.cgi

@@ -36,16 +36,9 @@ use Bugzilla::Constants;
 use Bugzilla::Util;
 use Bugzilla::Error;
 use Bugzilla::Bug;
-use Bugzilla::Series;
-use Bugzilla::Mailer;
 use Bugzilla::Product;
 use Bugzilla::Classification;
-use Bugzilla::Milestone;
-use Bugzilla::Group;
-use Bugzilla::User;
-use Bugzilla::Field;
 use Bugzilla::Token;
-use Bugzilla::Status;
 
 #
 # Preliminary checks:
@@ -76,7 +69,6 @@ $user->in_group('editcomponents')
 my $classification_name = trim($cgi->param('classification') || '');
 my $product_name = trim($cgi->param('product') || '');
 my $action  = trim($cgi->param('action')  || '');
-my $showbugcounts = (defined $cgi->param('showbugcounts'));
 my $token = $cgi->param('token');
 
 #
@@ -124,7 +116,7 @@ if (!$action && !$product_name) {
         }
     }
     $vars->{'products'} = $products;
-    $vars->{'showbugcounts'} = $showbugcounts;
+    $vars->{'showbugcounts'} = $cgi->param('showbugcounts') ? 1 : 0;
 
     $template->process("admin/products/list.html.tmpl", $vars)
       || ThrowTemplateError($template->error());
@@ -175,171 +167,27 @@ if ($action eq 'new') {
                                          object => "products"});
 
     check_token_data($token, 'add_product');
-    # Cleanups and validity checks
 
-    my $classification_id = 1;
-    if (Bugzilla->params->{'useclassification'}) {
-        my $classification = 
-            Bugzilla::Classification::check_classification($classification_name);
-        $classification_id = $classification->id;
-        $vars->{'classification'} = $classification;
-    }
-
-    unless ($product_name) {
-        ThrowUserError("product_blank_name");  
-    }
-
-    my $product = new Bugzilla::Product({name => $product_name});
-
-    if ($product) {
-
-        # Check for exact case sensitive match:
-        if ($product->name eq $product_name) {
-            ThrowUserError("product_name_already_in_use",
-                           {'product' => $product->name});
-        }
-
-        # Next check for a case-insensitive match:
-        if (lc($product->name) eq lc($product_name)) {
-            ThrowUserError("product_name_diff_in_case",
-                           {'product' => $product_name,
-                            'existing_product' => $product->name}); 
-        }
-    }
-
-    my $version = trim($cgi->param('version') || '');
-
-    if ($version eq '') {
-        ThrowUserError("product_must_have_version",
-                       {'product' => $product_name});
-    }
-
-    my $description  = trim($cgi->param('description')  || '');
-
-    if ($description eq '') {
-        ThrowUserError('product_must_have_description',
-                       {'product' => $product_name});
-    }
+    my $product =
+      Bugzilla::Product->create({classification   => $classification_name,
+                                 name             => $product_name,
+                                 description      => scalar $cgi->param('description'),
+                                 version          => scalar $cgi->param('version'),
+                                 defaultmilestone => scalar $cgi->param('defaultmilestone'),
+                                 milestoneurl     => scalar $cgi->param('milestoneurl'),
+                                 disallownew      => scalar $cgi->param('disallownew'),
+                                 votesperuser     => scalar $cgi->param('votesperuser'),
+                                 maxvotesperbug   => scalar $cgi->param('maxvotesperbug'),
+                                 votestoconfirm   => scalar $cgi->param('votestoconfirm'),
+                                 create_series    => scalar $cgi->param('createseries')});
 
-    my $milestoneurl = trim($cgi->param('milestoneurl') || '');
-    my $disallownew = $cgi->param('disallownew') ? 1 : 0;
-    my $votesperuser = $cgi->param('votesperuser') || 0;
-    my $maxvotesperbug = defined($cgi->param('maxvotesperbug')) ?
-        $cgi->param('maxvotesperbug') : 10000;
-    my $votestoconfirm = $cgi->param('votestoconfirm') || 0;
-    my $defaultmilestone = $cgi->param('defaultmilestone') || "---";
-
-    # The following variables are used in placeholders only.
-    trick_taint($product_name);
-    trick_taint($version);
-    trick_taint($description);
-    trick_taint($milestoneurl);
-    trick_taint($defaultmilestone);
-    detaint_natural($disallownew);
-    detaint_natural($votesperuser);
-    detaint_natural($maxvotesperbug);
-    detaint_natural($votestoconfirm);
-
-    # Add the new product.
-    $dbh->do('INSERT INTO products
-              (name, description, milestoneurl, disallownew, votesperuser,
-               maxvotesperbug, votestoconfirm, defaultmilestone, classification_id)
-              VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)',
-             undef, ($product_name, $description, $milestoneurl, $disallownew,
-             $votesperuser, $maxvotesperbug, $votestoconfirm, $defaultmilestone,
-             $classification_id));
-
-    $product = new Bugzilla::Product({name => $product_name});
-    
-    $dbh->do('INSERT INTO versions (value, product_id) VALUES (?, ?)',
-             undef, ($version, $product->id));
-
-    $dbh->do('INSERT INTO milestones (product_id, value) VALUES (?, ?)',
-             undef, ($product->id, $defaultmilestone));
-
-    # If we're using bug groups, then we need to create a group for this
-    # product as well.  -JMR, 2/16/00
-    if (Bugzilla->params->{"makeproductgroups"}) {
-        # Next we insert into the groups table
-        my $productgroup = $product->name;
-        while (new Bugzilla::Group({name => $productgroup})) {
-            $productgroup .= '_';
-        }
-        my $group_description = "Access to bugs in the " .
-                                $product->name . " product";
-
-        $dbh->do('INSERT INTO groups (name, description, isbuggroup)
-                  VALUES (?, ?, ?)',
-                  undef, ($productgroup, $group_description, 1));
-
-        my $gid = $dbh->bz_last_key('groups', 'id');
-
-        # If we created a new group, give the "admin" group privileges
-        # initially.
-        my $admin = Bugzilla::Group->new({name => 'admin'})->id();
-        
-        my $sth = $dbh->prepare('INSERT INTO group_group_map
-                                 (member_id, grantor_id, grant_type)
-                                 VALUES (?, ?, ?)');
-
-        $sth->execute($admin, $gid, GROUP_MEMBERSHIP);
-        $sth->execute($admin, $gid, GROUP_BLESS);
-        $sth->execute($admin, $gid, GROUP_VISIBLE);
-
-        # Associate the new group and new product.
-        $dbh->do('INSERT INTO group_control_map
-                  (group_id, product_id, entry, membercontrol,
-                   othercontrol, canedit)
-                  VALUES (?, ?, ?, ?, ?, ?)',
-                 undef, ($gid, $product->id, 
-                         Bugzilla->params->{'useentrygroupdefault'},
-                 CONTROLMAPDEFAULT, CONTROLMAPNA, 0));
-    }
-
-    if ($cgi->param('createseries')) {
-        # Insert default charting queries for this product.
-        # If they aren't using charting, this won't do any harm.
-        #
-        # $open_name and $product are sqlquoted by the series code 
-        # and never used again here, so we can trick_taint them.
-        my $open_name = $cgi->param('open_name');
-        trick_taint($open_name);
-    
-        my @series;
-    
-        # We do every status, every resolution, and an "opened" one as well.
-        foreach my $bug_status (@{get_legal_field_values('bug_status')}) {
-            push(@series, [$bug_status, 
-                           "bug_status=" . url_quote($bug_status)]);
-        }
-
-        foreach my $resolution (@{get_legal_field_values('resolution')}) {
-            next if !$resolution;
-            push(@series, [$resolution, "resolution=" .url_quote($resolution)]);
-        }
-
-        # For localization reasons, we get the name of the "global" subcategory
-        # and the title of the "open" query from the submitted form.
-        my @openedstatuses = BUG_STATE_OPEN;
-        my $query = 
-               join("&", map { "bug_status=" . url_quote($_) } @openedstatuses);
-        push(@series, [$open_name, $query]);
-    
-        foreach my $sdata (@series) {
-            my $series = new Bugzilla::Series(undef, $product->name, 
-                            scalar $cgi->param('subcategory'),
-                            $sdata->[0], $whoid, 1,
-                            $sdata->[1] . "&product=" .
-                            url_quote($product->name), 1);
-            $series->writeToDatabase();
-        }
-    }
     delete_token($token);
 
     $vars->{'message'} = 'product_created';
     $vars->{'product'} = $product;
-    $vars->{'classification'} = new Bugzilla::Classification($product->classification_id)
-      if Bugzilla->params->{'useclassification'};
+    if (Bugzilla->params->{'useclassification'}) {
+        $vars->{'classification'} = new Bugzilla::Classification($product->classification_id);
+    }
     $vars->{'token'} = issue_session_token('edit_product');
 
     $template->process("admin/products/edit.html.tmpl", $vars)
@@ -357,16 +205,8 @@ if ($action eq 'del') {
     my $product = $user->check_can_admin_product($product_name);
 
     if (Bugzilla->params->{'useclassification'}) {
-        my $classification = 
-            Bugzilla::Classification::check_classification($classification_name);
-        if ($classification->id != $product->classification_id) {
-            ThrowUserError('classification_doesnt_exist_for_product',
-                           { product => $product->name,
-                             classification => $classification->name });
-        }
-        $vars->{'classification'} = $classification;
+        $vars->{'classification'} = new Bugzilla::Classification($product->classification_id);
     }
-
     $vars->{'product'} = $product;
     $vars->{'token'} = issue_session_token('delete_product');
 
@@ -383,69 +223,7 @@ if ($action eq 'delete') {
     my $product = $user->check_can_admin_product($product_name);
     check_token_data($token, 'delete_product');
 
-    if (Bugzilla->params->{'useclassification'}) {
-        my $classification = 
-            Bugzilla::Classification::check_classification($classification_name);
-        if ($classification->id != $product->classification_id) {
-            ThrowUserError('classification_doesnt_exist_for_product',
-                           { product => $product->name,
-                             classification => $classification->name });
-        }
-        $vars->{'classification'} = $classification;
-    }
-
-    if ($product->bug_count) {
-        if (Bugzilla->params->{"allowbugdeletion"}) {
-            foreach my $bug_id (@{$product->bug_ids}) {
-                # Note that we allow the user to delete bugs he can't see,
-                # which is okay, because he's deleting the whole Product.
-                my $bug = new Bugzilla::Bug($bug_id);
-                $bug->remove_from_db();
-            }
-        }
-        else {
-            ThrowUserError("product_has_bugs", 
-                           { nb => $product->bug_count });
-        }
-    }
-
-    $dbh->bz_start_transaction();
-
-    my $comp_ids = $dbh->selectcol_arrayref('SELECT id FROM components
-                                             WHERE product_id = ?',
-                                             undef, $product->id);
-
-    $dbh->do('DELETE FROM component_cc WHERE component_id IN
-              (' . join(',', @$comp_ids) . ')') if scalar(@$comp_ids);
-
-    $dbh->do("DELETE FROM components WHERE product_id = ?",
-             undef, $product->id);
-
-    $dbh->do("DELETE FROM versions WHERE product_id = ?",
-             undef, $product->id);
-
-    $dbh->do("DELETE FROM milestones WHERE product_id = ?",
-             undef, $product->id);
-
-    $dbh->do("DELETE FROM group_control_map WHERE product_id = ?",
-             undef, $product->id);
-
-    $dbh->do("DELETE FROM flaginclusions WHERE product_id = ?",
-             undef, $product->id);
-             
-    $dbh->do("DELETE FROM flagexclusions WHERE product_id = ?",
-             undef, $product->id);
-             
-    $dbh->do("DELETE FROM products WHERE id = ?",
-             undef, $product->id);
-
-    $dbh->bz_commit_transaction();
-
-    # We have to delete these internal variables, else we get
-    # the old lists of products and classifications again.
-    delete $user->{selectable_products};
-    delete $user->{selectable_classifications};
-
+    $product->remove_from_db;
     delete_token($token);
 
     $vars->{'message'} = 'product_deleted';
@@ -484,20 +262,7 @@ if ($action eq 'edit' || (!$action && $product_name)) {
     my $product = $user->check_can_admin_product($product_name);
 
     if (Bugzilla->params->{'useclassification'}) {
-        my $classification; 
-        if (!$classification_name) {
-            $classification = 
-                new Bugzilla::Classification($product->classification_id);
-        } else {
-            $classification = 
-                Bugzilla::Classification::check_classification($classification_name);
-            if ($classification->id != $product->classification_id) {
-                ThrowUserError('classification_doesnt_exist_for_product',
-                               { product => $product->name,
-                                 classification => $classification->name });
-            }
-        }
-        $vars->{'classification'} = $classification;
+        $vars->{'classification'} = new Bugzilla::Classification($product->classification_id);
     }
     $vars->{'product'} = $product;
     $vars->{'token'} = issue_session_token('edit_product');
@@ -798,239 +563,26 @@ if ($action eq 'updategroupcontrols') {
 if ($action eq 'update') {
     check_token_data($token, 'edit_product');
     my $product_old_name = trim($cgi->param('product_old_name') || '');
-    my $description         = trim($cgi->param('description')         || '');
-    my $disallownew         = trim($cgi->param('disallownew')         || '');
-    my $milestoneurl        = trim($cgi->param('milestoneurl')        || '');
-    my $votesperuser        = trim($cgi->param('votesperuser')        || 0);
-    my $maxvotesperbug      = trim($cgi->param('maxvotesperbug')      || 0);
-    my $votestoconfirm      = trim($cgi->param('votestoconfirm')      || 0);
-    my $defaultmilestone    = trim($cgi->param('defaultmilestone')    || '---');
-
-    my $checkvotes = 0;
-
-    my $product_old = $user->check_can_admin_product($product_old_name);
-
-    if (Bugzilla->params->{'useclassification'}) {
-        my $classification; 
-        if (!$classification_name) {
-            $classification = 
-                new Bugzilla::Classification($product_old->classification_id);
-        } else {
-            $classification = 
-                Bugzilla::Classification::check_classification($classification_name);
-            if ($classification->id != $product_old->classification_id) {
-                ThrowUserError('classification_doesnt_exist_for_product',
-                               { product => $product_old->name,
-                                 classification => $classification->name });
-            }
-        }
-        $vars->{'classification'} = $classification;
-    }
-
-    unless ($product_name) {
-        ThrowUserError('product_cant_delete_name',
-                       {product => $product_old->name});
-    }
-
-    unless ($description) {
-        ThrowUserError('product_cant_delete_description',
-                       {product => $product_old->name});
-    }
-
-    my $stored_maxvotesperbug = $maxvotesperbug;
-    if (!detaint_natural($maxvotesperbug)) {
-        ThrowUserError('product_votes_per_bug_must_be_nonnegative',
-                       {maxvotesperbug => $stored_maxvotesperbug});
-    }
-
-    my $stored_votesperuser = $votesperuser;
-    if (!detaint_natural($votesperuser)) {
-        ThrowUserError('product_votes_per_user_must_be_nonnegative',
-                       {votesperuser => $stored_votesperuser});
-    }
-
-    my $stored_votestoconfirm = $votestoconfirm;
-    if (!detaint_natural($votestoconfirm)) {
-        ThrowUserError('product_votes_to_confirm_must_be_nonnegative',
-                       {votestoconfirm => $stored_votestoconfirm});
-    }
-
-    $dbh->bz_start_transaction();
-
-    my $testproduct = 
-        new Bugzilla::Product({name => $product_name});
-    if (lc($product_name) ne lc($product_old->name) &&
-        $testproduct) {
-        ThrowUserError('product_name_already_in_use',
-                       {product => $product_name});
-    }
-
-    # Only update milestone related stuff if 'usetargetmilestone' is on.
-    if (Bugzilla->params->{'usetargetmilestone'}) {
-        my $milestone = new Bugzilla::Milestone(
-            { product => $product_old, name => $defaultmilestone });
-
-        unless ($milestone) {
-            ThrowUserError('product_must_define_defaultmilestone',
-                           {product          => $product_old->name,
-                            defaultmilestone => $defaultmilestone,
-                            classification   => $classification_name});
-        }
-
-        if ($milestoneurl ne $product_old->milestone_url) {
-            trick_taint($milestoneurl);
-            $dbh->do('UPDATE products SET milestoneurl = ? WHERE id = ?',
-                     undef, ($milestoneurl, $product_old->id));
-        }
-
-        if ($milestone->name ne $product_old->default_milestone) {
-            $dbh->do('UPDATE products SET defaultmilestone = ? WHERE id = ?',
-                     undef, ($milestone->name, $product_old->id));
-        }
-    }
-
-    $disallownew = $disallownew ? 1 : 0;
-    if ($disallownew ne $product_old->disallow_new) {
-        $dbh->do('UPDATE products SET disallownew = ? WHERE id = ?',
-                 undef, ($disallownew, $product_old->id));
-    }
-
-    if ($description ne $product_old->description) {
-        trick_taint($description);
-        $dbh->do('UPDATE products SET description = ? WHERE id = ?',
-                 undef, ($description, $product_old->id));
-    }
-
-    if ($votesperuser ne $product_old->votes_per_user) {
-        $dbh->do('UPDATE products SET votesperuser = ? WHERE id = ?',
-                 undef, ($votesperuser, $product_old->id));
-        $checkvotes = 1;
-    }
+    my $product = $user->check_can_admin_product($product_old_name);
 
-    if ($maxvotesperbug ne $product_old->max_votes_per_bug) {
-        $dbh->do('UPDATE products SET maxvotesperbug = ? WHERE id = ?',
-                 undef, ($maxvotesperbug, $product_old->id));
-        $checkvotes = 1;
-    }
-
-    if ($votestoconfirm ne $product_old->votes_to_confirm) {
-        $dbh->do('UPDATE products SET votestoconfirm = ? WHERE id = ?',
-                 undef, ($votestoconfirm, $product_old->id));
-        $checkvotes = 1;
-    }
-
-    if ($product_name ne $product_old->name) {
-        trick_taint($product_name);
-        $dbh->do('UPDATE products SET name = ? WHERE id = ?',
-                 undef, ($product_name, $product_old->id));
-    }
+    $product->set_name($product_name);
+    $product->set_description(scalar $cgi->param('description'));
+    $product->set_default_milestone(scalar $cgi->param('defaultmilestone'));
+    $product->set_milestone_url(scalar $cgi->param('milestoneurl'));
+    $product->set_disallow_new(scalar $cgi->param('disallownew'));
+    $product->set_votes_per_user(scalar $cgi->param('votesperuser'));
+    $product->set_votes_per_bug(scalar $cgi->param('maxvotesperbug'));
+    $product->set_votes_to_confirm(scalar $cgi->param('votestoconfirm'));
 
-    $dbh->bz_commit_transaction();
-
-    my $product = new Bugzilla::Product({name => $product_name});
-
-    if ($checkvotes) {
-        $vars->{'checkvotes'} = 1;
+    my $changes = $product->update();
 
-        # 1. too many votes for a single user on a single bug.
-        my @toomanyvotes_list = ();
-        if ($maxvotesperbug < $votesperuser) {
-            my $votes = $dbh->selectall_arrayref(
-                        'SELECT votes.who, votes.bug_id
-                           FROM votes
-                     INNER JOIN bugs
-                             ON bugs.bug_id = votes.bug_id
-                          WHERE bugs.product_id = ?
-                            AND votes.vote_count > ?',
-                         undef, ($product->id, $maxvotesperbug));
-
-            foreach my $vote (@$votes) {
-                my ($who, $id) = (@$vote);
-                # If some votes are removed, RemoveVotes() returns a list
-                # of messages to send to voters.
-                my $msgs = RemoveVotes($id, $who, 'votes_too_many_per_bug');
-                foreach my $msg (@$msgs) {
-                    MessageToMTA($msg);
-                }
-                my $name = user_id_to_login($who);
-
-                push(@toomanyvotes_list,
-                     {id => $id, name => $name});
-            }
-        }
-        $vars->{'toomanyvotes'} = \@toomanyvotes_list;
-
-        # 2. too many total votes for a single user.
-        # This part doesn't work in the general case because RemoveVotes
-        # doesn't enforce votesperuser (except per-bug when it's less
-        # than maxvotesperbug).  See Bugzilla::Bug::RemoveVotes().
-
-        my $votes = $dbh->selectall_arrayref(
-                    'SELECT votes.who, votes.vote_count
-                       FROM votes
-                 INNER JOIN bugs
-                         ON bugs.bug_id = votes.bug_id
-                      WHERE bugs.product_id = ?',
-                     undef, $product->id);
-
-        my %counts;
-        foreach my $vote (@$votes) {
-            my ($who, $count) = @$vote;
-            if (!defined $counts{$who}) {
-                $counts{$who} = $count;
-            } else {
-                $counts{$who} += $count;
-            }
-        }
-        my @toomanytotalvotes_list = ();
-        foreach my $who (keys(%counts)) {
-            if ($counts{$who} > $votesperuser) {
-                my $bug_ids = $dbh->selectcol_arrayref(
-                              'SELECT votes.bug_id
-                                 FROM votes
-                           INNER JOIN bugs
-                                   ON bugs.bug_id = votes.bug_id
-                                WHERE bugs.product_id = ?
-                                  AND votes.who = ?',
-                               undef, ($product->id, $who));
-
-                foreach my $bug_id (@$bug_ids) {
-                    # RemoveVotes() returns a list of messages to send
-                    # in case some voters had too many votes.
-                    my $msgs = RemoveVotes($bug_id, $who, 'votes_too_many_per_user');
-                    foreach my $msg (@$msgs) {
-                        MessageToMTA($msg);
-                    }
-                    my $name = user_id_to_login($who);
-
-                    push(@toomanytotalvotes_list,
-                         {id => $bug_id, name => $name});
-                }
-            }
-        }
-        $vars->{'toomanytotalvotes'} = \@toomanytotalvotes_list;
-
-        # 3. enough votes to confirm
-        my $bug_list = $dbh->selectcol_arrayref(
-                       "SELECT bug_id FROM bugs
-                         WHERE product_id = ?
-                           AND bug_status = 'UNCONFIRMED'
-                           AND votes >= ?",
-                        undef, ($product->id, $votestoconfirm));
-
-        my @updated_bugs = ();
-        foreach my $bug_id (@$bug_list) {
-            my $confirmed = CheckIfVotedConfirmed($bug_id, $whoid);
-            push (@updated_bugs, $bug_id) if $confirmed;
-        }
-
-        $vars->{'confirmedbugs'} = \@updated_bugs;
-        $vars->{'changer'} = $user->login;
-    }
     delete_token($token);
 
-    $vars->{'old_product'} = $product_old;
+    if (Bugzilla->params->{'useclassification'}) {
+        $vars->{'classification'} = new Bugzilla::Classification($product->classification_id);
+    }
     $vars->{'product'} = $product;
+    $vars->{'changes'} = $changes;
 
     $template->process("admin/products/updated.html.tmpl", $vars)
         || ThrowTemplateError($template->error());

process_bug.cgi

@@ -540,13 +540,13 @@ foreach my $bug (@bug_objects) {
     # an error later.
     delete $changed_deps{''};
 
-    # $msgs will store emails which have to be sent to voters, if any.
-    my $msgs;
+    # @msgs will store emails which have to be sent to voters, if any.
+    my @msgs;
     if ($changes->{'product'}) {
         # If some votes have been removed, RemoveVotes() returns
         # a list of messages to send to voters.
-        # We delay the sending of these messages till tables are unlocked.
-        $msgs = RemoveVotes($bug->id, 0, 'votes_bug_moved');
+        # We delay the sending of these messages till changes are committed.
+        @msgs = RemoveVotes($bug->id, 0, 'votes_bug_moved');
         CheckIfVotedConfirmed($bug->id, Bugzilla->user->id);
     }
 
@@ -560,7 +560,7 @@ foreach my $bug (@bug_objects) {
     ###############
 
     # Now is a good time to send email to voters.
-    foreach my $msg (@$msgs) {
+    foreach my $msg (@msgs) {
         MessageToMTA($msg);
     }
 

template/en/default/admin/products/confirm-delete.html.tmpl

@@ -31,14 +31,6 @@
   style_urls = ['skins/standard/admin.css']
 %]
 
-[% IF classification %]
-  [% classification_url_part = BLOCK %]&classification=
-    [%- classification.name FILTER url_quote %]
-  [%- END %]
-[% ELSE %]
-  [% classification_url_part = "" %]
-[% END %]
-
 <table border="1" cellpadding="4" cellspacing="0">
   <tr bgcolor="#6666FF">
     <th valign="top" align="left">Field</th>
@@ -66,8 +58,7 @@
   <tr>
     <td valign="top">Product:</td>
     <td valign="top">
-      <a href="editproducts.cgi?product=[% product.name FILTER url_quote %]
-         [%- classification_url_part %]">
+      <a href="editproducts.cgi?product=[% product.name FILTER url_quote %]">
         [% product.name FILTER html %]
       </a>
     </td>
@@ -113,8 +104,7 @@
   <tr>
     <td>
       [% IF product.components.size > 0 %]
-        <a href="editcomponents.cgi?product=[% product.name FILTER url_quote %]
-           [%- classification_url_part %]"
+        <a href="editcomponents.cgi?product=[% product.name FILTER url_quote %]"
            title="Edit components for product '[% product.name FILTER html %]'">
           Components:
         </a>
@@ -148,8 +138,7 @@
   <tr>
     <td>
       [% IF product.versions.size > 0 %]
-        <a href="editversions.cgi?product=[%- product.name FILTER url_quote %]
-           [%- classification_url_part %]">
+        <a href="editversions.cgi?product=[%- product.name FILTER url_quote %]">
           Versions:
         </a>
       [% ELSE %]
@@ -172,8 +161,7 @@
     <tr>
       <td valign="top">
         [% IF product.milestones.size > 0 %]
-          <a href="editmilestones.cgi?product=[%- product.name FILTER url_quote %]
-             [%- classification_url_part -%]">
+          <a href="editmilestones.cgi?product=[%- product.name FILTER url_quote %]">
             Milestones:
           </a>
         [% ELSE %]
@@ -196,10 +184,8 @@
     <td>[% terms.Bugs %]:</td>
     <td>
       [% IF product.bug_count %]
-        <a href="buglist.cgi?product=[%- product.name FILTER url_quote %]
-           [%- classification_url_part %]"
-           title="List of [% terms.bugs %] for product '
-           [%- product.name FILTER html %]'">
+        <a href="buglist.cgi?product=[% product.name FILTER url_quote %]"
+           title="List of [% terms.bugs %] for product '[% product.name FILTER html %]'">
           [% product.bug_count FILTER html %]
         </a>
       [% ELSE %]
@@ -264,8 +250,6 @@
     <input type="hidden" name="action" value="delete">
     <input type="hidden" name="product" value="[% product.name FILTER html %]">
     <input type="hidden" name="token" value="[% token FILTER html %]">
-    <input type="hidden" name="classification"
-           value="[% classification.name FILTER html %]">
   </form>
 
 [% END %]

template/en/default/admin/products/create.html.tmpl

@@ -53,8 +53,6 @@
   </table>
 
   <input type="submit" value="Add">
-  <input type="hidden" name="subcategory" value="-All-">
-  <input type="hidden" name="open_name"   value="All Open">
   <input type="hidden" name="action" value="new">
   <input type="hidden" name="token" value="[% token FILTER html %]">
   <input type="hidden" name="classification"

template/en/default/admin/products/edit.html.tmpl

@@ -101,8 +101,7 @@ versions:</a>
     <tr>
       <th align="right" valign="top">
         <a href="editproducts.cgi?action=editgroupcontrols&product=
-          [%- product.name FILTER url_quote %]&classification=
-          [%- classification.name FILTER url_quote %]">
+          [%- product.name FILTER url_quote %]">
           Edit Group Access Controls:
         </a>
       </th>
@@ -139,8 +138,6 @@ versions:</a>
         value="[% product.name FILTER html %]">
   <input type="hidden" name="action" value="update">
   <input type="hidden" name="token" value="[% token FILTER html %]">
-  <input type="hidden" name="classification"
-         value="[% classification.name FILTER html %]">
   <input type="submit" name="submit" value="Save Changes">
 </form>
   

template/en/default/admin/products/footer.html.tmpl

@@ -61,9 +61,7 @@
   Edit product <a 
   title="Edit Product '[% product.name FILTER html %]' 
          [%- classification_text %]"
-  href="editproducts.cgi?action=edit&amp;product=
-        [%- product.name FILTER url_quote %]
-        [%- classification_url_part %]">
+  href="editproducts.cgi?action=edit&amp;product=[% product.name FILTER url_quote %]">
         '[% product.name FILTER html %]'</a>.
 [% END %]
 

template/en/default/admin/products/list.html.tmpl

@@ -39,14 +39,13 @@
 
 [% edit_contentlink = BLOCK %]
   editproducts.cgi?action=edit&product=%%name%%
-  [%- classification_url_part %]
 [% END %]
 [% delete_contentlink = BLOCK %]
   editproducts.cgi?action=del&product=%%name%%
-  [%- classification_url_part %]
 [% END %]
-[% bug_count_contentlink = BLOCK %]buglist.cgi?product=%%name%%
-  [%- classification_url_part %][% END %]
+[% bug_count_contentlink = BLOCK %]
+  buglist.cgi?product=%%name%%
+[% END %]
 
 
 [% columns = [

template/en/default/admin/products/updated.html.tmpl

@@ -16,37 +16,17 @@
   # Rights Reserved.
   #
   # Contributor(s): Gavin Shelley <bugzilla@chimpychompy.org>
+  #                 Frédéric Buclin <LpSolit@gmail.com>
   #%]
 
 [%# INTERFACE:
-  #
-  # old_product : Bugzilla::Product Object; old product.
   # product : Bugzilla::Product Object; new product.
-  #
   # classification: Bugzilla::Classification Object; The product classification (may be empty or missing)
-  #
-  # checkvotes: boolean; is true if vote related fields have changed. If so, 
-  #             then the following parameters will be specified:
-  #
-  # toomanyvotes: list of hashes, each one with an 'id' and a 'name' hash key
-  #               detailing the bug id and the username of users who had too
-  #               many votes for a bug
-  #
-  # toomanytotalvotes: list of hashes, each one with an 'id' and a 'name' hash key
-  #                    detailing the bug id and the username of users who had
-  #                    too many total votes
-  #
-  # confirmedbugs: list of bug ids, which were confirmed by votes
-  #
-  # changer: string; login of the user making the changes, used for mailing
-  #          bug changes if necessary
-  #
+  # changes: hashref with all changes made to the product. Each key is an edited field,
+  #          and its value is an arrayref of the form [old values, new values].
   #%]
 
 [% IF classification %]
-  [% classification_url_part = BLOCK %]&amp;classification=
-     [%- classification.name FILTER url_quote %]
-  [% END %]
   [% classification_text = BLOCK %]
     of classification '[% classification.name FILTER html %]'
   [% END %]
@@ -58,28 +38,24 @@
   title = title
   style_urls = ['skins/standard/admin.css']
 %]
-[% updated = 0 %]
 
-[% IF product.name != old_product.name %]
+[% IF changes.name.defined %]
   <p>
-  Updated product name from '[% old_product.name FILTER html %]' to 
-  <a href="editproducts.cgi?action=edit&amp;product=
-  [%- product.name FILTER url_quote %]
-  [%- classification_url_part FILTER none %]">[% product.name FILTER html %]</a>.
+  Updated product name from '[% changes.name.0 FILTER html %]' to
+  '<a href="editproducts.cgi?action=edit&amp;product=
+  [%- product.name FILTER url_quote %]">[% product.name FILTER html %]</a>'.
   </p>
-  [% updated = 1 %]
 [% END %]
 
 
-[% IF product.description != old_product.description %]
+[% IF changes.description.defined %]
   <p>
     Updated description to:
   </p>
   <p style="margin: 1em 3em 1em 3em">[% product.description FILTER html_light %]</p>
-  [% updated = 1 %]
 [% END %]
 
-[% IF product.disallow_new != old_product.disallow_new %]
+[% IF changes.disallownew.defined %]
   <p>
   Product is now
   [% IF product.disallow_new %]
@@ -89,15 +65,14 @@
   [% END %]
   new [% terms.bugs %].
   </p>
-  [% updated = 1 %]
 [% END %]
 
-[% IF product.milestone_url != old_product.milestone_url %]
+[% IF changes.milestoneurl.defined %]
   <p>
   Updated milestone URL 
-  [% IF old_product.milestone_url != '' %]
-    from<br> <a href="[%- old_product.milestone_url FILTER html %]">
-    [%- old_product.milestone_url FILTER html %]</a>
+  [% IF changes.milestoneurl.0 != '' %]
+    from<br> <a href="[%- changes.milestoneurl.0 FILTER html %]">
+    [%- changes.milestoneurl.0 FILTER html %]</a>
   [% END %]
   to
   [% IF product.milestone_url != '' %]
@@ -107,45 +82,43 @@
     be empty.
   [% END %]
   </p>
-  [% updated = 1 %]
 [% END %]
 
-[% IF product.default_milestone != old_product.default_milestone %]
+[% IF changes.defaultmilestone.defined %]
   <p>
-  Updated default milestone from '[% old_product.default_milestone FILTER html %]' to
+  Updated default milestone from '[% changes.defaultmilestone.0 FILTER html %]' to
   '[% product.default_milestone FILTER html %]'.
   </p>
-  [% updated = 1 %]
 [% END %]
   
-[% IF product.votes_per_user != old_product.votes_per_user %]
+[% IF changes.votesperuser.defined %]
   <p>
   Updated votes per user from
-  [%+ old_product.votes_per_user FILTER html %] to 
+  [%+ changes.votesperuser.0 FILTER html %] to
   [%+ product.votes_per_user FILTER html %].
   </p>
-  [% updated = 1 %]
+  [% checkvotes = 1 %]
 [% END %]
 
-[% IF product.max_votes_per_bug != old_product.max_votes_per_bug %]
+[% IF changes.maxvotesperbug.defined %]
   <p>
   Updated maximum votes per [% terms.bug %] from 
-  [%+ old_product.max_votes_per_bug FILTER html %] to 
+  [%+ changes.maxvotesperbug.0 FILTER html %] to
   [%+ product.max_votes_per_bug FILTER html %].
   </p>
-  [% updated = 1 %]
+  [% checkvotes = 1 %]
 [% END %]
 
-[% IF product.votes_to_confirm != old_product.votes_to_confirm %]
+[% IF changes.votestoconfirm.defined %]
   <p>
   Updated number of votes needed to confirm a [% terms.bug %] from
-  [%+ old_product.votes_to_confirm FILTER html %] to 
+  [%+ changes.votestoconfirm.0 FILTER html %] to
   [%+ product.votes_to_confirm FILTER html %].
   </p>
-  [% updated = 1 %]
+  [% checkvotes = 1 %]
 [% END %]
 
-[% UNLESS updated %]
+[% IF !changes.keys.size %]
   <p>Nothing changed for product '[% product.name FILTER html %]'.</p>
 [% END %]
 
@@ -159,8 +132,8 @@
 
   <p>Checking existing votes in this product for anybody who now
   has too many votes for [% terms.abug %]...<br>
-  [% IF toomanyvotes.size > 0 %]
-    [% FOREACH detail = toomanyvotes %]
+  [% IF changes.too_many_votes.size %]
+    [% FOREACH detail = changes.too_many_votes %]
       &rarr;removed votes for [% terms.bug %] <a href="show_bug.cgi?id=
      [%- detail.id FILTER url_quote %]">
      [%- detail.id FILTER html %]</a> from [% detail.name FILTER html %]<br>
@@ -172,8 +145,8 @@
 
   <p>Checking existing votes in this product for anybody
   who now has too many total votes...<br>
-  [% IF toomanytotalvotes.size > 0 %]
-    [% FOREACH detail = toomanytotalvotes %]
+  [% IF changes.too_many_total_votes.size %]
+    [% FOREACH detail = changes.too_many_total_votes %]
       &rarr;removed votes for [% terms.bug %] <a href="show_bug.cgi?id=
      [%- detail.id FILTER url_quote %]">
      [%- detail.id FILTER html %]</a> from [% detail.name FILTER html %]<br>
@@ -185,14 +158,14 @@
 
   <p>Checking unconfirmed [% terms.bugs %] in this product for any which now have
   sufficient votes...<br>
-  [% IF confirmedbugs.size > 0 %]
-    [% FOREACH id = confirmedbugs %]
+  [% IF changes.confirmed_bugs.size %]
+    [% FOREACH id = changes.confirmed_bugs %]
 
       [%# This is INCLUDED instead of PROCESSED to avoid variables getting
           overwritten, which happens otherwise %]
       [% INCLUDE bug/process/results.html.tmpl
         type = 'votes'
-        mailrecipients = { 'changer' => changer }
+        mailrecipients = { 'changer' => user.login }
         header_done = 1
         id = id
       %]

template/en/default/filterexceptions.pl

@@ -474,10 +474,6 @@
   'classification_url_part', 
 ],
 
-'admin/products/confirm-delete.html.tmpl' => [
-  'classification_url_part', 
-],
-
 'admin/products/footer.html.tmpl' => [
   'classification_url_part', 
   'classification_text', 

template/en/default/global/messages.html.tmpl

@@ -129,6 +129,9 @@
   [% ELSIF message_tag == "bug_has_duplicate" %]
     *** [% terms.Bug %] [%+ dupe FILTER html %] has been marked as a duplicate of this [% terms.bug %]. ***
 
+  [% ELSIF message_tag == "bug_group_description" %]
+    Access to [% terms.bugs %] in the [% product.name FILTER html %] product
+
   [% ELSIF message_tag == "bug_moved_to" %]
     <p>[% terms.Bug %] moved to [% Param("move-to-url") FILTER html %].</p>
     <p>If the move succeeded, [% login FILTER html %] will receive a mail
@@ -706,6 +709,9 @@
   [% ELSIF message_tag == "series_all_closed" %]
     All Closed
 
+  [% ELSIF message_tag == "series_subcategory" %]
+    -All-
+
   [% ELSIF message_tag == "sudo_started" %]
     [% title = "Sudo session started" %]
       The sudo session has been started.  For the next 6 hours, or until you 

template/en/default/global/user-error.html.tmpl

@@ -273,11 +273,6 @@
     [% title = "Classification Does Not Exist" %]
     The classification '[% name FILTER html %]' does not exist.
 
-  [% ELSIF error == "classification_doesnt_exist_for_product" %]
-    [% title = "Classification Does Not Exist For Product" %]
-    The classification '[% classification FILTER html %]' does not exist
-    for product '[% product FILTER html %]'.
-
   [% ELSIF error == "classification_invalid_sortkey" %]
     [% title = "Invalid Sortkey for Classification" %]
     The sortkey <em>[% sortkey FILTER html %]</em> for the '[% name FILTER html %]'
@@ -313,8 +308,8 @@
 
   [% ELSIF error == "component_name_too_long" %]
     [% title = "Component Name Is Too Long" %]
-    The name of a component is limited to 64 characters. 
-    '[% name FILTER html %]' is too long ([% name.length %] characters).
+    The name of a component is limited to [% constants.MAX_COMPONENT_SIZE FILTER html %]
+    characters. '[% name FILTER html %]' is too long ([% name.length %] characters).
 
   [% ELSIF error == "component_need_initialowner" %]
     [% title = "Component Requires Default Assignee" %]
@@ -952,8 +947,8 @@
 
   [% ELSIF error == "milestone_name_too_long" %]
     [% title = "Milestone Name Is Too Long" %]
-    The name of a milestone is limited to 20 characters. 
-    '[% name FILTER html %]' is too long ([% name.length %] characters).
+    The name of a milestone is limited to [% constants.MAX_MILESTONE_SIZE FILTER html %]
+    characters. '[% name FILTER html %]' is too long ([% name.length %] characters).
 
   [% ELSIF error == "milestone_required" %]
     [% title = "Milestone Required" %]
@@ -1241,37 +1236,19 @@
     [% title = "Specified Product Does Not Exist" %]
     The product '[% product FILTER html %]' does not exist.
 
-  [% ELSIF error == "product_votes_per_bug_must_be_nonnegative" %]
-    [% title = "Maximum Votes Must Be Non-negative" %]
-    [% admindocslinks = {'voting.html' => 'Setting up the voting feature'} %]
-    '[% maxvotesperbug FILTER html %]' is an invalid value for the 
-    <em>'Maximum Votes Per [% terms.Bug %]'</em> field, which should
-    contain a non-negative number.
-
-  [% ELSIF error == "product_votes_per_user_must_be_nonnegative" %]
-    [% title = "Votes Per User Must Be Non-negative" %]
-    [% admindocslinks = {'voting.html' => 'Setting up the voting feature'} %]
-    '[% votesperuser FILTER html %]' is an invalid value for the 
-    <em>'Votes Per User'</em> field, which should contain a
-    non-negative number.
-
-  [% ELSIF error == "product_votes_to_confirm_must_be_nonnegative" %]
-    [% title = "Votes To Confirm Must Be Non-negative" %]
+  [% ELSIF error == "product_illegal_votes" %]
+    [% title = "Votes Must Be Non-negative" %]
     [% admindocslinks = {'voting.html' => 'Setting up the voting feature'} %]
-    '[% votestoconfirm FILTER html %]' is an invalid value for the 
-    <em>'Votes To Confirm'</em> field, which should contain a
-    non-negative number.
-
-  [% ELSIF error == "product_cant_delete_description" %]
-    [% title = "Cannot delete product description" %]
-    [% admindocslinks = {'products.html' => 'Administering products'} %]
-    Cannot delete the description for product
-    '[% product FILTER html %]'.
-
-  [% ELSIF error == "product_cant_delete_name" %]
-    [% title = "Cannot delete product name" %]
-    [% admindocslinks = {'products.html' => 'Administering products'} %]
-    Cannot delete the product name for product '[% product FILTER html %]'.
+    '[% votes FILTER html %]' is an invalid value for the
+    <em>
+    [% IF field == "votesperuser" %]
+      Votes Per User
+    [% ELSIF field == "maxvotesperbug" %]
+      Maximum Votes Per [% terms.Bug %]
+    [% ELSIF field == "votestoconfirm" %]
+      Votes To Confirm
+    [% END %]
+    </em> field, which should contain a non-negative number.
 
   [% ELSIF error == "product_name_already_in_use" %]
     [% title = "Product name already in use" %]
@@ -1284,19 +1261,17 @@
     The product name '[% product FILTER html %]' differs from existing 
     product '[% existing_product FILTER html %]' only in case.
 
+  [% ELSIF error == "product_name_too_long" %]
+    [% title = "Product name too long" %]
+    The name of a product is limited to [% constants.MAX_PRODUCT_SIZE FILTER html %]
+    characters. '[% name FILTER html %]' is too long ([% name.length %] characters).
+
   [% ELSIF error == "product_must_define_defaultmilestone" %]
     [% title = "Must define new default milestone" %]
     [% admindocslinks = {'products.html' => 'Administering products',
                          'milestones.html' => 'About Milestones'} %]
-    [% IF classification %]
-      [% classification_url_part = BLOCK %]&amp;classification=
-        [%- classification FILTER url_quote %]
-      [% END %]
-    [% END %]
-    You must <a href="editmilestones.cgi?action=add&amp;product=
-    [%- product FILTER url_quote %]
-    [%- classification_url_part FILTER none %]">
-    create the milestone '[% defaultmilestone FILTER html %]'</a> before
+    You must <a href="editmilestones.cgi?action=add&amp;product=[% product FILTER url_quote %]">
+    create the milestone '[% milestone FILTER html %]'</a> before
     it can be made the default milestone for product '[% product FILTER html %]'.
 
   [% ELSIF error == "product_admin_denied" %]
@@ -1306,7 +1281,7 @@
   [% ELSIF error == "product_blank_name" %]
     [% title = "Blank Product Name Not Allowed" %]
     [% admindocslinks = {'products.html' => 'Administering products'} %]
-    You must enter a name for the new product.
+    You must enter a name for the product.
   
   [% ELSIF error == "product_disabled" %]
     [% title = BLOCK %]Product closed for [% terms.Bug %] Entry[% END %]
@@ -1331,13 +1306,13 @@
   [% ELSIF error == "product_must_have_description" %]
     [% title = "Product needs Description" %]
     [% admindocslinks = {'products.html' => 'Administering products'} %]
-    You must enter a description for product '[% product FILTER html %]'.
+    You must enter a description for this product.
 
   [% ELSIF error == "product_must_have_version" %]
     [% title = "Product needs Version" %]
     [% admindocslinks = {'products.html' => 'Administering products',
                          'versions.html' => 'Administering versions'} %]
-    You must enter a version for product '[% product FILTER html %]'.
+    You must enter a valid version to create a new product.
 
   [% ELSIF error == "product_not_specified" %]
     [% title = "No Product Specified" %]
@@ -1363,7 +1338,8 @@
 
   [% ELSIF error == "query_name_too_long" %]
     [% title = "Query Name Too Long" %]
-    The name of the query must be less than 64 characters long.
+    The name of the query must be less than [% constants.MAX_LEN_QUERY_NAME FILTER html %]
+    characters long.
 
   [% ELSIF error == "quicksearch_unknown_field" %]
     [% title = "Unknown QuickSearch Field" %]