From 30c7fa92ac2d0c76443fcc47907487a9ff0186b2 Mon Sep 17 00:00:00 2001 From: "lpsolit%gmail.com" <> Date: Tue, 12 Feb 2008 07:32:51 +0000 Subject: Bug 384009: Global fields (priority, severity, OS, and platform) are required when using WebService instead of using the defaults - Patch by Frédéric Buclin r/a=mkanat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Bugzilla/Bug.pm | 65 ++++++++++++++++++++++++---------------- Bugzilla/Status.pm | 52 ++------------------------------ Bugzilla/WebService/Bug.pm | 14 +++++---- Bugzilla/WebService/Constants.pm | 1 + email_in.pl | 56 ---------------------------------- 5 files changed, 51 insertions(+), 137 deletions(-) diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 364f5730c..9be3830cf 100755 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -103,13 +103,8 @@ sub DB_COLUMNS { } use constant REQUIRED_CREATE_FIELDS => qw( - bug_severity - comment component - op_sys - priority product - rep_platform short_desc version ); @@ -317,13 +312,25 @@ sub new { # C - For time-tracking. Will be ignored for the same # reasons as C. sub create { - my $class = shift; + my ($class, $params) = @_; my $dbh = Bugzilla->dbh; $dbh->bz_start_transaction(); - $class->check_required_create_fields(@_); - my $params = $class->run_create_validators(@_); + # These fields have default values which we can use if they are undefined. + $params->{bug_severity} = Bugzilla->params->{defaultseverity} + unless defined $params->{bug_severity}; + $params->{priority} = Bugzilla->params->{defaultpriority} + unless defined $params->{priority}; + $params->{op_sys} = Bugzilla->params->{defaultopsys} + unless defined $params->{op_sys}; + $params->{rep_platform} = Bugzilla->params->{defaultplatform} + unless defined $params->{rep_platform}; + # Make sure a comment is always defined. + $params->{comment} = '' unless defined $params->{comment}; + + $class->check_required_create_fields($params); + $params = $class->run_create_validators($params); # These are not a fields in the bugs table, so we don't pass them to # insert_create_data. @@ -905,40 +912,46 @@ sub _check_bug_severity { } sub _check_bug_status { - my ($invocant, $status, $product, $comment) = @_; + my ($invocant, $new_status, $product, $comment) = @_; my $user = Bugzilla->user; - - # Make sure this is a valid status. - my $new_status = ref $status ? $status : Bugzilla::Status->check($status); - + my @valid_statuses; my $old_status; # Note that this is undef for new bugs. + if (ref $invocant) { + @valid_statuses = @{$invocant->status->can_change_to}; $product = $invocant->product_obj; $old_status = $invocant->status; my $comments = $invocant->{added_comments} || []; $comment = $comments->[-1]; } - + else { + @valid_statuses = @{Bugzilla::Status->can_change_to()}; + } + + if (!$product->votes_to_confirm) { + # UNCONFIRMED becomes an invalid status if votes_to_confirm is 0, + # even if you are in editbugs. + @valid_statuses = grep {$_->name ne 'UNCONFIRMED'} @valid_statuses; + } + # Check permissions for users filing new bugs. if (!ref $invocant) { - my $default_status = Bugzilla::Status->can_change_to->[0]; - if ($user->in_group('editbugs', $product->id) || $user->in_group('canconfirm', $product->id)) { - # If the user with privs hasn't selected another status, - # select the first one of the list. - $new_status ||= $default_status; + # If the user with privs hasn't selected another status, + # select the first one of the list. + $new_status ||= $valid_statuses[0]; } else { # A user with no privs cannot choose the initial status. - $new_status = $default_status; + $new_status = $valid_statuses[0]; } } - - # Make sure this is a valid transition. - if (!$new_status->allow_change_from($old_status, $product)) { - ThrowUserError('illegal_bug_status_transition', - { old => $old_status, new => $new_status }); + # Time to validate the bug status. + $new_status = Bugzilla::Status->check($new_status) unless ref($new_status); + if (!grep {$_->name eq $new_status->name} @valid_statuses) { + ThrowUserError('illegal_bug_status_transition', + { old => $old_status, new => $new_status }); } # Check if a comment is required for this change. @@ -961,7 +974,7 @@ sub _check_bug_status { } return $new_status->name if ref $invocant; - return ($status, $status eq 'UNCONFIRMED' ? 0 : 1); + return ($new_status->name, $new_status->name eq 'UNCONFIRMED' ? 0 : 1); } sub _check_cc { diff --git a/Bugzilla/Status.pm b/Bugzilla/Status.pm index 5573fa836..f8b77331c 100644 --- a/Bugzilla/Status.pm +++ b/Bugzilla/Status.pm @@ -95,7 +95,8 @@ sub can_change_to { INNER JOIN bug_status ON id = new_status WHERE isactive = 1 - AND old_status $cond", + AND old_status $cond + ORDER BY sortkey", undef, @args); # Allow the bug status to remain unchanged. @@ -106,24 +107,6 @@ sub can_change_to { return $self->{'can_change_to'}; } -sub allow_change_from { - my ($self, $old_status, $product) = @_; - - # Always allow transitions from a status to itself. - return 1 if ($old_status && $old_status->id == $self->id); - - if ($self->name eq 'UNCONFIRMED' && !$product->votes_to_confirm) { - # UNCONFIRMED is an invalid status transition if votes_to_confirm is 0 - # in this product. - return 0; - } - - my ($cond, $values) = $self->_status_condition($old_status); - my ($transition_allowed) = Bugzilla->dbh->selectrow_array( - "SELECT 1 FROM status_workflow WHERE $cond", undef, @$values); - return $transition_allowed ? 1 : 0; -} - sub can_change_from { my $self = shift; my $dbh = Bugzilla->dbh; @@ -259,37 +242,6 @@ below. Returns: A list of Bugzilla::Status objects. -=item C - -=over - -=item B - -Tells you whether or not a change to this status from another status is -allowed. - -=item B - -=over - -=item C<$old_status> - The Bugzilla::Status you're changing from. - -=item C<$product> - A L representing the product of -the bug you're changing. Needed to check product-specific workflow -issues (such as whether or not the C status is enabled -in this product). - -=back - -=item B - -C<1> if you are allowed to change to this status from that status, or -C<0> if you aren't allowed. - -Note that changing from a status to itself is always allowed. - -=back - =item C =over diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm index 01d5c16eb..44180cc14 100755 --- a/Bugzilla/WebService/Bug.pm +++ b/Bugzilla/WebService/Bug.pm @@ -123,11 +123,6 @@ sub create { $field_values{$field_name} = $params->{$field}; } - # Make sure all the required fields are in the hash. - foreach my $field (Bugzilla::Bug::REQUIRED_CREATE_FIELDS) { - $field_values{$field} = undef unless exists $field_values{$field}; - } - # WebService users can't set the creation date of a bug. delete $field_values{'creation_ts'}; @@ -504,6 +499,15 @@ in them. The error message will have more details. =back +=item B + +=over + +=item Before B<3.0.4>, parameters marked as B were actually +B, due to a bug in Bugzilla. + +=back + =back =item C B diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm index f12c5ecd2..85640d1bc 100755 --- a/Bugzilla/WebService/Constants.pm +++ b/Bugzilla/WebService/Constants.pm @@ -52,6 +52,7 @@ use base qw(Exporter); use constant WS_ERROR_CODE => { # Generic Bugzilla::Object errors are 50-99. object_name_not_specified => 50, + param_required => 50, object_does_not_exist => 51, # Bug errors usually occupy the 100-200 range. improper_bug_id_field_value => 100, diff --git a/email_in.pl b/email_in.pl index ca7a29735..248b86bb8 100644 --- a/email_in.pl +++ b/email_in.pl @@ -56,45 +56,6 @@ use Bugzilla::Util; # in a message. RFC-compliant mailers use this. use constant SIGNATURE_DELIMITER => '-- '; -# These fields must all be defined or post_bug complains. They don't have -# to have values--they just have to be defined. There's not yet any -# way to require custom fields have values, for enter_bug, so we don't -# have to worry about those yet. -use constant REQUIRED_ENTRY_FIELDS => qw( - reporter - short_desc - product - component - version - - assigned_to - rep_platform - op_sys - priority - bug_severity - bug_file_loc -); - -# Fields that must be defined during process_bug. They *do* have to -# have values. The script will grab their values from the current -# bug object, if they're not specified. -use constant REQUIRED_PROCESS_FIELDS => qw( - dependson - blocked - version - product - target_milestone - rep_platform - op_sys - priority - bug_severity - bug_file_loc - component - short_desc - reporter_accessible - cclist_accessible -); - # $input_email is a global so that it can be used in die_handler. our ($input_email, %switch); @@ -180,15 +141,6 @@ sub post_bug { debug_print('Posting a new bug...'); - $fields{'rep_platform'} ||= Bugzilla->params->{'defaultplatform'}; - $fields{'op_sys'} ||= Bugzilla->params->{'defaultopsys'}; - $fields{'priority'} ||= Bugzilla->params->{'defaultpriority'}; - $fields{'bug_severity'} ||= Bugzilla->params->{'defaultseverity'}; - - foreach my $field (REQUIRED_ENTRY_FIELDS) { - $fields{$field} ||= ''; - } - my $cgi = Bugzilla->cgi; foreach my $field (keys %fields) { $cgi->param(-name => $field, -value => $fields{$field}); @@ -231,14 +183,6 @@ sub process_bug { $fields{'addtonewgroup'} = 0; } - foreach my $field (REQUIRED_PROCESS_FIELDS) { - my $value = $bug->$field; - if (ref $value) { - $value = join(',', @$value); - } - $fields{$field} ||= $value; - } - # Make it possible to remove CCs. if ($fields{'removecc'}) { $fields{'cc'} = [split(',', $fields{'removecc'})]; -- cgit v1.2.3-24-g4f1b