Commit b63fd277 authored by mkanat%bugzilla.org's avatar mkanat%bugzilla.org

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
parent 0436b3d4
......@@ -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
......
......@@ -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.
......
......@@ -139,12 +139,6 @@ if (defined $cgi->param('maketemplate')) {
umask 0;
# Some sanity checking
my $component =
Bugzilla::Bug::_check_component(scalar $cgi->param('component'), $product);
$cgi->param('short_desc',
Bugzilla::Bug::_check_short_desc($cgi->param('short_desc')));
# This has to go somewhere after 'maketemplate'
# or it breaks bookmarks with no comments.
$comment = Bugzilla::Bug::_check_comment($cgi->param('comment'));
......@@ -152,81 +146,19 @@ $comment = Bugzilla::Bug::_check_comment($cgi->param('comment'));
# OK except for the fact that it causes e-mail to be suppressed.
$comment = $comment ? $comment : " ";
$cgi->param('bug_file_loc',
Bugzilla::Bug::_check_bug_file_loc($cgi->param('bug_file_loc')));
$cgi->param('assigned_to',
Bugzilla::Bug::_check_assigned_to(scalar $cgi->param('assigned_to'),
$component));
my @enter_bug_field_names = map {$_->name} Bugzilla->get_fields({ custom => 1,
obsolete => 0, enter_bug => 1});
my @bug_fields = ("version", "rep_platform",
"bug_severity", "priority", "op_sys", "assigned_to",
"bug_status", "everconfirmed", "bug_file_loc", "short_desc",
"target_milestone", "status_whiteboard",
@enter_bug_field_names);
if (Bugzilla->params->{"usebugaliases"}) {
my $alias = Bugzilla::Bug::_check_alias($cgi->param('alias'));
if ($alias) {
$cgi->param('alias', $alias);
push(@bug_fields, "alias");
}
}
if (Bugzilla->params->{"useqacontact"}) {
my $qa_contact = Bugzilla::Bug::_check_qa_contact(
scalar $cgi->param('qa_contact'), $component);
if ($qa_contact) {
$cgi->param('qa_contact', $qa_contact);
push(@bug_fields, "qa_contact");
}
}
$cgi->param('bug_status', Bugzilla::Bug::_check_bug_status(
scalar $cgi->param('bug_status'), $product));
if (!defined $cgi->param('target_milestone')) {
$cgi->param(-name => 'target_milestone', -value => $product->default_milestone);
}
# Some more sanity checking
$cgi->param(-name => 'priority', -value => Bugzilla::Bug::_check_priority(
$cgi->param('priority')));
$cgi->param(-name => 'rep_platform',
-value => Bugzilla::Bug::_check_rep_platform($cgi->param('rep_platform')));
$cgi->param(-name => 'bug_severity',
-value => Bugzilla::Bug::_check_bug_severity($cgi->param('bug_severity')));
$cgi->param(-name => 'op_sys', -value => Bugzilla::Bug::_check_op_sys(
$cgi->param('op_sys')));
check_field('version', scalar $cgi->param('version'),
[map($_->name, @{$product->versions})]);
check_field('target_milestone', scalar $cgi->param('target_milestone'),
[map($_->name, @{$product->milestones})]);
my $everconfirmed = ($cgi->param('bug_status') eq 'UNCONFIRMED') ? 0 : 1;
$cgi->param(-name => 'everconfirmed', -value => $everconfirmed);
my @used_fields;
foreach my $field (@bug_fields) {
if (defined $cgi->param($field)) {
push (@used_fields, $field);
}
}
$cgi->param(-name => 'product_id', -value => $product->id);
push(@used_fields, "product_id");
$cgi->param(-name => 'component_id', -value => $component->id);
push(@used_fields, "component_id");
my $cc_ids = Bugzilla::Bug::_check_cc([$cgi->param('cc')]);
my @keyword_ids = @{Bugzilla::Bug::_check_keywords($cgi->param('keywords'))};
Bugzilla::Bug::_check_strict_isolation($product, $cc_ids,
$cgi->param('assigned_to'), $cgi->param('qa_contact'));
# 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'));
......@@ -234,54 +166,6 @@ my ($depends_on_ids, $blocks_ids) = Bugzilla::Bug::_check_dependencies(
# get current time
my $timestamp = $dbh->selectrow_array(q{SELECT NOW()});
# Build up SQL string to add bug.
# creation_ts will only be set when all other fields are defined.
my @fields_values;
foreach my $field (@used_fields) {
my $value = $cgi->param($field);
trick_taint($value);
push (@fields_values, $value);
}
my $sql_used_fields = join(", ", @used_fields);
my $sql_placeholders = "?, " x scalar(@used_fields);
my $query = qq{INSERT INTO bugs ($sql_used_fields, reporter, delta_ts,
estimated_time, remaining_time, deadline)
VALUES ($sql_placeholders ?, ?, ?, ?, ?)};
push (@fields_values, $user->id);
push (@fields_values, $timestamp);
my $est_time = 0;
my $deadline;
# Time Tracking
if (UserInGroup(Bugzilla->params->{"timetrackinggroup"}) &&
defined $cgi->param('estimated_time')) {
$est_time = $cgi->param('estimated_time');
Bugzilla::Bug::ValidateTime($est_time, 'estimated_time');
trick_taint($est_time);
}
push (@fields_values, $est_time, $est_time);
if ( UserInGroup(Bugzilla->params->{"timetrackinggroup"})
&& $cgi->param('deadline') )
{
validate_date($cgi->param('deadline'))
|| ThrowUserError('illegal_date', {date => $cgi->param('deadline'),
format => 'YYYY-MM-DD'});
$deadline = $cgi->param('deadline');
trick_taint($deadline);
}
push (@fields_values, $deadline);
# Groups
my @groupstoadd = ();
my $sth_othercontrol = $dbh->prepare(q{SELECT othercontrol
......@@ -339,17 +223,50 @@ foreach my $group (@$groups) {
}
}
my @bug_fields = map {$_->name} Bugzilla->get_fields(
{ custom => 1, obsolete => 0, enter_bug => 1});
push(@bug_fields, qw(
product
component
assigned_to
qa_contact
alias
bug_file_loc
bug_severity
bug_status
short_desc
op_sys
priority
rep_platform
version
target_milestone
status_whiteboard
estimated_time
deadline
));
my %bug_params;
foreach my $field (@bug_fields) {
$bug_params{$field} = $cgi->param($field);
}
$bug_params{'creation_ts'} = $timestamp;
# Add the bug report to the DB.
$dbh->bz_lock_tables('bugs WRITE', 'bug_group_map WRITE', 'longdescs WRITE',
'cc WRITE', 'keywords WRITE', 'dependencies WRITE',
'bugs_activity WRITE', 'groups READ',
'user_group_map READ', 'group_group_map READ',
'keyworddefs READ', 'fielddefs READ');
'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');
$dbh->do($query, undef, @fields_values);
my $bug = Bugzilla::Bug->create(\%bug_params);
# Get the bug ID back.
my $id = $dbh->bz_last_key('bugs', 'bug_id');
my $id = $bug->bug_id;
# Add the group restrictions
my $sth_addgroup = $dbh->prepare(q{
......@@ -420,7 +337,6 @@ $dbh->do("UPDATE bugs SET creation_ts = ? WHERE bug_id = ?",
$dbh->bz_unlock_tables();
my $bug = new Bugzilla::Bug($id);
# We don't have to check if the user can see the bug, because a user filing
# a bug can always see it. You can't change reporter_accessible until
# after the bug is filed.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment