Bug 313125: Implement validations and database persistence functions for Versions.pm and clean-up editversions.cgi - Patch by André Batosti <batosti@async.com.br> r=LpSolit a=myk

3 files changed, 125 insertions, 91 deletions

diff --git a/Bugzilla/Version.pm b/Bugzilla/Version.pm
index 9492e8135..4d47ecdeb 100644
--- a/Bugzilla/Version.pm
+++ b/Bugzilla/Version.pm
@@ -85,6 +85,53 @@ sub bug_count {
     return $self->{'bug_count'};
 }
 
+sub remove_from_db {
+    my $self = shift;
+    my $dbh = Bugzilla->dbh;
+
+    # The version cannot be removed if there are bugs
+    # associated with it.
+    if ($self->bug_count) {
+        ThrowUserError("version_has_bugs", { nb => $self->bug_count });
+    }
+
+    $dbh->do(q{DELETE FROM versions WHERE product_id = ? AND value = ?},
+              undef, ($self->product_id, $self->name));
+}
+
+sub update {
+    my $self = shift;
+    my ($name, $product) = @_;
+    my $dbh = Bugzilla->dbh;
+
+    $name || ThrowUserError('version_not_specified');
+
+    # Remove unprintable characters
+    $name = clean_text($name);
+
+    return 0 if ($name eq $self->name);
+    my $version = new Bugzilla::Version($self->product_id, $name);
+
+    if ($version) {
+        ThrowUserError('version_already_exists',
+                       {'name' => $version->name,
+                        'product' => $product->name});
+    }
+
+    trick_taint($name);
+    $dbh->do("UPDATE bugs SET version = ?
+              WHERE version = ? AND product_id = ?", undef,
+              ($name, $self->name, $self->product_id));
+
+    $dbh->do("UPDATE versions SET value = ?
+              WHERE product_id = ? AND value = ?", undef,
+              ($name, $self->product_id, $self->name));
+
+    $self->{'value'} = $name;
+
+    return 1;
+}
+
 ###############################
 #####     Accessors        ####
 ###############################
@@ -109,6 +156,32 @@ sub check_version {
     return $version;
 }
 
+sub create {
+    my ($name, $product) = @_;
+    my $dbh = Bugzilla->dbh;
+
+    # Cleanups and validity checks
+    $name || ThrowUserError('version_blank_name');
+
+    # Remove unprintable characters
+    $name = clean_text($name);
+
+    my $version = new Bugzilla::Version($product->id, $name);
+    if ($version) {
+        ThrowUserError('version_already_exists',
+                       {'name' => $version->name,
+                        'product' => $product->name});
+    }
+
+    # Add the new version
+    trick_taint($name);
+    $dbh->do(q{INSERT INTO versions (value, product_id)
+               VALUES (?, ?)}, undef, ($name, $product->id));
+
+    $version = new Bugzilla::Version($product->id, $name);
+    return $version;
+}
+
 1;
 
 __END__
@@ -126,11 +199,17 @@ Bugzilla::Version - Bugzilla product version class.
     my $product_id = $version->product_id;
     my $value = $version->value;
 
+    $version->remove_from_db;
+
+    my $updated = $version->update($version_name, $product);
+
     my $version = $hash_ref->{'version_value'};
 
     my $version = Bugzilla::Version::check_version($product_obj,
                                                    'acme_version');
 
+    my $version = Bugzilla::Version::create($version_name, $product);
+
 =head1 DESCRIPTION
 
 Version.pm represents a Product Version object.
@@ -157,6 +236,23 @@ Version.pm represents a Product Version object.
 
  Returns:     Integer with the number of bugs.
 
+=item C<remove_from_db()>
+
+ Description: Removes the version from the database.
+
+ Params:      none.
+
+ Retruns:     none.
+
+=item C<update($name, $product)>
+
+ Description: Update the value of the version.
+
+ Params:      $name - String with the new version value.
+              $product - Bugzilla::Product object the version belongs to.
+
+ Returns:     An integer - 1 if the version has been updated, else 0.
+
 =back
 
 =head1 SUBROUTINES
@@ -172,6 +268,15 @@ Version.pm represents a Product Version object.
 
  Returns:     Bugzilla::Version object.
 
+=item C<create($version_name, $product)>
+
+ Description: Create a new version for the given product.
+
+ Params:      $version_name - String with a version value.
+              $product - A Bugzilla::Product object.
+
+ Returns:     A Bugzilla::Version object.
+
 =back
 
 =cut
diff --git a/editversions.cgi b/editversions.cgi
index e8f7f4c35..0941896a5 100755
--- a/editversions.cgi
+++ b/editversions.cgi
@@ -92,8 +92,7 @@ $user->can_see_product($product->name)
 unless ($action) {
     $vars->{'showbugcounts'} = $showbugcounts;
     $vars->{'product'} = $product;
-    $template->process("admin/versions/list.html.tmpl",
-                       $vars)
+    $template->process("admin/versions/list.html.tmpl", $vars)
       || ThrowTemplateError($template->error());
 
     exit;
@@ -111,8 +110,7 @@ unless ($action) {
 if ($action eq 'add') {
 
     $vars->{'product'} = $product;
-    $template->process("admin/versions/create.html.tmpl",
-                       $vars)
+    $template->process("admin/versions/create.html.tmpl", $vars)
       || ThrowTemplateError($template->error());
 
     exit;
@@ -126,29 +124,11 @@ if ($action eq 'add') {
 
 if ($action eq 'new') {
 
-    # Cleanups and validity checks
-    $version_name || ThrowUserError('version_blank_name');
+    my $version = Bugzilla::Version::create($version_name, $product);
 
-    # Remove unprintable characters
-    $version_name = clean_text($version_name);
-
-    my $version = new Bugzilla::Version($product->id, $version_name);
-    if ($version) {
-        ThrowUserError('version_already_exists',
-                       {'name' => $version->name,
-                        'product' => $product->name});
-    }
-
-    # Add the new version
-    trick_taint($version_name);
-    $dbh->do("INSERT INTO versions (value, product_id)
-              VALUES (?, ?)", undef, ($version_name, $product->id));
-
-    $version = new Bugzilla::Version($product->id, $version_name);
     $vars->{'version'} = $version;
     $vars->{'product'} = $product;
-    $template->process("admin/versions/created.html.tmpl",
-                       $vars)
+    $template->process("admin/versions/created.html.tmpl", $vars)
       || ThrowTemplateError($template->error());
 
     exit;
@@ -165,13 +145,11 @@ if ($action eq 'new') {
 
 if ($action eq 'del') {
 
-    my $version = Bugzilla::Version::check_version($product,
-                                                   $version_name);
+    my $version = Bugzilla::Version::check_version($product, $version_name);
 
     $vars->{'version'} = $version;
     $vars->{'product'} = $product;
-    $template->process("admin/versions/confirm-delete.html.tmpl",
-                       $vars)
+    $template->process("admin/versions/confirm-delete.html.tmpl", $vars)
       || ThrowTemplateError($template->error());
 
     exit;
@@ -185,24 +163,15 @@ if ($action eq 'del') {
 
 if ($action eq 'delete') {
 
-    my $version = Bugzilla::Version::check_version($product,
-                                                   $version_name);
-
-    # The version cannot be removed if there are bugs
-    # associated with it.
-    if ($version->bug_count) {
-        ThrowUserError("version_has_bugs",
-                       { nb => $version->bug_count });
-    }
-
-    $dbh->do("DELETE FROM versions WHERE product_id = ? AND value = ?",
-              undef, ($product->id, $version->name));
+    my $version = Bugzilla::Version::check_version($product, $version_name);
+    $version->remove_from_db;
 
     $vars->{'version'} = $version;
     $vars->{'product'} = $product;
 
     $template->process("admin/versions/deleted.html.tmpl", $vars)
       || ThrowTemplateError($template->error());
+
     exit;
 }
 
@@ -216,14 +185,12 @@ if ($action eq 'delete') {
 
 if ($action eq 'edit') {
 
-    my $version = Bugzilla::Version::check_version($product,
-                                                   $version_name);
+    my $version = Bugzilla::Version::check_version($product, $version_name);
 
     $vars->{'version'} = $version;
     $vars->{'product'} = $product;
 
-    $template->process("admin/versions/edit.html.tmpl",
-                       $vars)
+    $template->process("admin/versions/edit.html.tmpl", $vars)
       || ThrowTemplateError($template->error());
 
     exit;
@@ -237,55 +204,19 @@ if ($action eq 'edit') {
 
 if ($action eq 'update') {
 
-    $version_name || ThrowUserError('version_not_specified');
-
-    # Remove unprintable characters
-    $version_name = clean_text($version_name);
-
     my $version_old_name = trim($cgi->param('versionold') || '');
-    my $version_old =
-        Bugzilla::Version::check_version($product,
-                                         $version_old_name);
-
-    # Note that the order of this tests is important. If you change
-    # them, be sure to test for WHERE='$version' or WHERE='$versionold'
+    my $version =
+        Bugzilla::Version::check_version($product, $version_old_name);
 
     $dbh->bz_lock_tables('bugs WRITE', 'versions WRITE');
 
-    if ($version_name ne $version_old->name) {
-        
-        my $version = new Bugzilla::Version($product->id,
-                                            $version_name);
-
-        if ($version) {
-            ThrowUserError('version_already_exists',
-                           {'name' => $version->name,
-                            'product' => $product->name});
-        }
-        
-        trick_taint($version_name);
-        $dbh->do("UPDATE bugs
-                  SET version = ?
-                  WHERE version = ? AND product_id = ?", undef,
-                  ($version_name, $version_old->name, $product->id));
-        
-        $dbh->do("UPDATE versions
-                  SET value = ?
-                  WHERE product_id = ? AND value = ?", undef,
-                  ($version_name, $product->id, $version_old->name));
-
-        $vars->{'updated_name'} = 1;
-    }
-
-    $dbh->bz_unlock_tables(); 
+    $vars->{'updated'} = $version->update($version_name, $product);
+
+    $dbh->bz_unlock_tables();
 
-    my $version =
-        Bugzilla::Version::check_version($product,
-                                         $version_name);
     $vars->{'version'} = $version;
     $vars->{'product'} = $product;
-    $template->process("admin/versions/updated.html.tmpl",
-                       $vars)
+    $template->process("admin/versions/updated.html.tmpl", $vars)
       || ThrowTemplateError($template->error());
 
     exit;
diff --git a/template/en/default/admin/versions/updated.html.tmpl b/template/en/default/admin/versions/updated.html.tmpl
index 5bae6bcf6..8f679c4aa 100644
--- a/template/en/default/admin/versions/updated.html.tmpl
+++ b/template/en/default/admin/versions/updated.html.tmpl
@@ -25,7 +25,7 @@
   # version: object; Bugzilla::Version object representing the
   #                    version the user updated.
   #
-  # updated_name: boolean; defined if the 'name' field was updated
+  # updated: boolean; defined if the 'name' field was updated
   #%]
   
 [% title = BLOCK %]Updating Version '[% version.name FILTER html %]' of Product
@@ -34,11 +34,9 @@
   title = title
 %]
 
-[% IF updated_name %]
+[% IF updated %]
   <p>Updated Version name to: '[% version.name FILTER html %]'.</p>
-[% END %]
-
-[% UNLESS updated_name %]
+[% ELSE %]
   <p>Nothing changed for version '[% version.name FILTER html %]'.
 [% END %]