Commit 30c7fa92 authored by lpsolit%gmail.com's avatar lpsolit%gmail.com

Bug 384009: Global fields (priority, severity, OS, and platform) are required…

Bug 384009: Global fields (priority, severity, OS, and platform) are required when using WebService instead of using the defaults - Patch by Fré©ric Buclin <LpSolit@gmail.com> r/a=mkanat
parent d2678313
...@@ -103,13 +103,8 @@ sub DB_COLUMNS { ...@@ -103,13 +103,8 @@ sub DB_COLUMNS {
} }
use constant REQUIRED_CREATE_FIELDS => qw( use constant REQUIRED_CREATE_FIELDS => qw(
bug_severity
comment
component component
op_sys
priority
product product
rep_platform
short_desc short_desc
version version
); );
...@@ -317,13 +312,25 @@ sub new { ...@@ -317,13 +312,25 @@ sub new {
# C<deadline> - For time-tracking. Will be ignored for the same # C<deadline> - For time-tracking. Will be ignored for the same
# reasons as C<estimated_time>. # reasons as C<estimated_time>.
sub create { sub create {
my $class = shift; my ($class, $params) = @_;
my $dbh = Bugzilla->dbh; my $dbh = Bugzilla->dbh;
$dbh->bz_start_transaction(); $dbh->bz_start_transaction();
$class->check_required_create_fields(@_); # These fields have default values which we can use if they are undefined.
my $params = $class->run_create_validators(@_); $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 # These are not a fields in the bugs table, so we don't pass them to
# insert_create_data. # insert_create_data.
...@@ -905,38 +912,44 @@ sub _check_bug_severity { ...@@ -905,38 +912,44 @@ sub _check_bug_severity {
} }
sub _check_bug_status { sub _check_bug_status {
my ($invocant, $status, $product, $comment) = @_; my ($invocant, $new_status, $product, $comment) = @_;
my $user = Bugzilla->user; my $user = Bugzilla->user;
my @valid_statuses;
# Make sure this is a valid status.
my $new_status = ref $status ? $status : Bugzilla::Status->check($status);
my $old_status; # Note that this is undef for new bugs. my $old_status; # Note that this is undef for new bugs.
if (ref $invocant) { if (ref $invocant) {
@valid_statuses = @{$invocant->status->can_change_to};
$product = $invocant->product_obj; $product = $invocant->product_obj;
$old_status = $invocant->status; $old_status = $invocant->status;
my $comments = $invocant->{added_comments} || []; my $comments = $invocant->{added_comments} || [];
$comment = $comments->[-1]; $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. # Check permissions for users filing new bugs.
if (!ref $invocant) { if (!ref $invocant) {
my $default_status = Bugzilla::Status->can_change_to->[0];
if ($user->in_group('editbugs', $product->id) if ($user->in_group('editbugs', $product->id)
|| $user->in_group('canconfirm', $product->id)) { || $user->in_group('canconfirm', $product->id)) {
# If the user with privs hasn't selected another status, # If the user with privs hasn't selected another status,
# select the first one of the list. # select the first one of the list.
$new_status ||= $default_status; $new_status ||= $valid_statuses[0];
} }
else { else {
# A user with no privs cannot choose the initial status. # A user with no privs cannot choose the initial status.
$new_status = $default_status; $new_status = $valid_statuses[0];
} }
} }
# Time to validate the bug status.
# Make sure this is a valid transition. $new_status = Bugzilla::Status->check($new_status) unless ref($new_status);
if (!$new_status->allow_change_from($old_status, $product)) { if (!grep {$_->name eq $new_status->name} @valid_statuses) {
ThrowUserError('illegal_bug_status_transition', ThrowUserError('illegal_bug_status_transition',
{ old => $old_status, new => $new_status }); { old => $old_status, new => $new_status });
} }
...@@ -961,7 +974,7 @@ sub _check_bug_status { ...@@ -961,7 +974,7 @@ sub _check_bug_status {
} }
return $new_status->name if ref $invocant; 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 { sub _check_cc {
......
...@@ -95,7 +95,8 @@ sub can_change_to { ...@@ -95,7 +95,8 @@ sub can_change_to {
INNER JOIN bug_status INNER JOIN bug_status
ON id = new_status ON id = new_status
WHERE isactive = 1 WHERE isactive = 1
AND old_status $cond", AND old_status $cond
ORDER BY sortkey",
undef, @args); undef, @args);
# Allow the bug status to remain unchanged. # Allow the bug status to remain unchanged.
...@@ -106,24 +107,6 @@ sub can_change_to { ...@@ -106,24 +107,6 @@ sub can_change_to {
return $self->{'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 { sub can_change_from {
my $self = shift; my $self = shift;
my $dbh = Bugzilla->dbh; my $dbh = Bugzilla->dbh;
...@@ -259,37 +242,6 @@ below. ...@@ -259,37 +242,6 @@ below.
Returns: A list of Bugzilla::Status objects. Returns: A list of Bugzilla::Status objects.
=item C<allow_change_from>
=over
=item B<Description>
Tells you whether or not a change to this status from another status is
allowed.
=item B<Params>
=over
=item C<$old_status> - The Bugzilla::Status you're changing from.
=item C<$product> - A L<Bugzilla::Product> representing the product of
the bug you're changing. Needed to check product-specific workflow
issues (such as whether or not the C<UNCONFIRMED> status is enabled
in this product).
=back
=item B<Returns>
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<comment_required_on_change_from> =item C<comment_required_on_change_from>
=over =over
......
...@@ -123,11 +123,6 @@ sub create { ...@@ -123,11 +123,6 @@ sub create {
$field_values{$field_name} = $params->{$field}; $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. # WebService users can't set the creation date of a bug.
delete $field_values{'creation_ts'}; delete $field_values{'creation_ts'};
...@@ -504,6 +499,15 @@ in them. The error message will have more details. ...@@ -504,6 +499,15 @@ in them. The error message will have more details.
=back =back
=item B<History>
=over
=item Before B<3.0.4>, parameters marked as B<Defaulted> were actually
B<Required>, due to a bug in Bugzilla.
=back
=back =back
=item C<add_comment> B<EXPERIMENTAL> =item C<add_comment> B<EXPERIMENTAL>
......
...@@ -52,6 +52,7 @@ use base qw(Exporter); ...@@ -52,6 +52,7 @@ use base qw(Exporter);
use constant WS_ERROR_CODE => { use constant WS_ERROR_CODE => {
# Generic Bugzilla::Object errors are 50-99. # Generic Bugzilla::Object errors are 50-99.
object_name_not_specified => 50, object_name_not_specified => 50,
param_required => 50,
object_does_not_exist => 51, object_does_not_exist => 51,
# Bug errors usually occupy the 100-200 range. # Bug errors usually occupy the 100-200 range.
improper_bug_id_field_value => 100, improper_bug_id_field_value => 100,
......
...@@ -56,45 +56,6 @@ use Bugzilla::Util; ...@@ -56,45 +56,6 @@ use Bugzilla::Util;
# in a message. RFC-compliant mailers use this. # in a message. RFC-compliant mailers use this.
use constant SIGNATURE_DELIMITER => '-- '; 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. # $input_email is a global so that it can be used in die_handler.
our ($input_email, %switch); our ($input_email, %switch);
...@@ -180,15 +141,6 @@ sub post_bug { ...@@ -180,15 +141,6 @@ sub post_bug {
debug_print('Posting a new 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; my $cgi = Bugzilla->cgi;
foreach my $field (keys %fields) { foreach my $field (keys %fields) {
$cgi->param(-name => $field, -value => $fields{$field}); $cgi->param(-name => $field, -value => $fields{$field});
...@@ -231,14 +183,6 @@ sub process_bug { ...@@ -231,14 +183,6 @@ sub process_bug {
$fields{'addtonewgroup'} = 0; $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. # Make it possible to remove CCs.
if ($fields{'removecc'}) { if ($fields{'removecc'}) {
$fields{'cc'} = [split(',', $fields{'removecc'})]; $fields{'cc'} = [split(',', $fields{'removecc'})];
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment