From 461d51d5078bd9971593296dbc35d3e37e48df9b Mon Sep 17 00:00:00 2001 From: "lpsolit%gmail.com" <> Date: Fri, 10 Apr 2009 09:36:43 +0000 Subject: Bug 471871: Bugzilla::Version has duplicated code compared to Bugzilla::Object (make Bugzilla::Version really a subclass of Bugzilla::Object) - Patch by Frédéric Buclin r/a=mkanat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Bugzilla/Install.pm | 3 +- Bugzilla/Product.pm | 2 +- Bugzilla/Version.pm | 199 +++++++++++------------- editversions.cgi | 7 +- template/en/default/global/messages.html.tmpl | 8 +- template/en/default/global/user-error.html.tmpl | 4 - 6 files changed, 105 insertions(+), 118 deletions(-) diff --git a/Bugzilla/Install.pm b/Bugzilla/Install.pm index 0a1f0e955..8365f4182 100644 --- a/Bugzilla/Install.pm +++ b/Bugzilla/Install.pm @@ -240,7 +240,8 @@ sub create_default_product { my $product = new Bugzilla::Product({name => $default_prod->{name}}); # The default version. - Bugzilla::Version::create(Bugzilla::Version::DEFAULT_VERSION, $product); + Bugzilla::Version->create({name => Bugzilla::Version::DEFAULT_VERSION, + product => $product}); # And we automatically insert the default milestone. $dbh->do(q{INSERT INTO milestones (product_id, value, sortkey) diff --git a/Bugzilla/Product.pm b/Bugzilla/Product.pm index 03ebe2639..488624c43 100644 --- a/Bugzilla/Product.pm +++ b/Bugzilla/Product.pm @@ -112,7 +112,7 @@ sub create { 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::Version->create({name => $version, product => $product}); Bugzilla::Milestone->create({name => $params->{defaultmilestone}, product => $product}); # Create groups and series for the new product, if requested. diff --git a/Bugzilla/Version.pm b/Bugzilla/Version.pm index a2ef6b01e..1c96003f1 100644 --- a/Bugzilla/Version.pm +++ b/Bugzilla/Version.pm @@ -14,6 +14,7 @@ # # Contributor(s): Tiago R. Mello # Max Kanat-Alexander +# Frédéric Buclin use strict; @@ -32,6 +33,10 @@ use Bugzilla::Error; use constant DEFAULT_VERSION => 'unspecified'; use constant DB_TABLE => 'versions'; +use constant NAME_FIELD => 'value'; +# This is "id" because it has to be filled in and id is probably the fastest. +# We do a custom sort in new_from_list below. +use constant LIST_ORDER => 'id'; use constant DB_COLUMNS => qw( id @@ -39,10 +44,26 @@ use constant DB_COLUMNS => qw( product_id ); -use constant NAME_FIELD => 'value'; -# This is "id" because it has to be filled in and id is probably the fastest. -# We do a custom sort in new_from_list below. -use constant LIST_ORDER => 'id'; +use constant REQUIRED_CREATE_FIELDS => qw( + name + product +); + +use constant UPDATE_COLUMNS => qw( + value +); + +use constant VALIDATORS => { + product => \&_check_product, +}; + +use constant UPDATE_VALIDATORS => { + value => \&_check_value, +}; + +################################ +# Methods +################################ sub new { my $class = shift; @@ -79,6 +100,18 @@ sub new_from_list { return [sort { vers_cmp(lc($a->name), lc($b->name)) } @$list]; } +sub run_create_validators { + my $class = shift; + my $params = $class->SUPER::run_create_validators(@_); + + my $product = delete $params->{product}; + $params->{product_id} = $product->id; + $params->{value} = $class->_check_value($params->{name}, $product); + delete $params->{name}; + + return $params; +} + sub bug_count { my $self = shift; my $dbh = Bugzilla->dbh; @@ -92,87 +125,71 @@ sub bug_count { return $self->{'bug_count'}; } -sub remove_from_db { +sub update { my $self = shift; - my $dbh = Bugzilla->dbh; + my ($changes, $old_self) = $self->SUPER::update(@_); - # The version cannot be removed if there are bugs - # associated with it. - if ($self->bug_count) { - ThrowUserError("version_has_bugs", { nb => $self->bug_count }); + if (exists $changes->{value}) { + my $dbh = Bugzilla->dbh; + $dbh->do('UPDATE bugs SET version = ? + WHERE version = ? AND product_id = ?', + undef, ($self->name, $old_self->name, $self->product_id)); } - - $dbh->do(q{DELETE FROM versions WHERE product_id = ? AND value = ?}, - undef, ($self->product_id, $self->name)); + return $changes; } -sub update { +sub remove_from_db { 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({ product => $product, name => $name }); - - if ($version) { - ThrowUserError('version_already_exists', - {'name' => $version->name, - 'product' => $product->name}); + # The version cannot be removed if there are bugs + # associated with it. + if ($self->bug_count) { + ThrowUserError("version_has_bugs", { nb => $self->bug_count }); } - - 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; + $self->SUPER::remove_from_db(); } ############################### ##### Accessors #### ############################### -sub name { return $_[0]->{'value'}; } sub product_id { return $_[0]->{'product_id'}; } -############################### -##### Subroutines ### -############################### +sub product { + my $self = shift; -sub create { - my ($name, $product) = @_; - my $dbh = Bugzilla->dbh; + require Bugzilla::Product; + $self->{'product'} ||= new Bugzilla::Product($self->product_id); + return $self->{'product'}; +} - # Cleanups and validity checks - $name || ThrowUserError('version_blank_name'); +################################ +# Validators +################################ +sub set_name { $_[0]->set('value', $_[1]); } + +sub _check_value { + my ($invocant, $name, $product) = @_; + + $name = trim($name); + $name || ThrowUserError('version_blank_name'); # Remove unprintable characters $name = clean_text($name); + $product = $invocant->product if (ref $invocant); my $version = new Bugzilla::Version({ product => $product, name => $name }); - if ($version) { - ThrowUserError('version_already_exists', - {'name' => $version->name, - 'product' => $product->name}); + if ($version && (!ref $invocant || $version->id != $invocant->id)) { + ThrowUserError('version_already_exists', { name => $version->name, + product => $product->name }); } + return $name; +} - # Add the new version - trick_taint($name); - $dbh->do(q{INSERT INTO versions (value, product_id) - VALUES (?, ?)}, undef, ($name, $product->id)); - - return new Bugzilla::Version($dbh->bz_last_key('versions', 'id')); +sub _check_product { + my ($invocant, $product) = @_; + return Bugzilla->user->check_can_admin_product($product->name); } 1; @@ -187,37 +204,33 @@ Bugzilla::Version - Bugzilla product version class. use Bugzilla::Version; - my $version = new Bugzilla::Version(1, 'version_value'); + my $version = new Bugzilla::Version({ name => $name, product => $product }); + my $value = $version->name; my $product_id = $version->product_id; - my $value = $version->value; + my $product = $version->product; - $version->remove_from_db; - - my $updated = $version->update($version_name, $product); + my $version = Bugzilla::Version->create( + { name => $name, product => $product }); - my $version = $hash_ref->{'version_value'}; + $version->set_name($new_name); + $version->update(); - my $version = Bugzilla::Version::create($version_name, $product); + $version->remove_from_db; =head1 DESCRIPTION -Version.pm represents a Product Version object. +Version.pm represents a Product Version object. It is an implementation +of L, and thus provides all methods that +L provides. + +The methods that are specific to C are listed +below. =head1 METHODS =over -=item C - - Description: The constructor is used to load an existing version - by passing a product id and a version value. - - Params: $product_id - Integer with a product id. - $value - String with a version value. - - Returns: A Bugzilla::Version object. - =item C Description: Returns the total of bugs that belong to the version. @@ -226,38 +239,6 @@ Version.pm represents a Product Version object. Returns: Integer with the number of bugs. -=item C - - Description: Removes the version from the database. - - Params: none. - - Retruns: none. - -=item C - - 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 - -=over - -=item C - - 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 85f4f8ca4..7e6b9247d 100755 --- a/editversions.cgi +++ b/editversions.cgi @@ -119,7 +119,8 @@ if ($action eq 'add') { if ($action eq 'new') { check_token_data($token, 'add_version'); - my $version = Bugzilla::Version::create($version_name, $product); + my $version = Bugzilla::Version->create( + {name => $version_name, product => $product}); delete_token($token); $vars->{'message'} = 'version_created'; @@ -202,7 +203,8 @@ if ($action eq 'update') { $dbh->bz_start_transaction(); - $vars->{'updated'} = $version->update($version_name, $product); + $version->set_name($version_name); + my $changes = $version->update(); $dbh->bz_commit_transaction(); delete_token($token); @@ -210,6 +212,7 @@ if ($action eq 'update') { $vars->{'message'} = 'version_updated'; $vars->{'version'} = $version; $vars->{'product'} = $product; + $vars->{'changes'} = $changes; $template->process("admin/versions/list.html.tmpl", $vars) || ThrowTemplateError($template->error()); diff --git a/template/en/default/global/messages.html.tmpl b/template/en/default/global/messages.html.tmpl index c8e4dd225..d2915780c 100644 --- a/template/en/default/global/messages.html.tmpl +++ b/template/en/default/global/messages.html.tmpl @@ -796,7 +796,13 @@ [% ELSIF message_tag == "version_updated" %] [% title = "Version Updated" %] - Version renamed as [% version.name FILTER html %]. + [% IF changes.size %] + [% IF changes.value.defined %] + Version renamed to [% version.name FILTER html %]. + [% END %] + [% ELSE %] + No changes made to version [% version.name FILTER html %]. + [% END %] [% ELSIF message_tag == "workflow_updated" %] The workflow has been updated. diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index 879e62ade..d1c3bd643 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -1579,10 +1579,6 @@ version! You must reassign those [% terms.bugs %] to another version before you can delete this one. - [% ELSIF error == "version_not_specified" %] - [% title = "No Version Specified" %] - No version specified when trying to edit versions. - [% ELSIF error == "users_deletion_disabled" %] [% title = "Deletion not activated" %] [% admindocslinks = {'useradmin.html' => 'User administration'} %] -- cgit v1.2.3-24-g4f1b