Move review history to version list page; load on demand via API.

docs/topics/api/addons.rst

@@ -284,6 +284,66 @@ This endpoint allows you to fetch a single version belonging to a specific add-o
     :>json string version: The version number string for the version.
 
 
+-----------------
+Review Notes List
+-----------------
+
+.. _review-notes-version-list:
+
+This endpoint allows you to list the approval/rejection review history for a version of an add-on.
+
+.. http:get:: /api/v3/addons/addon/(int:addon_id|string:addon_slug|string:addon_guid)/versions/(int:id)/reviewnotes/
+
+    .. note::
+        All add-ons require authentication and either
+        reviewer permissions or a user account listed as a developer of the
+        add-on.
+
+    :>json int count: The number of versions for this add-on.
+    :>json string next: The URL of the next page of results.
+    :>json string previous: The URL of the previous page of results.
+    :>json array results: An array of :ref:`per version review notes<review-notes-version-detail-object>`.
+
+
+-------------------
+Review Notes Detail
+-------------------
+
+.. _review-notes-version-detail:
+
+This endpoint allows you to fetch a single review note for a specific version of an add-on.
+
+.. http:get:: /api/v3/addons/addon/(int:addon_id|string:addon_slug|string:addon_guid)/versions/(int:id)/reviewnotes/(int:id)/
+
+    .. _review-notes-version-detail-object:
+
+    :>json int id: The id for a review note.
+    :>json string action: The :ref:`type of review note<review-note-action>`.
+    :>json string .action_label: The text label of the action.
+    :>json int user.id: The id of the reviewer or author who left the review note.
+    :>json string user.name: The name of the reviewer or author.
+    :>json string user.url: The link to the profile page for of the reviewer or author.
+    :>json string comments: The text content of the review note.
+    :>json string date: The date the review note was created.
+
+
+.. _review-note-action:
+
+    Possible values for the ``action`` field:
+
+    ==========================  ==========================================================
+                         Value  Description
+    ==========================  ==========================================================
+                      approved  Version, or file in the version, was approved
+                      rejected  Version, or file in the version, was rejected
+              review-requested  Developer requested review
+    more-information-requested  Reviewer requested more information from developer
+        super-review-requested  Add-on was referred to an admin for attention
+                       comment  Reviewer added comment for other reviewers
+                   review-note  Generic review comment 
+    ==========================  ==========================================================
+
+
 ----------------------------
 Add-on Feature Compatibility
 ----------------------------

src/olympia/activity/serializers.py

@@ -0,0 +1,28 @@
+from django.utils.translation import ugettext_lazy as _
+
+from rest_framework import serializers
+
+from olympia.users.serializers import BaseUserSerializer
+from olympia.devhub.models import ActivityLog
+
+
+class ActivityLogSerializer(serializers.ModelSerializer):
+    action = serializers.SerializerMethodField()
+    action_label = serializers.SerializerMethodField()
+    comments = serializers.SerializerMethodField()
+    date = serializers.DateTimeField(source='created')
+    user = BaseUserSerializer()
+
+    class Meta:
+        model = ActivityLog
+        fields = ('id', 'action', 'action_label', 'comments', 'user', 'date')
+
+    def get_comments(self, obj):
+        return obj.details['comments']
+
+    def get_action_label(self, obj):
+        log = obj.log()
+        return _(u'Review note') if not hasattr(log, 'short') else log.short
+
+    def get_action(self, obj):
+        return self.get_action_label(obj).replace(' ', '-').lower()

src/olympia/activity/tests/test_serializers.py

@@ -0,0 +1,43 @@
+# -*- coding: utf-8 -*-
+from rest_framework.test import APIRequestFactory
+
+from olympia import amo
+from olympia.activity.serializers import ActivityLogSerializer
+from olympia.amo.helpers import absolutify
+from olympia.amo.tests import (
+    addon_factory, TestCase, user_factory)
+
+
+class LogMixin(object):
+    def log(self, comments, action, created):
+        details = {'comments': comments}
+        details['version'] = self.addon.current_version.version
+        kwargs = {'user': self.user, 'created': created, 'details': details}
+        return amo.log(
+            action, self.addon, self.addon.current_version, **kwargs)
+
+
+class TestReviewNotesSerializerOutput(TestCase, LogMixin):
+    def setUp(self):
+        self.request = APIRequestFactory().get('/')
+        self.user = user_factory()
+        self.addon = addon_factory()
+
+    def serialize(self, id_):
+        serializer = ActivityLogSerializer(context={'request': self.request})
+        return serializer.to_representation(id_)
+
+    def test_basic(self):
+        now = self.days_ago(0)
+        entry = self.log(u'Oh nôes!', amo.LOG.REJECT_VERSION, now)
+
+        result = self.serialize(entry)
+
+        assert result['id'] == entry.pk
+        assert result['date'] == now.isoformat()
+        assert result['action'] == 'rejected'
+        assert result['action_label'] == 'Rejected'
+        assert result['comments'] == u'Oh nôes!'
+        assert result['user'] == {
+            'name': self.user.name,
+            'url': absolutify(self.user.get_url_path())}

src/olympia/activity/tests/test_views.py

@@ -0,0 +1,185 @@
+# -*- coding: utf-8 -*-
+import json
+
+from olympia import amo
+from olympia.activity.tests.test_serializers import LogMixin
+from olympia.amo.tests import (
+    addon_factory, user_factory, TestCase)
+from olympia.amo.urlresolvers import reverse
+from olympia.addons.models import AddonUser
+from olympia.addons.utils import generate_addon_guid
+from olympia.users.models import UserProfile
+
+
+class ReviewNotesViewSetDetailMixin(LogMixin):
+    """Tests that play with addon state and permissions. Shared between review
+    note viewset detail tests since both need to react the same way."""
+    def _test_url(self):
+        raise NotImplementedError
+
+    def _set_tested_url(self, pk=None, version_pk=None, addon_pk=None):
+        raise NotImplementedError
+
+    def _login_developer(self):
+        user = UserProfile.objects.create(username='author')
+        AddonUser.objects.create(user=user, addon=self.addon)
+        self.client.login_api(user)
+
+    def _login_reviewer(self, permission='Addons:Review'):
+        user = UserProfile.objects.create(username='reviewer')
+        self.grant_permission(user, permission)
+        self.client.login_api(user)
+
+    def test_get_by_id(self):
+        self._login_developer()
+        self._test_url()
+
+    def test_get_by_id_reviewer(self):
+        self._login_reviewer()
+        self._test_url()
+
+    def test_get_anonymous(self):
+        response = self.client.get(self.url)
+        assert response.status_code == 401
+
+    def test_get_no_rights(self):
+        self.client.login_api(UserProfile.objects.create(username='joe'))
+        response = self.client.get(self.url)
+        assert response.status_code == 403
+
+    def test_get_not_public_reviewer(self):
+        self.addon.update(status=amo.STATUS_NOMINATED)
+        self._login_reviewer()
+        response = self.client.get(self.url)
+        assert response.status_code == 200
+
+    def test_get_not_public_developer(self):
+        self.addon.update(status=amo.STATUS_NOMINATED)
+        self._login_developer()
+        response = self.client.get(self.url)
+        assert response.status_code == 200
+
+    def test_get_not_listed_simple_reviewer(self):
+        self.addon.update(is_listed=False)
+        self._login_reviewer()
+        response = self.client.get(self.url)
+        assert response.status_code == 403
+
+    def test_get_not_listed_specific_reviewer(self):
+        self.addon.update(is_listed=False)
+        self._login_reviewer(permission='Addons:ReviewUnlisted')
+        response = self.client.get(self.url)
+        assert response.status_code == 200
+
+    def test_get_not_listed_author(self):
+        self.addon.update(is_listed=False)
+        self._login_developer()
+        response = self.client.get(self.url)
+        assert response.status_code == 200
+
+    def test_get_deleted(self):
+        self.addon.delete()
+        self._login_developer()
+        response = self.client.get(self.url)
+        assert response.status_code == 404
+
+    def test_get_deleted_reviewer(self):
+        self.addon.delete()
+        self._login_reviewer()
+        response = self.client.get(self.url)
+        assert response.status_code == 404
+
+    def test_get_deleted_admin(self):
+        self.addon.delete()
+        self._login_reviewer(permission='*:*')
+        response = self.client.get(self.url)
+        assert response.status_code == 200
+
+    def test_disabled_version_reviewer(self):
+        self.version.files.update(status=amo.STATUS_DISABLED)
+        self._login_reviewer()
+        self._test_url()
+
+    def test_disabled_version_developer(self):
+        self.version.files.update(status=amo.STATUS_DISABLED)
+        self._login_developer()
+        self._test_url()
+
+    def test_deleted_version_reviewer(self):
+        self.version.delete()
+        self._login_reviewer()
+        self._test_url()
+
+    def test_deleted_version_developer(self):
+        self.version.delete()
+        self._login_developer()
+        self._test_url()
+
+    def test_get_version_not_found(self):
+        self._login_reviewer(permission='*:*')
+        self._set_tested_url(version_pk=self.version.pk + 27)
+        response = self.client.get(self.url)
+        assert response.status_code == 404
+
+
+class TestReviewNotesViewSetDetail(ReviewNotesViewSetDetailMixin, TestCase):
+    def setUp(self):
+        super(TestReviewNotesViewSetDetail, self).setUp()
+        self.addon = addon_factory(
+            guid=generate_addon_guid(), name=u'My Addôn', slug='my-addon')
+        self.user = user_factory()
+        self.version = self.addon.latest_version
+        self.note = self.log(u'noôo!', amo.LOG.APPROVE_VERSION,
+                             self.days_ago(0))
+        self._set_tested_url()
+
+    def _test_url(self):
+        response = self.client.get(self.url)
+        assert response.status_code == 200
+        result = json.loads(response.content)
+        assert result['id'] == self.note.pk
+        assert result['action_label'] == amo.LOG.APPROVE_VERSION.short
+        assert result['comments'] == u'noôo!'
+
+    def _set_tested_url(self, pk=None, version_pk=None, addon_pk=None):
+        self.url = reverse('version-reviewnotes-detail', kwargs={
+            'addon_pk': addon_pk or self.addon.pk,
+            'version_pk': version_pk or self.version.pk,
+            'pk': pk or self.note.pk})
+
+    def test_get_note_not_found(self):
+        self._login_reviewer(permission='*:*')
+        self._set_tested_url(self.note.pk + 42)
+        response = self.client.get(self.url)
+        assert response.status_code == 404
+
+
+class TestReviewNotesViewSetList(ReviewNotesViewSetDetailMixin, TestCase):
+    def setUp(self):
+        super(TestReviewNotesViewSetList, self).setUp()
+        self.addon = addon_factory(
+            guid=generate_addon_guid(), name=u'My Addôn', slug='my-addon')
+        self.user = user_factory()
+        self.note = self.log(u'noôo!', amo.LOG.APPROVE_VERSION,
+                             self.days_ago(1))
+        self.note2 = self.log(u'yéss!', amo.LOG.REJECT_VERSION,
+                              self.days_ago(0))
+
+        self.version = self.addon.latest_version
+        self._set_tested_url()
+
+    def _test_url(self, **kwargs):
+        response = self.client.get(self.url, data=kwargs)
+        assert response.status_code == 200
+        result = json.loads(response.content)
+        assert result['results']
+        assert len(result['results']) == 2
+        result_version = result['results'][0]
+        assert result_version['id'] == self.note2.pk
+        result_version = result['results'][1]
+        assert result_version['id'] == self.note.pk
+
+    def _set_tested_url(self, pk=None, version_pk=None, addon_pk=None):
+        self.url = reverse('version-reviewnotes-list', kwargs={
+            'addon_pk': addon_pk or self.addon.pk,
+            'version_pk': version_pk or self.version.pk})

