Bug 349741: Make Bugzilla::Bug able to do basic bug creation, and have post_bug.cgi use it

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=bkor, a=myk

2 files changed, 192 insertions, 21 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 8c61a657b..642a71d3f 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -96,6 +96,34 @@ sub DB_COLUMNS {
     Bugzilla->custom_field_names;
 }
 
+use constant REQUIRED_CREATE_FIELDS => qw(
+    bug_severity
+    component
+    creation_ts
+    op_sys
+    priority
+    product
+    rep_platform
+    short_desc
+    version
+);
+
+# There are also other, more complex validators that are called
+# from run_create_validators.
+use constant VALIDATORS => {
+    alias          => \&_check_alias,
+    bug_file_loc   => \&_check_bug_file_loc,
+    bug_severity   => \&_check_bug_severity,
+    deadline       => \&_check_deadline,
+    estimated_time => \&_check_estimated_time,
+    op_sys         => \&_check_op_sys,
+    priority       => \&_check_priority,
+    remaining_time => \&_check_remaining_time,
+    rep_platform   => \&_check_rep_platform,
+    short_desc     => \&_check_short_desc,
+    status_whiteboard => \&_check_status_whiteboard,
+};
+
 # Used in LogActivityEntry(). Gives the max length of lines in the
 # activity table.
 use constant MAX_LINE_LENGTH => 254;
@@ -157,6 +185,79 @@ sub new {
     return $self;
 }
 
+# Docs for create() (there's no POD in this file yet, but we very
+# much need this documented right now):
+#
+# The same as Bugzilla::Object->create. Parameters are only required
+# if they say so below.
+#
+# Params:
+#
+# C<product>     - B<Required> The name of the product this bug is being
+#                  filed against.
+# C<component>   - B<Required> The name of the component this bug is being
+#                  filed against.
+#
+# C<bug_severity> - B<Required> The severity for the bug, a string.
+# C<creation_ts>  - B<Required> A SQL timestamp for when the bug was created.
+# C<short_desc>   - B<Required> A summary for the bug.
+# C<op_sys>       - B<Required> The OS the bug was found against.
+# C<priority>     - B<Required> The initial priority for the bug.
+# C<rep_platform> - B<Required> The platform the bug was found against.
+# C<version>      - B<Required> The version of the product the bug was found in.
+#
+# C<alias>        - An alias for this bug. Will be ignored if C<usebugaliases>
+#                   is off.
+# C<target_milestone> - When this bug is expected to be fixed.
+# C<status_whiteboard> - A string.
+# C<bug_status>   - The initial status of the bug, a string.
+# C<bug_file_loc> - The URL field.
+#
+# C<assigned_to> - The full login name of the user who the bug is
+#                  initially assigned to.
+# C<qa_contact>  - The full login name of the QA Contact for this bug. 
+#                  Will be ignored if C<useqacontact> is off.
+#
+# C<estimated_time> - For time-tracking. Will be ignored if 
+#                     C<timetrackinggroup> is not set, or if the current
+#                     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 run_create_validators {
+    my $class  = shift;
+    my $params = shift;
+
+    my $product = _check_product($params->{product});
+    $params->{product_id} = $product->id;
+    delete $params->{product};
+
+    ($params->{bug_status}, $params->{everconfirmed})
+        = _check_bug_status($product, $params->{bug_status});
+
+    $params->{target_milestone} = _check_target_milestone($product,
+        $params->{target_milestone});
+
+    $params->{version} = _check_version($product, $params->{version});
+
+    my $component = _check_component($product, $params->{component});
+    $params->{component_id} = $component->id;
+    delete $params->{component};
+
+    $params->{assigned_to} = 
+        _check_assigned_to($component, $params->{assigned_to});
+    $params->{qa_contact} =
+        _check_qa_contact($component, $params->{qa_contact});
+    # Callers cannot set Reporter, currently.
+    $params->{reporter} = Bugzilla->user->id;
+
+    $params->{delta_ts} = $params->{creation_ts};
+    $params->{remaining_time} = $params->{estimated_time};
+
+    unshift @_, $params;
+    return $class->SUPER::run_create_validators(@_);
+}
+
 # This is the correct way to delete bugs from the DB.
 # No bug should be deleted from anywhere else except from here.
 #
