Bug 313123: Implement $component->create and $component->update based on Object.pm - Patch by Frédéric Buclin <LpSolit@gmail.com> r/a=mkanat

5 files changed, 386 insertions, 59 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index e318537a5..cc492ad41 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -930,7 +930,7 @@ sub _check_component {
     $name = trim($name);
     $name || ThrowUserError("require_component");
     ($product = $invocant->product_obj) if ref $invocant;
-    my $obj = Bugzilla::Component::check_component($product, $name);
+    my $obj = Bugzilla::Component->check({ product => $product, name => $name });
     return $obj;
 }
 
diff --git a/Bugzilla/Component.pm b/Bugzilla/Component.pm
index a615e7ae5..f5719e82c 100644
--- a/Bugzilla/Component.pm
+++ b/Bugzilla/Component.pm
@@ -23,10 +23,12 @@ package Bugzilla::Component;
 
 use base qw(Bugzilla::Object);
 
+use Bugzilla::Constants;
 use Bugzilla::Util;
 use Bugzilla::Error;
 use Bugzilla::User;
 use Bugzilla::FlagType;
+use Bugzilla::Series;
 
 ###############################
 ####    Initialization     ####
@@ -43,8 +45,32 @@ use constant DB_COLUMNS => qw(
     description
 );
 
-###############################
-####       Methods         ####
+use constant REQUIRED_CREATE_FIELDS => qw(
+    name
+    product
+    initialowner
+    description
+);
+
+use constant UPDATE_COLUMNS => qw(
+    name
+    initialowner
+    initialqacontact
+    description
+);
+
+use constant VALIDATORS => {
+    product          => \&_check_product,
+    initialowner     => \&_check_initialowner,
+    initialqacontact => \&_check_initialqacontact,
+    description      => \&_check_description,
+    initial_cc       => \&_check_cc_list,
+};
+
+use constant UPDATE_VALIDATORS => {
+    name => \&_check_name,
+};
+
 ###############################
 
 sub new {
@@ -79,6 +105,225 @@ sub new {
     return $component;
 }
 
+sub create {
+    my $class = shift;
+    my $dbh = Bugzilla->dbh;
+
+    $dbh->bz_start_transaction();
+
+    $class->check_required_create_fields(@_);
+    my $params = $class->run_create_validators(@_);
+    my $cc_list = delete $params->{initial_cc};
+
+    my $component = $class->insert_create_data($params);
+
+    # We still have to fill the component_cc table.
+    $component->_update_cc_list($cc_list);
+
+    # Create series for the new component.
+    $component->_create_series();
+
+    $dbh->bz_commit_transaction();
+    return $component;
+}
+
+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->{name} = $class->_check_name($params->{name}, $product);
+
+    return $params;
+}
+
+sub update {
+    my $self = shift;
+    my $changes = $self->SUPER::update(@_);
+
+    # Update the component_cc table if necessary.
+    if (defined $self->{cc_ids}) {
+        my $diff = $self->_update_cc_list($self->{cc_ids});
+        $changes->{cc_list} = $diff if defined $diff;
+    }
+    return $changes;
+}
+
+sub remove_from_db {
+    my $self = shift;
+    my $dbh = Bugzilla->dbh;
+
+    $dbh->bz_start_transaction();
+
+    if ($self->bug_count) {
+        if (Bugzilla->params->{'allowbugdeletion'}) {
+            require Bugzilla::Bug;
+            foreach my $bug_id (@{$self->bug_ids}) {
+                # Note: We allow admins to delete bugs even if they can't
+                # see them, as long as they can see the product.
+                my $bug = new Bugzilla::Bug($bug_id);
+                $bug->remove_from_db();
+            }
+        } else {
+            ThrowUserError('component_has_bugs', {nb => $self->bug_count});
+        }
+    }
+
+    $dbh->do('DELETE FROM flaginclusions WHERE component_id = ?',
+             undef, $self->id);
+    $dbh->do('DELETE FROM flagexclusions WHERE component_id = ?',
+             undef, $self->id);
+    $dbh->do('DELETE FROM component_cc WHERE component_id = ?',
+             undef, $self->id);
+    $dbh->do('DELETE FROM components WHERE id = ?', undef, $self->id);
+
+    $dbh->bz_commit_transaction();
+}
+
+################################
+# Validators
+################################
+
+sub _check_name {
+    my ($invocant, $name, $product) = @_;
+
+    $name = trim($name);
+    $name || ThrowUserError('component_blank_name');
+
+    if (length($name) > MAX_COMPONENT_SIZE) {
+        ThrowUserError('component_name_too_long', {'name' => $name});
+    }
+
+    $product = $invocant->product if (ref $invocant);
+    my $component = new Bugzilla::Component({product => $product, name => $name});
+    if ($component && (!ref $invocant || $component->id != $invocant->id)) {
+        ThrowUserError('component_already_exists', { name    => $component->name,
+                                                     product => $product });
+    }
+    return $name;
+}
+
+sub _check_description {
+    my ($invocant, $description) = @_;
+
+    $description = trim($description);
+    $description || ThrowUserError('component_blank_description');
+    return $description;
+}
+
+sub _check_initialowner {
+    my ($invocant, $owner) = @_;
+
+    $owner || ThrowUserError('component_need_initialowner');
+    my $owner_id = Bugzilla::User->check($owner)->id;
+    return $owner_id;
+}
+
+sub _check_initialqacontact {
+    my ($invocant, $qa_contact) = @_;
+
+    my $qa_contact_id;
+    if (Bugzilla->params->{'useqacontact'}) {
+        $qa_contact_id = Bugzilla::User->check($qa_contact)->id if $qa_contact;
+    }
+    elsif (ref $invocant) {
+        $qa_contact_id = $invocant->{initialqacontact};
+    }
+    return $qa_contact_id;
+}
+
+sub _check_product {
+    my ($invocant, $product) = @_;
+    return Bugzilla->user->check_can_admin_product($product->name);
+}
+
+sub _check_cc_list {
+    my ($invocant, $cc_list) = @_;
+
+    my %cc_ids;
+    foreach my $cc (@$cc_list) {
+        my $id = login_to_id($cc, THROW_ERROR);
+        $cc_ids{$id} = 1;
+    }
+    return [keys %cc_ids];
+}
+
+###############################
+####       Methods         ####
+###############################
+
+sub _update_cc_list {
+    my ($self, $cc_list) = @_;
+    my $dbh = Bugzilla->dbh;
+
+    my $old_cc_list =
+      $dbh->selectcol_arrayref('SELECT user_id FROM component_cc
+                                WHERE component_id = ?', undef, $self->id);
+
+    my ($removed, $added) = diff_arrays($old_cc_list, $cc_list);
+    my $diff;
+    if (scalar @$removed || scalar @$added) {
+        $diff = [join(', ', @$removed), join(', ', @$added)];
+    }
+
+    $dbh->do('DELETE FROM component_cc WHERE component_id = ?', undef, $self->id);
+
+    my $sth = $dbh->prepare('INSERT INTO component_cc
+                             (user_id, component_id) VALUES (?, ?)');
+    $sth->execute($_, $self->id) foreach (@$cc_list);
+
+    return $diff;
+}
+
+sub _create_series {
+    my $self = shift;
+
+    # Insert default charting queries for this product.
+    # If they aren't using charting, this won't do any harm.
+    my $prodcomp = "&product="   . url_quote($self->product->name) .
+                   "&component=" . url_quote($self->name);
+
+    my $open_query = 'field0-0-0=resolution&type0-0-0=notregexp&value0-0-0=.' .
+                     $prodcomp;
+    my $nonopen_query = 'field0-0-0=resolution&type0-0-0=regexp&value0-0-0=.' .
+                        $prodcomp;
+
+    my @series = ([get_text('series_all_open'), $open_query],
+                  [get_text('series_all_closed'), $nonopen_query]);
+
+    foreach my $sdata (@series) {
+        my $series = new Bugzilla::Series(undef, $self->product->name,
+                                          $self->name, $sdata->[0],
+                                          Bugzilla->user->id, 1, $sdata->[1], 1);
+        $series->writeToDatabase();
+    }
+}
+
+sub set_name { $_[0]->set('name', $_[1]); }
+sub set_description { $_[0]->set('description', $_[1]); }
+sub set_default_assignee {
+    my ($self, $owner) = @_;
+
+    $self->set('initialowner', $owner);
+    # Reset the default owner object.
+    delete $self->{default_assignee};
+}
+sub set_default_qa_contact {
+    my ($self, $qa_contact) = @_;
+
+    $self->set('initialqacontact', $qa_contact);
+    # Reset the default QA contact object.
+    delete $self->{default_qa_contact};
+}
+sub set_cc_list {
+    my ($self, $cc_list) = @_;
+
+    $self->{cc_ids} = $self->_check_cc_list($cc_list);
+    # Reset the list of CC user objects.
+    delete $self->{initial_cc};
+}
+
 sub bug_count {
     my $self = shift;
     my $dbh = Bugzilla->dbh;
@@ -143,15 +388,17 @@ sub flag_types {
 
 sub initial_cc {
     my $self = shift;
-
     my $dbh = Bugzilla->dbh;
 
     if (!defined $self->{'initial_cc'}) {
-        my $cc_ids = $dbh->selectcol_arrayref(
-            "SELECT user_id FROM component_cc WHERE component_id = ?",
-            undef, $self->id);
-        my $initial_cc = Bugzilla::User->new_from_list($cc_ids);
-        $self->{'initial_cc'} = $initial_cc;
+        # If set_cc_list() has been called but data are not yet written
+        # into the DB, we want the new values defined by it.
+        my $cc_ids = $self->{cc_ids}
+                     || $dbh->selectcol_arrayref('SELECT user_id FROM component_cc
+                                                  WHERE component_id = ?',
+                                                  undef, $self->id);
+
+        $self->{'initial_cc'} = Bugzilla::User->new_from_list($cc_ids);
     }
     return $self->{'initial_cc'};
 }
@@ -178,27 +425,6 @@ sub product_id  { return $_[0]->{'product_id'};  }
 ####      Subroutines      ####
 ###############################
 
-sub check_component {
-    my ($product, $comp_name) = @_;
-
-    $comp_name || ThrowUserError('component_blank_name');
-
-    if (length($comp_name) > 64) {
-        ThrowUserError('component_name_too_long',
-                       {'name' => $comp_name});
-    }
-
-    my $component =
-        new Bugzilla::Component({product => $product,
-                                 name    => $comp_name});
-    unless ($component) {
-        ThrowUserError('component_not_valid',
-                       {'product' => $product->name,
-                        'name' => $comp_name});
-    }
-    return $component;
-}
-
 1;
 
 __END__
@@ -211,9 +437,8 @@ Bugzilla::Component - Bugzilla product component class.
 
     use Bugzilla::Component;
 
-    my $component = new Bugzilla::Component(1);
-    my $component = new Bugzilla::Component({product => $product,
-                                             name    => 'AcmeComp'});
+    my $component = new Bugzilla::Component($comp_id);
+    my $component = new Bugzilla::Component({ product => $product, name => $name });
 
     my $bug_count          = $component->bug_count();
     my $bug_ids            = $component->bug_ids();
@@ -228,7 +453,23 @@ Bugzilla::Component - Bugzilla product component class.
     my $bug_flag_types     = $component->flag_types->{'bug'};
     my $attach_flag_types  = $component->flag_types->{'attachment'};
 
-    my $component  = Bugzilla::Component::check_component($product, 'AcmeComp');
+    my $component = Bugzilla::Component->check({ product => $product, name => $name });
+
+    my $component =
+      Bugzilla::Component->create({ name             => $name,
+                                    product          => $product,
+                                    initialowner     => $user_login1,
+                                    initialqacontact => $user_login2,
+                                    description      => $description});
+
+    $component->set_name($new_name);
+    $component->set_description($new_description);
+    $component->set_default_assignee($new_login_name);
+    $component->set_default_qa_contact($new_login_name);
+    $component->set_cc_list(\@new_login_names);
+    $component->update();
+
+    $component->remove_from_db;
 
 =head1 DESCRIPTION
 
@@ -241,14 +482,15 @@ Component.pm represents a Product Component object.
 =item C<new($param)>
 
  Description: The constructor is used to load an existing component
-              by passing a component id or a hash with the product
-              id and the component name.
+              by passing a component ID or a hash with the product
+              object the component belongs to and the component name.
 
  Params:      $param - If you pass an integer, the integer is the
-                       component id from the database that we want to
-                       read in. If you pass in a hash with 'name' key,
-                       then the value of the name key is the name of a
-                       component from the DB.
+                       component ID from the database that we want to
+                       read in. If you pass in a hash with the 'name'
+                       and 'product' keys, then the value of the name
+                       key is the name of a component being in the given
+                       product.
 
  Returns:     A Bugzilla::Component object.
 
@@ -288,41 +530,117 @@ Component.pm represents a Product Component object.
 
 =item C<initial_cc>
 
-Returns an arrayref of L<Bugzilla::User> objects representing the
-Initial CC List.
+ Description: Returns a list of user objects representing users being
+              in the initial CC list.
+
+ Params:      none.
+
+ Returns:     An arrayref of L<Bugzilla::User> objects.
 
 =item C<flag_types()>
 
-  Description: Returns all bug and attachment flagtypes available for
-               the component.
+ Description: Returns all bug and attachment flagtypes available for
+              the component.
 
-  Params:      none.
+ Params:      none.
 
-  Returns:     Two references to an array of flagtype objects.
+ Returns:     Two references to an array of flagtype objects.
 
 =item C<product()>
 
-  Description: Returns the product the component belongs to.
+ Description: Returns the product the component belongs to.
+
+ Params:      none.
+
+ Returns:     A Bugzilla::Product object.
+
+=item C<set_name($new_name)>
+
+ Description: Changes the name of the component.
+
+ Params:      $new_name - new name of the component (string). This name
+                          must be unique within the product.
+
+ Returns:     Nothing.
+
+=item C<set_description($new_desc)>
+
+ Description: Changes the description of the component.
+
+ Params:      $new_desc - new description of the component (string).
+
+ Returns:     Nothing.
+
+=item C<set_default_assignee($new_assignee)>
+
+ Description: Changes the default assignee of the component.
+
+ Params:      $new_owner - login name of the new default assignee of
+                           the component (string). This user account
+                           must already exist.
 
-  Params:      none.
+ Returns:     Nothing.
 
-  Returns:     A Bugzilla::Product object.
+=item C<set_default_qa_contact($new_qa_contact)>
+
+ Description: Changes the default QA contact of the component.
+
+ Params:      $new_qa_contact - login name of the new QA contact of
+                                the component (string). This user
+                                account must already exist.
+
+ Returns:     Nothing.
+
+=item C<set_cc_list(\@cc_list)>
+
+ Description: Changes the list of users being in the CC list by default.
+
+ Params:      \@cc_list - list of login names (string). All the user
+                          accounts must already exist.
+
+ Returns:     Nothing.
+
+=item C<update()>
+
+ Description: Write changes made to the component into the DB.
+
+ Params:      none.
+
+ Returns:     A hashref with changes made to the component object.
+
+=item C<remove_from_db()>
+
+ Description: Deletes the current component from the DB. The object itself
+              is not destroyed.
+
+ Params:      none.
+
+ Returns:     Nothing.
 
 =back
 
-=head1 SUBROUTINES
+=head1 CLASS METHODS
 
 =over
 
-=item C<check_component($product, $comp_name)>
+=item C<create(\%params)>
 
- Description: Checks if the component name was passed in and if it is a valid
-              component.
+ Description: Create a new component for the given product.
 
- Params:      $product - A Bugzilla::Product object.
-              $comp_name - String with a component name.
+ Params:      The hashref must have the following keys:
+              name            - name of the new component (string). This name
+                                must be unique within the product.
+              product         - a Bugzilla::Product object to which
+                                the Component is being added.
+              description     - description of the new component (string).
+              initialowner    - login name of the default assignee (string).
+              The following keys are optional:
+              initiaqacontact - login name of the default QA contact (string),
+                                or an empty string to clear it.
+              initial_cc      - an arrayref of login names to add to the
+                                CC list by default.
 
- Returns:     Bugzilla::Component object.
+ Returns:     A Bugzilla::Component object.
 
 =back
 
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index 99edb2f50..f84ece006 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -147,6 +147,7 @@ use File::Basename;
 
     MAX_LEN_QUERY_NAME
     MAX_MILESTONE_SIZE
+    MAX_COMPONENT_SIZE
 );
 
 @Bugzilla::Constants::EXPORT_OK = qw(contenttypes);
@@ -410,6 +411,9 @@ use constant MAX_LEN_QUERY_NAME => 64;
 # The longest milestone name allowed.
 use constant MAX_MILESTONE_SIZE => 20;
 
+# The longest component name allowed.
+use constant MAX_COMPONENT_SIZE => 64;
+
 sub bz_locations {
     # We know that Bugzilla/Constants.pm must be in %INC at this point.
     # So the only question is, what's the name of the directory
diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm
index 14ed2e7fd..8dacfe956 100755
--- a/Bugzilla/WebService/Bug.pm
+++ b/Bugzilla/WebService/Bug.pm
@@ -431,6 +431,10 @@ A hash with one element, C<id>. This is the id of the newly-filed bug.
 
 =over
 
+=item 51 (Invalid Object)
+
+The component you specified is not valid for this Product.
+
 =item 103 (Invalid Alias)
 
 The alias you specified is invalid for some reason. See the error message
@@ -443,8 +447,7 @@ have more detail.
 
 =item 105 (Invalid Component)
 
-Either you didn't specify a component, or the component you specified was
-invalid.
+You didn't specify a component.
 
 =item 106 (Invalid Product)
 
diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm
index 139ec1b7b..2dfb0b112 100755
--- a/Bugzilla/WebService/Constants.pm
+++ b/Bugzilla/WebService/Constants.pm
@@ -50,6 +50,9 @@ use base qw(Exporter);
 # comment that it was retired. Also, if an error changes its name, you'll
 # have to fix it here.
 use constant WS_ERROR_CODE => {
+    # Generic Bugzilla::Object errors are 50-99.
+    object_name_not_specified   => 50,
+    object_does_not_exist       => 51,
     # Bug errors usually occupy the 100-200 range.
     improper_bug_id_field_value => 100,
     bug_id_does_not_exist       => 101,
@@ -65,7 +68,6 @@ use constant WS_ERROR_CODE => {
     # Component errors
     require_component       => 105,
     component_name_too_long => 105,
-    component_not_valid     => 105,
     # Invalid Product
     no_products         => 106,
     entry_access_denied => 106,