622203 - Allow filing closed bugs via the WebService

Status: RESOLVED FIXED

(Bugzilla :: WebService, enhancement)

Description

[Max Kanat-Alexander]

This is simpler than bug 399483--we just have to fix the backend and update the POD. Basically this just involves allowing Bugzilla::Bug to use the validators when $invocant is a class.

Comment 1

[Max Kanat-Alexander]

Attachment: Work In Progress

This is a bit more work than I anticipated, because there are lots of things that _check_resolution depends on.

I've decided that, for now, I'm not going to make filing new bugs as DUPLICATE work. But all other resolutions and _check_resolution stuff will still work.

Comment 2

[Max Kanat-Alexander]

Attachment: v1

All right, here's a patch that allows filing closed bugs via the WebService, but not yet via the enter_bug UI. (Note that enter_bug already filters out closed statuses in its code, so we don't have to worry about modifying it.)

I had to update the way that _check_dependencies works, also, so that "blocked" and "dependson" could go into VALIDATOR_DEPENDENCIES (since they now have to be checked before "resolution" when a bug is filed, due to noresolveonopenblockers).

Comment 3

[Max Kanat-Alexander]

Attachment: v2

v1 broke filing new bugs with a dependson value, and also had a bad error message for noresolveonopenblockers for new bugs.

Comment 4

[Max Kanat-Alexander]

Attachment: v2.1

I removed an incorrect comment that I had added, about commentonchange_resolution.

Comment 5

[Max Kanat-Alexander]

dkl: Any hope of getting a review on this? It's been waiting a LONG time.

Comment 6

[David Lawrence [:dkl]]

Comment on attachment 500498 [details] [diff] [review]
v2.1

Review of attachment 500498 [details] [diff] [review]:
-----------------------------------------------------------------

Overall my testing has been positive with this patch except for the bug I found which I commented on below.

::: Bugzilla/Bug.pm
@@ +1450,5 @@
> +    my ($invocant, $new_value, $field, $params) = @_;
> +
> +    my $has_comment = ref($invocant) ? $invocant->{added_comments} 
> +                                     : (defined $params->{comment}
> +                                        and $params->{comment} ne '');

$params->{'comment'} is a hash by this point with an empty value for 'thetext' if the user omitted a comment on Bug.create. So this never throws error.

Comment 7

[Max Kanat-Alexander]

Attachment: v3

Thanks for the review, and thanks for catching that bug! I've updated the code exactly as you suggested.

Comment 8

[David Lawrence [:dkl]]

Comment on attachment 535777 [details] [diff] [review]
v3

Review of attachment 535777 [details] [diff] [review]:
-----------------------------------------------------------------

In addition to my comment below, user-error.html.tmpl in error == 'comment_required', the template is
looking for old.name and new.name. _check_commenton is passing the actual values and not an object so
when the error displays, you get blank values.

dkl

::: Bugzilla/Bug.pm
@@ +1470,5 @@
> +
> +    if ($is_changing && !$has_comment) {
> +        my $old_value = ref($invocant) ? $invocant->$field : undef;
> +        ThrowUserError('comment_required',
> +                       { old => $old_value, new => $new_value });

When closing an existing bug report that is currently open, to a closed state with resolution, you get the wrong error message.

When closing a bug, old_value will be empty and new_value will not be empty. So the error generated by user-error.html.tmpl will be 

"You have to specify a description for this bug" 

which is what should happend for a new bug, not an existing one.  Instead something like 

"You have to specify a comment when changing the status to RESOLVED"

The template is looking for [% IF old && new %] which is why the wrong error is occurring.

Comment 9

[Max Kanat-Alexander]

Attachment: v4

Ah, good point. Okay, here's my attempt to address that issue. My solution isn't perfect; let me know if you have any thoughts or suggestions.

Comment 10

[David Lawrence [:dkl]]

Comment on attachment 537672 [details] [diff] [review]
v4

Review of attachment 537672 [details] [diff] [review]:
-----------------------------------------------------------------

I think the solution you came up with is acceptable and works as expected. r=dkl

Comment 11

[Frédéric Buclin]

Comment on attachment 537672 [details] [diff] [review]
v4

>=== modified file 'Bugzilla/WebService/Bug.pm'

>+=item The ability to file new bugs with a C<resolution> was added in
>+Bugzilla B<4.2>.

4.2 -> 5.0 (or 4.4 for now).

Comment 12

[Max Kanat-Alexander]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified editworkflow.cgi
modified Bugzilla/Bug.pm
modified Bugzilla/WebService/Bug.pm
modified template/en/default/admin/workflow/edit.html.tmpl                     
modified template/en/default/global/user-error.html.tmpl                       
Committed revision 7905

Comment 13

[Frédéric Buclin]

Added to the relnotes for 4.4.