MozReview: Review Page Redesign: Update main content area contents (Bug 1309964). r?glob,smacleod draft
authorDavid Walsh <dwalsh@mozilla.com>
Mon, 21 Nov 2016 14:38:36 -0600
changeset 9982 75e6bd4195a3cace92e9ef3ba542f3c2409cb9d3
parent 9981 7fde75b31579e5311cc291d7e1db6976c53947e6
child 9983 646d54730cf8012408c5ce9ea87999887c113b37
push id1403
push userbmo:dwalsh@mozilla.com
push dateTue, 29 Nov 2016 16:23:11 +0000
reviewersglob, smacleod
bugs1309964
MozReview: Review Page Redesign: Update main content area contents (Bug 1309964). r?glob,smacleod MozReview-Commit-ID: 48eHFAIpvlD
pylib/mozreview/mozreview/extension.py
pylib/mozreview/mozreview/fields.py
pylib/mozreview/mozreview/static/mozreview/css/commits.less
pylib/mozreview/mozreview/static/mozreview/css/mozilla-theme-common.less
pylib/mozreview/mozreview/static/mozreview/css/mozilla-theme-reviews.less
pylib/mozreview/mozreview/static/mozreview/css/mozilla-theme.less
pylib/mozreview/mozreview/templates/mozreview/commit-main.html
pylib/mozreview/mozreview/templates/mozreview/hg-import.html
pylib/mozreview/mozreview/templates/mozreview/hg-pull.html
--- a/pylib/mozreview/mozreview/extension.py
+++ b/pylib/mozreview/mozreview/extension.py
@@ -43,20 +43,19 @@ from mozreview.diffviewer.opcode_generat
 )
 from mozreview.extra_data import (
     is_parent,
 )
 from mozreview.fields import (
     BaseCommitField,
     CombinedReviewersField,
     CommitAuthorField,
+    CommitDetailField,
     CommitsListField,
     FileDiffReviewerField,
-    ImportCommitField,
-    PullCommitField,
     TryField,
 )
 from mozreview.file_diff_reviewer.resources import (
     file_diff_reviewer_resource,
 )
 from mozreview.hooks import (
     CommitContextTemplateHook,
     MozReviewApprovalHook,
@@ -276,22 +275,21 @@ class MozReviewExtension(Extension):
 
         info_fieldset = get_review_request_fieldset('info')
         for field_name in ('branch', 'depends_on', 'blocks'):
             field = get_review_request_field(field_name)
             if field:
                 info_fieldset.remove_field(field)
 
         # We "monkey patch" (yes, I feel dirty) the should_render method on
-        # the description field so that it is not rendered for parent review
-        # requests.
+        # the description field so that it is not rendered
         description_field = get_review_request_field('description')
         if description_field:
-            description_field.should_render = (lambda self, value:
-                not is_parent(self.review_request_details))
+            self.old_desc_should_render = description_field.should_render
+            description_field.should_render = lambda self, value: False
 
         # All of our review request styling is injected via
         # review-stylings-css, which in turn loads the review.css static
         # bundle.
         TemplateHook(self, 'base-css', 'mozreview/review-stylings-css.html',
                      apply_to=review_request_url_names)
         TemplateHook(self, 'base-css', 'mozreview/viewdiff-stylings-css.html',
                      apply_to=diffviewer_url_names)
@@ -311,32 +309,29 @@ class MozReviewExtension(Extension):
                      'mozreview/user-data.html')
         TemplateHook(self, 'base-after-content',
                      'mozreview/repository.html')
         TemplateHook(self, 'base-after-content',
                      'mozreview/user_review_flag.html',
                      apply_to=review_request_url_names)
 
         ReviewRequestFieldsHook(self, 'main', [CommitsListField])
+        ReviewRequestFieldsHook(self, 'main', [CommitDetailField])
         CommitContextTemplateHook(self, 'mozreview-pre-review-request-box',
                                   'mozreview/commits.html',
                                   apply_to=review_request_url_names)
 
         # The above hack forced Commits at the top, but the rest of these
         # fields are fine below the Description.
         ReviewRequestFieldsHook(self, 'main', [CombinedReviewersField])
         ReviewRequestFieldsHook(self, 'main', [TryField])
         ReviewRequestFieldsHook(self, 'main', [BaseCommitField])
         ReviewRequestFieldsHook(self, 'main', [FileDiffReviewerField])
 
         ReviewRequestFieldsHook(self, 'info', [CommitAuthorField])
-        # We want pull to appear first as it is the more robust way of
-        # retrieving changesets.
-        ReviewRequestFieldsHook(self, 'info', [PullCommitField])
-        ReviewRequestFieldsHook(self, 'info', [ImportCommitField])
 
         # Use a custom method to calculate a review approval state.
         MozReviewApprovalHook(self)
 
         # Instantiate the various signal handlers
         initialize_signal_handlers(self)
 
         HostingServiceHook(self, HMORepository)
@@ -355,16 +350,20 @@ class MozReviewExtension(Extension):
         info_fieldset = get_review_request_fieldset('info')
         if not get_review_request_field('branch'):
             info_fieldset.add_field(BranchField)
         if not get_review_request_field('depends_on'):
             info_fieldset.add_field(DependsOnField)
         if not get_review_request_field('blocks'):
             info_fieldset.add_field(BlocksField)
 
+        if self.old_desc_should_render:
+            get_review_request_field('description').should_render = (
+                self.old_desc_should_render)
+
         super(MozReviewExtension, self).shutdown()
 
     def get_settings(self, key, default=None):
         """This gets settings from the mozreview-settings.json file
 
         It caches the settings in memory and reloads them if changes are
         detected on disk.
 
--- a/pylib/mozreview/mozreview/fields.py
+++ b/pylib/mozreview/mozreview/fields.py
@@ -87,25 +87,25 @@ class CommitsListField(CommitDataBackedF
     This field is injected in the details of a review request that
     is a "push" based review request.
     """
     field_id = COMMITS_KEY
     label = ""
 
     can_record_change_entry = True
 
+    def should_render(self, value):
+        return False
+
     def has_value_changed(self, old_value, new_value):
         # Just to be safe, we de-serialize the json and compare values
         if old_value is not None and new_value is not None:
             return json.loads(old_value) != json.loads(new_value)
         return old_value != new_value
 
-    def should_render(self, value):
-        return False
-
     def render_change_entry_html(self, info):
         old_value = json.loads(info.get('old', ['[]'])[0])
         old_commits = [c for c, r in old_value]
 
         new_value = json.loads(info.get('new', ['[]'])[0])
         new_commits = [c for c, r in new_value]
 
         # Pad the commit lists so they're equal in length.
