diff options
| -rwxr-xr-x | Bugzilla/Bug.pm | 70 | ||||
| -rw-r--r-- | Bugzilla/Object.pm | 12 | ||||
| -rwxr-xr-x | process_bug.cgi | 67 | ||||
| -rw-r--r-- | template/en/default/global/messages.html.tmpl | 4 |
4 files changed, 79 insertions, 74 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index ec2fd491a..de1884b7f 100755 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -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'})) { diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm index 3da4b9379..4d54b04e1 100644 --- a/Bugzilla/Object.pm +++ b/Bugzilla/Object.pm @@ -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 diff --git a/process_bug.cgi b/process_bug.cgi index dcc0d6543..f8b5201b3 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -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); diff --git a/template/en/default/global/messages.html.tmpl b/template/en/default/global/messages.html.tmpl index 2b6621069..898ade60d 100644 --- a/template/en/default/global/messages.html.tmpl +++ b/template/en/default/global/messages.html.tmpl @@ -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 |