src/olympia/activity/views.py

@@ -0,0 +1,30 @@
+from django.shortcuts import get_object_or_404
+
+from rest_framework.mixins import ListModelMixin, RetrieveModelMixin
+from rest_framework.viewsets import GenericViewSet
+
+from olympia import amo
+from olympia.activity.serializers import ActivityLogSerializer
+from olympia.addons.views import AddonChildMixin
+from olympia.api.permissions import (
+    AllowAddonAuthor, AllowReviewer, AllowReviewerUnlisted, AnyOf)
+from olympia.devhub.models import ActivityLog
+from olympia.versions.models import Version
+
+
+class VersionReviewNotesViewSet(AddonChildMixin, RetrieveModelMixin,
+                                ListModelMixin, GenericViewSet):
+    permission_classes = [
+        AnyOf(AllowAddonAuthor, AllowReviewer, AllowReviewerUnlisted),
+    ]
+    serializer_class = ActivityLogSerializer
+    queryset = ActivityLog.objects.all()
+    filter = amo.LOG_REVIEW_QUEUE_DEVELOPER
+
+    def get_queryset(self):
+        addon = self.get_addon_object()
+        version_object = get_object_or_404(
+            Version.unfiltered.filter(addon=addon),
+            pk=self.kwargs['version_pk'])
+        alog = ActivityLog.objects.for_version(version_object)
+        return alog.filter(action__in=self.filter)

src/olympia/addons/api_urls.py

@@ -3,11 +3,13 @@ from django.conf.urls import include, patterns, url
 from rest_framework.routers import SimpleRouter
 from rest_framework_nested.routers import NestedSimpleRouter
 
+from olympia.activity.views import VersionReviewNotesViewSet
 from olympia.reviews.views import ReviewViewSet
 
 from .views import (
     AddonFeaturedView, AddonSearchView, AddonVersionViewSet, AddonViewSet)
 
+
 addons = SimpleRouter()
 addons.register(r'addon', AddonViewSet)
 
@@ -15,12 +17,15 @@ addons.register(r'addon', AddonViewSet)
 sub_addons = NestedSimpleRouter(addons, r'addon', lookup='addon')
 sub_addons.register('versions', AddonVersionViewSet, base_name='addon-version')
 sub_addons.register('reviews', ReviewViewSet, base_name='addon-review')
+sub_versions = NestedSimpleRouter(sub_addons, r'versions', lookup='version')
+sub_versions.register(r'reviewnotes', VersionReviewNotesViewSet,
+                      base_name='version-reviewnotes')
 
 urlpatterns = patterns(
     '',
-
     url(r'', include(addons.urls)),
     url(r'', include(sub_addons.urls)),
+    url(r'', include(sub_versions.urls)),
     url(r'^search/$', AddonSearchView.as_view(), name='addon-search'),
     url(r'^featured/$', AddonFeaturedView.as_view(), name='addon-featured'),
 )

src/olympia/addons/views.py

@@ -668,7 +668,30 @@ class AddonViewSet(RetrieveModelMixin, GenericViewSet):
         return Response(serializer.data)
 
 
-class AddonVersionViewSet(RetrieveModelMixin, ListModelMixin, GenericViewSet):
+class AddonChildMixin(object):
+    """Some methods to get the parent add-on object and ensure permissions are
+    checked against it"""
+
+    def get_addon_object(self):
+        """Return the parent Addon object using the URL parameter passed
+        to the view."""
+        if hasattr(self, 'addon_object'):
+            return self.addon_object
+
+        self.addon_object = AddonViewSet(
+            request=self.request, permission_classes=self.permission_classes,
+            kwargs={'pk': self.kwargs['addon_pk']}).get_object()
+
+        return self.addon_object
+
+    def check_object_permissions(self, request, obj):
+        """Check object permissions against the add-on, not the version."""
+        super(AddonChildMixin, self).check_object_permissions(
+            request, self.get_addon_object())
+
+
+class AddonVersionViewSet(AddonChildMixin, RetrieveModelMixin,
+                          ListModelMixin, GenericViewSet):
     # Permissions are checked against the parent add-on - see
     # check_object_permissions() implementation below.
     permission_classes = AddonViewSet.permission_classes
@@ -679,23 +702,6 @@ class AddonVersionViewSet(RetrieveModelMixin, ListModelMixin, GenericViewSet):
     queryset = Version.objects.filter(
         files__status__in=amo.VALID_STATUSES).distinct()
 
-    def get_addon_object(self):
-        """Return the parent Addon object using the URL parameter passed
-        to the view."""
-        if hasattr(self, 'addon_object'):
-            return self.addon_object
-
-        self.addon_object = AddonViewSet(
-            request=self.request,
-            kwargs={'pk': self.kwargs['addon_pk']}).get_object()
-
-        return self.addon_object
-
-    def check_object_permissions(self, request, obj):
-        """Check object permissions against the add-on, not the version."""
-        super(AddonVersionViewSet, self).check_object_permissions(
-            request, self.get_addon_object())
-
     def get_queryset(self):
         """Return the right base queryset depending on the situation. Note that
         permissions checks still apply on top of that, against the add-on

src/olympia/amo/__init__.py

@@ -15,7 +15,7 @@ from olympia.constants.search import *  # noqa
 
 from .log import (LOG, LOG_BY_ID, LOG_ADMINS, LOG_EDITOR_REVIEW_ACTION,  # noqa
                   LOG_EDITORS, LOG_HIDE_DEVELOPER, LOG_KEEP, LOG_REVIEW_QUEUE,
-                  LOG_REVIEW_EMAIL_USER, log)
+                  LOG_REVIEW_QUEUE_DEVELOPER, LOG_REVIEW_EMAIL_USER, log)
 
 
 logger_log = commonware.log.getLogger('z.amo')

src/olympia/amo/log.py

@@ -201,7 +201,7 @@ class ESCALATE_VERSION(_LOG):
     # takes add-on, version, reviewtype
     id = 23
     format = _(u'{addon} {version} escalated.')
-    short = _(u'Escalated')
+    short = _(u'Super review requested')
     keep = True
     review_email_user = True
     review_queue = True
@@ -670,6 +670,9 @@ LOG_REVIEW_EMAIL_USER = [l.id for l in LOGS if hasattr(l, 'review_email_user')]
 LOG_HIDE_DEVELOPER = [l.id for l in LOGS
                       if (getattr(l, 'hide_developer', False) or
                           l.id in LOG_ADMINS)]
+# Review Queue logs to show to developer (i.e. hiding admin/private)
+LOG_REVIEW_QUEUE_DEVELOPER = list(set(LOG_EDITOR_REVIEW_ACTION) -
+                                  set(LOG_HIDE_DEVELOPER))
 
 
 def log(action, *args, **kw):

src/olympia/devhub/helpers.py

@@ -141,12 +141,11 @@ def status_choices(addon):
 
 
 @register.inclusion_tag('devhub/versions/file_status_message.html')
-def file_status_message(file, addon, file_history=False):
+def file_status_message(file, addon):
     choices = status_choices(addon)
     return {'fileid': file.id, 'platform': file.get_platform_display(),
             'created': datetime(file.created),
             'status': choices[file.status],
-            'file_history': file_history,
             'actions': amo.LOG_REVIEW_EMAIL_USER,
             'status_date': datetime(file.datestatuschanged)}
 

src/olympia/devhub/templates/devhub/versions/edit.html

@@ -101,7 +101,7 @@
               <ul>
                 {% for file in version.all_files %}
                 <li>
-                  {{ file_status_message(file, addon, file_history[file.id]) }}
+                  {{ file_status_message(file, addon) }}
                 </li>
                 {% endfor %}
               </ul>

src/olympia/devhub/templates/devhub/versions/file_status_message.html

@@ -11,28 +11,5 @@
     {% endtrans %}
   {% endif %}
   </div>
-  {% if file_history %}
-  <p>
-  (<a class="show_file_history" href="#">{{ _('Show File History') }}</a>)
-  </p>
-
-  <div>
-    {% for fh in file_history %}
-    <div class="version-comments">
-      <div><strong>{{ fh }}</strong></div>
-      {% if fh.action in actions %}
-        {# <pre> rather than <div> so Firefox will preserve whitespace on copy. #}
-        <pre class="email_comment">{{ fh.details['comments'] }}</pre>
-      {% endif %}
-      <div class="light">
-        {% trans user_name=fh.user|user_link, date=fh.modified|babel_datetime %}
-        By {{user_name}} on {{date}}
-        {% endtrans %}
-      </div>
-    </div>
-    {% endfor %}
-  </div>
-
-  {% endif %}
 
 </div>

src/olympia/devhub/templates/devhub/versions/list.html

@@ -45,6 +45,10 @@
           {% endif %}
         {% endwith %}
       {% endif %}
+        <div>
+            <a href="#" class="review-history-show" data-div="#{{ version.id }}-review-history">{{ _('Review History') }}</a>
+            <a href="#" class="review-history-hide hidden">{{ _('Close Review History') }}</a>
+        </div>
     </td>
     <td class="file-validation">
       <ul>
@@ -93,6 +97,24 @@
       </td>
     </tr>
   {% endif %}
+    <tr>
+        <td colspan="0" id="{{ version.id }}-review-history" class="review-history hidden"
+             data-api-url="{{ url('version-reviewnotes-list', addon.id, version.id) }}"
+             data-token="{{ token }}">
+            <div class="history-container">
+                <div class="review-entry-loading">{{ _('Loading Review History...') }}</div>
+                <div class="review-entry-failure hidden">{{ _('We had a problem retrieving review notes') }}</div>
+                <div class="review-entry-loadmore hidden">
+                    <p><a href="#" class="review-history-loadmore" data-div="#{{ version.id }}-review-history">{{ _('Load older...') }}</a></p>
+                </div>
+                <div class="review-entry-empty hidden">
+                    <p><span class="action">$action_label</span> {{ _('by') }}
+                    <a href="$user_profile">$user_name</a> {{ _('on') }} <time class="timeago" datetime=$date>$date</time></p>
+                    <pre>$comments</pre>
+                </div>
+            </div>
+        </td>
+    </tr>
 {% endmacro %}
 
 {% block content %}

src/olympia/devhub/tests/test_views.py

@@ -2705,61 +2705,6 @@ class TestVersionAddFile(UploadTest):
         new_file = self.version.files.get(platform=amo.PLATFORM_MAC.id)
         assert r.context['form'].instance == new_file
 
-    def test_show_item_history(self):
-        version = self.addon.latest_version
-        user = UserProfile.objects.get(email='editor@mozilla.com')
-
-        details = {'comments': 'yo', 'files': [version.files.all()[0].id]}
-        amo.log(amo.LOG.APPROVE_VERSION, self.addon,
-                self.addon.latest_version, user=user, created=datetime.now(),
-                details=details)
-
-        doc = pq(self.client.get(self.edit_url).content)
-        appr = doc('#approval_status')
-
-        assert appr.length == 1
-        assert appr.find('strong').eq(0).text() == "File  (Linux)"
-        assert appr.find('.version-comments').length == 1
-
-        comment = appr.find('.version-comments').eq(0)
-        assert comment.find('strong a').text() == (
-            'Delicious Bookmarks Version 0.1')
-        assert comment.find('pre.email_comment').length == 1
-        assert comment.find('pre.email_comment').text() == 'yo'
-
-    def test_show_item_history_hide_message(self):
-        """ Test to make sure comments not to the user aren't shown. """
-        version = self.addon.latest_version
-        user = UserProfile.objects.get(email='editor@mozilla.com')
-
-        details = {'comments': 'yo', 'files': [version.files.all()[0].id]}
-        amo.log(amo.LOG.REQUEST_SUPER_REVIEW, self.addon,
-                self.addon.latest_version, user=user, created=datetime.now(),
-                details=details)
-
-        doc = pq(self.client.get(self.edit_url).content)
-        comment = doc('#approval_status').find('.version-comments').eq(0)
-
-        assert comment.find('pre.email_comment').length == 0
-
-    def test_show_item_history_multiple(self):
-        version = self.addon.latest_version
-        user = UserProfile.objects.get(email='editor@mozilla.com')
-
-        details = {'comments': 'yo', 'files': [version.files.all()[0].id]}
-        amo.log(amo.LOG.APPROVE_VERSION, self.addon,
-                self.addon.latest_version, user=user, created=datetime.now(),
-                details=details)
-
-        amo.log(amo.LOG.REQUEST_SUPER_REVIEW, self.addon,
-                self.addon.latest_version, user=user, created=datetime.now(),
-                details=details)
-
-        doc = pq(self.client.get(self.edit_url).content)
-        comments = doc('#approval_status').find('.version-comments')
-
-        assert comments.length == 2
-
     def test_with_source(self):
         tdir = temp.gettempdir()
         source = temp.NamedTemporaryFile(suffix=".zip", dir=tdir)

src/olympia/devhub/tests/test_views_versions.py

@@ -447,6 +447,23 @@ class TestVersion(TestCase):
         assert set([i.attrib['type'] for i in doc('input.platform')]) == (
             set(['checkbox']))
 
+    def test_version_history(self):
+        self.client.cookies['jwt_api_auth_token'] = 'magicbeans'
+        v1 = self.version
+        v2, _ = self._extra_version_and_file(amo.STATUS_UNREVIEWED)
+        r = self.client.get(self.url)
+        assert r.status_code == 200
+        doc = pq(r.content)
+        show_links = doc('.review-history-show')
+        assert show_links.length == 2
+        assert show_links[0].attrib['data-div'] == '#%s-review-history' % v1.id
+        assert show_links[1].attrib['data-div'] == '#%s-review-history' % v2.id
+        review_history_td = doc('#%s-review-history' % v1.id)[0]
+        assert review_history_td.attrib['data-token'] == 'magicbeans'
+        assert review_history_td.attrib['data-api-url'] == reverse(
+            'version-reviewnotes-list', args=[self.addon.id, self.version.id])
+        assert doc('.review-history-hide').length == 2
+
 
 class TestVersionEditMixin(object):
 

src/olympia/devhub/views.py

@@ -1130,7 +1130,6 @@ def version_edit(request, addon_id, addon, version_id):
 
     file_form = forms.FileFormSet(request.POST or None, prefix='files',
                                   queryset=version.files.all())
-    file_history = _get_file_history(version)
 
     data = {'version_form': version_form, 'file_form': file_form}
 
@@ -1175,7 +1174,7 @@ def version_edit(request, addon_id, addon, version_id):
         return redirect('devhub.versions.edit', addon.slug, version_id)
 
     data.update(addon=addon, version=version, new_file_form=new_file_form,
-                file_history=file_history, is_admin=is_admin)
+                is_admin=is_admin)
     return render(request, 'devhub/versions/edit.html', data)
 
 
@@ -1187,22 +1186,6 @@ def _log_max_version_change(addon, version, appversion):
             addon, version, details=details)
 
 
-def _get_file_history(version):
-    file_ids = [f.id for f in version.all_files]
-    addon = version.addon
-    file_history = (ActivityLog.objects.for_addons(addon)
-                               .filter(action__in=amo.LOG_REVIEW_QUEUE))
-    files = dict([(fid, []) for fid in file_ids])
-    for log in file_history:
-        details = log.details
-        current_file_ids = details["files"] if 'files' in details else []
-        for fid in current_file_ids:
-            if fid in file_ids:
-                files[fid].append(log)
-
-    return files
-
-
 @dev_required
 @post_required
 @transaction.atomic
@@ -1373,10 +1356,13 @@ def version_list(request, addon_id, addon):
     new_file_form = forms.NewVersionForm(None, addon=addon, request=request)
     is_admin = acl.action_allowed(request, 'ReviewerAdminTools', 'View')
 
+    token = request.COOKIES.get('jwt_api_auth_token', None)
+
     data = {'addon': addon,
             'versions': versions,
             'new_file_form': new_file_form,
             'position': get_position(addon),
+            'token': token,
             'is_admin': is_admin}
     return render(request, 'devhub/versions/list.html', data)
 

src/olympia/lib/settings_base.py

