mozreview: use set_attachment_flag on review publishing (bug 1197879) r?smacleod r?mcote draft
authorMauro Doglio <mdoglio@mozilla.com>
Fri, 08 Apr 2016 13:02:20 +0100
changeset 8119 f65d74673981650c3d2e089207487a325b8d4151
parent 8118 e8917df4bf7748af97afbcbd53888ec65f6d2de5
child 8120 c7043a56fcd3dfbda95169199de561451275fab9
push id830
push usermdoglio@mozilla.com
push dateTue, 17 May 2016 10:03:33 +0000
reviewerssmacleod, mcote
bugs1197879
mozreview: use set_attachment_flag on review publishing (bug 1197879) r?smacleod r?mcote This is just to prepare the ground for when the ui will start setting r+/- from the review dialog MozReview-Commit-ID: 1Wr3pQrp49l
hgext/reviewboard/tests/test-bugzilla-review-flag-cleared.t
hgext/reviewboard/tests/test-bugzilla-review-interaction.t
hgext/reviewboard/tests/test-review-commit-rewrite.t
hgext/reviewboard/tests/test-review-request-approval.t
hgext/reviewboard/tests/test-review-request-summary.t
hgext/reviewboard/tests/test-reviewer-flags.t
pylib/mozreview/mozreview/extra_data.py
pylib/mozreview/mozreview/signal_handlers.py
pylib/rbbz/rbbz/extension.py
testing/vcttesting/reviewboard/mach_commands.py
--- a/hgext/reviewboard/tests/test-bugzilla-review-flag-cleared.t
+++ b/hgext/reviewboard/tests/test-bugzilla-review-flag-cleared.t
@@ -183,17 +183,17 @@ Posting a non Ship It review without a r
     platform: All
     product: TestProduct
     resolution: ''
     status: UNCONFIRMED
     summary: First Bug
 
 Posting a Ship It review will add an r+
 
-  $ rbmanage create-review 2 --body-top LGTM --public --ship-it
+  $ 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
@@ -409,17 +409,17 @@ Sanity check to ensure we have an r? fla
     product: TestProduct
     resolution: ''
     status: UNCONFIRMED
     summary: Second Bug
 
 Post a Ship It review so we can carry it forward
 
   $ exportbzauth reviewer@example.com password
-  $ rbmanage create-review 4 --body-top LGTM --public --ship-it
+  $ rbmanage create-review 4 --body-top LGTM --public --review-flag='r+'
   created review 4
 
 Sanity check to ensure we have an r+ flag set
 
   $ bugzilla dump-bug 2
   Bug 2:
     attachments:
     - attacher: l3author@example.com
--- a/hgext/reviewboard/tests/test-bugzilla-review-interaction.t
+++ b/hgext/reviewboard/tests/test-bugzilla-review-interaction.t
@@ -76,17 +76,17 @@ Adding a reviewer should result in a r? 
     product: TestProduct
     resolution: ''
     status: UNCONFIRMED
     summary: First Bug
 
 Adding a "Ship It" review will grant r+
 
   $ exportbzauth reviewer@example.com password
-  $ rbmanage create-review 2 --body-top LGTM --public --ship-it
+  $ rbmanage create-review 2 --body-top LGTM --public --review-flag='r+'
   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
@@ -387,17 +387,17 @@ review? sticks around when 1 person gran
 
   $ rbmanage add-reviewer 5 --user reviewer --user rev2
   2 people listed on review request
   $ rbmanage add-reviewer 6 --user reviewer --user rev2
   2 people listed on review request
   $ rbmanage publish 5
 
   $ exportbzauth reviewer@example.com password
-  $ rbmanage create-review 6 --body-top 'land it!' --public --ship-it
+  $ rbmanage create-review 6 --body-top 'land it!' --public --review-flag='r+'
   created review 3
 
   $ bugzilla dump-bug 3
   Bug 3:
     attachments:
     - attacher: author@example.com
       content_type: text/x-review-board-request
       data: http://$DOCKER_HOSTNAME:$HGPORT1/r/6/diff/#index_header
@@ -467,17 +467,17 @@ Random users can come along and grant re
 
   $ rbmanage add-reviewer 7 --user reviewer
   1 people listed on review request
   $ rbmanage add-reviewer 8 --user reviewer
   1 people listed on review request
   $ rbmanage publish 7
 
   $ exportbzauth troll@example.com password
-  $ rbmanage create-review 8 --body-top 'I am always watching' --public --ship-it
+  $ rbmanage create-review 8 --body-top 'I am always watching' --public --review-flag='r+'
   created review 4
 
   $ bugzilla dump-bug 4
   Bug 4:
     attachments:
     - attacher: author@example.com
       content_type: text/x-review-board-request
       data: http://$DOCKER_HOSTNAME:$HGPORT1/r/8/diff/#index_header
@@ -664,17 +664,17 @@ Test interaction with multiple commits.
     resolution: ''
     status: UNCONFIRMED
     summary: Parent Reviews
 
   $ exportbzauth reviewer@example.com password
 
 Verify that a single ship-it r+s only that attachment.
 
-  $ rbmanage create-review 11 --body-top 'land it!' --public --ship-it
+  $ 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
       content_type: text/x-review-board-request
       data: http://$DOCKER_HOSTNAME:$HGPORT1/r/10/diff/#index_header
@@ -786,17 +786,17 @@ Verify that a single ship-it r+s only th
     platform: All
     product: TestProduct
     resolution: ''
     status: UNCONFIRMED
     summary: Parent Reviews
 
 Ship-it reviews are not allowed on the parent.
 
-  $ rbmanage create-review 9 --body-top 'all good!' --public --ship-it
+  $ 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+.
 
   $ rbmanage create-review 11 --body-top 'there is a problem' --public
   created review 7
 
--- a/hgext/reviewboard/tests/test-review-commit-rewrite.t
+++ b/hgext/reviewboard/tests/test-review-commit-rewrite.t
@@ -47,17 +47,17 @@ Create bug and review
   (published review request 1)
   $ rbmanage add-reviewer 2 --user reviewer
   1 people listed on review request
   $ rbmanage publish 1
   $ rbmanage dump-rewrite-commit 1
   API Error: 405: 1008: Unable to continue as the review has not been approved.
   [1]
   $ exportbzauth reviewer@example.com password
-  $ rbmanage create-review 2 --body-top "Ship-it!" --public --ship-it
+  $ rbmanage create-review 2 --body-top "Ship-it!" --public --review-flag='r+'
   created review 1
 
 Check for rewrite on parent
 
   $ rbmanage dump-rewrite-commit 1
   commits:
   - commit: 63c61970184bac9e9ae1660344e26e98587b0103
     id: 2
--- a/hgext/reviewboard/tests/test-review-request-approval.t
+++ b/hgext/reviewboard/tests/test-review-request-approval.t
@@ -80,17 +80,17 @@ Create a review request from an L1 user
     - +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
 
   $ exportbzauth l1b@example.com password
-  $ rbmanage create-review 2 --body-top "Ship-it!" --public --ship-it
+  $ rbmanage create-review 2 --body-top "Ship-it!" --public --review-flag='r+'
   created review 1
   $ rbmanage dumpreview 2
   id: 2
   status: pending
   public: true
   bugs:
   - '1'
   commit: null
@@ -128,24 +128,26 @@ Have an L1 user provide a ship it review
     - ''
   approved: false
   approval_failure: A suitable reviewer has not given a "Ship It!"
   review_count: 1
   reviews:
   - id: 1
     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
 
   $ exportbzauth l3@example.com password
-  $ rbmanage create-review 2 --body-top "Ship-it!" --public --ship-it
+  $ rbmanage create-review 2 --body-top "Ship-it!" --public --review-flag='r+'
   created review 2
   $ rbmanage dumpreview 2
   id: 2
   status: pending
   public: true
   bugs:
   - '1'
   commit: null
@@ -183,22 +185,26 @@ Have an L3 user provide a ship it review
     - ''
   approved: true
   approval_failure: null
   review_count: 2
   reviews:
   - id: 1
     public: true
     ship_it: true
+    extra_data:
+      p2rb.review_flag: r+
     body_top: Ship-it!
     body_top_text_type: plain
     diff_comments: []
   - id: 2
     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
   $ rbmanage create-review 2 --body-top "Don't Land it!" --public
   created review 3
   $ rbmanage dumpreview 2
@@ -242,35 +248,41 @@ Posting a new review without ship it sho
     - ''
   approved: false
   approval_failure: A suitable reviewer has not given a "Ship It!"
   review_count: 3
   reviews:
   - id: 1
     public: true
     ship_it: true
+    extra_data:
+      p2rb.review_flag: r+
     body_top: Ship-it!
     body_top_text_type: plain
     diff_comments: []
   - id: 2
     public: true
     ship_it: true
+    extra_data:
+      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: ' '
     body_top: Don't Land it!
     body_top_text_type: plain
     diff_comments: []
 
 One more ship it should switch it back to approved
 
-  $ rbmanage create-review 2 --body-top "NVM, Ship-it!" --public --ship-it
+  $ 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:
   - '1'
   commit: null
@@ -308,34 +320,42 @@ One more ship it should switch it back t
     - ''
   approved: true
   approval_failure: null
   review_count: 4
   reviews:
   - id: 1
     public: true
     ship_it: true
+    extra_data:
+      p2rb.review_flag: r+
     body_top: Ship-it!
     body_top_text_type: plain
     diff_comments: []
   - id: 2
     public: true
     ship_it: true
+    extra_data:
+      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: ' '
     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: []
 
 Even though the author is L1, adding a new diff will not cancel approval
 
   $ echo modified > foo
   $ hg commit --amend > /dev/null
@@ -395,41 +415,49 @@ Even though the author is L1, adding a n
     - ''
   approved: true
   approval_failure: null
   review_count: 4
   reviews:
   - id: 1
     public: true
     ship_it: true
+    extra_data:
+      p2rb.review_flag: r+
     body_top: Ship-it!
     body_top_text_type: plain
     diff_comments: []
   - id: 2
     public: true
     ship_it: true
+    extra_data:
+      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: ' '
     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
 
-  $ rbmanage create-review 2 --body-top "Update looks good!" --public --ship-it
+  $ 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:
   - '1'
   commit: null
@@ -480,40 +508,50 @@ A new ship-it from L3 should give approv
     - ''
   approved: true
   approval_failure: null
   review_count: 5
   reviews:
   - id: 1
     public: true
     ship_it: true
+    extra_data:
+      p2rb.review_flag: r+
     body_top: Ship-it!
     body_top_text_type: plain
     diff_comments: []
   - id: 2
     public: true
     ship_it: true
+    extra_data:
+      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: ' '
     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: []
   - id: 5
     public: true
     ship_it: true
+    extra_data:
+      p2rb.review_flag: r+
     body_top: Update looks good!
     body_top_text_type: plain
     diff_comments: []
 
 Opening issues, even from an L1 user, should revoke approval until they're fixed
 
   $ exportbzauth l1b@example.com password
   $ rbmanage create-review 2 --body-top "I found issues"
@@ -576,46 +614,58 @@ Opening issues, even from an L1 user, sh
     - ''
   approved: false
   approval_failure: The review request has open issues.
   review_count: 6
   reviews:
   - id: 1
     public: true
     ship_it: true
+    extra_data:
+      p2rb.review_flag: r+
     body_top: Ship-it!
     body_top_text_type: plain
     diff_comments: []
   - id: 2
     public: true
     ship_it: true
+    extra_data:
+      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: ' '
     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: []
   - id: 5
     public: true
     ship_it: true
+    extra_data:
+      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: ' '
     body_top: I found issues
     body_top_text_type: plain
     diff_comments:
     - id: 1
       public: true
       user: level1b
       issue_opened: true
       issue_status: open
@@ -685,46 +735,58 @@ Fixing the issue should restore approval
     - ''
   approved: true
   approval_failure: null
   review_count: 6
   reviews:
   - id: 1
     public: true
     ship_it: true
+    extra_data:
+      p2rb.review_flag: r+
     body_top: Ship-it!
     body_top_text_type: plain
     diff_comments: []
   - id: 2
     public: true
     ship_it: true
+    extra_data:
+      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: ' '
     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: []
   - id: 5
     public: true
     ship_it: true
+    extra_data:
+      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: ' '
     body_top: I found issues
     body_top_text_type: plain
     diff_comments:
     - id: 1
       public: true
       user: level1b
       issue_opened: true
       issue_status: resolved
@@ -788,17 +850,17 @@ Review requests created by L3 users
     - +author2
     - ''
   approved: false
   approval_failure: A suitable reviewer has not given a "Ship It!"
 
 Even a ship-it from an L1 user will give approval to an L3 author
 
   $ exportbzauth l1a@example.com password
-  $ rbmanage create-review 4 --body-top "Ship-it!" --public --ship-it
+  $ rbmanage create-review 4 --body-top "Ship-it!" --public --review-flag='r+'
   created review 7
   $ rbmanage dumpreview 4
   id: 4
   status: pending
   public: true
   bugs:
   - '2'
   commit: null
@@ -836,16 +898,18 @@ Even a ship-it from an L1 user will give
     - ''
   approved: true
   approval_failure: null
   review_count: 1
   reviews:
   - id: 7
     public: true
     ship_it: true
+    extra_data:
+      p2rb.review_flag: r+
     body_top: Ship-it!
     body_top_text_type: plain
     diff_comments: []
 
 We trust L3 authors to update diffs and carry forward approval from previous
 ship-its. Posting a new diff should not clear approval
 
   $ exportbzauth l3@example.com password
@@ -907,16 +971,18 @@ ship-its. Posting a new diff should not 
     - ''
   approved: true
   approval_failure: null
   review_count: 1
   reviews:
   - id: 7
     public: true
     ship_it: true
+    extra_data:
+      p2rb.review_flag: r+
     body_top: Ship-it!
     body_top_text_type: plain
     diff_comments: []
 
   $ cd ..
 
 Cleanup
 
--- a/hgext/reviewboard/tests/test-review-request-summary.t
+++ b/hgext/reviewboard/tests/test-review-request-summary.t
@@ -158,17 +158,17 @@ Resolving an issue should decrement the 
     issue_open_count: 0
     status: pending
     reviewers: []
     reviewers_status: {}
 
 Giving a ship-it should result in a change in the reviewer status
 
   $ exportbzauth reviewer@example.com password1
-  $ rbmanage create-review 2 --ship-it --public
+  $ rbmanage create-review 2 --review-flag='r+' --public
   created review 2
 
   $ rbmanage dump-summary 1
   parent:
     summary: bz://1/mynick
     id: 1
     submitter: default+5
     issue_open_count: 0
--- a/hgext/reviewboard/tests/test-reviewer-flags.t
+++ b/hgext/reviewboard/tests/test-reviewer-flags.t
@@ -49,17 +49,17 @@ a shipit.
   (published review request 1)
 
   $ rbmanage list-reviewers 2
   cthulhu
 
 There are no warnings for reviewers who haved granted a ship-it when using r=
 
   $ exportbzauth cthulhu@example.com password
-  $ rbmanage create-review 2 --body-top "Ship-it!" --public --ship-it
+  $ rbmanage create-review 2 --body-top "Ship-it!" --public --review-flag='r+'
   created review 1
   $ exportbzauth default@example.com password
   $ echo foo >> foo
   $ hg commit --amend -l - << EOF
   > bug 1 - serious changes; r=cthulhu
   > 
   > MozReview-Commit-ID: 124Bxg
   > EOF
--- a/pylib/mozreview/mozreview/extra_data.py
+++ b/pylib/mozreview/mozreview/extra_data.py
@@ -15,16 +15,17 @@ from reviewboard.reviews.models import (
 from mozreview.models import (
     CommitData,
 )
 
 MOZREVIEW_KEY = 'p2rb'
 
 # Built-in extra_data keys:
 REVIEWER_MAP_KEY = MOZREVIEW_KEY + '.reviewer_map'
+REVIEW_FLAG_KEY = MOZREVIEW_KEY + '.review_flag'
 
 # CommitData field keys:
 AUTHOR_KEY = MOZREVIEW_KEY + '.author'
 BASE_COMMIT_KEY = MOZREVIEW_KEY + '.base_commit'
 COMMIT_ID_KEY = MOZREVIEW_KEY + '.commit_id'
 COMMITS_KEY = MOZREVIEW_KEY + '.commits'
 DISCARD_ON_PUBLISH_KEY = MOZREVIEW_KEY + '.discard_on_publish_rids'
 FIRST_PUBLIC_ANCESTOR_KEY = MOZREVIEW_KEY + '.first_public_ancestor'
--- a/pylib/mozreview/mozreview/signal_handlers.py
+++ b/pylib/mozreview/mozreview/signal_handlers.py
@@ -2,28 +2,30 @@ from __future__ import unicode_literals
 
 import copy
 import json
 import logging
 
 from django.db.models.signals import (
     post_save,
     pre_delete,
+    pre_save,
 )
 
 from djblets.siteconfig.models import (
     SiteConfiguration,
 )
 from reviewboard.reviews.errors import (
     PublishError,
 )
 from reviewboard.extensions.hooks import (
     SignalHook,
 )
 from reviewboard.reviews.models import (
+    Review,
     ReviewRequest,
     ReviewRequestDraft,
 )
 from reviewboard.reviews.signals import (
     review_request_closed,
     review_request_publishing,
     review_request_reopened,
 )
@@ -51,16 +53,17 @@ from mozreview.extra_data import (
     gen_child_rrs,
     gen_rrs_by_extra_data_key,
     gen_rrs_by_rids,
     get_parent_rr,
     IDENTIFIER_KEY,
     is_parent,
     is_pushed,
     REVIEWID_RE,
+    REVIEW_FLAG_KEY,
     UNPUBLISHED_KEY,
     update_parent_rr_reviewers,
 )
 from mozreview.messages import (
     AUTO_CLOSE_DESCRIPTION,
     AUTO_SUBMITTED_DESCRIPTION,
     NEVER_USED_DESCRIPTION,
     OBSOLETE_DESCRIPTION,
@@ -119,16 +122,22 @@ def initialize_signal_handlers(extension
         on_review_request_closed_submitted)
 
     SignalHook(
         extension,
         review_request_publishing,
         on_review_request_publishing,
         sandbox_errors=False)
 
+    SignalHook(
+        extension,
+        pre_save,
+        pre_save_review,
+        sender=Review)
+
 
 def post_save_review_request_draft(sender, **kwargs):
     """Handle post_save for a ReviewRequestDraft."""
     draft = kwargs["instance"]
 
     if kwargs["created"] and not kwargs["raw"]:
         copy_commit_data(draft)
 
@@ -539,8 +548,29 @@ def _close_child_review_requests(user, r
                                            commit_data=commit_data):
         child.close(ReviewRequest.DISCARDED,
                     user=user,
                     description=NEVER_USED_DESCRIPTION)
 
     commit_data.extra_data[UNPUBLISHED_KEY] = '[]'
     commit_data.extra_data[DISCARD_ON_PUBLISH_KEY] = '[]'
     commit_data.save(update_fields=['extra_data'])
+
+
+def pre_save_review(sender, *args, **kwargs):
+    """Handle pre_save for a Review.
+
+    This is needed to give a default value to the REVIEW_FLAG_KEY
+    extra_data key.
+    """
+    review = kwargs["instance"]
+
+    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] = ' '
--- a/pylib/rbbz/rbbz/extension.py
+++ b/pylib/rbbz/rbbz/extension.py
@@ -20,16 +20,17 @@ from mozreview.bugzilla.errors import (
     bugzilla_to_publish_errors,
 )
 from mozreview.errors import (
     ParentShipItError,
 )
 from mozreview.extra_data import (
     is_parent,
     is_pushed,
+    REVIEW_FLAG_KEY
 )
 from mozreview.models import (
     get_bugzilla_api_key,
 )
 from mozreview.rb_utils import (
     get_obj_url,
 )
 from rbbz.auth import BugzillaBackend
@@ -96,33 +97,45 @@ def on_review_publishing(user, review, *
                                                  siteconfig),
                                      {"user": user})
     b = Bugzilla(get_bugzilla_api_key(user))
 
     # TODO: Update all attachments in one call.  This is not possible right
     # now because we have to potentially mix changing and creating flags.
 
     if is_parent(review_request):
