зеркало из https://github.com/mozilla/pjs.git
Bug 388149: Move updating of time-tracking fields into Bugzilla::Bug
Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit
This commit is contained in:
mkanat%bugzilla.org
Родитель
c18706297e
Коммит
91dfa00dac
|
|
@ -160,12 +160,15 @@ sub UPDATE_COLUMNS {
|
|||
my @columns = qw(
|
||||
alias
|
||||
cclist_accessible
|
||||
deadline
|
||||
estimated_time
|
||||
everconfirmed
|
||||
bug_file_loc
|
||||
bug_severity
|
||||
bug_status
|
||||
op_sys
|
||||
priority
|
||||
remaining_time
|
||||
rep_platform
|
||||
reporter_accessible
|
||||
resolution
|
||||
|
|
@ -176,6 +179,11 @@ sub UPDATE_COLUMNS {
|
|||
return @columns;
|
||||
};
|
||||
|
||||
use constant NUMERIC_COLUMNS => qw(
|
||||
estimated_time
|
||||
remaining_time
|
||||
);
|
||||
|
||||
# This is used by add_comment to know what we validate before putting in
|
||||
# the DB.
|
||||
use constant UPDATE_COMMENT_COLUMNS => qw(
|
||||
|
|
@ -860,10 +868,15 @@ sub _check_component {
|
|||
|
||||
sub _check_deadline {
|
||||
my ($invocant, $date) = @_;
|
||||
$date = trim($date);
|
||||
|
||||
# Check time-tracking permissions.
|
||||
my $tt_group = Bugzilla->params->{"timetrackinggroup"};
|
||||
return undef unless $date && $tt_group
|
||||
&& Bugzilla->user->in_group($tt_group);
|
||||
my $current = ref $invocant ? $invocant->deadline : undef;
|
||||
return $current unless $tt_group && Bugzilla->user->in_group($tt_group);
|
||||
|
||||
# Validate entered deadline
|
||||
$date = trim($date);
|
||||
return undef if !$date;
|
||||
validate_date($date)
|
||||
|| ThrowUserError('illegal_date', { date => $date,
|
||||
format => 'YYYY-MM-DD' });
|
||||
|
|
@ -1115,8 +1128,14 @@ sub _check_target_milestone {
|
|||
|
||||
sub _check_time {
|
||||
my ($invocant, $time, $field) = @_;
|
||||
|
||||
my $current = 0;
|
||||
if (ref $invocant && $field ne 'work_time') {
|
||||
$current = $invocant->$field;
|
||||
}
|
||||
my $tt_group = Bugzilla->params->{"timetrackinggroup"};
|
||||
return 0 unless $tt_group && Bugzilla->user->in_group($tt_group);
|
||||
return $current unless $tt_group && Bugzilla->user->in_group($tt_group);
|
||||
|
||||
$time = trim($time) || 0;
|
||||
ValidateTime($time, $field);
|
||||
return $time;
|
||||
|
|
@ -1217,6 +1236,7 @@ sub set_custom_field {
|
|||
ThrowCodeError('field_not_custom', { field => $field }) if !$field->custom;
|
||||
$self->set($field->name, $value);
|
||||
}
|
||||
sub set_deadline { $_[0]->set('deadline', $_[1]); }
|
||||
sub set_dependencies {
|
||||
my ($self, $dependson, $blocked) = @_;
|
||||
($dependson, $blocked) = $self->_check_dependencies($dependson, $blocked);
|
||||
|
|
@ -1227,10 +1247,14 @@ sub set_dependencies {
|
|||
$self->{'dependson'} = $dependson;
|
||||
$self->{'blocked'} = $blocked;
|
||||
}
|
||||
sub set_estimated_time { $_[0]->set('estimated_time', $_[1]); }
|
||||
sub _set_everconfirmed { $_[0]->set('everconfirmed', $_[1]); }
|
||||
sub set_op_sys { $_[0]->set('op_sys', $_[1]); }
|
||||
sub set_platform { $_[0]->set('rep_platform', $_[1]); }
|
||||
sub set_priority { $_[0]->set('priority', $_[1]); }
|
||||
sub set_remaining_time { $_[0]->set('remaining_time', $_[1]); }
|
||||
# Used only when closing a bug or moving between closed states.
|
||||
sub _zero_remaining_time { $_[0]->{'remaining_time'} = 0; }
|
||||
sub set_reporter_accessible { $_[0]->set('reporter_accessible', $_[1]); }
|
||||
sub set_resolution { $_[0]->set('resolution', $_[1]); }
|
||||
sub set_severity { $_[0]->set('bug_severity', $_[1]); }
|
||||
|
|
@ -1905,10 +1929,11 @@ sub check_status_transition {
|
|||
# Make sure all checks triggered by the workflow are successful.
|
||||
# Some are hardcoded and come from older versions of Bugzilla.
|
||||
sub check_status_change_triggers {
|
||||
my ($self, $action, $bug_ids, $vars) = @_;
|
||||
my ($self, $action, $bugs, $vars) = @_;
|
||||
my $dbh = Bugzilla->dbh;
|
||||
$vars ||= {};
|
||||
|
||||
my @bug_ids = map {$_->id} @$bugs;
|
||||
# First, make sure no comment is required if there is none.
|
||||
# If a comment is given, then this check is useless.
|
||||
if (!$vars->{comment_exists}) {
|
||||
|
|
@ -1916,8 +1941,8 @@ sub check_status_change_triggers {
|
|||
# 'commentonnone' doesn't exist, so this is safe.
|
||||
ThrowUserError('comment_required') if Bugzilla->params->{"commenton$action"};
|
||||
}
|
||||
elsif (!scalar(@$bug_ids)) {
|
||||
# The bug is being created; that's why $bug_ids is undefined.
|
||||
elsif (!scalar @bug_ids) {
|
||||
# The bug is being created; that's why @bug_ids is undefined.
|
||||
my $comment_required =
|
||||
$dbh->selectrow_array('SELECT require_comment
|
||||
FROM status_workflow
|
||||
|
|
@ -1939,7 +1964,7 @@ sub check_status_change_triggers {
|
|||
ON bug_status.id = old_status
|
||||
INNER JOIN bug_status b_s
|
||||
ON b_s.id = new_status
|
||||
WHERE bug_id IN (' . join (',', @$bug_ids). ')
|
||||
WHERE bug_id IN (' . join (',', @bug_ids). ')
|
||||
AND b_s.value = ?
|
||||
AND require_comment = 1',
|
||||
undef, $action);
|
||||
|
|
@ -1957,7 +1982,7 @@ sub check_status_change_triggers {
|
|||
|
||||
# Also leave now if we are creating a new bug (we only want to check
|
||||
# if a comment is required on bug creation).
|
||||
return unless scalar(@$bug_ids);
|
||||
return unless scalar @bug_ids;
|
||||
|
||||
if ($action eq 'duplicate') {
|
||||
# You cannot mark bugs as duplicates when changing
|
||||
|
|
@ -1995,14 +2020,15 @@ sub check_status_change_triggers {
|
|||
$vars->{DuplicateUserConfirm} = 1;
|
||||
|
||||
# DUPLICATE bugs should have no time remaining.
|
||||
$vars->{remove_remaining_time} = 1;
|
||||
$_->_zero_remaining_time() foreach @$bugs;
|
||||
$vars->{'message'} = "remaining_time_zeroed";
|
||||
}
|
||||
elsif ($action eq 'change_resolution' || !is_open_state($action)) {
|
||||
# don't resolve as fixed while still unresolved blocking bugs
|
||||
if (Bugzilla->params->{"noresolveonopenblockers"}
|
||||
&& $vars->{resolution} eq 'FIXED')
|
||||
{
|
||||
my @dependencies = Bugzilla::Bug::CountOpenDependencies(@$bug_ids);
|
||||
my @dependencies = Bugzilla::Bug::CountOpenDependencies(@bug_ids);
|
||||
if (scalar @dependencies > 0) {
|
||||
ThrowUserError("still_unresolved_bugs",
|
||||
{ dependencies => \@dependencies,
|
||||
|
|
@ -2013,7 +2039,7 @@ sub check_status_change_triggers {
|
|||
# You cannot use change_resolution if there is at least one open bug
|
||||
# nor can you close open bugs if no resolution is given.
|
||||
my $open_states = join(',', map {$dbh->quote($_)} BUG_STATE_OPEN);
|
||||
my $idlist = join(',', @$bug_ids);
|
||||
my $idlist = join(',', @bug_ids);
|
||||
my $is_open =
|
||||
$dbh->selectrow_array("SELECT 1 FROM bugs WHERE bug_id IN ($idlist)
|
||||
AND bug_status IN ($open_states)");
|
||||
|
|
@ -2026,7 +2052,14 @@ sub check_status_change_triggers {
|
|||
check_field('resolution', $vars->{resolution},
|
||||
Bugzilla::Bug->settable_resolutions) if $vars->{resolution};
|
||||
|
||||
$vars->{remove_remaining_time} = 1 if ($action ne 'change_resolution');
|
||||
if ($action ne 'change_resolution') {
|
||||
foreach my $b (@$bugs) {
|
||||
if ($b->bug_status ne $action) {
|
||||
$b->_zero_remaining_time;
|
||||
$vars->{'message'} = "remaining_time_zeroed";
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
elsif ($action eq 'ASSIGNED'
|
||||
&& Bugzilla->params->{"usetargetmilestone"}
|
||||
|
|
@ -2652,7 +2685,7 @@ sub check_can_change_field {
|
|||
return 1;
|
||||
# numeric fields need to be compared using ==
|
||||
} elsif (($field eq 'estimated_time' || $field eq 'remaining_time')
|
||||
&& $newvalue ne $data->{'dontchange'}
|
||||
&& (!$data || $newvalue ne $data->{'dontchange'})
|
||||
&& $oldvalue == $newvalue)
|
||||
{
|
||||
return 1;
|
||||
|
|
@ -2671,6 +2704,15 @@ sub check_can_change_field {
|
|||
# $PrivilegesRequired = 1 : the reporter, assignee or an empowered user;
|
||||
# $PrivilegesRequired = 2 : the assignee or an empowered user;
|
||||
# $PrivilegesRequired = 3 : an empowered user.
|
||||
|
||||
# Only users in the time-tracking group can change time-tracking fields.
|
||||
if ( grep($_ eq $field, qw(deadline estimated_time remaining_time)) ) {
|
||||
my $tt_group = Bugzilla->params->{timetrackinggroup};
|
||||
if (!$tt_group || !$user->in_group($tt_group)) {
|
||||
$$PrivilegesRequired = 3;
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
|
||||
# Allow anyone with (product-specific) "editbugs" privs to change anything.
|
||||
if ($user->in_group('editbugs', $self->{'product_id'})) {
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ use constant ID_FIELD => 'id';
|
|||
use constant LIST_ORDER => NAME_FIELD;
|
||||
|
||||
use constant UPDATE_VALIDATORS => {};
|
||||
use constant NUMERIC_COLUMNS => ();
|
||||
|
||||
###############################
|
||||
#### Initialization ####
|
||||
|
|
@ -216,6 +217,7 @@ sub update {
|
|||
|
||||
my $old_self = $self->new($self->id);
|
||||
|
||||
my %numeric = map { $_ => 1 } $self->NUMERIC_COLUMNS;
|
||||
my (@update_columns, @values, %changes);
|
||||
foreach my $column ($self->UPDATE_COLUMNS) {
|
||||
my ($old, $new) = ($old_self->{$column}, $self->{$column});
|
||||
|
|
@ -225,7 +227,7 @@ sub update {
|
|||
if (!defined $new || !defined $old) {
|
||||
next if !defined $new && !defined $old;
|
||||
}
|
||||
elsif ($old eq $new) {
|
||||
elsif ( ($numeric{$column} && $old == $new) || $old eq $new ) {
|
||||
next;
|
||||
}
|
||||
|
||||
|
|
@ -445,6 +447,14 @@ A list of columns to update when L</update> is called.
|
|||
If a field can't be changed, it shouldn't be listed here. (For example,
|
||||
the L</ID_FIELD> usually can't be updated.)
|
||||
|
||||
=item C<NUMERIC_COLUMNS>
|
||||
|
||||
When L</update> is called, it compares each column in the object to its
|
||||
current value in the database. It only updates columns that have changed.
|
||||
|
||||
Any column listed in NUMERIC_COLUMNS is treated as a number, not as a string,
|
||||
during these comparisons.
|
||||
|
||||
=back
|
||||
|
||||
=head1 METHODS
|
||||
|
|
|
|||
|
|
@ -144,13 +144,12 @@ $cgi->param('dontchange','') unless defined $cgi->param('dontchange');
|
|||
# Make sure the 'knob' param is defined; else set it to 'none'.
|
||||
$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)
|
||||
&& $cgi->param($field) ne $cgi->param('dontchange'))
|
||||
{
|
||||
$cgi->param($field, $bug->_check_time($cgi->param($field), $field));
|
||||
}
|
||||
# Validate work_time
|
||||
if (defined $cgi->param('work_time')
|
||||
&& $cgi->param('work_time') ne $cgi->param('dontchange'))
|
||||
{
|
||||
$cgi->param('work_time', $bug->_check_time($cgi->param('work_time'),
|
||||
'work_time'));
|
||||
}
|
||||
|
||||
if (Bugzilla->user->in_group(Bugzilla->params->{'timetrackinggroup'})) {
|
||||
|
|
@ -456,7 +455,8 @@ my %methods = (
|
|||
);
|
||||
foreach my $b (@bug_objects) {
|
||||
foreach my $field_name (qw(op_sys rep_platform priority bug_severity
|
||||
bug_file_loc status_whiteboard short_desc))
|
||||
bug_file_loc status_whiteboard short_desc
|
||||
deadline remaining_time estimated_time))
|
||||
{
|
||||
# We only update the field if it's defined and it's not set
|
||||
# to dontchange.
|
||||
|
|
@ -550,22 +550,6 @@ $::comma = "";
|
|||
local our @values;
|
||||
umask(0);
|
||||
|
||||
sub _remove_remaining_time {
|
||||
my $cgi = Bugzilla->cgi;
|
||||
if (Bugzilla->user->in_group(Bugzilla->params->{'timetrackinggroup'})) {
|
||||
if ( defined $cgi->param('remaining_time')
|
||||
&& $cgi->param('remaining_time') > 0 )
|
||||
{
|
||||
$cgi->param('remaining_time', 0);
|
||||
$vars->{'message'} = "remaining_time_zeroed";
|
||||
}
|
||||
}
|
||||
else {
|
||||
DoComma();
|
||||
$::query .= "remaining_time = 0";
|
||||
}
|
||||
}
|
||||
|
||||
sub DoComma {
|
||||
$::query .= "$::comma\n ";
|
||||
$::comma = ",";
|
||||
|
|
@ -857,14 +841,13 @@ $vars->{comment_exists} = comment_exists();
|
|||
$vars->{bug_id} = $cgi->param('id');
|
||||
$vars->{dup_id} = $cgi->param('dup_id');
|
||||
$vars->{resolution} = $cgi->param('resolution') || '';
|
||||
Bugzilla::Bug->check_status_change_triggers($knob, \@idlist, $vars);
|
||||
Bugzilla::Bug->check_status_change_triggers($knob, \@bug_objects, $vars);
|
||||
|
||||
# Some triggers require extra actions.
|
||||
$duplicate = $vars->{dup_id} if ($knob eq 'duplicate');
|
||||
$requiremilestone = $vars->{requiremilestone};
|
||||
# $vars->{DuplicateUserConfirm} is true only if a single bug is being edited.
|
||||
DuplicateUserConfirm($bug, $duplicate) if $vars->{DuplicateUserConfirm};
|
||||
_remove_remaining_time() if $vars->{remove_remaining_time};
|
||||
|
||||
my $any_keyword_changes;
|
||||
if (defined $cgi->param('keywords')) {
|
||||
|
|
@ -885,36 +868,6 @@ if ($::comma eq ""
|
|||
}
|
||||
}
|
||||
|
||||
# Process data for Time Tracking fields
|
||||
if (Bugzilla->user->in_group(Bugzilla->params->{'timetrackinggroup'})) {
|
||||
foreach my $field ("estimated_time", "remaining_time") {
|
||||
if (defined $cgi->param($field)) {
|
||||
my $er_time = trim($cgi->param($field));
|
||||
if ($er_time ne $cgi->param('dontchange')) {
|
||||
DoComma();
|
||||
$::query .= "$field = ?";
|
||||
trick_taint($er_time);
|
||||
push(@values, $er_time);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (defined $cgi->param('deadline')) {
|
||||
DoComma();
|
||||
if ($cgi->param('deadline')) {
|
||||
validate_date($cgi->param('deadline'))
|
||||
|| ThrowUserError('illegal_date', {date => $cgi->param('deadline'),
|
||||
format => 'YYYY-MM-DD'});
|
||||
$::query .= "deadline = ?";
|
||||
my $deadline = $cgi->param('deadline');
|
||||
trick_taint($deadline);
|
||||
push(@values, $deadline);
|
||||
} else {
|
||||
$::query .= "deadline = NULL";
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
my $basequery = $::query;
|
||||
|
||||
local our $delta_ts;
|
||||
|
|
@ -981,7 +934,6 @@ if ($prod_changed && Bugzilla->params->{"strict_isolation"}) {
|
|||
}
|
||||
}
|
||||
|
||||
|
||||
my %bug_objects = map {$_->id => $_} @bug_objects;
|
||||
|
||||
# This loop iterates once for each bug to be processed (i.e. all the
|
||||
|
|
@ -1368,6 +1320,7 @@ foreach my $id (@idlist) {
|
|||
# Bugzilla::Bug does these for us already.
|
||||
next if grep($_ eq $col, qw(keywords op_sys rep_platform priority
|
||||
bug_severity short_desc alias
|
||||
deadline estimated_time remaining_time
|
||||
reporter_accessible cclist_accessible
|
||||
status_whiteboard bug_file_loc),
|
||||
Bugzilla->custom_field_names);
|
||||
|
|
|
|||
|
|
@ -434,8 +434,8 @@
|
|||
|
||||
[% ELSIF message_tag == "remaining_time_zeroed" %]
|
||||
The [% field_descs.remaining_time FILTER html %] field has been
|
||||
set to zero automatically as part of marking this [% terms.bug %]
|
||||
as either RESOLVED or CLOSED.
|
||||
set to zero automatically as part of closing this [% terms.bug %]
|
||||
or moving it from one closed state to another.
|
||||
|
||||
[% ELSIF message_tag == "sanitycheck" %]
|
||||
[%# We use this way to call sanitycheck-specific messages so that
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче