1253483 - MozReview.attachments() doesn't create flags on new attachments

Status: RESOLVED FIXED

(bugzilla.mozilla.org Graveyard :: Extensions: MozReview Integration, defect)

Description

[Mark Côté [:mcote]]

I can successfully update an attachment and set/clear flags, but the flags are not being set when I create a new attachment.

Example:

I'm sending this:

{'bug_id': 1, 'api_key': u'XXXX', 'attachments': [{'comment': u'Review commit: http://172.16.130.136:64784/r/22/diff/#index_header\nSee other reviews: http://172.16.130.136:64784/r/22/', 'file_name': 'reviewboard-22-url.txt', 'summary': u'MozReview Request: Bug 1 - Add foo. r?jrandom', 'flags': [{'status': '?', 'requestee': u'[e-mail]', 'name': 'review', 'new': True}], 'content_type': 'text/x-review-board-request', 'data': u'http://172.16.130.136:64784/r/22/diff/#index_header'}]}

And receiving this:

{'attachments_created': {'12': {'description': 'MozReview Request: Bug 1 - Add foo. r?jrandom', 'creator': '[e-mail]', 'is_obsolete': 0, 'is_patch': 0, 'creation_time': <DateTime '20160304T01:43:56' at 7fa31fae7b48>, 'summary': 'MozReview Request: Bug 1 - Add foo. r?jrandom', 'bug_id': 1, 'flags': [], 'last_change_time': <DateTime '20160304T01:43:56' at 7fa31fae7bd8>, 'content_type': 'text/x-review-board-request', 'file_name': 'reviewboard-22-url.txt', 'attacher': '[e-mail]', 'data': <xmlrpclib.Binary instance at 0x7fa31fae7b00>, 'id': 12, 'is_private': 0, 'size': 51}}, 'attachments_modified': {}}

Note that the response has an empty value for 'flags'.

Comment 1

[Dylan Hardison [:dylan] (he/him)]

Attachment: 1253483_1.patch

Comment 2

[David Lawrence [:dkl]]

Comment on attachment 8726528 [details] [diff] [review]
1253483_1.patch

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

Testing of adding attachments and flags worked fine. When updating the flag status of the attachment I just created I got the following error:

Can't call method "value" without a package or object reference at /loader/0x3c73b60/Bugzilla/Extension/MozReview/WebService.pm line 181.
 at /loader/0x3c73b60/Bugzilla/Extension/MozReview/WebService.pm line 181.

Changing extensions/MozReview/lib/WebService.pm line 101 fixed the problem for me. Take a look. Not sure how I would have missed it in the earlier review.

    my %attachments_modified = map { (ref $_->{id} ? $_->{id}->value : $_->{id}) => $_ } @modified;

::: Bugzilla/WebService/Util.pm
@@ -57,4 @@
>      # Copy the user provided $flags as we may call extract_flags more than
>      # once when editing multiple bugs or attachments.
>      my $flags_copy = dclone($flags);
> -

nit: unnecessary change for this patch

::: extensions/MozReview/lib/WebService.pm
@@ +155,4 @@
>                  isprivate   => $attachment->{is_private},
>              });
>  
> +            

nit: remove extra newline (whitespace)

Comment 3

[Dylan Hardison [:dylan] (he/him)]

Attachment: 1253483_2.patch

Comment 4

[David Lawrence [:dkl]]

Comment on attachment 8727843 [details] [diff] [review]
1253483_2.patch

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

r=dkl

Comment 5

[Dylan Hardison [:dylan] (he/him)]

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   30143b3..6ec9ecf  master -> master