1140575 - Comment-creation WebService API should allow tags to be specified

Status: RESOLVED FIXED

(Bugzilla :: WebService, enhancement)

Description

[Mark Côté [:mcote]]

Spun off of bug 975339, which is about the UI.  This bug is to add the ability to specify one or more tags to the comment-creation WebService API.

Marking this as a blocker to the other, since it's likely we would fix this before the UI, so the modifications to Bugzilla core will happen here.

Comment 1

[Frédéric Buclin]

(In reply to Mark Côté [:mcote] from comment #0)
> Marking this as a blocker to the other, since it's likely we would fix this
> before the UI, so the modifications to Bugzilla core will happen here.

Except that the comment will be submitted with other bug changes, not with new tags. So that's not the same code.

Comment 3

[David Lawrence [:dkl]]

Attachment: 1140575_1.patch

Comment 4

[David Lawrence [:dkl]]

Attachment: 1140575_2.patch

This has been revised to use Bug->set_all and Bug->update for adding comment tags to a new comment. This should allow the backend to be set up for adding the front end bits later for adding comment tags to a new comment.

dkl

Comment 5

[:glob ✱]

Comment on attachment 8600024 [details] [diff] [review]
1140575_2.patch

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

much nicer than the previous patch!

r=glob, with issues fixed on commit.

::: Bugzilla/Bug.pm
@@ +814,5 @@
> +    my $comment = Bugzilla::Comment->insert_create_data($creation_comment);
> +
> +    # Add comment tags
> +    if (defined $comment_tags && Bugzilla->user->can_tag_comments) {
> +        $comment_tags = ref $comment_tags ? $comment_tags : [ $comment_tags];

nit: missing a space before ]

@@ +815,5 @@
> +
> +    # Add comment tags
> +    if (defined $comment_tags && Bugzilla->user->can_tag_comments) {
> +        $comment_tags = ref $comment_tags ? $comment_tags : [ $comment_tags];
> +        foreach my $tag (@{$comment_tags || [] }) {

you don't need "|| []" because the previous line ensured it's a ref

@@ +1060,5 @@
>                               $user->id, $delta_ts);
>          }
> +        foreach my $tag (@{$self->{added_comment_tags} || []}) {
> +            $comment->add_tag($tag) if defined $tag;
> +            $comment->update() if @{$self->{added_comment_tags}};

move ->update() outside of the foreach loop, and add || [] to guard against casting undef as an array

@@ +2459,4 @@
>                is_markdown => $params->{'comment'}->{'is_markdown'} });
>      }
>  
> +    if (exists $params->{comment_tags} && Bugzilla->user->can_tag_comments()) {

you're using "exists" here, but "defined" earlier.  perform the same check both places.

::: docs/en/rst/api/core/v1/comment.rst
@@ +142,5 @@
> +ids           array    List of integer bug IDs to add the comment to.
> +**comment**   string   The comment to append to the bug. If this is empty
> +                       or all whitespace, an error will be thrown saying that you
> +                       did not set the ``comment`` parameter.
> +comment_tags  array    An array of comment tags to add to the new comment.

this should read "Array of strings to add as comment tags" or similar (ie. indicate that we want strings)

Comment 6

[David Lawrence [:dkl]]

To ssh://[e-mail]/bugzilla/bugzilla.git
   e4362da..f805627  master -> master