-        # Mirror the comment to the bug, unless it's a ship-it, in which
-        # case throw an error.  Ship-its are allowed only on child commits.
-        if review.ship_it:
+        # Mirror the comment to the bug, unless it's a ship-it or the review
+        # flag was set, in which case throw an error.
+        # Ship-its and review flags are allowed only on child commits.
+        if review.ship_it or review.extra_data.get(REVIEW_FLAG_KEY):
             raise ParentShipItError
 
         [b.post_comment(int(bug_id), comment) for bug_id in
          review_request.get_bug_list()]
     else:
         diff_url = '%sdiff/#index_header' % get_obj_url(review_request)
         bug_id = int(review_request.get_bug_list()[0])
 
-        if review.ship_it:
-            commented = b.r_plus_attachment(bug_id, review.user.email,
-                                            diff_url, comment)
+        commented = False
+        flag = review.extra_data.get(REVIEW_FLAG_KEY)
+
+        if flag is not None:
+            commented = b.set_review_flag(bug_id, flag, review.user.email,
+                                              diff_url, comment)
         else:
-            commented = b.cancel_review_request(bug_id, review.user.email,
+            # If for some reasons we don't have the flag set in extra_data,
+            # fall back to ship_it
+            logger.warning('Review flag not set on review %s, '
+                           'updating attachment based on ship_it' % review.id)
+            if review.ship_it:
+                commented = b.r_plus_attachment(bug_id, review.user.email,
                                                 diff_url, comment)
+            else:
+                commented = b.cancel_review_request(bug_id, review.user.email,
+                                                    diff_url, comment)
 
         if comment and not commented:
             b.post_comment(bug_id, comment)
 
 
 @bugzilla_to_publish_errors
 def on_reply_publishing(user, reply, **kwargs):
     review_request = reply.review_request
--- a/testing/vcttesting/reviewboard/mach_commands.py
+++ b/testing/vcttesting/reviewboard/mach_commands.py
@@ -408,26 +408,31 @@ class ReviewBoardCommands(object):
         description='Create a new review on a review request')
     @CommandArgument('rrid', help='Review request to create the review on')
     @CommandArgument('--body-bottom',
             help='Review content below comments')
     @CommandArgument('--body-top',
             help='Review content above comments')
     @CommandArgument('--public', action='store_true',
             help='Whether to make this review public')
-    @CommandArgument('--ship-it', action='store_true',
-            help='Whether to mark the review "Ship It"')
+    @CommandArgument('--review-flag', action='store',
+            help='Bugzilla-style review flag to set')
     def create_review(self, rrid, body_bottom=None, body_top=None, public=False,
-            ship_it=False):
+            review_flag=None):
         from rbtools.api.errors import APIError
         root = self._get_root()
         reviews = root.get_reviews(review_request_id=rrid)
         # rbtools will convert body_* to str() and insert "None" if we pass
         # an argument.
-        args = {'public': public, 'ship_it': ship_it}
+        args = {'public': public}
+
+        if review_flag:
+            args['ship_it'] = (review_flag == 'r+')
+            args['extra_data.p2rb.review_flag'] = review_flag
+
         if body_bottom:
             args['body_bottom'] = body_bottom
         if body_top:
             args['body_top'] = body_top
 
         try:
             r = reviews.create(**args)
         except APIError as e: