Save commit message as a file attachment (bug 1248008) draft
authorPiotr Zalewa <pzalewa@mozilla.com>
Fri, 13 May 2016 12:31:53 +0200
changeset 9154 1579b6f1441a6fe3ea6de3f6431a321d22e73f60
parent 9074 89155088fa7023f295eef4752c4520ae5dfaac51
push id1097
push userbmo:pzalewa@mozilla.com
push dateMon, 15 Aug 2016 13:45:30 +0000
bugs1248008
Save commit message as a file attachment (bug 1248008) MozReview does not have a way to comment on commit message. This patch creates a plain text attachment so reviewer can comment on it. If a commit message has been amended attachment will be updated. `file_attachment_review.js` is created to fire `mozreview_ready event`. It is also preparing `MozReview` object so certain actions will not take place in `review_flag.js`. `dump-attachments` reviewboard command has been added. It is displaying all revisions of attachments. This might be changed so only the last revision is printed. `create-attachment-comment` reviewboard command has been added. It creates a review on an attachment and creates a comment. Review needs then to be published. A separate test file is created for the commit attachment creation and commenting. Reviewing a text attachment has the same UI as reviewing a diff. Only the comments pushed to bugzilla show the last revision of the file. Showing diff in bugzilla comment is pushed for a follow-up bug. MozReview-Commit-ID: 2oYfKXbUpND
hgext/reviewboard/tests/test-commits-message-attachment.t
pylib/mozreview/mozreview/diffs.py
pylib/mozreview/mozreview/extension.py
pylib/mozreview/mozreview/extra_data.py
pylib/mozreview/mozreview/resources/batch_review_request.py
pylib/mozreview/mozreview/static/mozreview/css/review.less
pylib/mozreview/mozreview/static/mozreview/js/file_attachment_review.js
pylib/mozreview/mozreview/static/mozreview/js/review.js
testing/vcttesting/reviewboard/mach_commands.py
new file mode 100644
--- /dev/null
+++ b/hgext/reviewboard/tests/test-commits-message-attachment.t
@@ -0,0 +1,192 @@
+#require mozreviewdocker
+  $ . $TESTDIR/hgext/reviewboard/tests/helpers.sh
+  $ commonenv
+
+  $ bugzilla create-bug TestProduct TestComponent 'Initial Bug'
+
+  $ cd client
+  $ echo 'foo1' > foo
+  $ hg commit -A -m 'Bug 1 - Foo 1'
+  adding foo
+  $ echo 'foo2' > foo
+  $ hg commit -m 'Bug 1 - Foo 2'
+  $ hg push
+  pushing to ssh://$DOCKER_HOSTNAME:$HGPORT6/test-repo
+  (adding commit id to 2 changesets)
+  saved backup bundle to $TESTTMP/client/.hg/strip-backup/1f0bb75ea473-1d48b186-addcommitid.hg (glob)
+  searching for changes
+  remote: adding changesets
+  remote: adding manifests
+  remote: adding file changes
+  remote: added 2 changesets with 2 changes to 1 files
+  remote: recorded push in pushlog
+  submitting 2 changesets for review
+  
+  changeset:  0:e3278fd3e255
+  summary:    Bug 1 - Foo 1
+  review:     http://$DOCKER_HOSTNAME:$HGPORT1/r/2 (draft)
+  
+  changeset:  1:bef76735ddf6
+  summary:    Bug 1 - Foo 2
+  review:     http://$DOCKER_HOSTNAME:$HGPORT1/r/3 (draft)
+  
+  review id:  bz://1/mynick
+  review url: http://$DOCKER_HOSTNAME:$HGPORT1/r/1 (draft)
+  
+  (review requests lack reviewers; visit review url to assign reviewers)
+  
+  publish these review requests now (Yn)?  y
+  (published review request 1)
+
+Commit message attachments should be created
+  $ reviewboard dump-attachments 2
+  - id: 1
+    caption: Commit message
+    filename: * (glob)
+    mimetype: text/plain
+    thumbnail: '<div class="file-thumbnail"> <div class="file-thumbnail-clipped"><pre>Bug
+      1 - Foo 1</pre><pre></pre><pre>MozReview-Commit-ID: 124Bxg</pre></div></div>'
+
+  $ reviewboard dump-attachments 3
+  - id: 2
+    caption: Commit message
+    filename: * (glob)
+    mimetype: text/plain
+    thumbnail: '<div class="file-thumbnail"> <div class="file-thumbnail-clipped"><pre>Bug
+      1 - Foo 2</pre><pre></pre><pre>MozReview-Commit-ID: 5ijR9k</pre></div></div>'
+
+Amended commit message should result with a changed attachment
+
+  $ hg commit --amend -m 'Bug 1 - Foo 2 - amended'
+  saved backup bundle to $TESTTMP/client/.hg/strip-backup/bef76735ddf6-725aca3a-amend-backup.hg (glob)
+  $ hg push
+  pushing to ssh://$DOCKER_HOSTNAME:$HGPORT6/test-repo
+  (adding commit id to 1 changesets)
+  saved backup bundle to $TESTTMP/client/.hg/strip-backup/ed1449e358cf-2899daf8-addcommitid.hg (glob)
+  searching for changes
+  remote: adding changesets
+  remote: adding manifests
+  remote: adding file changes
+  remote: added 1 changesets with 0 changes to 1 files (+1 heads)
+  remote: recorded push in pushlog
+  submitting 2 changesets for review
+  
+  changeset:  0:e3278fd3e255
+  summary:    Bug 1 - Foo 1
+  review:     http://$DOCKER_HOSTNAME:$HGPORT1/r/2
+  
+  changeset:  1:1677b1c36d63
+  summary:    Bug 1 - Foo 2 - amended
+  review:     http://$DOCKER_HOSTNAME:$HGPORT1/r/3 (draft)
+  
+  review id:  bz://1/mynick
+  review url: http://$DOCKER_HOSTNAME:$HGPORT1/r/1 (draft)
+  
+  (review requests lack reviewers; visit review url to assign reviewers)
+  
+  publish these review requests now (Yn)?  y
+  (published review request 1)
+  $ reviewboard dump-attachments 3
+  - id: 2
+    caption: Commit message
+    filename: * (glob)
+    mimetype: text/plain
+    thumbnail: '<div class="file-thumbnail"> <div class="file-thumbnail-clipped"><pre>Bug
+      1 - Foo 2</pre><pre></pre><pre>MozReview-Commit-ID: 5ijR9k</pre></div></div>'
+  - id: 3
+    caption: Commit message
+    filename: * (glob)
+    mimetype: text/plain
+    thumbnail: '<div class="file-thumbnail"> <div class="file-thumbnail-clipped"><pre>Bug
+      1 - Foo 2 - amended</pre><pre></pre><pre>MozReview-Commit-ID: F63vXs</pre></div></div>'
+
+Add a review on the commit message attachment and check how it is represented in
+Bugzilla
+
+  $ reviewboard create-review 3
+  created review 1
+  $ reviewboard create-attachment-comment 3 3 "This is a comment"
+  created file attachment comment 1 in review 1
+  $ reviewboard publish-review 3 1
+  published review 1
+  $ bugzilla dump-bug 1
+  Bug 1:
+    attachments:
+    - attacher: default@example.com
+      content_type: text/x-review-board-request
+      data: http://$DOCKER_HOSTNAME:$HGPORT1/r/2/diff/#index_header
+      description: Bug 1 - Foo 1
+      file_name: reviewboard-2-url.txt
+      flags: []
+      id: 1
+      is_obsolete: false
+      is_patch: false
+      summary: Bug 1 - Foo 1
+    - attacher: default@example.com
+      content_type: text/x-review-board-request
+      data: http://$DOCKER_HOSTNAME:$HGPORT1/r/3/diff/#index_header
+      description: Bug 1 - Foo 2 - amended
+      file_name: reviewboard-3-url.txt
+      flags: []
+      id: 2
+      is_obsolete: false
+      is_patch: false
+      summary: Bug 1 - Foo 2 - amended
+    blocks: []
+    cc: []
+    comments:
+    - author: default@example.com
+      id: 1
+      tags: []
+      text: ''
+    - author: default@example.com
+      id: 2
+      tags: []
+      text:
+      - Created attachment 1
+      - Bug 1 - Foo 1
+      - ''
+      - 'Review commit: http://$DOCKER_HOSTNAME:$HGPORT1/r/2/diff/#index_header'
+      - 'See other reviews: http://$DOCKER_HOSTNAME:$HGPORT1/r/2/'
+    - author: default@example.com
+      id: 3
+      tags: []
+      text:
+      - Created attachment 2
+      - Bug 1 - Foo 2 - amended
+      - ''
+      - 'Review commit: http://$DOCKER_HOSTNAME:$HGPORT1/r/3/diff/#index_header'
+      - 'See other reviews: http://$DOCKER_HOSTNAME:$HGPORT1/r/3/'
+    - author: default@example.com
+      id: 4
+      tags: []
+      text:
+      - Comment on attachment 2
+      - Bug 1 - Foo 2 - amended
+      - ''
+      - 'Review request updated; see interdiff: http://$DOCKER_HOSTNAME:$HGPORT1/r/3/diff/1-2/'
+    - author: default@example.com
+      id: 5
+      tags: []
+      text:
+      - http://$DOCKER_HOSTNAME:$HGPORT1/r/3/#review1
+      - ''
+      - 'Comment on Commit message:'
+      - ''
+      - '> Bug 1 - Foo 2 - amended'
+      - '> '
+      - '> MozReview-Commit-ID: F63vXs'
+      - This is a comment
+    component: TestComponent
+    depends_on: []
+    platform: All
+    product: TestProduct
+    resolution: ''
+    status: NEW
+    summary: Initial Bug
+
+
+Cleanup
+
+  $ mozreview stop
+  stopped 9 containers
--- a/pylib/mozreview/mozreview/diffs.py
+++ b/pylib/mozreview/mozreview/diffs.py
@@ -317,23 +317,60 @@ def render_comment_plain(comment, contex
             for line in chunk['lines']:
                 lines.append("> +%s" % parser.unescape(line[5]))
 
     lines.append(u"\n%s" % comment)
 
     return u"\n".join(lines)
 
 
+def render_plain_fileattachment_comment(comment):
+    """Render a comment on an attached file"""
+    # Do not provide additional data if not commenting on source
+    if 'viewMode' in comment.extra_data and comment.extra_data['viewMode'] != 'source':
+        return 'Comment on "%s"\n\n%s' % (comment.file_attachment.caption,
+                                          comment.text)
+
+    # If the reviewer selected a single line, comment with
+    # 5 lines of context.
+    # TODO: use diff_against_file_attachment to create a diff view in comments
+    lines = comment.review_ui.get_text().split('\n')
+    begin_line_num = 1
+    end_line_num = len(lines)
+    if 'beginLineNum' in comment.extra_data:
+        begin_line_num = comment.extra_data['beginLineNum']
+    if 'endLineNum' in comment.extra_data:
+        end_line_num = comment.extra_data['endLineNum'] or len(lines)
+    if begin_line_num == end_line_num:
+        first_line = max(1, begin_line_num - 5)
+    else:
+        first_line = begin_line_num
+
+    num_lines = end_line_num - first_line + 1
+
+    caption = comment.file_attachment.caption
+    # Blockquote commented lines
+    first_line_index = first_line - 1
+    last_line_index = min(first_line_index + num_lines, len(lines)) - 1
+    ret = ['Comment on %s:\n' % caption]
+    ret += ['> %s' % line for line in lines[first_line_index:last_line_index + 1]]
+    code = u"\n".join(ret)
+    return u"%s\n%s" % (code, comment.text)
+
+
 def build_plaintext_review(review, url, context):
     """Create a plaintext patch style representation of a review"""
     review_text = [url]
 
     if review.body_top:
         review_text.append(review.body_top)
 
     for comment in review.comments.order_by('filediff', 'first_line'):
         review_text.append(render_comment_plain(comment, context,
                                                 review.is_reply()))
 
+    for comment in review.file_attachment_comments.all():
+        review_text.append(render_plain_fileattachment_comment(comment))
+
     if review.body_bottom:
         review_text.append(review.body_bottom)
 
     return "\n\n".join(review_text)
--- a/pylib/mozreview/mozreview/extension.py
+++ b/pylib/mozreview/mozreview/extension.py
@@ -20,17 +20,18 @@ from reviewboard.extensions.hooks import
                                           URLHook)
 from reviewboard.reviews.builtin_fields import (TestingDoneField,
                                                 BranchField,
                                                 DependsOnField,
                                                 BlocksField)
 from reviewboard.reviews.fields import (get_review_request_field,
                                         get_review_request_fieldset)
 from reviewboard.urls import (diffviewer_url_names,
-                              review_request_url_names)
+                              review_request_url_names,
+                              reviewable_url_names)
 
 from mozreview.autoland.resources import (
     autoland_enable_resource,
     autoland_request_update_resource,
     autoland_trigger_resource,
     try_autoland_trigger_resource,
 )
 from mozreview.batchreview.resources import (
@@ -107,25 +108,31 @@ from mozreview.signal_handlers import (
 
 
 SETTINGS_PATH = os.path.join('/', 'mozreview-settings.json')
 SETTINGS = None
 
 
 logger = logging.getLogger(__name__)
 
+# These url names are used to set file_attachment_review JS
+file_attachment_url_names = [
+    'file-attachment',
+    'screenshot',
+]
+rr_and_attachment_url_names = review_request_url_names + file_attachment_url_names
 
 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
+    apply_to = rr_and_attachment_url_names
 
 
 class MozReviewExtension(Extension):
     metadata = {
         'Name': 'mozreview',
         'Summary': 'MozReview extension to Review Board',
     }
 
@@ -172,16 +179,27 @@ class MozReviewExtension(Extension):
                                  'mozreview/js/commits.js',
                                  'mozreview/js/review.js',
                                  'mozreview/js/autoland.js',
                                  'mozreview/js/ui.mozreviewautocomplete.js',
                                  'mozreview/js/review_flag.js',
                                  ],
             'apply_to': review_request_url_names,
         },