@@ -233,12 +334,13 @@ sub remove_from_db {
 sub _check_alias {
    my ($alias) = @_;
    $alias = trim($alias);
-   ValidateBugAlias($alias) if (defined $alias && $alias ne '');
+   return undef if (!Bugzilla->params->{'usebugaliases'} || !$alias);
+   ValidateBugAlias($alias);
    return $alias;
 }
 
 sub _check_assigned_to {
-    my ($name, $component) = @_;
+    my ($component, $name) = @_;
     my $user = Bugzilla->user;
 
     $name = trim($name);
@@ -254,9 +356,6 @@ sub _check_assigned_to {
 
 sub _check_bug_file_loc {
     my ($url) = @_;
-    if (!defined $url) {
-        ThrowCodeError('undefined_field', { field => 'bug_file_loc' });
-    }
     # If bug_file_loc is "http://", the default, use an empty value instead.
     $url = '' if $url eq 'http://';
     return $url;
@@ -270,7 +369,7 @@ sub _check_bug_severity {
 }
 
 sub _check_bug_status {
-    my ($status, $product) = @_;
+    my ($product, $status) = @_;
     my $user = Bugzilla->user;
 
     my @valid_statuses = VALID_ENTRY_STATUS;
@@ -293,7 +392,7 @@ sub _check_bug_status {
     shift @valid_statuses if !$product->votes_to_confirm;
 
     check_field('bug_status', $status, \@valid_statuses);
-    return $status;
+    return ($status, $status eq 'UNCONFIRMED' ? 0 : 1);
 }
 
 sub _check_cc {
@@ -331,7 +430,7 @@ sub _check_comment {
 }
 
 sub _check_component {
-    my ($name, $product) = @_;
+    my ($product, $name) = @_;
     $name = trim($name);
     $name || ThrowUserError("require_component");
     my $obj = Bugzilla::Component::check_component($product, $name);
@@ -341,6 +440,18 @@ sub _check_component {
     return $obj;
 }
 
+sub _check_deadline {
+    my ($date) = @_;
+    $date = trim($date);
+    my $tt_group = Bugzilla->params->{"timetrackinggroup"};
+    return undef unless $date && $tt_group 
+                        && Bugzilla->user->in_group($tt_group);
+    validate_date($date)
+        || ThrowUserError('illegal_date', { date   => $date,
+                                            format => 'YYYY-MM-DD' });
+    return $date;
+}
+
 # Takes two comma/space-separated strings and returns arrayrefs
 # of valid bug IDs.
 sub _check_dependencies {
@@ -370,6 +481,10 @@ sub _check_dependencies {
     return ($deps{'dependson'}, $deps{'blocked'});
 }
 
+sub _check_estimated_time {
+    return _check_time($_[0], 'estimated_time');
+}
+
 sub _check_keywords {
     my ($keyword_string) = @_;
     $keyword_string = trim($keyword_string);
@@ -417,6 +532,10 @@ sub _check_priority {
     return $priority;
 }
 
+sub _check_remaining_time {
+    return _check_time($_[0], 'remaining_time');
+}
+
 sub _check_rep_platform {
     my ($platform) = @_;
     $platform = trim($platform);
@@ -435,6 +554,8 @@ sub _check_short_desc {
     return $short_desc;
 }
 
+sub _check_status_whiteboard { return defined $_[0] ? $_[0] : ''; }
+
 # Unlike other checkers, this one doesn't return anything.
 sub _check_strict_isolation {
     my ($product, $cc_ids, $assignee_id, $qa_contact_id) = @_;
@@ -466,10 +587,30 @@ sub _check_strict_isolation {
     }
 }
 
+sub _check_target_milestone {
+    my ($product, $target) = @_;
+    $target = trim($target);
+    $target = $product->default_milestone if !defined $target;
+    check_field('target_milestone', $target,
+            [map($_->name, @{$product->milestones})]);
+    return $target;
+}
+
+sub _check_time {
+    my ($time, $field) = @_;
+    my $tt_group = Bugzilla->params->{"timetrackinggroup"};
+    return 0 unless $tt_group && Bugzilla->user->in_group($tt_group);
+    $time = trim($time) || 0;
+    ValidateTime($time, $field);
+    return $time;
+}
+
 sub _check_qa_contact {
-    my ($name, $component) = @_;
+    my ($component, $name) = @_;
     my $user = Bugzilla->user;
 
+    return undef unless Bugzilla->params->{'useqacontact'};
+
     $name = trim($name);
 
     my $id;
@@ -483,6 +624,13 @@ sub _check_qa_contact {
     return $id;
 }
 
+sub _check_version {
+    my ($product, $version) = @_;
+    $version = trim($version);
+    check_field('version', $version, [map($_->name, @{$product->versions})]);
+    return $version;
+}
+
 
 #####################################################################
 # Class Accessors
diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm
index 219658a92..833cdcd2f 100644
--- a/Bugzilla/Object.pm
+++ b/Bugzilla/Object.pm
@@ -123,16 +123,29 @@ sub create {
     my ($class, $params) = @_;
     my $dbh = Bugzilla->dbh;
 
-    my $required   = $class->REQUIRED_CREATE_FIELDS;
-    my $validators = $class->VALIDATORS;
-    my $table      = $class->DB_TABLE;
-
     foreach my $field ($class->REQUIRED_CREATE_FIELDS) {
         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 {
+    my ($class, $params) = @_;
+
+    my $validators = $class->VALIDATORS;
+
     my (@field_names, @values);
     # We do the sort just to make sure that validation always
     # happens in a consistent order.
@@ -144,18 +157,14 @@ sub create {
         else {
             $value = $params->{$field};
         }
-        trick_taint($value);
+        # 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;
         push(@field_names, $field);
         push(@values, $value);
     }
 
-    my $qmarks = '?,' x @values;
-    chop($qmarks);
-    $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);
+    return (\@field_names, \@values);
 }
 
 sub get_all {
@@ -307,6 +316,20 @@ 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)>
+
+Description: Runs the validation of input parameters for L</create>.
+             This subroutine exists so that it can be overridden
+             by subclasses who need to do special validations
+             of their input parameters. This method is B<only> called
+             by 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).
+
 =item C<get_all>
 
  Description: Returns all objects in this table from the database.