@@ -120,86 +120,66 @@ class CommitsListField(CommitDataBackedF
         return get_template('mozreview/commits_changedescription.html').render(
             Context({
                 'commit_changes': commit_changes,
                 'review_request': review_request,
                 'repository_path': review_request.repository.path
             }))
 
 
-class ImportCommitField(BaseReviewRequestField):
-    """This field provides some information on how to pull down the commit"""
-    # RB validation requires this to be unique, so we fake a field id
-    field_id = "p2rb.ImportCommitField"
-    label = _("Import")
-
-    def __init__(self, review_request_details, *args, **kwargs):
-        self.commit_data = fetch_commit_data(review_request_details)
-
-        super(ImportCommitField, self).__init__(review_request_details,
-                                                *args, **kwargs)
+class CommitDetailField(BaseReviewRequestField):
+    """This field provides the main content for review requests"""
+    label = ""
+    field_id = "p2rb.CommitDetail"
 
     def should_render(self, value):
-        return not is_parent(self.review_request_details, self.commit_data)
+        return is_pushed(self.review_request_details)
 
     def as_html(self):
-        commit_id = self.commit_data.extra_data.get(COMMIT_ID_KEY)
+        user = self.request.user
+
+        commit_data = fetch_commit_data(self.review_request_details)
+        commit_id = commit_data.get_for(self.review_request_details, COMMIT_ID_KEY)
+
         review_request = self.review_request_details.get_review_request()
+        parent = get_parent_rr(review_request)
+        parent_details = parent.get_draft(user) or parent
+
+        author = commit_data.extra_data.get(AUTHOR_KEY, None)
+
+        # If a user can view the parent draft they should also have
+        # permission to view every child. We check if the child is
+        # accessible anyways in case it has been restricted for other
+        # reasons.
+        children_details = [
+            child for child in gen_child_rrs(parent_details, user=user)
+            if child.is_accessible_by(user)]
+
+        # Generate the import and pull input field contents
+        import_text = pull_text = ""
         repo_path = review_request.repository.path
 
-        if not commit_id:
-            logger.error('No commit_id for review request: %d' % (
-                review_request.id))
-            return ''
-
-        return get_template('mozreview/hg-import.html').render(Context({
-                'commit_id': commit_id,
-                'repo_path': repo_path,
-        }))
-
+        if commit_id:
+            import_text = "hg import %s/rev/%s" % (repo_path, commit_id)
 
-class PullCommitField(BaseReviewRequestField):
-    """This field provides some information on how to pull down the commit"""
-    # RB validation requires this to be unique, so we fake a field id
-    field_id = "p2rb.PullCommitField"
-    label = _("Pull")
-
-    def __init__(self, review_request_details, *args, **kwargs):
-        self.commit_data = fetch_commit_data(review_request_details)
-
-        super(PullCommitField, self).__init__(review_request_details,
-                                              *args, **kwargs)
+        last_child_commit_id = commit_id
+        if is_parent(self.review_request_details, commit_data=commit_data):
+            last_child_commit_data = fetch_commit_data(children_details[-1])
+            last_child_commit_id = (
+                last_child_commit_data.extra_data.get(COMMIT_ID_KEY))
 
-    def as_html(self):
-        commit_id = self.commit_data.extra_data.get(COMMIT_ID_KEY)
-
-        if is_parent(self.review_request_details, self.commit_data):
-            user = self.request.user
-            parent = get_parent_rr(
-                self.review_request_details.get_review_request(),
-                self.commit_data)
-            parent_details = parent.get_draft() or parent
-            children = [
-                child for child in gen_child_rrs(parent_details, user=user)
-                if child.is_accessible_by(user)]
+        pull_text = "hg pull -r %s %s" % (last_child_commit_id, repo_path)
 
-            commit_data = fetch_commit_data(children[-1])
-            commit_id = commit_data.extra_data.get(COMMIT_ID_KEY)
-
-        review_request = self.review_request_details.get_review_request()
-        repo_path = review_request.repository.path
-
-        if not commit_id:
-            logger.error('No commit_id for review request: %d' % (
-                review_request.id))
-            return ''
-
-        return get_template('mozreview/hg-pull.html').render(Context({
-                'commit_id': commit_id,
-                'repo_path': repo_path,
+        return get_template('mozreview/commit-main.html').render(Context({
+            'review_request_details': self.review_request_details,
+            'parent_details': parent_details,
+            'user': user,
+            'author': author,
+            'pull_text': pull_text,
+            'import_text': import_text,
         }))
 
 
 class CommitAuthorField(CommitDataBackedField):
     """Field for the author of the review request's commit"""
     field_id = AUTHOR_KEY
     label = _("Author")
 
--- a/pylib/mozreview/mozreview/static/mozreview/css/commits.less
+++ b/pylib/mozreview/mozreview/static/mozreview/css/commits.less
@@ -1,28 +1,17 @@
-/*
- * We need to copy a few definitions from rb because there's a bug
- * that prevents to import a less file from another application.
- * See https://bugzilla.mozilla.org/show_bug.cgi?id=1168436 for more info.
- */
-.border-radius(...) {
-  -moz-border-radius: @arguments;
-  -webkit-border-radius: @arguments;
-  border-radius: @arguments;
-}
-
 @issue-opened-bg: #fff4B0;
 @issue-opened-border-color: darken(@issue-opened-bg, 40%);
 @issue-opened-link-color: darken(@issue-opened-bg, 60%);
 
 /*******************************************************/
 
 @import (reference) 'mozilla-theme-common.less';
 
-@approval-bg: #DFFFD7;
+@approval-background-color: #DFFFD7;
 @approval-border-color: #478A06;
 @table-large-font-size: 18px;
 
 #mozreview-request-series{
   margin-top:5px;
 }
 
 .help-icon {
@@ -97,17 +86,17 @@
     min-width: 100px;
   }
 
   .reviewer-name {
     min-height: 10px;
     display: inline-block;
 
     &.reviewer-ship-it {
-      background-color: @approval-bg;
+      background-color: @approval-background-color;
       border: 1px solid @approval-border-color;
       padding: 1px 5px;
       .border-radius(2px);
     }
     &.review-pending::after {
       content: " (r?)";
     }
     &.review-granted::after {
--- a/pylib/mozreview/mozreview/static/mozreview/css/mozilla-theme-common.less
+++ b/pylib/mozreview/mozreview/static/mozreview/css/mozilla-theme-common.less
@@ -1,8 +1,11 @@
+@approval-background-color: #DFFFD7;
+@approval-border-color: #478A06;
+
 @moz-blue-dark: #d4dde4;
 @moz-blue-light: #eaeff2;
 
 @light-white: rgba(255, 255, 255, 0.6);
 
 .border-radius(...) {
   -moz-border-radius: @arguments;
   -webkit-border-radius: @arguments;
--- a/pylib/mozreview/mozreview/static/mozreview/css/mozilla-theme-reviews.less
+++ b/pylib/mozreview/mozreview/static/mozreview/css/mozilla-theme-reviews.less
@@ -40,16 +40,84 @@
 
   .review-request-meta {
     color: #777;
     text-align: center;
     line-height: 1.6;
     font-size: 110%;
   }
 
+  .status {
+    .review-status-icon();
+    font-size: 20px;
+    margin-bottom: 10px;
+
+    &.no-approval {
+      cursor: default;
+    }
+
+    &.approval {
+      cursor: default;
+      background-color: @approval-bg;
+    }
+
+    &.approval-issues {
+      background: #fff4B0;
+    }
+
+    .issue-count {
+      text-decoration: none;
+      color: inherit;
+     }
+
+    .shipit-count {
+      background-image: url("../shipit_bg.png");
+      background-position: top left;
+      background-repeat: repeat-x;
+      background-color: #6bc810;
+      border: 1px solid @approval-border-color;
+      color: #205003;
+    }
+  }
+
+  #review-request-inputs {
+    margin-top: 20px;
+
+    input {
+      .border-radius(3px);
+      padding: 2px 6px;
+      background: #F9F9F9;
+      border: 1px solid #ddd;
+      font-family: monospace;
+      font-size: 16px !important;
+      width: 300px;
+      margin-right: 20px;
+      text-overflow: ellipsis;
+    }
+  }
+}
+
+#mozreview-review-content {
+  text-align: center;
+
+  h1 {
+    font-size: 28px;
+    font-weight: normal;
+    display: block;
+    line-height: auto;
+    margin-bottom: 10px;
+  }
+
+  .review-request-meta {
+    color: #777;
+    text-align: center;
+    line-height: 1.6;
+    font-size: 110%;
+  }
+
   #review-request-inputs {
     margin-top: 20px;
 
     input {
       .border-radius(3px);
       padding: 2px 6px;
       background: #F9F9F9;
       border: 1px solid #ddd;
@@ -67,17 +135,17 @@
   margin-bottom: 10px;
 
   &.no-approval {
     cursor: default;
   }
 
   &.approval {
     cursor: default;
-    background-color: @approval-bg;
+    background-color: @approval-background-color;
   }
 
   &.approval-issues {
     background: #fff4B0;
   }
 
   .issue-count {
     text-decoration: none;
--- a/pylib/mozreview/mozreview/static/mozreview/css/mozilla-theme.less
+++ b/pylib/mozreview/mozreview/static/mozreview/css/mozilla-theme.less
@@ -247,18 +247,21 @@ body.datagrid-page {
   padding: 10px;
 }
 
 #view_controls {
   font-size: 12px;
 }
 
 .review-request #review_request_details {
-  border-left: 1px solid @moz-blue-light;
-  padding: 0 10px;
+  display: none;
+}
+
+.review-request #review_request_main {
+  margin-right: 0;
 }
 
 /* Review Actions Bar */
 .actions-container, .actions.page-tabs {
   background: @moz-blue-dark;
   border: 0;
 }
 
@@ -331,25 +334,47 @@ body.datagrid-page {
   background: @moz-blue-light;
   border-bottom-color: @moz-blue-light;
 }
 
 /* Diffs in Reviews */
 #diffs .diff-box {
   .border-radius(0);
   border: 0;
+  margin-top: 0;
+}
+
+#diff-details #diff_revision_label h1,
+.review-request label {
+  color: #000;
+}
+
+#diff_index, .diff-index {
+  border: 0;
+  margin: 0;
 }
 
-.sidebyside thead {
-  .filename-row th {
-    background: @moz-blue-dark;
-  }
+#diff_index table tr:first-child td, .diff-index table tr:first-child td {
+  vertical-align: middle;
+}
+
+#diff_index table td, .diff-index table td {
+  background: none;
+  border-top: 1px solid #eee;
+}
 
-  .revision-row th .rb-icon {
-    vertical-align: sub;
+.sidebyside {
+  thead {
+    .filename-row th {
+      background: @moz-blue-dark;
+    }
+
+    .revision-row th .rb-icon {
+      vertical-align: sub;
+    }
   }
 }
 
 /* Dialogs */
 .modalbox-inner, .modalbox-title, .modalbox {
   background-image: none;
   .border-radius(0);
 }
new file mode 100644
--- /dev/null
+++ b/pylib/mozreview/mozreview/templates/mozreview/commit-main.html
@@ -0,0 +1,50 @@
+{% load djblets_utils i18n reviewtags staticfiles %}
+
+<div id="mozreview-review-content">
+
+<h1>{{ review_request_details.get_review_request.summary }}</h1>
+
+<div class="review-request-meta">
+  Submitted by {{review_request_details.get_review_request.submitter.username}} &lt;{{review_request_details.get_review_request.submitter.email}}&gt;
+  {% if author %}
+    &bull; Authored by {{author}}
+  {% endif %}
+  <br>
+
+  {% with review_request_details.time_added|date as dateadded and review_request_details.last_updated as lastupdated and review_request_details.last_updated|date:"c" as lastupdatedraw and latest_diffset.timestamp|date:"c" as lastdiffdateraw and latest_diffset.timestamp as lastdiffdate %}
+  {%  if review_request_details.status == 'S' %}
+  {%   blocktrans %}
+    Created {{dateadded}} and submitted
+    <time class="timesince" datetime="{{lastupdatedraw}}">{{lastupdated}}</time>
+  {%   endblocktrans %}
+  {%  elif review_request_details.status == 'D' %}
+  {%   blocktrans %}
+    Created {{dateadded}} and discarded
+    <time class="timesince" datetime="{{lastupdatedraw}}">{{lastupdated}}</time>
+  {%   endblocktrans %}
+  {%  elif review_request_details.status == 'P' %}
+  {%   blocktrans %}
+    Created {{dateadded}} and updated
+    <time class="timesince" datetime="{{lastupdatedraw}}">{{lastupdated}}</time>
+  {%   endblocktrans %}
+  {%  endif %}
+  {%  if latest_diffset %}
+  {%   blocktrans %}
+    - Latest diff uploaded
+    <time class="timesince" datetime="{{lastdiffdateraw}}">{{lastdiffdate}}</time>
+  {%   endblocktrans %}
+  {%  endif %}
+  {% endwith %}
+</div>
+
+<div id="review-request-inputs">
+  {% if pull_text %}
+    <input type="text" value="{{ pull_text }}" onclick="this.select();" readonly>
+  {% endif %}
+
+  {% if import_text %}
+    <input type="text" value="{{ import_text }}" onclick="this.select();" readonly>
+  {% endif %}
+</div>
+
+</div>
deleted file mode 100644
--- a/pylib/mozreview/mozreview/templates/mozreview/hg-import.html
+++ /dev/null
@@ -1,8 +0,0 @@
-{% comment %}
-This is the template for the hg import commit command.
-{% endcomment %}
-
-{% load i18n %}
-{% load djblets_utils %}
-
-<input style="font-size: smaller; position: relative; top:-3px" value="hg import {{repo_path}}/rev/{{commit_id}}" onclick="this.select();" readonly>
deleted file mode 100644
--- a/pylib/mozreview/mozreview/templates/mozreview/hg-pull.html
+++ /dev/null
@@ -1,8 +0,0 @@
-{% comment %}
-This is the template for the hg pull commit command.
-{% endcomment %}
-
-{% load i18n %}
-{% load djblets_utils %}
-
-<input style="font-size: smaller; position: relative; top:-3px" value="hg pull -r {{commit_id}} {{repo_path}}" onclick="this.select();" readonly>