summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authormkanat%bugzilla.org <>2006-08-20 08:21:43 +0200
committermkanat%bugzilla.org <>2006-08-20 08:21:43 +0200
commit36fcbae1f486223e61e1457b2af12a1fab0fe388 (patch)
tree61c9d5329b402e3af7d72182050eaba5bfcb0a6d
parent015e33633728805a62d61e49a49ee74077d4f888 (diff)
downloadbugzilla-36fcbae1f486223e61e1457b2af12a1fab0fe388.tar.gz
bugzilla-36fcbae1f486223e61e1457b2af12a1fab0fe388.tar.xz
Bug 348477: Move simple validations from post_bug.cgi to Bugzilla::Bug
Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=bkor, a=myk
-rwxr-xr-xBugzilla/Bug.pm110
-rwxr-xr-xpost_bug.cgi81
2 files changed, 133 insertions, 58 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index b89b0cf37..08677967a 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -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
diff --git a/post_bug.cgi b/post_bug.cgi
index d8f2150e9..7ceeffc3a 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -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);