summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorlpsolit%gmail.com <>2008-02-12 08:32:51 +0100
committerlpsolit%gmail.com <>2008-02-12 08:32:51 +0100
commit30c7fa92ac2d0c76443fcc47907487a9ff0186b2 (patch)
tree30743e0fee6f292ae812ed373d1078aedbe5fb29
parentd26783131dda06d57fb12342eb8d4df0bbfe774f (diff)
downloadbugzilla-30c7fa92ac2d0c76443fcc47907487a9ff0186b2.tar.gz
bugzilla-30c7fa92ac2d0c76443fcc47907487a9ff0186b2.tar.xz
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 <LpSolit@gmail.com> r/a=mkanat
-rwxr-xr-xBugzilla/Bug.pm65
-rw-r--r--Bugzilla/Status.pm52
-rwxr-xr-xBugzilla/WebService/Bug.pm14
-rwxr-xr-xBugzilla/WebService/Constants.pm1
-rw-r--r--email_in.pl56
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<deadline> - For time-tracking. Will be ignored for the same
# reasons as C<estimated_time>.
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<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>
=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<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
=item C<add_comment> B<EXPERIMENTAL>
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'})];