Skip to content

bugzilla
bugzilla
Commit 36fcbae1 authored by mkanat%bugzilla.org's avatar mkanat%bugzilla.org
Browse files
Options

Bug 348477: Move simple validations from post_bug.cgi to Bugzilla::Bug

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=bkor, a=myk
parent 015e3363
Hide whitespace changes
Inline Side-by-side
Showing with 133 additions and 58 deletions
Bugzilla/Bug.pm
View file @ 36fcbae1
......@@ -217,6 +217,116 @@ sub remove_from_db {
return $self;
}
#####################################################################
# Validators
#####################################################################
sub _check_alias {
my ($alias) = @_;
$alias = trim($alias);
ValidateBugAlias($alias) if (defined $alias && $alias ne '');
return $alias;
}
sub _check_assigned_to {
my ($name, $component) = @_;
my $user = Bugzilla->user;
$name = trim($name);
# Default assignee is the component owner.
my $id;
if (!$user->in_group("editbugs") || !$name) {
$id = $component->default_assignee->id;
} else {
$id = login_to_id($name, THROW_ERROR);
}
return $id;
}
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;
}
sub _check_comment {
my ($comment) = @_;
if (!defined $comment) {
ThrowCodeError('undefined_field', { field => 'comment' });
}
# Remove any trailing whitespace. Leading whitespace could be
# a valid part of the comment.
$comment =~ s/\s*$//s;
$comment =~ s/\r\n?/\n/g; # Get rid of \r.
ValidateComment($comment);
if (Bugzilla->params->{"commentoncreate"} && !$comment) {
ThrowUserError("description_required");
}
return $comment;
}
sub _check_component {
my ($name, $product) = @_;
$name = trim($name);
$name || ThrowUserError("require_component");
my $obj = Bugzilla::Component::check_component($product, $name);
# XXX Right now, post_bug needs this to return an object. However,
# when we move to Bugzilla::Bug->create, this should just return
# what it was passed.
return $obj;
}
sub _check_product {
my ($name) = @_;
# Check that the product exists and that the user
# is allowed to enter bugs into this product.
Bugzilla->user->can_enter_product($name, THROW_ERROR);
# can_enter_product already does everything that check_product
# would do for us, so we don't need to use it.
my $obj = new Bugzilla::Product({ name => $name });
# XXX Right now, post_bug needs this to return an object. However,
# when we move to Bugzilla::Bug->create, this should just return
# what it was passed.
return $obj;
}
sub _check_short_desc {
my ($short_desc) = @_;
# Set the parameter to itself, but cleaned up
$short_desc = clean_text($short_desc) if $short_desc;
if (!defined $short_desc || $short_desc eq '') {
ThrowUserError("require_summary");
}
return $short_desc;
}
sub _check_qa_contact {
my ($name, $component) = @_;
my $user = Bugzilla->user;
$name = trim($name);
my $id;
if (!$user->in_group("editbugs") || !$name) {
# We want to insert NULL into the database if we get a 0.
$id = $component->default_qa_contact->id || undef;
} else {
$id = login_to_id($name, THROW_ERROR);
}
return $id;
}
#####################################################################
# Class Accessors
......
post_bug.cgi
View file @ 36fcbae1
......@@ -113,13 +113,9 @@ my $format = $template->get_format("bug/create/comment",
$template->process($format->{'template'}, $vars, \$comment)
|| ThrowTemplateError($template->error());
ValidateComment($comment);
# Check that the product exists and that the user
# is allowed to enter bugs into this product.
$user->can_enter_product(scalar $cgi->param('product'), 1);
my $product = Bugzilla::Product::check_product(scalar $cgi->param('product'));
my $product = Bugzilla::Bug::_check_product($cgi->param('product'));
# Set cookies
if (defined $cgi->param('product')) {
......@@ -143,35 +139,23 @@ if (defined $cgi->param('maketemplate')) {
umask 0;
# Some sanity checking
$cgi->param('component') || ThrowUserError("require_component");
my $component = Bugzilla::Component::check_component($product, scalar $cgi->param('component'));
# Set the parameter to itself, but cleaned up
$cgi->param('short_desc', clean_text($cgi->param('short_desc')));
my $component =
Bugzilla::Bug::_check_component(scalar $cgi->param('component'), $product);
$cgi->param('short_desc',
Bugzilla::Bug::_check_short_desc($cgi->param('short_desc')));
if (!defined $cgi->param('short_desc')
|| $cgi->param('short_desc') eq "") {
ThrowUserError("require_summary");
}
# Check that if required a description has been provided
# This has to go somewhere after 'maketemplate'
# or it breaks bookmarks with no comments.
if (Bugzilla->params->{"commentoncreate"} && !trim($cgi->param('comment'))) {
ThrowUserError("description_required");
}
# If bug_file_loc is "http://", the default, use an empty value instead.
$cgi->param('bug_file_loc', '') if $cgi->param('bug_file_loc') eq 'http://';
# or it breaks bookmarks with no comments.
$comment = Bugzilla::Bug::_check_comment($cgi->param('comment'));
# If comment is all whitespace, it'll be null at this point. That's
# OK except for the fact that it causes e-mail to be suppressed.
$comment = $comment ? $comment : " ";
# Default assignee is the component owner.
if (!UserInGroup("editbugs") || $cgi->param('assigned_to') eq "") {
$cgi->param(-name => 'assigned_to', -value => $component->default_assignee->id);
} else {
$cgi->param(-name => 'assigned_to',
-value => login_to_id(trim($cgi->param('assigned_to')), THROW_ERROR));
}
$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,
......@@ -184,26 +168,18 @@ my @bug_fields = ("version", "rep_platform",
@enter_bug_field_names);
if (Bugzilla->params->{"usebugaliases"}) {
my $alias = trim($cgi->param('alias') || "");
if ($alias ne "") {
ValidateBugAlias($alias);
$cgi->param('alias', $alias);
push (@bug_fields,"alias");
}
my $alias = Bugzilla::Bug::_check_alias($cgi->param('alias'));
if ($alias) {
$cgi->param('alias', $alias);
push(@bug_fields, "alias");
}
}
# Retrieve the default QA contact if the field is empty
if (Bugzilla->params->{"useqacontact"}) {
my $qa_contact;
if (!UserInGroup("editbugs") || !defined $cgi->param('qa_contact')
|| trim($cgi->param('qa_contact')) eq "") {
$qa_contact = $component->default_qa_contact->id;
} else {
$qa_contact = login_to_id(trim($cgi->param('qa_contact')), THROW_ERROR);
}
my $qa_contact = Bugzilla::Bug::_check_qa_contact(
scalar $cgi->param('qa_contact'), $component);
if ($qa_contact) {
$cgi->param(-name => 'qa_contact', -value => $qa_contact);
$cgi->param('qa_contact', $qa_contact);
push(@bug_fields, "qa_contact");
}
}
......@@ -245,11 +221,6 @@ check_field('version', scalar $cgi->param('version'),
check_field('target_milestone', scalar $cgi->param('target_milestone'),
[map($_->name, @{$product->milestones})]);
foreach my $field_name ('assigned_to', 'bug_file_loc', 'comment') {
defined($cgi->param($field_name))
|| ThrowCodeError('undefined_field', { field => $field_name });
}
my $everconfirmed = ($cgi->param('bug_status') eq 'UNCONFIRMED') ? 0 : 1;
$cgi->param(-name => 'everconfirmed', -value => $everconfirmed);
......@@ -364,12 +335,6 @@ my $query = qq{INSERT INTO bugs ($sql_used_fields, reporter, delta_ts,
estimated_time, remaining_time, deadline)
VALUES ($sql_placeholders ?, ?, ?, ?, ?)};
$comment =~ s/\r\n?/\n/g; # Get rid of \r.
$comment = trim($comment);
# If comment is all whitespace, it'll be null at this point. That's
# OK except for the fact that it causes e-mail to be suppressed.
$comment = $comment ? $comment : " ";
push (@fields_values, $user->id);
push (@fields_values, $timestamp);
......
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