+        # HTML is different in file attachment review page. It does not contain
+        # parent review element. review_flag depends on existance of the
+        # MozReview object (only to check if parent review exists) so in
+        # file_attachment_review.js such object is created.
+        'file_attachment_review': {
+            'source_filenames': ['mozreview/js/file_attachment_review.js',
+                                 'mozreview/js/review.js',
+                                 'mozreview/js/review_flag.js',
+                                 ],
+            'apply_to': file_attachment_url_names,
+        },
         'parent-review-requests': {
             'source_filenames': ['mozreview/js/parents.js'],
             'apply_to': review_request_url_names,
         },
     }
 
     resources = [
         autoland_enable_resource,
@@ -281,24 +299,29 @@ class MozReviewExtension(Extension):
         if description_field:
             description_field.should_render = (lambda self, value:
                 not is_parent(self.review_request_details))
 
         # 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)
+                     apply_to=rr_and_attachment_url_names)
         TemplateHook(self, 'base-css', 'mozreview/viewdiff-stylings-css.html',
-                     apply_to=diffviewer_url_names)
+                     apply_to=reviewable_url_names)
         TemplateHook(self, 'base-css', 'mozreview/admin-stylings-css.html',
                      apply_to=['reviewboard.extensions.views.configure_extension'])
         TemplateHook(self, 'base-scripts-post',
                      'mozreview/review-scripts-js.html',
                      apply_to=review_request_url_names)
+        # TODO check if there is anything which replaced it. If not
+        #      find out why it was here
+        # TemplateHook(self, 'file_attachment_review',
+        #              'mozreview/file_attachment_review-js.html',
+        #              apply_to=file_attachment_url_names)
         TemplateHook(self, 'base-extrahead',
                      'mozreview/base-extrahead-login-form.html',
                      apply_to=['login'])
         TemplateHook(self, 'before-login-form',
                      'mozreview/before-login-form.html', apply_to=['login'])
         TemplateHook(self, 'after-login-form',
                      'mozreview/after-login-form.html', apply_to=['login'])
         TemplateHook(self, 'base-after-content',
--- a/pylib/mozreview/mozreview/extra_data.py
+++ b/pylib/mozreview/mozreview/extra_data.py
@@ -29,16 +29,18 @@ COMMIT_ID_KEY = MOZREVIEW_KEY + '.commit
 COMMITS_KEY = MOZREVIEW_KEY + '.commits'
 DISCARD_ON_PUBLISH_KEY = MOZREVIEW_KEY + '.discard_on_publish_rids'
 FIRST_PUBLIC_ANCESTOR_KEY = MOZREVIEW_KEY + '.first_public_ancestor'
 IDENTIFIER_KEY = MOZREVIEW_KEY + '.identifier'
 SQUASHED_KEY = MOZREVIEW_KEY + '.is_squashed'
 UNPUBLISHED_KEY = MOZREVIEW_KEY + '.unpublished_rids'
 PUBLISH_AS_KEY = MOZREVIEW_KEY + '.publish_as'
 
+COMMIT_MESSAGE_ID = 'commit_message_id'
+
 # CommitData fields which should be automatically copied from
 # draft_extra_data to extra_data when a review request is published.
 DRAFTED_COMMIT_DATA_KEYS = (
     AUTHOR_KEY,
     FIRST_PUBLIC_ANCESTOR_KEY,
     IDENTIFIER_KEY,
     COMMIT_ID_KEY,
 )
--- a/pylib/mozreview/mozreview/resources/batch_review_request.py
+++ b/pylib/mozreview/mozreview/resources/batch_review_request.py
@@ -1,14 +1,20 @@
 from __future__ import absolute_import, unicode_literals
 
 import json
 import logging
+import os
 
 from django.contrib.auth.models import User
+from django.core.files.base import (
+    File,
+    ContentFile,
+)
+from django.core.files.temp import NamedTemporaryFile
 from django.db import transaction
 from django.db.models import Q
 from djblets.webapi.decorators import (
     webapi_login_required,
     webapi_request_fields,
     webapi_response_errors,
 )
 from djblets.webapi.errors import (
@@ -22,16 +28,20 @@ from mozautomation.commitparser import (
     parse_commit_id,
 )
 from reviewboard.accounts.backends import (
     get_enabled_auth_backends,
 )
 from reviewboard.accounts.errors import (
     UserQueryError,
 )
+from reviewboard.attachments.models import (
+    FileAttachment,
+    FileAttachmentHistory,
+)
 from reviewboard.diffviewer.models import (
     DiffSet,
 )
 from reviewboard.reviews.models import (
     ReviewRequest,
     ReviewRequestDraft,
 )
 from reviewboard.scmtools.models import (
@@ -54,16 +64,17 @@ from mozreview.extra_data import (
     COMMIT_ID_KEY,
     DISCARD_ON_PUBLISH_KEY,
     fetch_commit_data,
     FIRST_PUBLIC_ANCESTOR_KEY,
     IDENTIFIER_KEY,
     MOZREVIEW_KEY,
     SQUASHED_KEY,
     UNPUBLISHED_KEY,
+    COMMIT_MESSAGE_ID,
 )
 from mozreview.models import (
     DiffSetVerification,
 )
 from mozreview.resources.bugzilla_login import (
     auth_api_key
 )
 from mozreview.review_helpers import (
@@ -909,21 +920,70 @@ def update_review_request(local_site, re
         draft = rr.draft.get()
     except ReviewRequestDraft.DoesNotExist:
         draft = ReviewRequestDraft.create(rr)
 
     draft.summary = commit['message'].splitlines()[0]
     draft.description = commit['message']
     draft.bugs_closed = commit['bug']
 
+    # Create/update a new commit message attachment.
+    # cm_* corresponds to commit_message_*
+    cm_file = ContentFile(draft.description)
+    cm_caption = 'Commit message'
+    cm_atts = draft.file_attachments.filter(caption=cm_caption)
+
+    if not cm_atts:
+        # There is no commit message attachment in review request.
+        # Let's create a new one along with corresponding file attachment history.
+        att_history = FileAttachmentHistory()
+        att_revision = 1
+        att_history.display_position = FileAttachmentHistory.compute_next_display_position(rr)
+        att_history.save()
+        rr.file_attachment_histories.add(att_history)
+    else:
+        # At least one commit attachment has been found.
+        # Find attachment history and update attachment.
+        cm_att = cm_atts[0]
+        att_history = cm_att.attachment_history
+
+        try:
+            latest = att_history.file_attachments.latest()
+        except FileAttachment.DoesNotExist:
+            latest = None
+
+        if latest is None:
+            # This should theoretically never happen, but who knows.
+            att_revision = 1
+            logger.error('No latest file attachment in history %s' % att_history)
+        elif latest.review_request.exists():
+            # This is a new update in the draft.
+            att_revision = latest.attachment_revision + 1
+        else:
+            # Switch the latest attachment in the history to the new one
+            att_revision = latest.attachment_revision
+            latest.delete()
+
+    att_kwargs = {
+        'attachment_history': att_history,
+        'attachment_revision': att_revision,
+        'draft_caption': cm_caption,
+        'mimetype': 'text/plain',
+    }
+    file_att = FileAttachment(**att_kwargs)
+    file_att.file.save('commit_message.txt', cm_file, save=True)
+    draft.file_attachments.add(file_att)
+    # Commit message attachment block finishes here.
+
     commit_data = fetch_commit_data(draft)
     commit_data.draft_extra_data.update({
         AUTHOR_KEY: commit['author'],
         COMMIT_ID_KEY: commit['id'],
         FIRST_PUBLIC_ANCESTOR_KEY: commit['first_public_ancestor'],
+        COMMIT_MESSAGE_ID: file_att.id,
     })
     commit_data.save(
         update_fields=['draft_extra_data'])
 
     reviewer_users, unrecognized_reviewers = \
         resolve_reviewers(reviewer_cache, commit.get('reviewers', []))
     requal_reviewer_users, unrecognized_requal_reviewers = \
         resolve_reviewers(reviewer_cache, commit.get('requal_reviewers', []))
--- a/pylib/mozreview/mozreview/static/mozreview/css/review.less
+++ b/pylib/mozreview/mozreview/static/mozreview/css/review.less
@@ -139,16 +139,24 @@ label[for="mozreview-autoland-try-syntax
 }
 
 .parent-warning {
   padding-left: 10px;
   padding-right: 10px;
   font-size: 10pt;
 }
 
+// Hide attachments (these will show in JS under condition)
+#review_request_extra div.clearfix {
+  display: none !important;
+}
+#review_request_extra div.more_attachments {
+  display: block;
+}
+
 // Hide the "Ship it" checkbox, since we use the r?, r+, r- flags provided
 // by MRReviewFlag.
 #id_shipit,
 label[for="id_shipit"] {
   display: none;
 }
 
 // Don't show gravatar for meta changes; RB is hardcoded to always use the
@@ -220,8 +228,15 @@ label[for="id_shipit"] {
   border-left-color: #b09700;  /* arrow color */
 
   /* positioning */
   position: absolute;
   top: 8px;
   right: -16px;
   z-index: 1;
 }
+
+// Style review commit message button
+.description-review {
+  float: right;
+  font-weight: normal;
+  text-decoration: none;
+}
new file mode 100644
--- /dev/null
+++ b/pylib/mozreview/mozreview/static/mozreview/js/file_attachment_review.js
@@ -0,0 +1,10 @@
+// HTML is different in file attachment review page. It does not contain
+// parent review element. review_flag.js depends on existance of the
+// MozReview object (only to check if parent review exists) so in
+// such object is created here.
+
+var MozReview = window.MozReview || { isParent: false };
+
+$(document).ready(function() {
+  $(document).trigger("mozreview_ready");
+});
--- a/pylib/mozreview/mozreview/static/mozreview/js/review.js
+++ b/pylib/mozreview/mozreview/static/mozreview/js/review.js
@@ -14,16 +14,38 @@
   // Change string of "Edit Review" button in the review banner that
   // shows up when a pending review is waiting to be published.
   $("#review-banner-edit").val("Finish...");
 
   // Change string of "Review" button to be a verb so people better
   // understand what clicking it does.
   $("#review-link").text("Finish Review...");
 
+  // Find commit message attachment's review button.
+  // Movit to the review request's description.
+  var attachments = $('.main .file-container');
+  attachments.each(function() {
+    if ($(this).find('.file-caption a.edit').text() === 'Commit message') {
+      // $(this) is the Commit message attachment
+      var descriptionLabel = $('[for="field_description"]');
+      var reviewButton = $(this).find('.file-review');
+      reviewButton.removeClass('file-review')
+                  .addClass('description-review')
+                  .text('Review')
+                  .appendTo(descriptionLabel);
+      $(this).hide();
+    }
+  });
+
+  if (attachments.length > 1) {
+    $('#file-list').parent()
+      .removeClass('clearfix')
+      .addClass('more_atachments');
+  }
+
   if (MozReview.isParent) {
     $('#review_request_extra').prepend(MRParents.parentWarning);
   }
 
   var reviewRequest = RB.PageManager.getPage().reviewRequest;
   RB.apiCall({
     type: 'GET',
     prefix: reviewRequest.get('sitePrefix'),
--- a/testing/vcttesting/reviewboard/mach_commands.py
+++ b/testing/vcttesting/reviewboard/mach_commands.py
@@ -162,16 +162,31 @@ def short_review_request_dict(rr):
             d['reviewers_status'][reviewer] = dict(status.iteritems())
 
     if 'diff' in rr:
         d['diff'] = dict(rr[u'diff'].iteritems())
 
     return d
 
 
+def serialize_file_attachments(attachments):
+    """serialize FileAttachmentListResource"""
+    l = []
+    for attachment in attachments:
+        dd = OrderedDict()
+        dd['id'] = attachment['id']
+        dd['caption'] = attachment['caption']
+        dd['filename'] = attachment['filename']
+        dd['mimetype'] = attachment['mimetype']
+        dd['thumbnail'] = attachment['thumbnail']
+        l.append(dd)
+
+    return yaml.safe_dump(l, default_flow_style=False).rstrip()
+
+
 @CommandProvider
 class ReviewBoardCommands(object):
     def __init__(self, context):
         from vcttesting.mozreview import MozReview
         if 'MOZREVIEW_HOME' in os.environ:
             self.mr = MozReview(os.environ['MOZREVIEW_HOME'])
         else:
             self.mr = None
@@ -224,16 +239,31 @@ class ReviewBoardCommands(object):
                 pulse_port=os.environ.get('PULSE_PORT'))
         elif 'REVIEWBOARD_URL' in os.environ:
             return MozReviewBoard(None, None, os.environ['REVIEWBOARD_URL'])
         else:
             raise Exception('Do not know about Bugzilla URL. Cannot talk to '
                             'Review Board. Try running `mozreview start` and '
                             'setting MOZREVIEW_HOME.')
 
+    @Command ('dump-attachments', category='reviewboard',
+        description='Print all attachments of a review request')
+    @CommandArgument('rrid', help='Review request id')
+    def dump_attachments(self, rrid):
+        from rbtools.api.errors import APIError
+        root = self._get_root()
+        try:
+            r = root.get_review_request(review_request_id=rrid)
+        except APIError:
+            print('No such review request (%s)' % rrid)
+            sys.exit(1)
+
+        attachments = r.get_file_attachments()
+        print serialize_file_attachments(attachments)
+
     @Command('dumpreview', category='reviewboard',
         description='Print a representation of a review request.')
     @CommandArgument('rrid', help='Review request id to dump')
     def dumpreview(self, rrid):
         client = self._get_client()
         root = client.get_root()
         r = root.get_review_request(review_request_id=rrid)
         print(serialize_review_requests(client, r))
@@ -454,16 +484,17 @@ class ReviewBoardCommands(object):
     def publish_review(self, rrid, rid):
         from rbtools.api.errors import APIError
         root = self._get_root()
         review = root.get_review(review_request_id=rrid, review_id=rid)
 
         try:
             review.update(public=True)
         except APIError as e:
+            # XXX this message was throwing a KeyError.
             print('API Error: %s: %s: %s' % (e.http_status, e.error_code,
                                              e.rsp['err']['msg']))
             return 1
 
         print('published review %s' % review.id)
 
     @Command('create-review-reply', category='reviewboard',
         description='Create a reply to an existing review')
@@ -520,16 +551,47 @@ class ReviewBoardCommands(object):
         reviews = root.get_reviews(review_request_id=rrid)
         review = reviews.create()
         comments = review.get_diff_comments()
         comment = comments.create(filediff_id=file_id, first_line=first_line,
                                   num_lines=1, text=text,
                                   issue_opened=open_issue)
         print('created diff comment %s' % comment.id)
 
+    @Command('create-attachment-comment', category='reviewboard',
+             description='Create a comment on an attachment')
+    @CommandArgument('rrid', help='Review request id')
+    @CommandArgument('attid', help='Attachment id')
+    @CommandArgument('text', help='Comment text')
+    def create_attachment_comment(self, rrid, attid, text):
+        from rbtools.api.errors import APIError
+        root = self._get_root()
+        try:
+            rr = root.get_review_request(review_request_id=rrid)
+        except APIError:
+            print('No such review request (%s)' % rrid)
+            sys.exit(1)
+
+        reviews = root.get_reviews(review_request_id=rrid)
+        review = reviews.create()
+
+        try:
+            # attachment = root.get_file_attachment(review_request_id=rrid, file_attachment_id=attid)
+            comment = review.get_file_attachment_comments().create(
+                file_attachment_id=attid,
+                issue_opened=False,
+                extra_data={ 'viewMode': 'source' },
+                text=text)
+
+        except APIError:
+            print('No such attachment (%s)' % attid)
+            sys.exit(1)
+
+        print('created file attachment comment %s in review %s' % (comment.id, review.id))
+
     @Command('update-issue-status', category='reviewboard',
              description='Update issue status on a diff comment.')
     @CommandArgument('rrid', help='Review request for the diff comment review')
     @CommandArgument('rid', help='Review for the diff comment')
     @CommandArgument('cid', help='Diff comment of issue to be updated')
     @CommandArgument('status', help='Desired issue status ("open", "dropped", '
                      'or "resolved")')
     def update_issue_status(self, rrid, rid, cid, status):