summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMax Kanat-Alexander <mkanat@bugzilla.org>2011-08-09 23:10:41 +0200
committerMax Kanat-Alexander <mkanat@bugzilla.org>2011-08-09 23:10:41 +0200
commit89cf83ecf9ec62f82887569e71b22b39956aeb46 (patch)
treed7eb8a7f36fa65e9d0c489104747cde9512cdda9
parentb308699b2c0453392c86215cecc4fe508a0e1762 (diff)
downloadbugzilla-89cf83ecf9ec62f82887569e71b22b39956aeb46.tar.gz
bugzilla-89cf83ecf9ec62f82887569e71b22b39956aeb46.tar.xz
Bug 622203 - Allow filing closed bugs via the WebService
r=dkl, a=mkanat
-rw-r--r--Bugzilla/Bug.pm103
-rw-r--r--Bugzilla/WebService/Bug.pm8
-rwxr-xr-xeditworkflow.cgi2
-rw-r--r--template/en/default/admin/workflow/edit.html.tmpl2
-rw-r--r--template/en/default/global/user-error.html.tmpl13
5 files changed, 91 insertions, 37 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 62099c423..aa4eddd4c 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -124,6 +124,7 @@ sub VALIDATORS {
my $validators = {
alias => \&_check_alias,
assigned_to => \&_check_assigned_to,
+ blocked => \&_check_dependencies,
bug_file_loc => \&_check_bug_file_loc,
bug_severity => \&_check_select_field,
bug_status => \&_check_bug_status,
@@ -132,6 +133,7 @@ sub VALIDATORS {
component => \&_check_component,
creation_ts => \&_check_creation_ts,
deadline => \&_check_deadline,
+ dependson => \&_check_dependencies,
dup_id => \&_check_dup_id,
estimated_time => \&_check_time_field,
everconfirmed => \&Bugzilla::Object::check_boolean,
@@ -187,14 +189,16 @@ sub VALIDATOR_DEPENDENCIES {
my %deps = (
assigned_to => ['component'],
+ blocked => ['product'],
bug_status => ['product', 'comment', 'target_milestone'],
cc => ['component'],
comment => ['creation_ts'],
component => ['product'],
+ dependson => ['product'],
dup_id => ['bug_status', 'resolution'],
groups => ['product'],
keywords => ['product'],
- resolution => ['bug_status'],
+ resolution => ['bug_status', 'dependson'],
qa_contact => ['component'],
target_milestone => ['product'],
version => ['product'],
@@ -749,12 +753,7 @@ sub run_create_validators {
$class->_check_strict_isolation($params->{cc}, $params->{assigned_to},
$params->{qa_contact}, $product);
- ($params->{dependson}, $params->{blocked}) =
- $class->_check_dependencies($params->{dependson}, $params->{blocked},
- $product);
-
- # You can't set these fields on bug creation (or sometimes ever).
- delete $params->{resolution};
+ # You can't set these fields.
delete $params->{lastdiffed};
delete $params->{bug_id};
@@ -769,6 +768,11 @@ sub run_create_validators {
$params);
}
+ # And this is not a valid DB field, it's just used as part of
+ # _check_dependencies to avoid running it twice for both blocked
+ # and dependson.
+ delete $params->{_dependencies_validated};
+
return $params;
}
@@ -1370,9 +1374,9 @@ sub _check_bug_status {
# Check if a comment is required for this change.
if ($new_status->comment_required_on_change_from($old_status) && !$comment)
{
- ThrowUserError('comment_required', { old => $old_status,
- new => $new_status });
-
+ ThrowUserError('comment_required',
+ { old => $old_status->name, new => $new_status->name,
+ field => 'bug_status' });
}
if (ref $invocant
@@ -1453,7 +1457,24 @@ sub _check_comment {
);
return $comment;
+}
+
+sub _check_commenton {
+ my ($invocant, $new_value, $field, $params) = @_;
+
+ my $has_comment =
+ ref($invocant) ? $invocant->{added_comments}
+ : (defined $params->{comment}
+ and $params->{comment}->{thetext} ne '');
+
+ my $is_changing = ref($invocant) ? $invocant->$field ne $new_value
+ : $new_value ne '';
+ if ($is_changing && !$has_comment) {
+ my $old_value = ref($invocant) ? $invocant->$field : undef;
+ ThrowUserError('comment_required',
+ { field => $field, old => $old_value, new => $new_value });
+ }
}
sub _check_component {
@@ -1492,15 +1513,23 @@ sub _check_deadline {
# Takes two comma/space-separated strings and returns arrayrefs
# of valid bug IDs.
sub _check_dependencies {
- my ($invocant, $depends_on, $blocks, $product) = @_;
+ my ($invocant, $value, $field, $params) = @_;
+
+ return $value if $params->{_dependencies_validated};
if (!ref $invocant) {
# Only editbugs users can set dependencies on bug entry.
- return ([], []) unless Bugzilla->user->in_group('editbugs',
- $product->id);
+ return ([], []) unless Bugzilla->user->in_group(
+ 'editbugs', $params->{product}->id);
}
- my %deps_in = (dependson => $depends_on || '', blocked => $blocks || '');
+ # This is done this way so that dependson and blocked can be in
+ # VALIDATORS, meaning that they can be in VALIDATOR_DEPENDENCIES,
+ # which means that they can be checked in the right order during
+ # bug creation.
+ my $opposite = $field eq 'dependson' ? 'blocked' : 'dependson';
+ my %deps_in = ($field => $value || '',
+ $opposite => $params->{$opposite} || '');
foreach my $type (qw(dependson blocked)) {
my @bug_ids = ref($deps_in{$type})
@@ -1546,9 +1575,12 @@ sub _check_dependencies {
# And finally, check for dependency loops.
my $bug_id = ref($invocant) ? $invocant->id : 0;
- my %deps = ValidateDependencies($deps_in{dependson}, $deps_in{blocked}, $bug_id);
+ my %deps = ValidateDependencies($deps_in{dependson}, $deps_in{blocked},
+ $bug_id);
- return ($deps{'dependson'}, $deps{'blocked'});
+ $params->{$opposite} = $deps{$opposite};
+ $params->{_dependencies_validated} = 1;
+ return $deps{$field};
}
sub _check_dup_id {
@@ -1760,41 +1792,46 @@ sub _check_reporter {
}
sub _check_resolution {
- my ($self, $resolution) = @_;
+ my ($invocant, $resolution, undef, $params) = @_;
$resolution = trim($resolution);
+ my $status = ref($invocant) ? $invocant->status->name
+ : $params->{bug_status};
+ my $is_open = ref($invocant) ? $invocant->status->is_open
+ : is_open_state($status);
# Throw a special error for resolving bugs without a resolution
# (or trying to change the resolution to '' on a closed bug without
# using clear_resolution).
- ThrowUserError('missing_resolution', { status => $self->status->name })
- if !$resolution && !$self->status->is_open;
+ ThrowUserError('missing_resolution', { status => $status })
+ if !$resolution && !$is_open;
# Make sure this is a valid resolution.
- $resolution = $self->_check_select_field($resolution, 'resolution');
+ $resolution = $invocant->_check_select_field($resolution, 'resolution');
# Don't allow open bugs to have resolutions.
- ThrowUserError('resolution_not_allowed') if $self->status->is_open;
+ ThrowUserError('resolution_not_allowed') if $is_open;
# Check noresolveonopenblockers.
+ my $dependson = ref($invocant) ? $invocant->dependson
+ : ($params->{dependson} || []);
if (Bugzilla->params->{"noresolveonopenblockers"}
&& $resolution eq 'FIXED'
- && (!$self->resolution || $resolution ne $self->resolution)
- && scalar @{$self->dependson})
+ && (!ref $invocant or !$invocant->resolution
+ or $resolution ne $invocant->resolution)
+ && scalar @$dependson)
{
- my $dep_bugs = Bugzilla::Bug->new_from_list($self->dependson);
+ my $dep_bugs = Bugzilla::Bug->new_from_list($dependson);
my $count_open = grep { $_->isopened } @$dep_bugs;
if ($count_open) {
+ my $bug_id = ref($invocant) ? $invocant->id : undef;
ThrowUserError("still_unresolved_bugs",
- { bug_id => $self->id, dep_count => $count_open });
+ { bug_id => $bug_id, dep_count => $count_open });
}
}
# Check if they're changing the resolution and need to comment.
- if (Bugzilla->params->{'commentonchange_resolution'}
- && $self->resolution && $resolution ne $self->resolution
- && !$self->{added_comments})
- {
- ThrowUserError('comment_required');
+ if (Bugzilla->params->{'commentonchange_resolution'}) {
+ $invocant->_check_commenton($resolution, 'resolution', $params);
}
return $resolution;
@@ -2327,7 +2364,9 @@ sub set_custom_field {
sub set_deadline { $_[0]->set('deadline', $_[1]); }
sub set_dependencies {
my ($self, $dependson, $blocked) = @_;
- ($dependson, $blocked) = $self->_check_dependencies($dependson, $blocked);
+ my %extra = ( blocked => $blocked );
+ $dependson = $self->_check_dependencies($dependson, 'dependson', \%extra);
+ $blocked = $extra{blocked};
# These may already be detainted, but all setters are supposed to
# detaint their input if they've run a validator (just as though
# we had used Bugzilla::Object::set), so we do that here.
@@ -3854,6 +3893,8 @@ sub map_fields {
my %field_values;
foreach my $field (keys %$params) {
+ # Don't allow setting private fields via email_in or the WebService.
+ next if $field =~ /^_/;
my $field_name;
if ($except->{$field}) {
$field_name = $field;
diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm
index da2393033..1cc52f556 100644
--- a/Bugzilla/WebService/Bug.pm
+++ b/Bugzilla/WebService/Bug.pm
@@ -2288,6 +2288,11 @@ the component's default QA Contact.
=item C<status> (string) - The status that this bug should start out as.
Note that only certain statuses can be set on bug creation.
+=item C<resolution> (string) - If you are filing a closed bug, then
+you will have to specify a resolution. You cannot currently specify
+a resolution of C<DUPLICATE> for new bugs, though. That must be done
+with L</update>.
+
=item C<target_milestone> (string) - A valid target milestone for this
product.
@@ -2370,6 +2375,9 @@ argument.
=item Error 116 was added in Bugzilla B<4.0>. Before that, dependency
loop errors had a generic code of C<32000>.
+=item The ability to file new bugs with a C<resolution> was added in
+Bugzilla B<4.2>.
+
=back
=back
diff --git a/editworkflow.cgi b/editworkflow.cgi
index 321f077fe..805900abf 100755
--- a/editworkflow.cgi
+++ b/editworkflow.cgi
@@ -87,7 +87,7 @@ elsif ($action eq 'update') {
# Part 1: Initial bug statuses.
foreach my $new (@$statuses) {
- if ($new->is_open && $cgi->param('w_0_' . $new->id)) {
+ if ($cgi->param('w_0_' . $new->id)) {
$sth_insert->execute(undef, $new->id)
unless defined $workflow->{0}->{$new->id};
}
diff --git a/template/en/default/admin/workflow/edit.html.tmpl b/template/en/default/admin/workflow/edit.html.tmpl
index 5f2b21263..7fdcb3577 100644
--- a/template/en/default/admin/workflow/edit.html.tmpl
+++ b/template/en/default/admin/workflow/edit.html.tmpl
@@ -68,7 +68,7 @@
</th>
[% FOREACH new_status = statuses %]
- [% IF status.id != new_status.id && (status.id || new_status.is_open) %]
+ [% IF status.id != new_status.id %]
[% checked = workflow.${status.id}.${new_status.id}.defined ? 1 : 0 %]
[% mandatory = (status.id && new_status.name == Param("duplicate_or_move_bug_status")) ? 1 : 0 %]
<td align="center" class="checkbox-cell[% " checked" IF checked || mandatory %]"
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index dc0481a67..387a7df03 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -308,9 +308,10 @@
[% ELSIF error == "comment_required" %]
[% title = "Comment Required" %]
You have to specify a
- [% IF old && new %]
- <b>comment</b> when changing the status of [% terms.abug %] from
- [%+ old.name FILTER html %] to [% new.name FILTER html %].
+ [% IF old.defined && new.defined %]
+ <b>comment</b> when changing the [% field_descs.$field FILTER html %]
+ of [% terms.abug %] from [% (old || "(empty)") FILTER html %]
+ to [% (new || "(empty)") FILTER html %].
[% ELSIF new %]
description for this [% terms.bug %].
[% ELSE %]
@@ -1527,7 +1528,11 @@
[% ELSIF error == "still_unresolved_bugs" %]
[% title = "Unresolved Dependencies" %]
- [% terms.Bug %] [%+ bug_id FILTER bug_link(bug_id) FILTER none %]
+ [% IF bug_id %]
+ [% terms.Bug %] [%+ bug_id FILTER bug_link(bug_id) FILTER none %]
+ [% ELSE %]
+ This [% terms.bug %]
+ [% END %]
has [% dep_count FILTER none %] unresolved
[% IF dep_count == 1 %]
dependency