summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authormkanat%bugzilla.org <>2007-03-08 06:54:01 +0100
committermkanat%bugzilla.org <>2007-03-08 06:54:01 +0100
commit3448aa4db7fd9d034436df34cdaa053a81f191c1 (patch)
tree22298d8031eec25e2649f165e5ee593a1dabf10b
parentc423290c20a482c6984df52999eacf2a5242d7f3 (diff)
downloadbugzilla-3448aa4db7fd9d034436df34cdaa053a81f191c1.tar.gz
bugzilla-3448aa4db7fd9d034436df34cdaa053a81f191c1.tar.xz
Bug 371070: Move basic validations from process_bug.cgi into Bugzilla::Bug
Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit
-rwxr-xr-xBugzilla/Bug.pm48
-rwxr-xr-xprocess_bug.cgi66
2 files changed, 66 insertions, 48 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 8ee0d87e2..a530687fe 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -495,8 +495,13 @@ sub _check_assigned_to {
sub _check_bug_file_loc {
my ($invocant, $url) = @_;
- # If bug_file_loc is "http://", the default, use an empty value instead.
- $url = '' if (!defined($url) || $url eq 'http://');
+ $url = '' if !defined($url);
+ # On bug entry, if bug_file_loc is "http://", the default, use an
+ # empty value instead. However, on bug editing people can set that
+ # back if they *really* want to.
+ if (!ref $invocant && $url eq 'http://') {
+ $url = '';
+ }
return $url;
}
@@ -564,13 +569,16 @@ sub _check_comment {
ValidateComment($comment);
- if (Bugzilla->params->{"commentoncreate"} && !$comment) {
- ThrowUserError("description_required");
- }
+ # Creation-only checks
+ if (!ref $invocant) {
+ if (Bugzilla->params->{"commentoncreate"} && !$comment) {
+ ThrowUserError("description_required");
+ }
- # On creation only, there must be a single-space comment, or
- # email will be supressed.
- $comment = ' ' if $comment eq '' && !ref($invocant);
+ # On creation only, there must be a single-space comment, or
+ # email will be supressed.
+ $comment = ' ' if $comment eq '';
+ }
return $comment;
}
@@ -699,12 +707,19 @@ sub _check_keywords {
sub _check_product {
my ($invocant, $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 });
+ my $obj;
+ if (ref $invocant) {
+ $obj = Bugzilla::Product::check_product($name);
+ }
+ else {
+ # 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.
+ $obj = new Bugzilla::Product({ name => $name });
+ }
+
return $obj;
}
@@ -717,7 +732,7 @@ sub _check_op_sys {
sub _check_priority {
my ($invocant, $priority) = @_;
- if (!Bugzilla->params->{'letsubmitterchoosepriority'}) {
+ if (!ref $invocant && !Bugzilla->params->{'letsubmitterchoosepriority'}) {
$priority = Bugzilla->params->{'defaultpriority'};
}
$priority = trim($priority);
@@ -784,6 +799,9 @@ sub _check_strict_isolation {
sub _check_target_milestone {
my ($invocant, $product, $target) = @_;
$target = trim($target);
+ if (ref $invocant) {
+ return undef if !Bugzilla->params->{'usetargetmilestone'};
+ }
$target = $product->default_milestone if !defined $target;
check_field('target_milestone', $target,
[map($_->name, @{$product->milestones})]);
diff --git a/process_bug.cgi b/process_bug.cgi
index 6c0384973..00ff4c759 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -151,10 +151,8 @@ if (defined $cgi->param('id')) {
# Make sure there are bugs to process.
scalar(@idlist) || ThrowUserError("no_bugs_chosen");
-# Build a bug object using $cgi->param('id') as ID.
-# If there are more than one bug changed at once, the bug object will be
-# empty, which doesn't matter.
-my $bug = new Bugzilla::Bug(scalar $cgi->param('id'));
+# Build a bug object using the first bug id, for validations.
+my $bug = new Bugzilla::Bug($idlist[0]);
# Make sure form param 'dontchange' is defined so it can be compared to easily.
$cgi->param('dontchange','') unless defined $cgi->param('dontchange');
@@ -164,11 +162,10 @@ $cgi->param('knob', 'none') unless defined $cgi->param('knob');
# Validate all timetracking fields
foreach my $field ("estimated_time", "work_time", "remaining_time") {
- if (defined $cgi->param($field)) {
- my $er_time = trim($cgi->param($field));
- if ($er_time ne $cgi->param('dontchange')) {
- Bugzilla::Bug::ValidateTime($er_time, $field);
- }
+ if (defined $cgi->param($field)
+ && $cgi->param($field) ne $cgi->param('dontchange'))
+ {
+ $cgi->param($field, $bug->_check_time($cgi->param($field), $field));
}
}
@@ -179,7 +176,7 @@ if (Bugzilla->user->in_group(Bugzilla->params->{'timetrackinggroup'})) {
}
}
-ValidateComment(scalar $cgi->param('comment'));
+$cgi->param('comment', $bug->_check_comment($cgi->param('comment')));
# If the bug(s) being modified have dependencies, validate them
# and rebuild the list with the validated values. This is important
@@ -520,30 +517,33 @@ if (defined $cgi->param('id')) {
# values that have been changed instead of submitting all the new values.
# (XXX those error checks need to happen too, but implementing them
# is more work in the current architecture of this script...)
- my $prod_obj = Bugzilla::Product::check_product($cgi->param('product'));
- check_field('component', scalar $cgi->param('component'),
- [map($_->name, @{$prod_obj->components})]);
- check_field('version', scalar $cgi->param('version'),
- [map($_->name, @{$prod_obj->versions})]);
- if ( Bugzilla->params->{"usetargetmilestone"} ) {
- check_field('target_milestone', scalar $cgi->param('target_milestone'),
- [map($_->name, @{$prod_obj->milestones})]);
- }
- check_field('rep_platform', scalar $cgi->param('rep_platform'));
- check_field('op_sys', scalar $cgi->param('op_sys'));
- check_field('priority', scalar $cgi->param('priority'));
- check_field('bug_severity', scalar $cgi->param('bug_severity'));
-
- # Those fields only have to exist. We don't validate their value here.
- foreach my $field_name ('bug_file_loc', 'short_desc', 'longdesclength') {
- defined($cgi->param($field_name))
- || ThrowCodeError('undefined_field', { field => $field_name });
- }
- $cgi->param('short_desc', clean_text($cgi->param('short_desc')));
- if (trim($cgi->param('short_desc')) eq "") {
- ThrowUserError("require_summary");
- }
+ my $prod = $bug->_check_product($cgi->param('product'));
+ $cgi->param('product', $prod->name);
+
+ my $comp = $bug->_check_component($prod,
+ scalar $cgi->param('component'));
+ $cgi->param('component', $comp->name);
+
+ $cgi->param('version', $bug->_check_version($prod,
+ scalar $cgi->param('version')));
+ $cgi->param('target_milestone', $bug->_check_target_milestone($prod,
+ scalar $cgi->param('target_milestone')));
+ $cgi->param('rep_platform',
+ $bug->_check_rep_platform($cgi->param('rep_platform')));
+ $cgi->param('op_sys',
+ $bug->_check_op_sys($cgi->param('op_sys')));
+ $cgi->param('priority',
+ $bug->_check_priority($cgi->param('priority')));
+ $cgi->param('bug_severity',
+ $bug->_check_bug_severity($cgi->param('bug_severity')));
+
+ ThrowCodeError('undefined_field', { field => 'longdesclength' })
+ if !defined $cgi->param('longdesclength');
+ $cgi->param('bug_file_loc',
+ $bug->_check_bug_file_loc($cgi->param('bug_file_loc')));
+ $cgi->param('short_desc',
+ $bug->_check_short_desc($cgi->param('short_desc')));
}
my $action = trim($cgi->param('action') || '');