Bug 323239: Move CC insertion from post_bug.cgi to Bugzilla::Bug

Bugzilla/Bug.pm

@@ -114,10 +114,12 @@ use constant VALIDATORS => {
     alias          => \&_check_alias,
     bug_file_loc   => \&_check_bug_file_loc,
     bug_severity   => \&_check_bug_severity,
+    cc             => \&_check_cc,
     deadline       => \&_check_deadline,
     estimated_time => \&_check_estimated_time,
     op_sys         => \&_check_op_sys,
     priority       => \&_check_priority,
+    product        => \&_check_product,
     remaining_time => \&_check_remaining_time,
     rep_platform   => \&_check_rep_platform,
     short_desc     => \&_check_short_desc,
@@ -223,12 +225,34 @@ sub new {
 #                     user is not a member of the timetrackinggroup.
 # C<deadline>       - For time-tracking. Will be ignored for the same
 #                     reasons as C<estimated_time>.
+sub create {
+    my $class  = shift;
+    my $dbh = Bugzilla->dbh;
+
+    $class->check_required_create_fields(@_);
+    my $params = $class->run_create_validators(@_);
+
+    # "cc" is not a field in the bugs table, so we don't pass it to
+    # insert_create_data.
+    my $cc_ids = $params->{cc};
+    delete $params->{cc};
+
+    my $bug = $class->insert_create_data($params);
+
+    my $sth_cc = $dbh->prepare('INSERT INTO cc (bug_id, who) VALUES (?,?)');
+    foreach my $user_id (@$cc_ids) {
+        $sth_cc->execute($bug->bug_id, $user_id);
+    }
+
+    return $bug;
+}
+
 
 sub run_create_validators {
     my $class  = shift;
-    my $params = shift;
+    my $params = $class->SUPER::run_create_validators(@_);
 
-    my $product = $class->_check_product($params->{product});
+    my $product = $params->{product};
     $params->{product_id} = $product->id;
     delete $params->{product};
 
@@ -254,8 +278,10 @@ sub run_create_validators {
     $params->{delta_ts} = $params->{creation_ts};
     $params->{remaining_time} = $params->{estimated_time};
 
-    unshift @_, $params;
-    return $class->SUPER::run_create_validators(@_);
+    $class->_check_strict_isolation($product, $params->{cc},
+    $params->{assigned_to}, $params->{qa_contact});
+
+    return $params;
 }
 
 # This is the correct way to delete bugs from the DB.

Bugzilla/Object.pm

@@ -169,22 +169,19 @@ sub create {
     my ($class, $params) = @_;
     my $dbh = Bugzilla->dbh;
 
+    $class->check_required_create_fields($params);
+    my $field_values = $class->run_create_validators($params);
+    return $class->insert_create_data($field_values);
+}
+
+sub check_required_create_fields {
+    my ($class, $params) = @_;
+
     foreach my $field ($class->REQUIRED_CREATE_FIELDS) {
-        ThrowCodeError('param_required', 
+        ThrowCodeError('param_required',
             { function => "${class}->create", param => $field })
             if !exists $params->{$field};
     }
-
-    my ($field_names, $values) = $class->run_create_validators($params);
-
-    my $qmarks = '?,' x @$values;
-    chop($qmarks);
-    my $table = $class->DB_TABLE;
-    $dbh->do("INSERT INTO $table (" . join(', ', @$field_names) 
-             . ") VALUES ($qmarks)", undef, @$values);
-    my $id = $dbh->bz_last_key($table, $class->ID_FIELD);
-
-    return $class->new($id);
 }
 
 sub run_create_validators {
@@ -192,7 +189,7 @@ sub run_create_validators {
 
     my $validators = $class->VALIDATORS;
 
-    my (@field_names, @values);
+    my %field_values;
     # We do the sort just to make sure that validation always
     # happens in a consistent order.
     foreach my $field (sort keys %$params) {
@@ -206,12 +203,30 @@ sub run_create_validators {
         }
         # We want people to be able to explicitly set fields to NULL,
         # and that means they can be set to undef.
-        trick_taint($value) if defined $value;
+        trick_taint($value) if defined $value && !ref($value);
+        $field_values{$field} = $value;
+    }
+
+    return \%field_values;
+}
+
+sub insert_create_data {
+    my ($class, $field_values) = @_;
+    my $dbh = Bugzilla->dbh;
+
+    my (@field_names, @values);
+    while (my ($field, $value) = each %$field_values) {
         push(@field_names, $field);
         push(@values, $value);
     }
 
-    return (\@field_names, \@values);
+    my $qmarks = '?,' x @field_names;
+    chop($qmarks);
+    my $table = $class->DB_TABLE;
+    $dbh->do("INSERT INTO $table (" . join(', ', @field_names)
+             . ") VALUES ($qmarks)", undef, @values);
+    my $id = $dbh->bz_last_key($table, $class->ID_FIELD);
+    return $class->new($id);
 }
 
 sub get_all {
@@ -382,7 +397,35 @@ Notes:       In order for this function to work in your subclass,
              type in the database. Your subclass also must
              define L</REQUIRED_CREATE_FIELDS> and L</VALIDATORS>.
 
-=item C<run_create_validators($params)>
+             Subclass Implementors: This function basically just
+             calls L</check_required_create_fields>, then
+             L</run_create_validators>, and then finally
+             L</insert_create_data>. So if you have a complex system that
+             you need to implement, you can do it by calling these
+             three functions instead of C<SUPER::create>.
+
+=item C<check_required_create_fields>
+
+=over
+
+=item B<Description>
+
+Part of L</create>. Throws an error if any of the L</REQUIRED_CREATE_FIELDS>
+have not been specified in C<$params>
+
+=item B<Params>
+
+=over
+
+=item C<$params> - The same as C<$params> from L</create>.
+
+=back
+
+=item B<Returns> (nothing)
+
+=back
+
+=item C<run_create_validators>
 
 Description: Runs the validation of input parameters for L</create>.
              This subroutine exists so that it can be overridden
@@ -392,9 +435,16 @@ Description: Runs the validation of input parameters for L</create>.
 
 Params:      The same as L</create>.
 
-Returns:     Two arrayrefs. The first is an array of database field names.
-             The second is an untainted array of values that should go 
-             into those fields (in the same order).
+Returns:     A hash, in a similar format as C<$params>, except that
+             these are the values to be inserted into the database,
+             not the values that were input to L</create>.
+
+=item C<insert_create_data>
+
+Part of L</create>.
+
+Takes the return value from L</run_create_validators> and inserts the
+data into the database. Returns a newly created object. 
 
 =item C<update>
 

post_bug.cgi

@@ -146,20 +146,8 @@ $comment = Bugzilla::Bug->_check_comment($cgi->param('comment'));
 # OK except for the fact that it causes e-mail to be suppressed.
 $comment = $comment ? $comment : " ";
 
-my $cc_ids = Bugzilla::Bug->_check_cc([$cgi->param('cc')]);
 my @keyword_ids = @{Bugzilla::Bug->_check_keywords($cgi->param('keywords'))};
 
-# XXX These checks are only here until strict_isolation can move fully
-# into Bugzilla::Bug.
-my $component = Bugzilla::Bug->_check_component($product, 
-    $cgi->param('component'));
-my $assigned_to_id = Bugzilla::Bug->_check_assigned_to($component,
-    $cgi->param('assigned_to'));
-my $qa_contact_id = Bugzilla::Bug->_check_qa_contact($component,
-    $cgi->param('qa_contact'));
-Bugzilla::Bug->_check_strict_isolation($product, $cc_ids, $assigned_to_id, 
-    $qa_contact_id);
-
 my ($depends_on_ids, $blocks_ids) = Bugzilla::Bug->_check_dependencies(
     scalar $cgi->param('dependson'), scalar $cgi->param('blocked'));
 
@@ -252,6 +240,7 @@ foreach my $field (@bug_fields) {
     $bug_params{$field} = $cgi->param($field);
 }
 $bug_params{'creation_ts'} = $timestamp;
+$bug_params{'cc'}          = [$cgi->param('cc')];
 
 # Add the bug report to the DB.
 $dbh->bz_lock_tables('bugs WRITE', 'bug_group_map WRITE', 'longdescs WRITE',
@@ -261,7 +250,8 @@ $dbh->bz_lock_tables('bugs WRITE', 'bug_group_map WRITE', 'longdescs WRITE',
                      'keyworddefs READ', 'fielddefs READ',
                      'products READ', 'versions READ', 'milestones READ',
                      'components READ', 'profiles READ', 'bug_severity READ',
-                     'op_sys READ', 'priority READ', 'rep_platform READ');
+                     'op_sys READ', 'priority READ', 'rep_platform READ',
+                     'group_control_map READ');
 
 my $bug = Bugzilla::Bug->create(\%bug_params);
 
@@ -288,12 +278,6 @@ $dbh->do(q{INSERT INTO longdescs (bug_id, who, bug_when, thetext,isprivate)
            VALUES (?, ?, ?, ?, ?)}, undef, ($id, $user->id, $timestamp,
                                             $comment, $privacy));
 
-# Insert the cclist into the database
-my $sth_cclist = $dbh->prepare(q{INSERT INTO cc (bug_id, who) VALUES (?,?)});
-foreach my $ccid (@$cc_ids) {
-    $sth_cclist->execute($id, $ccid);
-}
-
 my @all_deps;
 my $sth_addkeyword = $dbh->prepare(q{
             INSERT INTO keywords (bug_id, keywordid) VALUES (?, ?)});