summaryrefslogtreecommitdiffstats
path: root/Bugzilla
diff options
context:
space:
mode:
Diffstat (limited to 'Bugzilla')
-rw-r--r--Bugzilla/Bug.pm225
-rw-r--r--Bugzilla/Comment.pm105
-rw-r--r--Bugzilla/Object.pm63
3 files changed, 235 insertions, 158 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 327e55ccc..70f3f607d 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -26,6 +26,7 @@
# Frédéric Buclin <LpSolit@gmail.com>
# Lance Larsh <lance.larsh@oracle.com>
# Elliotte Martin <elliotte_martin@yahoo.com>
+# Christian Legnitto <clegnitto@mozilla.com>
package Bugzilla::Bug;
@@ -125,11 +126,11 @@ sub VALIDATORS {
bug_status => \&_check_bug_status,
cc => \&_check_cc,
comment => \&_check_comment,
- commentprivacy => \&_check_commentprivacy,
component => \&_check_component,
+ creation_ts => \&_check_creation_ts,
deadline => \&_check_deadline,
dup_id => \&_check_dup_id,
- estimated_time => \&_check_estimated_time,
+ estimated_time => \&Bugzilla::Object::check_time,
everconfirmed => \&Bugzilla::Object::check_boolean,
groups => \&_check_groups,
keywords => \&_check_keywords,
@@ -137,7 +138,7 @@ sub VALIDATORS {
priority => \&_check_priority,
product => \&_check_product,
qa_contact => \&_check_qa_contact,
- remaining_time => \&_check_remaining_time,
+ remaining_time => \&Bugzilla::Object::check_time,
rep_platform => \&_check_select_field,
resolution => \&_check_resolution,
short_desc => \&_check_short_desc,
@@ -185,6 +186,7 @@ sub VALIDATOR_DEPENDENCIES {
assigned_to => ['component'],
bug_status => ['product', 'comment', 'target_milestone'],
cc => ['component'],
+ comment => ['creation_ts'],
component => ['product'],
dup_id => ['bug_status', 'resolution'],
groups => ['product'],
@@ -246,16 +248,6 @@ sub DATE_COLUMNS {
return map { $_->name } @fields;
}
-# This is used by add_comment to know what we validate before putting in
-# the DB.
-use constant UPDATE_COMMENT_COLUMNS => qw(
- thetext
- work_time
- type
- extra_data
- isprivate
-);
-
# Used in LogActivityEntry(). Gives the max length of lines in the
# activity table.
use constant MAX_LINE_LENGTH => 254;
@@ -292,6 +284,9 @@ use constant REQUIRED_FIELD_MAP => {
component_id => 'component',
};
+# Creation timestamp is here because it needs to be validated
+# but it can be NULL in the database (see comments in create above)
+#
# Target Milestone is here because it has a default that the validator
# creates (product.defaultmilestone) that is different from the database
# default.
@@ -305,7 +300,7 @@ use constant REQUIRED_FIELD_MAP => {
#
# Groups are in a separate table, but must always be validated so that
# mandatory groups get set on bugs.
-use constant EXTRA_REQUIRED_FIELDS => qw(target_milestone cc qa_contact groups);
+use constant EXTRA_REQUIRED_FIELDS => qw(creation_ts target_milestone cc qa_contact groups);
#####################################################################
@@ -615,14 +610,12 @@ sub create {
# These are not a fields in the bugs table, so we don't pass them to
# insert_create_data.
- my $cc_ids = delete $params->{cc};
- my $groups = delete $params->{groups};
- my $depends_on = delete $params->{dependson};
- my $blocked = delete $params->{blocked};
- my $keywords = delete $params->{keywords};
- my ($comment, $privacy) = ($params->{comment}, $params->{commentprivacy});
- delete $params->{comment};
- delete $params->{commentprivacy};
+ my $cc_ids = delete $params->{cc};
+ my $groups = delete $params->{groups};
+ my $depends_on = delete $params->{dependson};
+ my $blocked = delete $params->{blocked};
+ my $keywords = delete $params->{keywords};
+ my $creation_comment = delete $params->{comment};
# We don't want the bug to appear in the system until it's correctly
# protected by groups.
@@ -686,20 +679,14 @@ sub create {
}
}
- # And insert the comment. We always insert a comment on bug creation,
+ # Comment #0 handling...
+
+ # We now have a bug id so we can fill this out
+ $creation_comment->{'bug_id'} = $bug->id;
+
+ # Insert the comment. We always insert a comment on bug creation,
# but sometimes it's blank.
- my @columns = qw(bug_id who bug_when thetext);
- my @values = ($bug->bug_id, $bug->{reporter_id}, $timestamp, $comment);
- # We don't include the "isprivate" column unless it was specified.
- # This allows it to fall back to its database default.
- if (defined $privacy) {
- push(@columns, 'isprivate');
- push(@values, $privacy);
- }
- my $qmarks = "?," x @columns;
- chop($qmarks);
- $dbh->do('INSERT INTO longdescs (' . join(',', @columns) . ")
- VALUES ($qmarks)", undef, @values);
+ Bugzilla::Comment->insert_create_data($creation_comment);
Bugzilla::Hook::process('bug_end_of_create', { bug => $bug,
timestamp => $timestamp,
@@ -726,8 +713,6 @@ sub run_create_validators {
# Callers cannot set reporter, creation_ts, or delta_ts.
$params->{reporter} = $class->_check_reporter();
- $params->{creation_ts} =
- Bugzilla->dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
$params->{delta_ts} = $params->{creation_ts};
if ($params->{estimated_time}) {
@@ -903,26 +888,21 @@ sub update {
# Comments
foreach my $comment (@{$self->{added_comments} || []}) {
- my $columns = join(',', keys %$comment);
- my @values = values %$comment;
- my $qmarks = join(',', ('?') x @values);
- $dbh->do("INSERT INTO longdescs (bug_id, who, bug_when, $columns)
- VALUES (?,?,?,$qmarks)", undef,
- $self->bug_id, Bugzilla->user->id, $delta_ts, @values);
+ $comment = Bugzilla::Comment->insert_create_data($comment);
if ($comment->{work_time}) {
LogActivityEntry($self->id, "work_time", "", $comment->{work_time},
Bugzilla->user->id, $delta_ts);
}
}
-
+
# Comment Privacy
- foreach my $comment_id (keys %{$self->{comment_isprivate} || {}}) {
- $dbh->do("UPDATE longdescs SET isprivate = ? WHERE comment_id = ?",
- undef, $self->{comment_isprivate}->{$comment_id}, $comment_id);
+ foreach my $comment (@{$self->{comment_isprivate} || []}) {
+ $comment->update();
+
my ($from, $to)
- = $self->{comment_isprivate}->{$comment_id} ? (0, 1) : (1, 0);
+ = $comment->is_private ? (0, 1) : (1, 0);
LogActivityEntry($self->id, "longdescs.isprivate", $from, $to,
- Bugzilla->user->id, $delta_ts, $comment_id);
+ Bugzilla->user->id, $delta_ts, $comment->id);
}
# Insert the values into the multiselect value tables
@@ -1417,33 +1397,43 @@ sub _check_cc {
}
sub _check_comment {
- my ($invocant, $comment) = @_;
+ my ($invocant, $comment_txt, undef, $params) = @_;
- $comment = '' unless defined $comment;
+ # Comment can be empty. We should force it to be empty if the text is undef
+ if (!defined $comment_txt) {
+ $comment_txt = '';
+ }
- # 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.
+ # Load up some data
+ my $isprivate = $params->{commentprivacy};
+ my $timestamp = $params->{creation_ts};
- ThrowUserError('comment_too_long') if length($comment) > MAX_COMMENT_LENGTH;
- return $comment;
-}
+ # Create the new comment so we can check it
+ my $comment = {
+ thetext => $comment_txt,
+ bug_when => $timestamp,
+ };
-sub _check_commentprivacy {
- my ($invocant, $comment_privacy) = @_;
- if ($comment_privacy && !Bugzilla->user->is_insider) {
- ThrowUserError('user_not_insider');
+ # We don't include the "isprivate" column unless it was specified.
+ # This allows it to fall back to its database default.
+ if (defined $isprivate) {
+ $comment->{isprivate} = $isprivate;
}
- return $comment_privacy ? 1 : 0;
-}
-sub _check_comment_type {
- my ($invocant, $type) = @_;
- detaint_natural($type)
- || ThrowCodeError('bad_arg', { argument => 'type',
- function => caller });
- return $type;
+ # Don't need this anymore as it is now in the comment hash
+ delete $params->{commentprivacy};
+
+ # Validate comment. We have to do this special as a comment normally
+ # requires a bug to be already created. For a new bug, the first comment
+ # obviously can't get the bug if the bug is created after this
+ # (see bug 590334)
+ Bugzilla::Comment->check_required_create_fields($comment);
+ $comment = Bugzilla::Comment->run_create_validators($comment,
+ { skip => ['bug_id'] }
+ );
+
+ return $comment;
+
}
sub _check_component {
@@ -1456,6 +1446,10 @@ sub _check_component {
return $obj;
}
+sub _check_creation_ts {
+ return Bugzilla->dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
+}
+
sub _check_deadline {
my ($invocant, $date) = @_;
@@ -1605,10 +1599,6 @@ sub _check_dup_id {
return $dupe_of;
}
-sub _check_estimated_time {
- return $_[0]->_check_time($_[1], 'estimated_time');
-}
-
sub _check_groups {
my ($invocant, $group_names, undef, $params) = @_;
my $product = blessed($invocant) ? $invocant->product_obj
@@ -1728,10 +1718,6 @@ sub _check_qa_contact {
return $id || undef;
}
-sub _check_remaining_time {
- return $_[0]->_check_time($_[1], 'remaining_time');
-}
-
sub _check_reporter {
my $invocant = shift;
my $reporter;
@@ -1886,20 +1872,6 @@ sub _check_target_milestone {
return $object->name;
}
-sub _check_time {
- my ($invocant, $time, $field) = @_;
-
- my $current = 0;
- if (ref $invocant && $field ne 'work_time') {
- $current = $invocant->$field;
- }
- return $current unless Bugzilla->user->is_timetracker;
-
- $time = trim($time) || 0;
- ValidateTime($time, $field);
- return $time;
-}
-
sub _check_version {
my ($invocant, $version, undef, $params) = @_;
$version = trim($version);
@@ -1910,10 +1882,6 @@ sub _check_version {
return $object->name;
}
-sub _check_work_time {
- return $_[0]->_check_time($_[1], 'work_time');
-}
-
# Custom Field Validators
sub _check_field_is_mandatory {
@@ -2265,8 +2233,9 @@ sub set_comment_is_private {
$isprivate = $isprivate ? 1 : 0;
if ($isprivate != $comment->is_private) {
- $self->{comment_isprivate} ||= {};
- $self->{comment_isprivate}->{$comment_id} = $isprivate;
+ $self->{comment_isprivate} ||= [];
+ $comment->set_is_private($isprivate);
+ push @{$self->{comment_isprivate}}, $comment;
}
}
sub set_component {
@@ -2636,23 +2605,22 @@ sub remove_cc {
sub add_comment {
my ($self, $comment, $params) = @_;
- $comment = $self->_check_comment($comment);
-
$params ||= {};
- $params->{work_time} = $self->_check_work_time($params->{work_time});
- if (exists $params->{type}) {
- $params->{type} = $self->_check_comment_type($params->{type});
- }
- if (exists $params->{isprivate}) {
- $params->{isprivate} =
- $self->_check_commentprivacy($params->{isprivate});
- }
- # XXX We really should check extra_data, too.
+ # This makes it so we won't create new comments when there is nothing
+ # to add
if ($comment eq '' && !($params->{type} || abs($params->{work_time}))) {
return;
}
+ # Fill out info that doesn't change and callers may not pass in
+ $params->{'bug_id'} = $self;
+ $params->{'thetext'} = $comment;
+
+ # Validate all the entered data
+ Bugzilla::Comment->check_required_create_fields($params);
+ $params = Bugzilla::Comment->run_create_validators($params);
+
# If the user has explicitly set remaining_time, this will be overridden
# later in set_all. But if they haven't, this keeps remaining_time
# up-to-date.
@@ -2660,23 +2628,9 @@ sub add_comment {
$self->set_remaining_time(max($self->remaining_time - $params->{work_time}, 0));
}
- # So we really want to comment. Make sure we are allowed to do so.
- my $privs;
- $self->check_can_change_field('longdesc', 0, 1, \$privs)
- || ThrowUserError('illegal_change', { field => 'longdesc', privs => $privs });
-
$self->{added_comments} ||= [];
- my $add_comment = dclone($params);
- $add_comment->{thetext} = $comment;
- # We only want to trick_taint fields that we know about--we don't
- # want to accidentally let somebody set some field that's not OK
- # to set!
- foreach my $field (UPDATE_COMMENT_COLUMNS) {
- trick_taint($add_comment->{$field}) if defined $add_comment->{$field};
- }
-
- push(@{$self->{added_comments}}, $add_comment);
+ push(@{$self->{added_comments}}, $params);
}
# There was a lot of duplicate code when I wrote this as three separate
@@ -3638,29 +3592,6 @@ sub EmitDependList {
return $list_ref;
}
-sub ValidateTime {
- my ($time, $field) = @_;
-
- # regexp verifies one or more digits, optionally followed by a period and
- # zero or more digits, OR we have a period followed by one or more digits
- # (allow negatives, though, so people can back out errors in time reporting)
- if ($time !~ /^-?(?:\d+(?:\.\d*)?|\.\d+)$/) {
- ThrowUserError("number_not_numeric",
- {field => "$field", num => "$time"});
- }
-
- # Only the "work_time" field is allowed to contain a negative value.
- if ( ($time < 0) && ($field ne "work_time") ) {
- ThrowUserError("number_too_small",
- {field => "$field", num => "$time", min_num => "0"});
- }
-
- if ($time > 99999.99) {
- ThrowUserError("number_too_large",
- {field => "$field", num => "$time", max_num => "99999.99"});
- }
-}
-
# Get the activity of a bug, starting from $starttime (if given).
# This routine assumes Bugzilla::Bug->check has been previously called.
sub GetBugActivity {
diff --git a/Bugzilla/Comment.pm b/Bugzilla/Comment.pm
index b4f94e213..e8bdb4e89 100644
--- a/Bugzilla/Comment.pm
+++ b/Bugzilla/Comment.pm
@@ -17,6 +17,7 @@
# All rights reserved.
#
# Contributor(s): James Robson <arbingersys@gmail.com>
+# Christian Legnitto <clegnitto@mozilla.com>
use strict;
@@ -50,6 +51,7 @@ use constant DB_COLUMNS => qw(
);
use constant UPDATE_COLUMNS => qw(
+ isprivate
type
extra_data
);
@@ -62,12 +64,21 @@ use constant ID_FIELD => 'comment_id';
use constant LIST_ORDER => 'bug_when, comment_id';
use constant VALIDATORS => {
- extra_data => \&_check_extra_data,
- type => \&_check_type,
+ bug_id => \&_check_bug_id,
+ who => \&_check_who,
+ bug_when => \&_check_bug_when,
+ work_time => \&_check_work_time,
+ thetext => \&_check_thetext,
+ isprivate => \&_check_isprivate,
+ extra_data => \&_check_extra_data,
+ type => \&_check_type,
};
use constant VALIDATOR_DEPENDENCIES => {
extra_data => ['type'],
+ bug_id => ['who'],
+ work_time => ['who'],
+ isprivate => ['who'],
};
#########################
@@ -157,12 +168,9 @@ sub body_full {
# Mutators #
############
-sub set_extra_data { $_[0]->set('extra_data', $_[1]); }
-
-sub set_type {
- my ($self, $type) = @_;
- $self->set('type', $type);
-}
+sub set_is_private { $_[0]->set('isprivate', $_[1]); }
+sub set_type { $_[0]->set('type', $_[1]); }
+sub set_extra_data { $_[0]->set('extra_data', $_[1]); }
##############
# Validators #
@@ -209,6 +217,87 @@ sub _check_type {
return $type;
}
+sub _check_bug_id {
+ my ($invocant, $bug_id) = @_;
+
+ ThrowCodeError('param_required', {function => 'Bugzilla::Comment->create',
+ param => 'bug_id'}) unless $bug_id;
+
+ my $bug;
+ if (blessed $bug_id) {
+ # We got a bug object passed in, use it
+ $bug = $bug_id;
+ $bug->check_is_visible;
+ }
+ else {
+ # We got a bug id passed in, check it and get the bug object
+ $bug = Bugzilla::Bug->check({ id => $bug_id });
+ }
+
+ # Make sure the user can edit the product
+ Bugzilla->user->can_edit_product($bug->{product_id});
+
+ # Make sure the user can comment
+ my $privs;
+ $bug->check_can_change_field('longdesc', 0, 1, \$privs)
+ || ThrowUserError('illegal_change',
+ { field => 'longdesc', privs => $privs });
+ return $bug->id;
+}
+
+sub _check_who {
+ my ($invocant, $who) = @_;
+ Bugzilla->login(LOGIN_REQUIRED);
+ return Bugzilla->user->id;
+}
+
+sub _check_bug_when {
+ my ($invocant, $when) = @_;
+
+ # Make sure the timestamp is defined, default to a timestamp from the db
+ if (!defined $when) {
+ $when = Bugzilla->dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
+ }
+
+ # Make sure the timestamp parses
+ if (!datetime_from($when)) {
+ ThrowCodeError('invalid_timestamp', { timestamp => $when });
+ }
+
+ return $when;
+}
+
+sub _check_work_time {
+ my ($invocant, $value_in, $field, $params) = @_;
+
+ # Call down to Bugzilla::Object, letting it know negative
+ # values are ok
+ return $invocant->check_time( $value_in, $field, $params, 1);
+}
+
+sub _check_thetext {
+ my ($invocant, $thetext) = @_;
+
+ ThrowCodeError('param_required',{function => 'Bugzilla::Comment->create',
+ param => 'thetext'}) unless defined $thetext;
+
+ # Remove any trailing whitespace. Leading whitespace could be
+ # a valid part of the comment.
+ $thetext =~ s/\s*$//s;
+ $thetext =~ s/\r\n?/\n/g; # Get rid of \r.
+
+ ThrowUserError('comment_too_long') if length($thetext) > MAX_COMMENT_LENGTH;
+ return $thetext;
+}
+
+sub _check_isprivate {
+ my ($invocant, $isprivate) = @_;
+ if ($isprivate && !Bugzilla->user->is_insider) {
+ ThrowUserError('user_not_insider');
+ }
+ return $isprivate ? 1 : 0;
+}
+
sub count {
my ($self) = @_;
diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm
index 5b8576512..281143889 100644
--- a/Bugzilla/Object.pm
+++ b/Bugzilla/Object.pm
@@ -30,6 +30,7 @@ use Bugzilla::Error;
use Date::Parse;
use List::MoreUtils qw(part);
+use Scalar::Util qw(blessed);
use constant NAME_FIELD => 'name';
use constant ID_FIELD => 'id';
@@ -462,13 +463,21 @@ sub check_required_create_fields {
}
sub run_create_validators {
- my ($class, $params) = @_;
+ my ($class, $params, $options) = @_;
my $validators = $class->_get_validators;
my %field_values = %$params;
+ # Make a hash skiplist for easier searching later
+ my %skip_list = map { $_ => 1 } @{ $options->{skip} || [] };
+
+ # Get the sorted field names
my @sorted_names = $class->_sort_by_dep(keys %field_values);
- foreach my $field (@sorted_names) {
+
+ # Remove the skipped names
+ my @unskipped = grep { !$skip_list{$_} } @sorted_names;
+
+ foreach my $field (@unskipped) {
my $value;
if (exists $validators->{$field}) {
my $validator = $validators->{$field};
@@ -527,10 +536,54 @@ sub get_all {
sub check_boolean { return $_[1] ? 1 : 0 }
+sub check_time {
+ my ($invocant, $value, $field, $params, $allow_negative) = @_;
+
+ # If we don't have a current value default to zero
+ my $current = blessed($invocant) ? $invocant->{$field}
+ : 0;
+ $current ||= 0;
+
+ # Don't let the user set the value if they aren't a timetracker
+ return $current unless Bugzilla->user->is_timetracker;
+
+ # Get the new value or zero if it isn't defined
+ $value = trim($value) || 0;
+
+ # Make sure the new value is well formed
+ _validate_time($value, $field, $allow_negative);
+
+ return $value;
+}
+
+
###################
# General Helpers #
###################
+sub _validate_time {
+ my ($time, $field, $allow_negative) = @_;
+
+ # regexp verifies one or more digits, optionally followed by a period and
+ # zero or more digits, OR we have a period followed by one or more digits
+ # (allow negatives, though, so people can back out errors in time reporting)
+ if ($time !~ /^-?(?:\d+(?:\.\d*)?|\.\d+)$/) {
+ ThrowUserError("number_not_numeric",
+ {field => $field, num => "$time"});
+ }
+
+ # Callers can optionally allow negative times
+ if ( ($time < 0) && !$allow_negative ) {
+ ThrowUserError("number_too_small",
+ {field => $field, num => "$time", min_num => "0"});
+ }
+
+ if ($time > 99999.99) {
+ ThrowUserError("number_too_large",
+ {field => $field, num => "$time", max_num => "99999.99"});
+ }
+}
+
# Sorts fields according to VALIDATOR_DEPENDENCIES. This is not a
# traditional topological sort, because a "dependency" does not
# *have* to be in the list--it just has to be earlier than its dependent
@@ -1036,7 +1089,11 @@ Description: Runs the validation of input parameters for L</create>.
of their input parameters. This method is B<only> called
by L</create>.
-Params: The same as L</create>.
+Params: C<$params> - hashref - A value to put in each database
+ field for this object.
+ C<$options> - hashref - Processing options. Currently
+ the only option supported is B<skip>, which can be
+ used to specify a list of fields to not validate.
Returns: A hash, in a similar format as C<$params>, except that
these are the values to be inserted into the database,