@@ -383,6 +383,7 @@ INSTALLED_APPS = (
     'olympia.abuse',
     'olympia.access',
     'olympia.accounts',
+    'olympia.activity',
     'olympia.addons',
     'olympia.api',
     'olympia.applications',
@@ -819,6 +820,7 @@ MINIFY_BUNDLES = {
             'js/impala/formset.js',
             'js/zamboni/devhub.js',
             'js/zamboni/validator.js',
+            'js/lib/jquery.timeago.js',
         ),
         'zamboni/editors': (
             'js/lib/highcharts.src.js',

static/css/zamboni/developers.css

@@ -637,6 +637,15 @@ a.extra {
     margin: 0;
 }
 
+div.history-container {
+    margin: 0em 2em;
+}
+
+div.review-entry p {
+    color: #999;
+    margin-bottom: 0px;
+}
+
 .status-lite-nom i,
 .status-nominated b,
 .status-unreviewed b {

static/js/lib/jquery.timeago.js

@@ -0,0 +1,231 @@
+/**
+ * Timeago is a jQuery plugin that makes it easy to support automatically
+ * updating fuzzy timestamps (e.g. "4 minutes ago" or "about 1 day ago").
+ *
+ * @name timeago
+ * @version 1.5.3
+ * @requires jQuery v1.2.3+
+ * @author Ryan McGeary
+ * @license MIT License - http://www.opensource.org/licenses/mit-license.php
+ *
+ * For usage and examples, visit:
+ * http://timeago.yarp.com/
+ *
+ * Copyright (c) 2008-2015, Ryan McGeary (ryan -[at]- mcgeary [*dot*] org)
+ */
+
+(function (factory) {
+  if (typeof define === 'function' && define.amd) {
+    // AMD. Register as an anonymous module.
+    define(['jquery'], factory);
+  } else if (typeof module === 'object' && typeof module.exports === 'object') {
+    factory(require('jquery'));
+  } else {
+    // Browser globals
+    factory(jQuery);
+  }
+}(function ($) {
+  $.timeago = function(timestamp) {
+    if (timestamp instanceof Date) {
+      return inWords(timestamp);
+    } else if (typeof timestamp === "string") {
+      return inWords($.timeago.parse(timestamp));
+    } else if (typeof timestamp === "number") {
+      return inWords(new Date(timestamp));
+    } else {
+      return inWords($.timeago.datetime(timestamp));
+    }
+  };
+  var $t = $.timeago;
+
+  $.extend($.timeago, {
+    settings: {
+      refreshMillis: 60000,
+      allowPast: true,
+      allowFuture: false,
+      localeTitle: false,
+      cutoff: 0,
+      autoDispose: true,
+      strings: {
+        prefixAgo: null,
+        prefixFromNow: null,
+        suffixAgo: "ago",
+        suffixFromNow: "from now",
+        inPast: 'any moment now',
+        seconds: "less than a minute",
+        minute: "about a minute",
+        minutes: "%d minutes",
+        hour: "about an hour",
+        hours: "about %d hours",
+        day: "a day",
+        days: "%d days",
+        month: "about a month",
+        months: "%d months",
+        year: "about a year",
+        years: "%d years",
+        wordSeparator: " ",
+        numbers: []
+      }
+    },
+
+    inWords: function(distanceMillis) {
+      if (!this.settings.allowPast && ! this.settings.allowFuture) {
+          throw 'timeago allowPast and allowFuture settings can not both be set to false.';
+      }
+
+      var $l = this.settings.strings;
+      var prefix = $l.prefixAgo;
+      var suffix = $l.suffixAgo;
+      if (this.settings.allowFuture) {
+        if (distanceMillis < 0) {
+          prefix = $l.prefixFromNow;
+          suffix = $l.suffixFromNow;
+        }
+      }
+
+      if (!this.settings.allowPast && distanceMillis >= 0) {
+        return this.settings.strings.inPast;
+      }
+
+      var seconds = Math.abs(distanceMillis) / 1000;
+      var minutes = seconds / 60;
+      var hours = minutes / 60;
+      var days = hours / 24;
+      var years = days / 365;
+
+      function substitute(stringOrFunction, number) {
+        var string = $.isFunction(stringOrFunction) ? stringOrFunction(number, distanceMillis) : stringOrFunction;
+        var value = ($l.numbers && $l.numbers[number]) || number;
+        return string.replace(/%d/i, value);
+      }
+
+      var words = seconds < 45 && substitute($l.seconds, Math.round(seconds)) ||
+        seconds < 90 && substitute($l.minute, 1) ||
+        minutes < 45 && substitute($l.minutes, Math.round(minutes)) ||
+        minutes < 90 && substitute($l.hour, 1) ||
+        hours < 24 && substitute($l.hours, Math.round(hours)) ||
+        hours < 42 && substitute($l.day, 1) ||
+        days < 30 && substitute($l.days, Math.round(days)) ||
+        days < 45 && substitute($l.month, 1) ||
+        days < 365 && substitute($l.months, Math.round(days / 30)) ||
+        years < 1.5 && substitute($l.year, 1) ||
+        substitute($l.years, Math.round(years));
+
+      var separator = $l.wordSeparator || "";
+      if ($l.wordSeparator === undefined) { separator = " "; }
+      return $.trim([prefix, words, suffix].join(separator));
+    },
+
+    parse: function(iso8601) {
+      var s = $.trim(iso8601);
+      s = s.replace(/\.\d+/,""); // remove milliseconds
+      s = s.replace(/-/,"/").replace(/-/,"/");
+      s = s.replace(/T/," ").replace(/Z/," UTC");
+      s = s.replace(/([\+\-]\d\d)\:?(\d\d)/," $1$2"); // -04:00 -> -0400
+      s = s.replace(/([\+\-]\d\d)$/," $100"); // +09 -> +0900
+      return new Date(s);
+    },
+    datetime: function(elem) {
+      var iso8601 = $t.isTime(elem) ? $(elem).attr("datetime") : $(elem).attr("title");
+      return $t.parse(iso8601);
+    },
+    isTime: function(elem) {
+      // jQuery's `is()` doesn't play well with HTML5 in IE
+      return $(elem).get(0).tagName.toLowerCase() === "time"; // $(elem).is("time");
+    }
+  });
+
+  // functions that can be called via $(el).timeago('action')
+  // init is default when no action is given
+  // functions are called with context of a single element
+  var functions = {
+    init: function() {
+      var refresh_el = $.proxy(refresh, this);
+      refresh_el();
+      var $s = $t.settings;
+      if ($s.refreshMillis > 0) {
+        this._timeagoInterval = setInterval(refresh_el, $s.refreshMillis);
+      }
+    },
+    update: function(timestamp) {
+      var date = (timestamp instanceof Date) ? timestamp : $t.parse(timestamp);
+      $(this).data('timeago', { datetime: date });
+      if ($t.settings.localeTitle) {
+        $(this).attr("title", date.toLocaleString());
+      }
+      refresh.apply(this);
+    },
+    updateFromDOM: function() {
+      $(this).data('timeago', { datetime: $t.parse( $t.isTime(this) ? $(this).attr("datetime") : $(this).attr("title") ) });
+      refresh.apply(this);
+    },
+    dispose: function () {
+      if (this._timeagoInterval) {
+        window.clearInterval(this._timeagoInterval);
+        this._timeagoInterval = null;
+      }
+    }
+  };
+
+  $.fn.timeago = function(action, options) {
+    var fn = action ? functions[action] : functions.init;
+    if (!fn) {
+      throw new Error("Unknown function name '"+ action +"' for timeago");
+    }
+    // each over objects here and call the requested function
+    this.each(function() {
+      fn.call(this, options);
+    });
+    return this;
+  };
+
+  function refresh() {
+    var $s = $t.settings;
+
+    //check if it's still visible
+    if ($s.autoDispose && !$.contains(document.documentElement,this)) {
+      //stop if it has been removed
+      $(this).timeago("dispose");
+      return this;
+    }
+
+    var data = prepareData(this);
+
+    if (!isNaN(data.datetime)) {
+      if ( $s.cutoff === 0 || Math.abs(distance(data.datetime)) < $s.cutoff) {
+        $(this).text(inWords(data.datetime));
+      } else {
+        if ($(this).attr('title').length > 0) {
+            $(this).text($(this).attr('title'));
+        }
+      }
+    }
+    return this;
+  }
+
+  function prepareData(element) {
+    element = $(element);
+    if (!element.data("timeago")) {
+      element.data("timeago", { datetime: $t.datetime(element) });
+      var text = $.trim(element.text());
+      if ($t.settings.localeTitle) {
+        element.attr("title", element.data('timeago').datetime.toLocaleString());
+      } else if (text.length > 0 && !($t.isTime(element) && element.attr("title"))) {
+        element.attr("title", text);
+      }
+    }
+    return element.data("timeago");
+  }
+
+  function inWords(date) {
+    return $t.inWords(distance(date));
+  }
+
+  function distance(date) {
+    return (new Date().getTime() - date.getTime());
+  }
+
+  // fix for IE6 suckage
+  document.createElement("abbr");
+  document.createElement("time");
+}));

static/js/zamboni/devhub.js

@@ -691,6 +691,89 @@ function initVersions() {
             $('.current-version-warning', this).toggle(is_current);
             return true;
         }});
+
+    function loadReviewHistory(div, nextLoad) {
+        div.removeClass("hidden");
+        var token = div.data('token');
+        var container = div.children('.history-container');
+        container.children('.review-entry-loading').removeClass("hidden");
+        container.children('.review-entry-failure').addClass("hidden");
+        if (!nextLoad) {
+            container.children('.review-entry').empty();
+            var api_url = div.data('api-url');
+        } else {
+            var api_url = div.data('next-url');
+        }
+        var success = function (json) {
+            var empty_note = container.children('.review-entry-empty');
+            json["results"].forEach(function(note) {
+                var clone = empty_note.clone(true, true);
+                clone.attr('class', 'review-entry');
+                clone.find('span.action')[0].textContent = note["action_label"];
+                var user = clone.find('a:contains("$user_name")');
+                user[0].textContent = note["user"]["name"];
+                user.attr('href', note["user"]["url"]);
+                var date = clone.find('time.timeago');
+                date[0].textContent = note["date"];
+                date.attr('datetime', note["date"]);
+                date.attr('title', note["date"]);
+                clone.find('pre:contains("$comments")')[0].textContent = note["comments"];
+                clone.insertAfter(container.children('.review-entry-failure'));
+            });
+            var loadmorediv = container.children('div.review-entry-loadmore');
+            if (json["next"]) {
+                loadmorediv.removeClass("hidden");
+                container.prepend(loadmorediv);
+                div.attr('data-next-url', json["next"]);
+            } else {
+                loadmorediv.addClass("hidden");
+            }
+            $("time.timeago").timeago("updateFromDOM");
+        }
+        var fail =  function(xhr) {
+            container.children('.review-entry-failure').removeClass("hidden");
+            container.children('.review-entry-failure').append(
+                "<pre>"+api_url+", "+xhr.statusText+", "+xhr.responseText+"</pre>")
+        }
+        $.ajax({
+            url: api_url,
+            type: 'get',
+            beforeSend: function (xhr) {
+                xhr.setRequestHeader ("Authorization", 'Bearer '+token)
+            },
+            complete: function (xhr) {
+                container.children('.review-entry-loading').addClass("hidden")
+            },
+            processData: false,
+            contentType: false,
+            success: success,
+            error: fail
+        });
+    }
+    $('.review-history-show').click(function (e) {
+        e.preventDefault();
+        var $tgt = $(this);
+        $tgt.addClass("hidden");
+        $tgt.next().removeClass("hidden");
+        loadReviewHistory($($tgt.data('div')));
+    });
+    $('.review-history-hide').click(function (e) {
+        e.preventDefault();
+        var $tgt = $(this);
+        $tgt.addClass("hidden");
+        var prev = $tgt.prev();
+        prev.removeClass("hidden");
+        $(prev.data('div')).addClass("hidden");
+    });
+    $('a.review-history-loadmore').click(function (e) {
+        e.preventDefault();
+        var $tgt = $(this);
+        loadReviewHistory($($tgt.data('div')), true);
+    });
+    $('.review-history-hide').prop("style", "");
+    $('.review-history.hidden').prop("style", "");
+    $('.history-container .hidden').prop("style", "");
+    $("time.timeago").timeago();
 }
 
 function initSubmit() {