mozreview: add r?, r+, r- UI to the Review Dialog. (bug 1197879) r?smacleod r?mconley draft
authorMike Conley <mconley@mozilla.com>
Tue, 03 May 2016 19:22:11 +0100
changeset 8120 c7043a56fcd3dfbda95169199de561451275fab9
parent 8119 f65d74673981650c3d2e089207487a325b8d4151
child 8121 84ca3ce51ec1bb6aa5f012913e370f72c4e05523
push id830
push usermdoglio@mozilla.com
push dateTue, 17 May 2016 10:03:33 +0000
reviewerssmacleod, mconley
bugs1197879
mozreview: add r?, r+, r- UI to the Review Dialog. (bug 1197879) r?smacleod r?mconley MozReview-Commit-ID: 1j8eQgtY5qo
pylib/mozreview/mozreview/extension.py
pylib/mozreview/mozreview/static/mozreview/css/review.less
pylib/mozreview/mozreview/static/mozreview/js/review_flag.js
pylib/mozreview/mozreview/templates/mozreview/review-scripts-js.html
--- a/pylib/mozreview/mozreview/extension.py
+++ b/pylib/mozreview/mozreview/extension.py
@@ -97,27 +97,35 @@ SETTINGS = None
 logger = logging.getLogger(__name__)
 
 
 class ParentJSExtension(JSExtension):
     model_class = 'MRParents.Extension'
     apply_to = review_request_url_names
 
 
+class MRReviewFlagExtension(JSExtension):
+    model_class = 'MRReviewFlag.Extension'
+    apply_to = review_request_url_names
+
+
 class MozReviewExtension(Extension):
     metadata = {
         'Name': 'mozreview',
         'Summary': 'MozReview extension to Review Board',
     }
 
     default_settings = {}
 
     is_configurable = True
 
-    js_extensions = [ParentJSExtension]
+    js_extensions = [
+        MRReviewFlagExtension,
+        ParentJSExtension,
+    ]
 
     css_bundles = {
         'default': {
             'source_filenames': ['mozreview/css/common.less'],
         },
         'review': {
             'source_filenames': ['mozreview/css/review.less',
                                  'mozreview/css/commits.less'],
@@ -141,17 +149,20 @@ class MozReviewExtension(Extension):
         },
         'reviews': {
             # TODO: Everything will break if init_rr.js is not first in this
             # list.
             'source_filenames': ['mozreview/js/init_rr.js',
                                  'mozreview/js/commits.js',
                                  'mozreview/js/review.js',
                                  'mozreview/js/autoland.js',
-                                 'mozreview/js/ui.mozreviewautocomplete.js',]
+                                 'mozreview/js/ui.mozreviewautocomplete.js',
+                                 'mozreview/js/review_flag.js',
+                                 ],
+            'apply_to': review_request_url_names,
         },
         'parent-review-requests': {
             'source_filenames': ['mozreview/js/parents.js'],
             'apply_to': review_request_url_names,
         },
     }
 
     resources = [
--- a/pylib/mozreview/mozreview/static/mozreview/css/review.less
+++ b/pylib/mozreview/mozreview/static/mozreview/css/review.less
@@ -119,23 +119,16 @@ label[for="mozreview-autoland-try-syntax
   }
 }
 
 #try-syntax-error {
   display: none;
   color: red;
 }
 
-.parent-request {
-  #id_shipit,
-  label[for="id_shipit"] {
-    display: none;
-  }
-}
-
 .parent-request #review-form .edit-field {
   // Without the Ship It checkbox, the edit field gets squished up
   // with the Markdown Reference link unless we clear the latter's float.
   clear: right;
 }
 
 // Hide the "Reviewers" section; unfortunately these don't have IDs
 // TODO: Update this to use fieldset IDs once they are available upstream
@@ -146,8 +139,15 @@ label[for="mozreview-autoland-try-syntax
   }
 }
 
 .parent-warning {
   padding-left: 10px;
   padding-right: 10px;
   font-size: 10pt;
 }
+
+// Hide the "Ship it" checkbox, since we use the r?, r+, r- flags provided
+// by MRReviewFlag.
+#id_shipit,
+label[for="id_shipit"] {
+  display: none;
+}
new file mode 100644
--- /dev/null
+++ b/pylib/mozreview/mozreview/static/mozreview/js/review_flag.js
@@ -0,0 +1,97 @@
+/**
+ * MRReviewFlag is responsible for showing the r?, r+, r- select dropdown
+ * in the review dialog. It will also do the work of setting the extraData
+ * in a Review model with the value that the user selects.
+ */
+MRReviewFlag = {};
+
+
+MRReviewFlag.View = Backbone.View.extend({
+  /**
+   * If we save state, it'll be to a Review's extraData using this
+   * key. This is the js counterpart of mozreview.extra_data.REVIEW_FLAG_KEY
+   */
+  key: 'p2rb.review_flag',
+
+  /**
+   * These are the possible states that will be shown in the dropdown,
+   * in order. These are also the values that will be stored in the
+   * extraData field.
+   */
+  states: [' ', 'r?', 'r+', 'r-'],
+
+  template: _.template([
+      '<label for="mr-review-flag">Review state:</label>',
+      '<select id="mr-review-flag">',
+      '<% _(states).each(function(state) { %>',
+      '  <option <% if (state === val) { %> selected <% } %> >',
+      '    <%= state %>',
+      '  </option>',
+      '<% }); %>',
+      '</select>'
+  ].join('')),
+
+  events: {
+    'change #mr-review-flag': 'updateReviewState'
+  },
+
+  initialize: function() {
+    _super(this).initialize.call(this);
+
+    /**
+    *  The first time a ReviewDialogHook is rendered, it will then retrieve
+    *  the model data off the server and update the extraData. We have to
+    *  wait for that update, and then re-render in order to show the value
+    *  that is being stored server-side.
+    */
+
+    this.listenTo(this.model, 'change:extraData', _.bind(this.render, this));
+  },
+
+  /**
+  *  Under some circumstances a model save will replace the comment textarea
+  *  with a 'Add text' link. This is a hack to circumvent that problem.
+  */
+  save: function(){
+    this.model.save({
+      success: _.bind(function(){
+        $('#review-form-comments .body-top a.add-link').click();
+      }, this)
+    });
+  },
+
+  render: function() {
+    this.$el.html(this.template({
+      states: this.states,
+      val: this.model.get('extraData')[this.key] || 'r?'
+    }));
+    return this;
+  },
+
+  updateReviewState: function() {
+    var val = this.$el.find('#mr-review-flag').val();
+    // We have to send the 'clear the flag' as a non empty string because
+    // the api endpoint will ignore it otherwise.
+    if (val === '') {
+        val = ' ';
+    }
+    this.model.get('extraData')[this.key] = val;
+    $('#id_shipit').prop('checked', (val == 'r+'));
+    this.save()
+  }
+});
+
+
+MRReviewFlag.Extension = RB.Extension.extend({
+  initialize: function() {
+    _super(this).initialize.call(this);
+    $(document).on("mozreview_ready", _.bind(function() {
+      if (!MozReview.isParent) {
+        new RB.ReviewDialogHook({
+          extension: this,
+          viewType: MRReviewFlag.View
+        });
+      }
+    }, this));
+  }
+});
--- a/pylib/mozreview/mozreview/templates/mozreview/review-scripts-js.html
+++ b/pylib/mozreview/mozreview/templates/mozreview/review-scripts-js.html
@@ -1,12 +1,8 @@
 {% load djblets_extensions %}
 {% load mozreview %}
 
-{% if review_request|isPush %}
-{%   ext_js_bundle extension "reviews" %}
-
 {%   if review_request|isSquashed %}
 {%     ext_js_bundle extension "try" %}
 {%   endif %}
 
-{% endif %}