mozreview: warn when autolanding commits that have changed since last review(Bug 1239798) r?smacleod draft
authorDan Minor <dminor@mozilla.com>
Thu, 21 Jan 2016 05:54:49 -0500
changeset 6945 6da1817e43e9cbd8898077996db45b9caf242c6c
parent 6944 1bfee6c9b44d613296e04b8385997540f8fc342e
push id551
push userdminor@mozilla.com
push dateMon, 25 Jan 2016 20:21:17 +0000
reviewerssmacleod
bugs1239798
mozreview: warn when autolanding commits that have changed since last review(Bug 1239798) r?smacleod This causes an warning to be displayed during autoland for each commit that has been amended since last review.
pylib/mozreview/mozreview/resources/commit_rewrite.py
pylib/mozreview/mozreview/static/mozreview/css/review.less
pylib/mozreview/mozreview/static/mozreview/js/autoland.js
--- a/pylib/mozreview/mozreview/resources/commit_rewrite.py
+++ b/pylib/mozreview/mozreview/resources/commit_rewrite.py
@@ -6,18 +6,18 @@ import logging
 from djblets.webapi.decorators import (webapi_response_errors)
 from djblets.webapi.errors import (DOES_NOT_EXIST, INVALID_ATTRIBUTE,
                                    NOT_LOGGED_IN, PERMISSION_DENIED)
 from reviewboard.reviews.models import ReviewRequest
 from reviewboard.webapi.resources import WebAPIResource
 
 from mozreview.extra_data import (COMMITS_KEY, is_parent, get_parent_rr)
 from mozreview.errors import (NOT_PARENT, AUTOLAND_REVIEW_NOT_APPROVED)
-from mozreview.review_helpers import (gen_latest_reviews)
-
+from mozreview.review_helpers import (gen_latest_reviews,
+                                      has_shipit_carryforward)
 
 from mozautomation.commitparser import (replace_reviewers,)
 
 
 class CommitRewriteResource(WebAPIResource):
     """Provides interface to commit description rewriting."""
 
     name = 'commit_rewrite'
@@ -63,20 +63,24 @@ class CommitRewriteResource(WebAPIResour
             if not reviewers and child_request.approved:
                 # This review request is approved (the repeated check is
                 # to ensure this is guaranteed if other parts of the code
                 # change) but we have an empty list of reviewers. We'll
                 # assume the author has just approved this themself and
                 # set r=me
                 reviewers.append('me')
 
+            # Detect if the commit has been changed since the last review.
+            shipit_carryforward = has_shipit_carryforward(child_request)
+
             result.append({
                 'commit': child[0],
                 'id': child[1],
                 'reviewers': reviewers,
+                'shipit_carryforward': shipit_carryforward,
                 'summary': replace_reviewers(child_request.description,
                                              reviewers)
             })
 
         return 200, {
             'commits': result,
             'total_results': len(result),
             'links': self.get_links(request=request),
--- a/pylib/mozreview/mozreview/static/mozreview/css/review.less
+++ b/pylib/mozreview/mozreview/static/mozreview/css/review.less
@@ -100,16 +100,22 @@ label[for="mozreview-autoland-try-syntax
     font-weight: bold;
     margin-top: 1em;
   }
 
   .autoland-commit-rev {
     font-family: monospace;
     margin-right: 2ch;
   }
+
+  .autoland-commit-warning {
+    color: red;
+    font-weight: bold;
+    margin-left: 1em;
+  }
 }
 
 #try-syntax-error {
   display: none;
   color: red;
 }
 
 .parent-request {
--- a/pylib/mozreview/mozreview/static/mozreview/js/autoland.js
+++ b/pylib/mozreview/mozreview/static/mozreview/js/autoland.js
@@ -135,17 +135,17 @@
       })
       .width('60em')
       .append(
         $('<div id="autoland-content"/>)')
           .append('<i>Loading commits...</i>'))
       .modalBox({
         title: "Land Commits",
         buttons: [
-          $('<input type="button"/>')
+          $('<input type="button" id="autoland-cancel"/>')
             .val(gettext("Cancel")),
           $('<input type="button" id="autoland-submit" disabled/>')
             .val("Land")
             .click(send_to_autoland)
         ]
       });
 
     // fetch rewritten commit messages
@@ -176,16 +176,26 @@
                 .append(
                   $('<span/>')
                     .addClass('autoland-commit-rev')
                     .text(this.commit.substr(0, 12)))
                 .append(
                   $('<span/>')
                     .addClass('autoland-commit-reviewers')
                     .text('reviewers: ' + this.reviewers.join(', '))));
+          if (this.shipit_carryforward === true) {
+            $('#autoland-commits')
+              .append(
+                $('<span/>')
+                  .addClass('rb-icon rb-icon-warning'))
+              .append(
+                $('<span/>')
+                  .addClass('autoland-commit-warning')
+                  .text('Warning: commit has changed since review was granted'));
+          }
           // store commit descriptions in a form ready to pass to autoland
           commit_descriptions[this.commit.substr(0, 12)] = this.summary;
         });
         $('#autoland-submit')
           .data('commits', commit_descriptions)
           .prop('disabled', false);
       },
       error: function(res) {