mozreview: stop clearing bugzilla flags on comments (
bug 1197879); r?smacleod r?mcote
Now that the reviewer can explicitly set review flags, we can stop clearing
those flags on comment. As a result, a flag set on mozreview is always mirrored to bugzilla.
In case of a r? -> r+ -> r? status transition, the final flag will have the reviewer set as
both the setter and the requestee.
MozReview-Commit-ID: Ao2OiaAOsSd
--- a/hgext/reviewboard/tests/test-bugzilla-review-flag-cleared.t
+++ b/hgext/reviewboard/tests/test-bugzilla-review-flag-cleared.t
@@ -67,31 +67,36 @@ Sanity check to ensure we have a review
component: TestComponent
depends_on: []
platform: All
product: TestProduct
resolution: ''
status: UNCONFIRMED
summary: First Bug
-Publishing a review will clear the r? flag
+Publishing a non-'clear-the-flag' review will not clear the r? flag
$ exportbzauth reviewer@example.com password
$ rbmanage create-review 2 --body-top 'I have reservations' --public
created review 1
$ bugzilla dump-bug 1
Bug 1:
attachments:
- attacher: author@example.com
content_type: text/x-review-board-request
data: http://$DOCKER_HOSTNAME:$HGPORT1/r/2/diff/#index_header
description: 'MozReview Request: Bug 1 - Initial commit to review'
file_name: reviewboard-2-url.txt
- flags: []
+ flags:
+ - id: 1
+ name: review
+ requestee: reviewer@example.com
+ setter: author@example.com
+ status: '?'
id: 1
is_obsolete: false
is_patch: false
summary: 'MozReview Request: Bug 1 - Initial commit to review'
blocks: []
cc:
- reviewer@example.com
comments:
@@ -107,44 +112,46 @@ Publishing a review will clear the r? fl
- 'MozReview Request: Bug 1 - Initial commit to review'
- ''
- 'Review commit: http://$DOCKER_HOSTNAME:$HGPORT1/r/2/diff/#index_header'
- 'See other reviews: http://$DOCKER_HOSTNAME:$HGPORT1/r/2/'
- author: reviewer@example.com
id: 3
tags: []
text:
- - Comment on attachment 1
- - 'MozReview Request: Bug 1 - Initial commit to review'
- - ''
- http://$DOCKER_HOSTNAME:$HGPORT1/r/2/#review1
- ''
- I have reservations
component: TestComponent
depends_on: []
platform: All
product: TestProduct
resolution: ''
status: UNCONFIRMED
summary: First Bug
-Posting a non Ship It review without a review flag adds a comment
+Posting an r? review with a comment only adds a comment
$ rbmanage create-review 2 --body-top 'One more thing...' --public
created review 2
$ bugzilla dump-bug 1
Bug 1:
attachments:
- attacher: author@example.com
content_type: text/x-review-board-request
data: http://$DOCKER_HOSTNAME:$HGPORT1/r/2/diff/#index_header
description: 'MozReview Request: Bug 1 - Initial commit to review'
file_name: reviewboard-2-url.txt
- flags: []
+ flags:
+ - id: 1
+ name: review
+ requestee: reviewer@example.com
+ setter: author@example.com
+ status: '?'
id: 1
is_obsolete: false
is_patch: false
summary: 'MozReview Request: Bug 1 - Initial commit to review'
blocks: []
cc:
- reviewer@example.com
comments:
@@ -160,19 +167,16 @@ Posting a non Ship It review without a r
- 'MozReview Request: Bug 1 - Initial commit to review'
- ''
- 'Review commit: http://$DOCKER_HOSTNAME:$HGPORT1/r/2/diff/#index_header'
- 'See other reviews: http://$DOCKER_HOSTNAME:$HGPORT1/r/2/'
- author: reviewer@example.com
id: 3
tags: []
text:
- - Comment on attachment 1
- - 'MozReview Request: Bug 1 - Initial commit to review'
- - ''
- http://$DOCKER_HOSTNAME:$HGPORT1/r/2/#review1
- ''
- I have reservations
- author: reviewer@example.com
id: 4
tags: []
text:
- http://$DOCKER_HOSTNAME:$HGPORT1/r/2/#review2
@@ -181,31 +185,31 @@ Posting a non Ship It review without a r
component: TestComponent
depends_on: []
platform: All
product: TestProduct
resolution: ''
status: UNCONFIRMED
summary: First Bug
-Posting a Ship It review will add an r+
+Posting a r+ review will add a '+' review flag
$ rbmanage create-review 2 --body-top LGTM --public --review-flag='r+'
created review 3
$ bugzilla dump-bug 1
Bug 1:
attachments:
- attacher: author@example.com
content_type: text/x-review-board-request
data: http://$DOCKER_HOSTNAME:$HGPORT1/r/2/diff/#index_header
description: 'MozReview Request: Bug 1 - Initial commit to review'
file_name: reviewboard-2-url.txt
flags:
- - id: 2
+ - id: 1
name: review
requestee: null
setter: reviewer@example.com
status: +
id: 1
is_obsolete: false
is_patch: false
summary: 'MozReview Request: Bug 1 - Initial commit to review'
@@ -225,19 +229,16 @@ Posting a Ship It review will add an r+
- 'MozReview Request: Bug 1 - Initial commit to review'
- ''
- 'Review commit: http://$DOCKER_HOSTNAME:$HGPORT1/r/2/diff/#index_header'
- 'See other reviews: http://$DOCKER_HOSTNAME:$HGPORT1/r/2/'
- author: reviewer@example.com
id: 3
tags: []
text:
- - Comment on attachment 1
- - 'MozReview Request: Bug 1 - Initial commit to review'
- - ''
- http://$DOCKER_HOSTNAME:$HGPORT1/r/2/#review1
- ''
- I have reservations
- author: reviewer@example.com
id: 4
tags: []
text:
- http://$DOCKER_HOSTNAME:$HGPORT1/r/2/#review2
@@ -273,17 +274,17 @@ Updating the review request as an L1 aut
Bug 1:
attachments:
- attacher: author@example.com
content_type: text/x-review-board-request
data: http://$DOCKER_HOSTNAME:$HGPORT1/r/2/diff/#index_header
description: 'MozReview Request: Bug 1 - Initial commit to review'
file_name: reviewboard-2-url.txt
flags:
- - id: 2
+ - id: 1
name: review
requestee: null
setter: reviewer@example.com
status: +
id: 1
is_obsolete: false
is_patch: false
summary: 'MozReview Request: Bug 1 - Initial commit to review'
@@ -303,19 +304,16 @@ Updating the review request as an L1 aut
- 'MozReview Request: Bug 1 - Initial commit to review'
- ''
- 'Review commit: http://$DOCKER_HOSTNAME:$HGPORT1/r/2/diff/#index_header'
- 'See other reviews: http://$DOCKER_HOSTNAME:$HGPORT1/r/2/'
- author: reviewer@example.com
id: 3
tags: []
text:
- - Comment on attachment 1
- - 'MozReview Request: Bug 1 - Initial commit to review'
- - ''
- http://$DOCKER_HOSTNAME:$HGPORT1/r/2/#review1
- ''
- I have reservations
- author: reviewer@example.com
id: 4
tags: []
text:
- http://$DOCKER_HOSTNAME:$HGPORT1/r/2/#review2
@@ -372,17 +370,17 @@ Sanity check to ensure we have an r? fla
Bug 2:
attachments:
- attacher: l3author@example.com
content_type: text/x-review-board-request
data: http://$DOCKER_HOSTNAME:$HGPORT1/r/4/diff/#index_header
description: 'MozReview Request: Bug 2 - Initial commit to review'
file_name: reviewboard-4-url.txt
flags:
- - id: 3
+ - id: 2
name: review
requestee: reviewer@example.com
setter: l3author@example.com
status: '?'
id: 2
is_obsolete: false
is_patch: false
summary: 'MozReview Request: Bug 2 - Initial commit to review'
@@ -423,17 +421,17 @@ Sanity check to ensure we have an r+ fla
Bug 2:
attachments:
- attacher: l3author@example.com
content_type: text/x-review-board-request
data: http://$DOCKER_HOSTNAME:$HGPORT1/r/4/diff/#index_header
description: 'MozReview Request: Bug 2 - Initial commit to review'
file_name: reviewboard-4-url.txt
flags:
- - id: 3
+ - id: 2
name: review
requestee: null
setter: reviewer@example.com
status: +
id: 2
is_obsolete: false
is_patch: false
summary: 'MozReview Request: Bug 2 - Initial commit to review'
@@ -488,17 +486,17 @@ We should have an r+ flag already set.
Bug 2:
attachments:
- attacher: l3author@example.com
content_type: text/x-review-board-request
data: http://$DOCKER_HOSTNAME:$HGPORT1/r/4/diff/#index_header
description: 'MozReview Request: Bug 2 - Modified commit to review'
file_name: reviewboard-4-url.txt
flags:
- - id: 3
+ - id: 2
name: review
requestee: null
setter: reviewer@example.com
status: +
id: 2
is_obsolete: false
is_patch: false
summary: 'MozReview Request: Bug 2 - Modified commit to review'
@@ -540,14 +538,94 @@ We should have an r+ flag already set.
component: TestComponent
depends_on: []
platform: All
product: TestProduct
resolution: ''
status: UNCONFIRMED
summary: Second Bug
+Posting a r- review will add a '-' review flag
+
+ $ rbmanage create-review 4 --body-top 'I changed my mind' --public --review-flag='r-'
+ created review 5
+ $ bugzilla dump-bug 2
+ Bug 2:
+ attachments:
+ - attacher: l3author@example.com
+ content_type: text/x-review-board-request
+ data: http://$DOCKER_HOSTNAME:$HGPORT1/r/4/diff/#index_header
+ description: 'MozReview Request: Bug 2 - Modified commit to review'
+ file_name: reviewboard-4-url.txt
+ flags:
+ - id: 2
+ name: review
+ requestee: null
+ setter: reviewer@example.com
+ status: +
+ - id: 3
+ name: review
+ requestee: null
+ setter: l3author@example.com
+ status: '-'
+ id: 2
+ is_obsolete: false
+ is_patch: false
+ summary: 'MozReview Request: Bug 2 - Modified commit to review'
+ blocks: []
+ cc:
+ - reviewer@example.com
+ comments:
+ - author: l3author@example.com
+ id: 7
+ tags: []
+ text: ''
+ - author: l3author@example.com
+ id: 8
+ tags: []
+ text:
+ - Created attachment 2
+ - 'MozReview Request: Bug 2 - Modified commit to review'
+ - ''
+ - 'Review commit: http://$DOCKER_HOSTNAME:$HGPORT1/r/4/diff/#index_header'
+ - 'See other reviews: http://$DOCKER_HOSTNAME:$HGPORT1/r/4/'
+ - author: reviewer@example.com
+ id: 9
+ tags: []
+ text:
+ - Comment on attachment 2
+ - 'MozReview Request: Bug 2 - Modified commit to review'
+ - ''
+ - http://$DOCKER_HOSTNAME:$HGPORT1/r/4/#review4
+ - ''
+ - LGTM
+ - author: l3author@example.com
+ id: 10
+ tags: []
+ text:
+ - Comment on attachment 2
+ - 'MozReview Request: Bug 2 - Modified commit to review'
+ - ''
+ - 'Review request updated; see interdiff: http://$DOCKER_HOSTNAME:$HGPORT1/r/4/diff/1-2/'
+ - author: l3author@example.com
+ id: 11
+ tags: []
+ text:
+ - Comment on attachment 2
+ - 'MozReview Request: Bug 2 - Modified commit to review'
+ - ''
+ - http://$DOCKER_HOSTNAME:$HGPORT1/r/4/#review5
+ - ''
+ - I changed my mind
+ component: TestComponent
+ depends_on: []
+ platform: All
+ product: TestProduct
+ resolution: ''
+ status: UNCONFIRMED
+ summary: Second Bug
+
$ cd ..
Cleanup
$ mozreview stop
stopped 9 containers
--- a/hgext/reviewboard/tests/test-bugzilla-review-interaction.t
+++ b/hgext/reviewboard/tests/test-bugzilla-review-interaction.t
@@ -73,17 +73,17 @@ Adding a reviewer should result in a r?
component: TestComponent
depends_on: []
platform: All
product: TestProduct
resolution: ''
status: UNCONFIRMED
summary: First Bug
-Adding a "Ship It" review will grant r+
+Adding a r+ review will grant a '+' review flag on bugzilla
$ exportbzauth reviewer@example.com password
$ rbmanage create-review 2 --body-top LGTM --public --review-flag='r+'
created review 1
$ bugzilla dump-bug 1
Bug 1:
attachments:
@@ -662,17 +662,17 @@ Test interaction with multiple commits.
platform: All
product: TestProduct
resolution: ''
status: UNCONFIRMED
summary: Parent Reviews
$ exportbzauth reviewer@example.com password
-Verify that a single ship-it r+s only that attachment.
+Verify that a single r+ affects only that attachment.
$ rbmanage create-review 11 --body-top 'land it!' --public --review-flag='r+'
created review 5
$ bugzilla dump-bug 5
Bug 5:
attachments:
- attacher: author@example.com
@@ -784,23 +784,23 @@ Verify that a single ship-it r+s only th
component: TestComponent
depends_on: []
platform: All
product: TestProduct
resolution: ''
status: UNCONFIRMED
summary: Parent Reviews
-Ship-it reviews are not allowed on the parent.
+r+ reviews are not allowed on the parent.
$ rbmanage create-review 9 --body-top 'all good!' --public --review-flag='r+'
API Error: 500: 225: Error publishing: "Ship it" reviews on parent review requests are not allowed. Please review individual commits.
[1]
-A non-ship-it review on a child should clear only that attachment's r+.
+A non 'clear flag' review shouldn't clear the attachment's review flag.
$ rbmanage create-review 11 --body-top 'there is a problem' --public
created review 7
$ bugzilla dump-bug 5
Bug 5:
attachments:
- attacher: author@example.com
@@ -829,16 +829,21 @@ A non-ship-it review on a child should c
description: 'MozReview Request: Bug 5 - Parent reviews, second commit'
file_name: reviewboard-11-url.txt
flags:
- id: 10
name: review
requestee: reviewer2@example.com
setter: author@example.com
status: '?'
+ - id: 11
+ name: review
+ requestee: reviewer@example.com
+ setter: reviewer@example.com
+ status: '?'
id: 6
is_obsolete: false
is_patch: false
summary: 'MozReview Request: Bug 5 - Parent reviews, second commit'
- attacher: author@example.com
content_type: text/x-review-board-request
data: http://$DOCKER_HOSTNAME:$HGPORT1/r/12/diff/#index_header
description: 'MozReview Request: Bug 5 - Parent reviews, third commit'
@@ -917,20 +922,20 @@ A non-ship-it review on a child should c
component: TestComponent
depends_on: []
platform: All
product: TestProduct
resolution: ''
status: UNCONFIRMED
summary: Parent Reviews
-A non-ship-it review on a child should also clear the attachment's r?.
+A 'clear flag' review on a child should also clear the attachment's r?.
$ exportbzauth reviewer2@example.com password
- $ rbmanage create-review 10 --body-top 'this is not good' --public
+ $ rbmanage create-review 10 --body-top 'this is not good' --public --review-flag=' '
created review 8
$ bugzilla dump-bug 5
Bug 5:
attachments:
- attacher: author@example.com
content_type: text/x-review-board-request
data: http://$DOCKER_HOSTNAME:$HGPORT1/r/10/diff/#index_header
@@ -952,16 +957,21 @@ A non-ship-it review on a child should a
description: 'MozReview Request: Bug 5 - Parent reviews, second commit'
file_name: reviewboard-11-url.txt
flags:
- id: 10
name: review
requestee: reviewer2@example.com
setter: author@example.com
status: '?'
+ - id: 11
+ name: review
+ requestee: reviewer@example.com
+ setter: reviewer@example.com
+ status: '?'
id: 6
is_obsolete: false
is_patch: false
summary: 'MozReview Request: Bug 5 - Parent reviews, second commit'
- attacher: author@example.com
content_type: text/x-review-board-request
data: http://$DOCKER_HOSTNAME:$HGPORT1/r/12/diff/#index_header
description: 'MozReview Request: Bug 5 - Parent reviews, third commit'
@@ -1050,17 +1060,17 @@ A non-ship-it review on a child should a
component: TestComponent
depends_on: []
platform: All
product: TestProduct
resolution: ''
status: UNCONFIRMED
summary: Parent Reviews
-A non-ship-it review on a parent should post a comment only.
+A non-r+ review on a parent should post a comment only.
$ exportbzauth reviewer2@example.com password
$ rbmanage create-review 9 --body-top 'actually none of this is good' --public
created review 9
$ bugzilla dump-bug 5
Bug 5:
attachments:
@@ -1085,16 +1095,21 @@ A non-ship-it review on a parent should
description: 'MozReview Request: Bug 5 - Parent reviews, second commit'
file_name: reviewboard-11-url.txt
flags:
- id: 10
name: review
requestee: reviewer2@example.com
setter: author@example.com
status: '?'
+ - id: 11
+ name: review
+ requestee: reviewer@example.com
+ setter: reviewer@example.com
+ status: '?'
id: 6
is_obsolete: false
is_patch: false
summary: 'MozReview Request: Bug 5 - Parent reviews, second commit'
- attacher: author@example.com
content_type: text/x-review-board-request
data: http://$DOCKER_HOSTNAME:$HGPORT1/r/12/diff/#index_header
description: 'MozReview Request: Bug 5 - Parent reviews, third commit'
--- a/hgext/reviewboard/tests/test-review-request-approval.t
+++ b/hgext/reviewboard/tests/test-review-request-approval.t
@@ -77,17 +77,17 @@ Create a review request from an L1 user
- +++ b/foo
- '@@ -1,1 +1,1 @@'
- -foo
- +initial
- ''
approved: false
approval_failure: A suitable reviewer has not given a "Ship It!"
-Have an L1 user provide a ship it review which should not grant approval
+Have an L1 user provide a r+ review which should not grant approval
$ exportbzauth l1b@example.com password
$ rbmanage create-review 2 --body-top "Ship-it!" --public --review-flag='r+'
created review 1
$ rbmanage dumpreview 2
id: 2
status: pending
public: true
@@ -134,17 +134,17 @@ Have an L1 user provide a ship it review
public: true
ship_it: true
extra_data:
p2rb.review_flag: r+
body_top: Ship-it!
body_top_text_type: plain
diff_comments: []
-Have an L3 user provide a ship it review which should grant approval
+Have an L3 user provide a r+ review which should grant approval
$ exportbzauth l3@example.com password
$ rbmanage create-review 2 --body-top "Ship-it!" --public --review-flag='r+'
created review 2
$ rbmanage dumpreview 2
id: 2
status: pending
public: true
@@ -199,17 +199,17 @@ Have an L3 user provide a ship it review
public: true
ship_it: true
extra_data:
p2rb.review_flag: r+
body_top: Ship-it!
body_top_text_type: plain
diff_comments: []
-Posting a new review without ship it should cancel the previous approval
+Posting a new review without r+ should cancel the previous approval
$ rbmanage create-review 2 --body-top "Don't Land it!" --public
created review 3
$ rbmanage dumpreview 2
id: 2
status: pending
public: true
bugs:
- '1'
@@ -265,22 +265,22 @@ Posting a new review without ship it sho
p2rb.review_flag: r+
body_top: Ship-it!
body_top_text_type: plain
diff_comments: []
- id: 3
public: true
ship_it: false
extra_data:
- p2rb.review_flag: ' '
+ p2rb.review_flag: r?
body_top: Don't Land it!
body_top_text_type: plain
diff_comments: []
-One more ship it should switch it back to approved
+One more r+ should switch it back to approved
$ rbmanage create-review 2 --body-top "NVM, Ship-it!" --public --review-flag='r+'
created review 4
$ rbmanage dumpreview 2
id: 2
status: pending
public: true
bugs:
@@ -337,17 +337,17 @@ One more ship it should switch it back t
p2rb.review_flag: r+
body_top: Ship-it!
body_top_text_type: plain
diff_comments: []
- id: 3
public: true
ship_it: false
extra_data:
- p2rb.review_flag: ' '
+ p2rb.review_flag: r?
body_top: Don't Land it!
body_top_text_type: plain
diff_comments: []
- id: 4
public: true
ship_it: true
extra_data:
p2rb.review_flag: r+
@@ -432,30 +432,30 @@ Even though the author is L1, adding a n
p2rb.review_flag: r+
body_top: Ship-it!
body_top_text_type: plain
diff_comments: []
- id: 3
public: true
ship_it: false
extra_data:
- p2rb.review_flag: ' '
+ p2rb.review_flag: r?
body_top: Don't Land it!
body_top_text_type: plain
diff_comments: []
- id: 4
public: true
ship_it: true
extra_data:
p2rb.review_flag: r+
body_top: NVM, Ship-it!
body_top_text_type: plain
diff_comments: []
-A new ship-it from L3 should give approval
+A new r+ from L3 should give approval
$ rbmanage create-review 2 --body-top "Update looks good!" --public --review-flag='r+'
created review 5
$ rbmanage dumpreview 2
id: 2
status: pending
public: true
bugs:
@@ -525,17 +525,17 @@ A new ship-it from L3 should give approv
p2rb.review_flag: r+
body_top: Ship-it!
body_top_text_type: plain
diff_comments: []
- id: 3
public: true
ship_it: false
extra_data:
- p2rb.review_flag: ' '
+ p2rb.review_flag: r?
body_top: Don't Land it!
body_top_text_type: plain
diff_comments: []
- id: 4
public: true
ship_it: true
extra_data:
p2rb.review_flag: r+
@@ -631,17 +631,17 @@ Opening issues, even from an L1 user, sh
p2rb.review_flag: r+
body_top: Ship-it!
body_top_text_type: plain
diff_comments: []
- id: 3
public: true
ship_it: false
extra_data:
- p2rb.review_flag: ' '
+ p2rb.review_flag: r?
body_top: Don't Land it!
body_top_text_type: plain
diff_comments: []
- id: 4
public: true
ship_it: true
extra_data:
p2rb.review_flag: r+
@@ -655,17 +655,17 @@ Opening issues, even from an L1 user, sh
p2rb.review_flag: r+
body_top: Update looks good!
body_top_text_type: plain
diff_comments: []
- id: 6
public: true
ship_it: false
extra_data:
- p2rb.review_flag: ' '
+ p2rb.review_flag: r?
body_top: I found issues
body_top_text_type: plain
diff_comments:
- id: 1
public: true
user: level1b
issue_opened: true
issue_status: open
@@ -752,17 +752,17 @@ Fixing the issue should restore approval
p2rb.review_flag: r+
body_top: Ship-it!
body_top_text_type: plain
diff_comments: []
- id: 3
public: true
ship_it: false
extra_data:
- p2rb.review_flag: ' '
+ p2rb.review_flag: r?
body_top: Don't Land it!
body_top_text_type: plain
diff_comments: []
- id: 4
public: true
ship_it: true
extra_data:
p2rb.review_flag: r+
@@ -776,17 +776,17 @@ Fixing the issue should restore approval
p2rb.review_flag: r+
body_top: Update looks good!
body_top_text_type: plain
diff_comments: []
- id: 6
public: true
ship_it: false
extra_data:
- p2rb.review_flag: ' '
+ p2rb.review_flag: r?
body_top: I found issues
body_top_text_type: plain
diff_comments:
- id: 1
public: true
user: level1b
issue_opened: true
issue_status: resolved
--- a/pylib/mozreview/mozreview/bugzilla/client.py
+++ b/pylib/mozreview/mozreview/bugzilla/client.py
@@ -418,44 +418,41 @@ class Bugzilla(object):
review_flags = [f for f in flag_list if f['name'] == 'review']
for f in review_flags:
# Bugzilla attachments have a requestee only if the status is `?`.
# In the other cases requestee == setter.
if ((reviewer == f.get('requestee') and f['status'] == '?') or
(reviewer == f.get('setter') and f['status'] != '?')):
- # Always clear the flag if desired status is not r+.
- if flag == '?':
- flag_obj['status'] = 'X'
-
# Flag is already set, don't change it.
- elif f['status'] == flag:
+ if f['status'] == flag:
logger.info('r%s already set.' % flag)
return False
flag_obj['id'] = f['id']
break
else:
# The reviewer did not have a flag on the attachment.
if flag == 'X':
# This shouldn't happen under normal circumstances, but if it
# does log it.
logger.info('No flag to clear for %s on %s' % (
reviewer, rb_attachment
))
return False
- elif flag not in ('+', '-'):
- # If the reviewer has no +/- set and they submit a comment
- # don't create any flag.
- return False
# Flag not found, let's create a new one.
flag_obj['new'] = True
+ # If the reviewer is setting the flag back to ?,
+ # they will then be both the setter and the requestee
+ if flag == '?':
+ flag_obj['requestee'] = reviewer
+
logging.info('sending flag: %s' % flag_obj)
params = self._auth_params({
'ids': [rb_attachment['id']],
'flags': [flag_obj],
})
if comment:
--- a/pylib/mozreview/mozreview/signal_handlers.py
+++ b/pylib/mozreview/mozreview/signal_handlers.py
@@ -565,12 +565,9 @@ def pre_save_review(sender, *args, **kwa
if (REVIEW_FLAG_KEY not in review.extra_data and
not is_parent(review.review_request)):
if review.pk:
# The review create endpoint calls save twice: the first time it
# gets or creates the review and the second time it updates the
# object retrieved/created. This condition let us execute the code
# below only when all the fields are set.
- if review.ship_it:
- review.extra_data[REVIEW_FLAG_KEY] = 'r+'
- else:
- review.extra_data[REVIEW_FLAG_KEY] = ' '
+ review.extra_data[REVIEW_FLAG_KEY] = 'r?'