Add new message indicator to review history for messages since last reply.
This commit is contained in:
Andrew Williamson
Родитель
505bcdbb6e
Коммит
a138cced28
|
|
@ -12,10 +12,16 @@ class ActivityLogSerializer(serializers.ModelSerializer):
|
|||
comments = serializers.SerializerMethodField()
|
||||
date = serializers.DateTimeField(source='created')
|
||||
user = BaseUserSerializer()
|
||||
highlight = serializers.SerializerMethodField()
|
||||
|
||||
class Meta:
|
||||
model = ActivityLog
|
||||
fields = ('id', 'action', 'action_label', 'comments', 'user', 'date')
|
||||
fields = ('id', 'action', 'action_label', 'comments', 'user', 'date',
|
||||
'highlight')
|
||||
|
||||
def __init__(self, *args, **kwargs):
|
||||
super(ActivityLogSerializer, self).__init__(*args, **kwargs)
|
||||
self.to_highlight = kwargs.get('context', []).get('to_highlight', [])
|
||||
|
||||
def get_comments(self, obj):
|
||||
return obj.details['comments']
|
||||
|
|
@ -26,3 +32,6 @@ class ActivityLogSerializer(serializers.ModelSerializer):
|
|||
|
||||
def get_action(self, obj):
|
||||
return self.get_action_label(obj).replace(' ', '-').lower()
|
||||
|
||||
def get_highlight(self, obj):
|
||||
return obj in self.to_highlight
|
||||
|
|
|
|||
|
|
@ -9,7 +9,9 @@ from olympia.amo.tests import (
|
|||
|
||||
|
||||
class LogMixin(object):
|
||||
def log(self, comments, action, created):
|
||||
def log(self, comments, action, created=None):
|
||||
if not created:
|
||||
created = self.days_ago(0)
|
||||
details = {'comments': comments}
|
||||
details['version'] = self.addon.current_version.version
|
||||
kwargs = {'user': self.user, 'created': created, 'details': details}
|
||||
|
|
@ -22,22 +24,36 @@ class TestReviewNotesSerializerOutput(TestCase, LogMixin):
|
|||
self.request = APIRequestFactory().get('/')
|
||||
self.user = user_factory()
|
||||
self.addon = addon_factory()
|
||||
self.now = self.days_ago(0)
|
||||
self.entry = self.log(u'Oh nôes!', amo.LOG.REJECT_VERSION, self.now)
|
||||
|
||||
def serialize(self, id_):
|
||||
serializer = ActivityLogSerializer(context={'request': self.request})
|
||||
return serializer.to_representation(id_)
|
||||
def serialize(self, context={}):
|
||||
context['request'] = self.request
|
||||
serializer = ActivityLogSerializer(context=context)
|
||||
return serializer.to_representation(self.entry)
|
||||
|
||||
def test_basic(self):
|
||||
now = self.days_ago(0)
|
||||
entry = self.log(u'Oh nôes!', amo.LOG.REJECT_VERSION, now)
|
||||
result = self.serialize()
|
||||
|
||||
result = self.serialize(entry)
|
||||
|
||||
assert result['id'] == entry.pk
|
||||
assert result['date'] == now.isoformat()
|
||||
assert result['id'] == self.entry.pk
|
||||
assert result['date'] == self.now.isoformat()
|
||||
assert result['action'] == 'rejected'
|
||||
assert result['action_label'] == 'Rejected'
|
||||
assert result['comments'] == u'Oh nôes!'
|
||||
assert result['comments'] == u'Oh nôes!'
|
||||
assert result['user'] == {
|
||||
'name': self.user.name,
|
||||
'url': absolutify(self.user.get_url_path())}
|
||||
|
||||
def test_should_highlight(self):
|
||||
result = self.serialize(context={'to_highlight': [self.entry]})
|
||||
|
||||
assert result['id'] == self.entry.pk
|
||||
assert result['highlight']
|
||||
|
||||
def test_should_not_highlight(self):
|
||||
no_highlight = self.log(u'something élse', amo.LOG.REJECT_VERSION)
|
||||
|
||||
result = self.serialize(context={'to_highlight': [no_highlight]})
|
||||
|
||||
assert result['id'] == self.entry.pk
|
||||
assert not result['highlight']
|
||||
|
|
|
|||
|
|
@ -142,6 +142,7 @@ class TestReviewNotesViewSetDetail(ReviewNotesViewSetDetailMixin, TestCase):
|
|||
assert result['id'] == self.note.pk
|
||||
assert result['action_label'] == amo.LOG.APPROVE_VERSION.short
|
||||
assert result['comments'] == u'noôo!'
|
||||
assert result['highlight'] # Its the first reply so highlight
|
||||
|
||||
def _set_tested_url(self, pk=None, version_pk=None, addon_pk=None):
|
||||
self.url = reverse('version-reviewnotes-detail', kwargs={
|
||||
|
|
@ -165,8 +166,10 @@ class TestReviewNotesViewSetList(ReviewNotesViewSetDetailMixin, TestCase):
|
|||
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(2))
|
||||
self.note2 = self.log(u'réply!', amo.LOG.DEVELOPER_REPLY_VERSION,
|
||||
self.days_ago(1))
|
||||
self.note3 = self.log(u'yéss!', amo.LOG.REJECT_VERSION,
|
||||
self.days_ago(0))
|
||||
|
||||
self.version = self.addon.latest_version
|
||||
|
|
@ -177,11 +180,19 @@ class TestReviewNotesViewSetList(ReviewNotesViewSetDetailMixin, TestCase):
|
|||
assert response.status_code == 200
|
||||
result = json.loads(response.content)
|
||||
assert result['results']
|
||||
assert len(result['results']) == 2
|
||||
assert len(result['results']) == 3
|
||||
|
||||
result_version = result['results'][0]
|
||||
assert result_version['id'] == self.note2.pk
|
||||
assert result_version['id'] == self.note3.pk
|
||||
assert result_version['highlight'] # This note is after the dev reply.
|
||||
|
||||
result_version = result['results'][1]
|
||||
assert result_version['id'] == self.note2.pk
|
||||
assert not result_version['highlight'] # This note is the dev reply.
|
||||
|
||||
result_version = result['results'][2]
|
||||
assert result_version['id'] == self.note.pk
|
||||
assert not result_version['highlight'] # The dev replied so read it.
|
||||
|
||||
def _set_tested_url(self, pk=None, version_pk=None, addon_pk=None):
|
||||
self.url = reverse('version-reviewnotes-list', kwargs={
|
||||
|
|
|
|||
|
|
@ -37,3 +37,16 @@ class VersionReviewNotesViewSet(AddonChildMixin, RetrieveModelMixin,
|
|||
# Just loading the add-on object triggers permission checks, because
|
||||
# the implementation in AddonChildMixin calls AddonViewSet.get_object()
|
||||
self.get_addon_object()
|
||||
|
||||
def get_serializer_context(self):
|
||||
ctx = super(VersionReviewNotesViewSet, self).get_serializer_context()
|
||||
ctx['to_highlight'] = self.pending_queryset(
|
||||
amo.LOG.DEVELOPER_REPLY_VERSION)
|
||||
return ctx
|
||||
|
||||
def pending_queryset(self, log_type):
|
||||
version_qs = self.get_queryset()
|
||||
latest_reply = version_qs.filter(action=log_type.id).first()
|
||||
if not latest_reply:
|
||||
return version_qs
|
||||
return version_qs.filter(created__gt=latest_reply.created)
|
||||
|
|
|
|||
|
|
@ -650,6 +650,14 @@ class PRELIMINARY_ADDON_MIGRATED(_LOG):
|
|||
review_queue = True
|
||||
|
||||
|
||||
class DEVELOPER_REPLY_VERSION(_LOG):
|
||||
id = 140
|
||||
format = _(u'Reply by developer on {addon} {version}.')
|
||||
short = _(u'Developer Reply')
|
||||
keep = True
|
||||
review_queue = True
|
||||
|
||||
|
||||
LOGS = [x for x in vars().values()
|
||||
if isclass(x) and issubclass(x, _LOG) and x != _LOG]
|
||||
# Make sure there's no duplicate IDs.
|
||||
|
|
@ -671,7 +679,7 @@ 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) -
|
||||
LOG_REVIEW_QUEUE_DEVELOPER = list(set(LOG_REVIEW_QUEUE) -
|
||||
set(LOG_HIDE_DEVELOPER))
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ from olympia.amo.helpers import breadcrumbs, impala_breadcrumbs, page_title
|
|||
from olympia.access import acl
|
||||
from olympia.addons.helpers import new_context
|
||||
from olympia.addons.models import Addon
|
||||
from olympia.devhub.models import ActivityLog
|
||||
from olympia.compat.models import CompatReport
|
||||
from olympia.files.models import File
|
||||
|
||||
|
|
@ -225,3 +226,15 @@ def version_disabled(version):
|
|||
disabled = [status == amo.STATUS_DISABLED
|
||||
for _id, status in version.statuses]
|
||||
return all(disabled)
|
||||
|
||||
|
||||
@register.function
|
||||
def pending_activity_log_count_for_developer(version):
|
||||
alog = ActivityLog.objects.for_version(version).filter(
|
||||
action__in=amo.LOG_REVIEW_QUEUE_DEVELOPER)
|
||||
|
||||
latest_reply = alog.filter(
|
||||
action=amo.LOG.DEVELOPER_REPLY_VERSION.id).first()
|
||||
if not latest_reply:
|
||||
return alog.count()
|
||||
return alog.filter(created__gt=latest_reply.created).count()
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@
|
|||
{% block title %}{{ dev_page_title(title, addon) }}{% endblock %}
|
||||
|
||||
{% macro version_details(version, full_info=False) %}
|
||||
{% set is_last_version = (version == version.addon.latest_or_rejected_version) %}
|
||||
<tr{% if version_disabled(version) %} class="version-disabled"{% endif %}>
|
||||
<td>
|
||||
<strong>
|
||||
|
|
@ -48,6 +49,9 @@
|
|||
<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>
|
||||
{% if is_last_version %}
|
||||
<b class="review-history-pending-count">{{ pending_activity_log_count_for_developer(version) }}</b>
|
||||
{% endif %}
|
||||
</div>
|
||||
</td>
|
||||
<td class="file-validation">
|
||||
|
|
|
|||
|
|
@ -8,7 +8,8 @@ from mock import Mock
|
|||
from pyquery import PyQuery as pq
|
||||
|
||||
from olympia import amo
|
||||
from olympia.amo.tests import TestCase
|
||||
from olympia.amo import LOG
|
||||
from olympia.amo.tests import addon_factory, days_ago, TestCase, user_factory
|
||||
from olympia.amo.urlresolvers import reverse
|
||||
from olympia.amo.tests.test_helpers import render
|
||||
from olympia.addons.models import Addon
|
||||
|
|
@ -212,3 +213,22 @@ class TestDevFilesStatus(TestCase):
|
|||
self.addon.status = amo.STATUS_PUBLIC
|
||||
self.file.status = amo.STATUS_DISABLED
|
||||
self.expect(File.STATUS_CHOICES[amo.STATUS_DISABLED])
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
'action1,action2,action3,count',
|
||||
((LOG.REQUEST_INFORMATION, LOG.REJECT_VERSION, LOG.APPROVE_VERSION, 3),
|
||||
(LOG.DEVELOPER_REPLY_VERSION, LOG.REJECT_VERSION, LOG.REJECT_VERSION, 2),
|
||||
(LOG.APPROVE_VERSION, LOG.DEVELOPER_REPLY_VERSION, LOG.REJECT_VERSION, 1),
|
||||
(LOG.APPROVE_VERSION, LOG.REJECT_VERSION, LOG.DEVELOPER_REPLY_VERSION, 0))
|
||||
)
|
||||
def test_pending_activity_log_count_for_developer(
|
||||
action1, action2, action3, count):
|
||||
user = user_factory()
|
||||
addon = addon_factory()
|
||||
version = addon.latest_version
|
||||
amo.log(action1, addon, version, user=user, created=days_ago(2))
|
||||
amo.log(action2, addon, version, user=user, created=days_ago(1))
|
||||
amo.log(action3, addon, version, user=user, created=days_ago(0))
|
||||
|
||||
assert helpers.pending_activity_log_count_for_developer(version) == count
|
||||
|
|
|
|||
|
|
@ -469,6 +469,10 @@ class TestVersion(TestCase):
|
|||
self.client.cookies['jwt_api_auth_token'] = 'magicbeans'
|
||||
v1 = self.version
|
||||
v2, _ = self._extra_version_and_file(amo.STATUS_UNREVIEWED)
|
||||
# Add some activity log messages
|
||||
amo.log(amo.LOG.REJECT_VERSION, v2.addon, v2, user=self.user)
|
||||
amo.log(amo.LOG.REJECT_VERSION, v2.addon, v2, user=self.user)
|
||||
|
||||
r = self.client.get(self.url)
|
||||
assert r.status_code == 200
|
||||
doc = pq(r.content)
|
||||
|
|
@ -482,6 +486,11 @@ class TestVersion(TestCase):
|
|||
'version-reviewnotes-list', args=[self.addon.id, self.version.id])
|
||||
assert doc('.review-history-hide').length == 2
|
||||
|
||||
pending_activity_count = doc('.review-history-pending-count')
|
||||
# Only one, for the latest/deleted version
|
||||
assert pending_activity_count.length == 1
|
||||
assert pending_activity_count.text() == '2'
|
||||
|
||||
|
||||
class TestVersionEditMixin(object):
|
||||
|
||||
|
|
|
|||
|
|
@ -1428,7 +1428,7 @@ class TestPerformance(QueueTest):
|
|||
doc = pq(r.content)
|
||||
data = json.loads(doc('#monthly').attr('data-chart'))
|
||||
label = datetime.now().strftime('%Y-%m')
|
||||
assert data[label]['usercount'] == 19
|
||||
assert data[label]['usercount'] == len(amo.LOG_REVIEW_QUEUE) - 1
|
||||
|
||||
def _test_performance_other_user_as_admin(self):
|
||||
userid = amo.get_user().pk
|
||||
|
|
|
|||
|
|
@ -637,8 +637,24 @@ a.extra {
|
|||
margin: 0;
|
||||
}
|
||||
|
||||
b.review-history-pending-count {
|
||||
background-color: #000;
|
||||
border-radius: 5px;
|
||||
padding: 0 0.3em;
|
||||
color: #FFF;
|
||||
}
|
||||
|
||||
div.history-container {
|
||||
margin: 0em 2em;
|
||||
margin: 0em 1em;
|
||||
}
|
||||
|
||||
div.review-entry {
|
||||
padding: 0.5em 1em;
|
||||
margin-bottom: 0.25em;
|
||||
}
|
||||
|
||||
div.review-entry.new {
|
||||
background-color: #FFFFD5;
|
||||
}
|
||||
|
||||
div.review-entry p {
|
||||
|
|
@ -649,6 +665,7 @@ div.review-entry p {
|
|||
div.review-entry pre {
|
||||
white-space: pre-wrap;
|
||||
word-break: break-all;
|
||||
margin-bottom: 0px;
|
||||
}
|
||||
|
||||
.status-lite-nom i,
|
||||
|
|
|
|||
|
|
@ -699,7 +699,7 @@ function initVersions() {
|
|||
container.children('.review-entry-loading').removeClass("hidden");
|
||||
container.children('.review-entry-failure').addClass("hidden");
|
||||
if (!nextLoad) {
|
||||
container.children('.review-entry').empty();
|
||||
container.children('.review-entry').remove();
|
||||
var api_url = div.data('api-url');
|
||||
} else {
|
||||
var api_url = div.data('next-url');
|
||||
|
|
@ -709,6 +709,9 @@ function initVersions() {
|
|||
json["results"].forEach(function(note) {
|
||||
var clone = empty_note.clone(true, true);
|
||||
clone.attr('class', 'review-entry');
|
||||
if (note["highlight"] == true) {
|
||||
clone.addClass("new");
|
||||
}
|
||||
clone.find('span.action')[0].textContent = note["action_label"];
|
||||
var user = clone.find('a:contains("$user_name")');
|
||||
user[0].textContent = note["user"]["name"];
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче