mozilla
/
addons-server
зеркало из https://github.com/mozilla/addons-server.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Merge pull request #7562 from diox/deadline-for-info-requests 

Add a (soft) deadline for requests for information
This commit is contained in:
Mathieu Pillard 2018-02-19 12:35:02 +01:00 коммит произвёл GitHub
Родитель f11f838383 0511e5cafb
Коммит 8789f550e5
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
33 изменённых файлов: 957 добавлений и 426 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

15
docs/topics/api/reviewers.rst
Просмотреть файл

@ -103,7 +103,20 @@ flag that reviewers can set on an add-on.
Requires authentication and the current user to have ``Reviews:Admin``
permission.
.. http:post::/api/v3/reviewers/addon/1/clear_admin_review_flag/
.. http:post::/api/v3/reviewers/addon/(int:addon_id)/clear_admin_review_flag/
:query string flag_type: The flag to clear. Can be either ``code`` or
``content``.
----------------------------------
Clear Information Request Deadline
----------------------------------
This endpoint allows you to clear the request for more information deadline
that reviewers can set on an add-on.
.. note::
Requires authentication and the current user to have ``Reviews:Admin``
permission.
.. http:post::/api/v3/reviewers/addon/(int:addon_id)/clear_pending_info_request/

2
src/olympia/activity/templates/activity/emails/from_reviewer.txt
Просмотреть файл

@ -9,7 +9,7 @@ Hello,
*********
To respond, please reply to this email or visit {{ url }}. {% if is_info_request %}If we do not hear from you within seven (7) days of this notification, this listing may be removed from addons.mozilla.org.{% endif %}
To respond, please reply to this email or visit {{ url }}. {% if is_info_request and number_of_days_left %}If we do not hear from you within {{ number_of_days_left }} of this notification, this listing may be removed from addons.mozilla.org.{% endif %}
Thank you for your attention.

123
src/olympia/activity/tests/test_utils.py
Просмотреть файл

@ -3,6 +3,8 @@ import copy
import json
import os
from datetime import datetime, timedelta
from django.conf import settings
from django.core import mail
@ -18,7 +20,9 @@ from olympia.activity.models import (
from olympia.activity.utils import (
ACTIVITY_MAIL_GROUP, ActivityEmailEncodingError, ActivityEmailParser,
ActivityEmailTokenError, ActivityEmailUUIDError, add_email_to_activity_log,
add_email_to_activity_log_wrapper, log_and_notify, send_activity_mail)
add_email_to_activity_log_wrapper, log_and_notify,
notify_about_activity_log, send_activity_mail)
from olympia.addons.models import Addon, AddonReviewerFlags
from olympia.amo.templatetags.jinja_helpers import absolutify
from olympia.amo.tests import TestCase, addon_factory, user_factory
from olympia.amo.urlresolvers import reverse
@ -247,14 +251,16 @@ class TestLogAndNotify(TestCase):
assert reply_to.endswith(settings.INBOUND_EMAIL_DOMAIN)
return recipients
def _check_email_info_request(self, call, url, reason_text):
def _check_email_info_request(self, call, url, reason_text, days_text):
subject = call[0][0]
body = call[0][1]
assert subject == u'Mozilla Add-ons: Action Required for %s %s' % (
self.addon.name, self.version.version)
assert ('visit %s' % url) in body
assert ('receiving this email because %s' % reason_text) in body
assert 'If we do not hear from you within seven (7) days' in body
if days_text is not None:
assert 'If we do not hear from you within' in body
assert days_text in body
assert 'reviewing version %s of the add-on %s' % (
self.version.version, self.addon.name) in body
@ -265,10 +271,13 @@ class TestLogAndNotify(TestCase):
self.addon.name, self.version.version)
assert ('visit %s' % url) in body
assert ('receiving this email because %s' % reason_text) in body
assert 'If we do not hear from you within seven (7) days' not in body
assert 'If we do not hear from you within' not in body
@mock.patch('olympia.activity.utils.send_mail')
def test_reviewer_request_for_information(self, send_mail_mock):
AddonReviewerFlags.objects.create(
addon=self.addon,
pending_info_request=datetime.now() + timedelta(days=7))
self._create(amo.LOG.REQUEST_INFORMATION, self.reviewer)
log_and_notify(
amo.LOG.REQUEST_INFORMATION, 'blah', self.reviewer, self.version)
@ -287,17 +296,83 @@ class TestLogAndNotify(TestCase):
self._check_email_info_request(
send_mail_mock.call_args_list[0],
absolutify(self.addon.get_dev_url('versions')),
'you are listed as an author of this add-on.')
'you are listed as an author of this add-on.',
'seven (7) days of this notification')
self._check_email_info_request(
send_mail_mock.call_args_list[1],
absolutify(self.addon.get_dev_url('versions')),
'you are listed as an author of this add-on.')
'you are listed as an author of this add-on.',
'seven (7) days of this notification')
@mock.patch('olympia.activity.utils.send_mail')
def test_reviewer_request_for_information_close_date(self, send_mail_mock):
AddonReviewerFlags.objects.create(
addon=self.addon,
pending_info_request=datetime.now() + timedelta(days=1))
self._create(amo.LOG.REQUEST_INFORMATION, self.reviewer)
log_and_notify(
amo.LOG.REQUEST_INFORMATION, 'blah', self.reviewer, self.version)
assert send_mail_mock.call_count == 2 # Both authors.
sender = '%s <notifications@%s>' % (
self.reviewer.name, settings.INBOUND_EMAIL_DOMAIN)
assert sender == send_mail_mock.call_args_list[0][1]['from_email']
recipients = self._recipients(send_mail_mock)
assert len(recipients) == 2
assert self.developer.email in recipients
assert self.developer2.email in recipients
# The reviewer who sent it doesn't get their email back.
assert self.reviewer.email not in recipients
self._check_email_info_request(
send_mail_mock.call_args_list[0],
absolutify(self.addon.get_dev_url('versions')),
'you are listed as an author of this add-on.',
'one (1) day of this notification')
self._check_email_info_request(
send_mail_mock.call_args_list[1],
absolutify(self.addon.get_dev_url('versions')),
'you are listed as an author of this add-on.',
'one (1) day of this notification')
@mock.patch('olympia.activity.utils.send_mail')
def test_reviewer_request_for_information_far_date(self, send_mail_mock):
AddonReviewerFlags.objects.create(
addon=self.addon,
pending_info_request=datetime.now() + timedelta(days=21))
self._create(amo.LOG.REQUEST_INFORMATION, self.reviewer)
log_and_notify(
amo.LOG.REQUEST_INFORMATION, 'blah', self.reviewer, self.version)
assert send_mail_mock.call_count == 2 # Both authors.
sender = '%s <notifications@%s>' % (
self.reviewer.name, settings.INBOUND_EMAIL_DOMAIN)
assert sender == send_mail_mock.call_args_list[0][1]['from_email']
recipients = self._recipients(send_mail_mock)
assert len(recipients) == 2
assert self.developer.email in recipients
assert self.developer2.email in recipients
# The reviewer who sent it doesn't get their email back.
assert self.reviewer.email not in recipients
self._check_email_info_request(
send_mail_mock.call_args_list[0],
absolutify(self.addon.get_dev_url('versions')),
'you are listed as an author of this add-on.',
'21 days of this notification')
self._check_email_info_request(
send_mail_mock.call_args_list[1],
absolutify(self.addon.get_dev_url('versions')),
'you are listed as an author of this add-on.',
'21 days of this notification')
@mock.patch('olympia.activity.utils.send_mail')
def test_developer_reply(self, send_mail_mock):
# Set info-requested flag to make sure
# Set pending info request flag to make sure
# it has been dropped after the reply.
self.version.has_info_request = True
AddonReviewerFlags.objects.create(
addon=self.addon,
pending_info_request=datetime.now() + timedelta(days=1))
# One from the reviewer.
self._create(amo.LOG.REJECT_VERSION, self.reviewer)
# One from the developer. So the developer is on the 'thread'
@ -334,7 +409,8 @@ class TestLogAndNotify(TestCase):
send_mail_mock.call_args_list[1],
review_url, 'you reviewed this add-on.')
assert not self.version.has_info_request
self.addon = Addon.objects.get(pk=self.addon.pk)
assert not self.addon.pending_info_request
@mock.patch('olympia.activity.utils.send_mail')
def test_reviewer_reply(self, send_mail_mock):
@ -548,6 +624,35 @@ class TestLogAndNotify(TestCase):
settings.INBOUND_EMAIL_DOMAIN)
assert sender == send_mail_mock.call_args_list[0][1]['from_email']
@mock.patch('olympia.activity.utils.send_mail')
def test_notify_about_previous_activity(self, send_mail_mock):
# Create an activity to use when notifying.
activity = self._create(amo.LOG.REQUEST_INFORMATION, self.reviewer)
notify_about_activity_log(self.addon, self.version, activity)
assert ActivityLog.objects.count() == 1 # No new activity created.
assert send_mail_mock.call_count == 2 # Both authors.
sender = '%s <notifications@%s>' % (
self.reviewer.name, settings.INBOUND_EMAIL_DOMAIN)
assert sender == send_mail_mock.call_args_list[0][1]['from_email']
recipients = self._recipients(send_mail_mock)
assert len(recipients) == 2
assert self.developer.email in recipients
assert self.developer2.email in recipients
# The reviewer who sent it doesn't get their email back.
assert self.reviewer.email not in recipients
self._check_email_info_request(
send_mail_mock.call_args_list[0],
absolutify(self.addon.get_dev_url('versions')),
'you are listed as an author of this add-on.',
days_text=None)
self._check_email_info_request(
send_mail_mock.call_args_list[1],
absolutify(self.addon.get_dev_url('versions')),
'you are listed as an author of this add-on.',
days_text=None)
@pytest.mark.django_db
def test_send_activity_mail():

163
src/olympia/activity/utils.py
Просмотреть файл

@ -1,9 +1,10 @@
import datetime
import re
from datetime import datetime, timedelta
from email.utils import formataddr
from django.conf import settings
from django.contrib.humanize.templatetags.humanize import apnumber
from django.template import loader
from django.utils import translation
@ -16,6 +17,7 @@ import olympia.core.logger
from olympia import amo
from olympia.access import acl
from olympia.activity.models import ActivityLog, ActivityLogToken
from olympia.addons.models import AddonReviewerFlags
from olympia.amo.templatetags.jinja_helpers import absolutify
from olympia.amo.urlresolvers import reverse
from olympia.amo.utils import no_translation, send_mail
@ -188,21 +190,17 @@ def template_from_user(user, version):
def log_and_notify(action, comments, note_creator, version, perm_setting=None,
detail_kwargs=None):
"""Record an action through ActivityLog and notify relevant users about it.
"""
log_kwargs = {
'user': note_creator,
'created': datetime.datetime.now(),
'created': datetime.now(),
}
if detail_kwargs is None:
detail_kwargs = {}
if comments:
detail_kwargs['version'] = version.version
detail_kwargs['comments'] = comments
else:
# Just use the name of the action if no comments provided. Alas we
# can't know the locale of recipient, and our templates are English
# only so prevent language jumble by forcing into en-US.
with no_translation():
comments = '%s' % action.short
if detail_kwargs:
log_kwargs['details'] = detail_kwargs
@ -210,77 +208,120 @@ def log_and_notify(action, comments, note_creator, version, perm_setting=None,
if not note:
return
# Collect reviewers involved with this version.
notify_about_activity_log(
version.addon, version, note, perm_setting=perm_setting)
if action == amo.LOG.DEVELOPER_REPLY_VERSION:
# When a developer repies by email, we automatically clear the
# corresponding info request.
AddonReviewerFlags.objects.update_or_create(
addon=version.addon, defaults={'pending_info_request': None}
)
return note
def notify_about_activity_log(addon, version, note, perm_setting=None,
send_to_reviewers=True, send_to_staff=True):
"""Notify relevant users about an ActivityLog note."""
comments = (note.details or {}).get('comments')
if not comments:
# Just use the name of the action if no comments provided. Alas we
# can't know the locale of recipient, and our templates are English
# only so prevent language jumble by forcing into en-US.
with no_translation():
comments = '%s' % amo.LOG_BY_ID[note.action].short
# Collect add-on authors (excl. the person who sent the email.) and build
# the context for them.
addon_authors = set(addon.authors.all()) - {note.user}
author_context_dict = {
'name': addon.name,
'number': version.version,
'author': note.user.name,
'comments': comments,
'url': absolutify(addon.get_dev_url('versions')),
'SITE_URL': settings.SITE_URL,
'email_reason': 'you are listed as an author of this add-on',
'is_info_request': note.action == amo.LOG.REQUEST_INFORMATION.id,
}
# Not being localised because we don't know the recipients locale.
with translation.override('en-US'):
if note.action == amo.LOG.REQUEST_INFORMATION.id:
if addon.pending_info_request:
days_left = (
# We pad the time left with an extra hour so that the email
# does not end up saying "6 days left" because a few
# seconds or minutes passed between the datetime was saved
# and the email was sent.
addon.pending_info_request + timedelta(hours=1) -
datetime.now()
).days
if days_left > 9:
author_context_dict['number_of_days_left'] = (
'%d days' % days_left)
elif days_left > 1:
author_context_dict['number_of_days_left'] = (
'%s (%d) days' % (apnumber(days_left), days_left))
else:
author_context_dict['number_of_days_left'] = 'one (1) day'
subject = u'Mozilla Add-ons: Action Required for %s %s' % (
addon.name, version.version)
else:
subject = u'Mozilla Add-ons: %s %s' % (
addon.name, version.version)
# Build and send the mail for authors.
template = template_from_user(note.user, version)
from_email = formataddr((note.user.name, NOTIFICATIONS_FROM_EMAIL))
send_activity_mail(
subject, template.render(author_context_dict),
version, addon_authors, from_email, note.id, perm_setting)
if send_to_reviewers or send_to_staff:
# If task_user doesn't exist that's no big issue (i.e. in tests)
try:
task_user = {get_task_user()}
except UserProfile.DoesNotExist:
task_user = set()
if send_to_reviewers:
# Collect reviewers on the thread (excl. the email sender and task user
# for automated messages), build the context for them and send them
# their copy.
review_perm = (amo.permissions.ADDONS_REVIEW
if version.channel == amo.RELEASE_CHANNEL_LISTED
else amo.permissions.ADDONS_REVIEW_UNLISTED)
log_users = {
alog.user for alog in ActivityLog.objects.for_version(version) if
acl.action_allowed_user(alog.user, review_perm)}
# Collect add-on authors (excl. the person who sent the email.)
addon_authors = set(version.addon.authors.all()) - {note_creator}
# Collect staff that want a copy of the email
staff = set(
UserProfile.objects.filter(groups__name=ACTIVITY_MAIL_GROUP))
# If task_user doesn't exist that's no big issue (i.e. in tests)
try:
task_user = {get_task_user()}
except UserProfile.DoesNotExist:
task_user = set()
# Collect reviewers on the thread (excl. the email sender and task user for
# automated messages).
reviewers = log_users - addon_authors - task_user - {note_creator}
staff_cc = staff - reviewers - addon_authors - task_user - {note_creator}
author_context_dict = {
'name': version.addon.name,
'number': version.version,
'author': note_creator.name,
'comments': comments,
'url': absolutify(version.addon.get_dev_url('versions')),
'SITE_URL': settings.SITE_URL,
'email_reason': 'you are listed as an author of this add-on',
'is_info_request': action == amo.LOG.REQUEST_INFORMATION,
}
reviewers = log_users - addon_authors - task_user - {note.user}
reviewer_context_dict = author_context_dict.copy()
reviewer_context_dict['url'] = absolutify(
reverse('reviewers.review',
kwargs={'addon_id': version.addon.pk,
'channel': amo.CHANNEL_CHOICES_API[version.channel]},
add_prefix=False))
kwargs={
'addon_id': version.addon.pk,
'channel': amo.CHANNEL_CHOICES_API[version.channel]
}, add_prefix=False))
reviewer_context_dict['email_reason'] = 'you reviewed this add-on'
staff_cc_context_dict = reviewer_context_dict.copy()
staff_cc_context_dict['email_reason'] = (
'you are member of the activity email cc group')
# Not being localised because we don't know the recipients locale.
with translation.override('en-US'):
if action == amo.LOG.REQUEST_INFORMATION:
subject = u'Mozilla Add-ons: Action Required for %s %s' % (
version.addon.name, version.version)
else:
subject = u'Mozilla Add-ons: %s %s' % (
version.addon.name, version.version)
template = template_from_user(note_creator, version)
from_email = formataddr((note_creator.name, NOTIFICATIONS_FROM_EMAIL))
send_activity_mail(
subject, template.render(author_context_dict),
version, addon_authors, from_email, note.id, perm_setting)
send_activity_mail(
subject, template.render(reviewer_context_dict),
version, reviewers, from_email, note.id, perm_setting)
if send_to_staff:
# Collect staff that want a copy of the email, build the context for
# them and send them their copy.
staff = set(
UserProfile.objects.filter(groups__name=ACTIVITY_MAIL_GROUP))
staff_cc = (
staff - reviewers - addon_authors - task_user - {note.user})
staff_cc_context_dict = reviewer_context_dict.copy()
staff_cc_context_dict['email_reason'] = (
'you are member of the activity email cc group')
send_activity_mail(
subject, template.render(staff_cc_context_dict),
version, staff_cc, from_email, note.id, perm_setting)
if action == amo.LOG.DEVELOPER_REPLY_VERSION:
version.update(has_info_request=False)
return note
def send_activity_mail(subject, message, version, recipients, from_email,
unique_id, perm_setting=None):

6
src/olympia/addons/fixtures/addons/base_es.json
Просмотреть файл

@ -249,7 +249,6 @@
"pk": 1,
"model": "versions.version",
"fields": {
"has_info_request": false,
"license": null,
"created": "2014-12-08T06:39:45",
"has_reviewer_comment": false,
@ -269,7 +268,6 @@
"pk": 2,
"model": "versions.version",
"fields": {
"has_info_request": false,
"license": null,
"created": "2014-12-08T06:40:07",
"has_reviewer_comment": false,
@ -289,7 +287,6 @@
"pk": 3,
"model": "versions.version",
"fields": {
"has_info_request": false,
"license": null,
"created": "2014-12-08T06:40:16",
"has_reviewer_comment": false,
@ -309,7 +306,6 @@
"pk": 4,
"model": "versions.version",
"fields": {
"has_info_request": false,
"license": null,
"created": "2014-12-08T06:40:23",
"has_reviewer_comment": false,
@ -329,7 +325,6 @@
"pk": 5,
"model": "versions.version",
"fields": {
"has_info_request": false,
"license": null,
"created": "2014-12-08T06:40:24",
"has_reviewer_comment": false,
@ -349,7 +344,6 @@
"pk": 6,
"model": "versions.version",
"fields": {
"has_info_request": false,
"license": null,
"created": "2014-12-08T06:40:25",
"has_reviewer_comment": false,

4
src/olympia/addons/fixtures/addons/default-to-compat.json
Просмотреть файл

@ -166,7 +166,6 @@
"pk": 1268881,
"model": "versions.version",
"fields": {
"has_info_request": false,
"license": null,
"created": "2011-12-05 14:46:43",
"has_reviewer_comment": false,
@ -194,7 +193,6 @@
"pk": 1268882,
"model": "versions.version",
"fields": {
"has_info_request": false,
"license": null,
"created": "2011-12-05 14:46:44",
"has_reviewer_comment": false,
@ -222,7 +220,6 @@
"pk": 1268883,
"model": "versions.version",
"fields": {
"has_info_request": false,
"license": null,
"created": "2011-12-05 14:46:45",
"has_reviewer_comment": false,
@ -250,7 +247,6 @@
"pk": 1268884,
"model": "versions.version",
"fields": {
"has_info_request": false,
"license": null,
"created": "2011-12-05 14:46:46",
"has_reviewer_comment": false,

28
src/olympia/addons/models.py
Просмотреть файл

@ -1400,6 +1400,18 @@ class Addon(OnChangeMixin, ModelBase):
except AddonReviewerFlags.DoesNotExist:
return None
@property
def pending_info_request(self):
try:
return self.addonreviewerflags.pending_info_request
except AddonReviewerFlags.DoesNotExist:
return None
@property
def expired_info_request(self):
info_request = self.pending_info_request
return info_request and info_request < datetime.now()
dbsignals.pre_save.connect(save_signal, sender=Addon,
dispatch_uid='addon_translations')
@ -1476,20 +1488,6 @@ def watch_disabled(old_attr=None, new_attr=None, instance=None, sender=None,
f.hide_disabled_file()
@Addon.on_change
def watch_developer_notes(old_attr=None, new_attr=None, instance=None,
sender=None, **kwargs):
if old_attr is None:
old_attr = {}
if new_attr is None:
new_attr = {}
developer_comments_changed = (new_attr.get('_developer_comments_cache') and
old_attr.get('_developer_comments_cache') !=
new_attr.get('_developer_comments_cache'))
if developer_comments_changed:
instance.versions.update(has_info_request=False)
def attach_translations(addons):
"""Put all translations into a translations dict."""
attach_trans_dict(Addon, addons)
@ -1509,6 +1507,8 @@ class AddonReviewerFlags(ModelBase):
needs_admin_code_review = models.BooleanField(default=False)
needs_admin_content_review = models.BooleanField(default=False)
auto_approval_disabled = models.BooleanField(default=False)
pending_info_request = models.DateTimeField(default=None, null=True)
notified_about_expiring_info_request = models.BooleanField(default=False)
class Persona(caching.CachingMixin, models.Model):

93
src/olympia/addons/tests/test_models.py
Просмотреть файл

@ -1512,6 +1512,37 @@ class TestAddonModels(TestCase):
flags.update(needs_admin_content_review=True)
assert addon.needs_admin_content_review is True
def test_pending_info_request_property(self):
addon = Addon.objects.get(pk=3615)
# No flags: None
assert addon.pending_info_request is None
# Flag present, value is None (default): None.
flags = AddonReviewerFlags.objects.create(addon=addon)
assert flags.pending_info_request is None
assert addon.pending_info_request is None
# Flag present, value is a date.
in_the_past = self.days_ago(1)
flags.update(pending_info_request=in_the_past)
assert addon.pending_info_request == in_the_past
def test_expired_info_request_property(self):
addon = Addon.objects.get(pk=3615)
# No flags: None
assert addon.expired_info_request is None
# Flag present, value is None (default): None.
flags = AddonReviewerFlags.objects.create(addon=addon)
assert flags.pending_info_request is None
assert addon.expired_info_request is None
# Flag present, value is a date in the past.
in_the_past = self.days_ago(1)
flags.update(pending_info_request=in_the_past)
assert addon.expired_info_request
# Flag present, value is a date in the future.
in_the_future = datetime.now() + timedelta(days=2)
flags.update(pending_info_request=in_the_future)
assert not addon.expired_info_request
class TestShouldRedirectToSubmitFlow(TestCase):
fixtures = ['base/addon_3615']
@ -2594,68 +2625,6 @@ class TestAddonWatchDisabled(TestCase):
assert not mock.hide_disabled_file.called
class TestAddonWatchDeveloperNotes(TestCase):
def make_addon(self, **kwargs):
addon = Addon(type=amo.ADDON_EXTENSION, status=amo.STATUS_PUBLIC,
**kwargs)
addon.save()
addon.versions.create(has_info_request=True)
addon.versions.create(has_info_request=False)
addon.versions.create(has_info_request=True)
return addon
def assertHasInfoSet(self, addon):
assert any([v.has_info_request for v in addon.versions.all()])
def assertHasInfoNotSet(self, addon):
assert all([not v.has_info_request for v in addon.versions.all()])
def test_has_info_save(self):
"""Test saving without a change doesn't clear has_info_request."""
addon = self.make_addon()
self.assertHasInfoSet(addon)
addon.save()
self.assertHasInfoSet(addon)
def test_has_info_update_developer_comments(self):
"""Test saving with a change to developer_comments clears
has_info_request."""
addon = self.make_addon()
self.assertHasInfoSet(addon)
addon.developer_comments = 'Things are thing-like.'
addon.save()
self.assertHasInfoNotSet(addon)
def test_has_info_update_developer_comments_again(self):
"""Test saving a change to developer_comments when developer_comments
was already set clears has_info_request (developer_comments is a
PurifiedField so it is really just an id)."""
addon = self.make_addon(developer_comments='Wat things like.')
self.assertHasInfoSet(addon)
addon.developer_comments = 'Things are thing-like.'
addon.save()
self.assertHasInfoNotSet(addon)
def test_has_info_update_developer_comments_no_change(self):
"""Test saving without a change to developer_comments doesn't clear
has_info_request."""
addon = self.make_addon(developer_comments='Things are thing-like.')
self.assertHasInfoSet(addon)
addon.developer_comments = 'Things are thing-like.'
addon.save()
self.assertHasInfoSet(addon)
def test_has_info_remove_developer_comments(self):
"""Test saving with an empty developer_comments doesn't clear
has_info_request."""
addon = self.make_addon(developer_comments='Things are thing-like.')
self.assertHasInfoSet(addon)
addon.developer_comments = ''
addon.save()
self.assertHasInfoSet(addon)
class TestTrackAddonStatusChange(TestCase):
def create_addon(self, **kwargs):

9
src/olympia/constants/activity.py
Просмотреть файл

@ -586,6 +586,15 @@ class REJECT_CONTENT(_LOG):
reviewer_review_action = True
class CLEAR_INFO_REQUEST(_LOG):
id = 149
format = _(u'{addon} information request removed.')
short = _(u'Information request removed')
keep = True
reviewer_review_action = 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.

29
src/olympia/devhub/forms.py
Просмотреть файл

@ -16,7 +16,8 @@ from olympia.access import acl
from olympia.activity.models import ActivityLog
from olympia.addons.forms import AddonFormBase
from olympia.addons.models import (
Addon, AddonCategory, AddonDependency, AddonUser, Preview)
Addon, AddonCategory, AddonDependency, AddonReviewerFlags, AddonUser,
Preview)
from olympia.amo.fields import HttpHttpsOnlyURLField
from olympia.amo.forms import AMOModelForm
from olympia.amo.templatetags.jinja_helpers import mark_safe_lazy
@ -304,10 +305,34 @@ class VersionForm(WithSourceMixin, happyforms.ModelForm):
approvalnotes = forms.CharField(
widget=TranslationTextarea(attrs={'rows': 4}), required=False)
source = forms.FileField(required=False, widget=SourceFileInput)
clear_pending_info_request = forms.BooleanField(required=False)
class Meta:
model = Version
fields = ('releasenotes', 'approvalnotes', 'source')
fields = ('releasenotes', 'clear_pending_info_request',
'approvalnotes', 'source',)
def __init__(self, *args, **kwargs):
super(VersionForm, self).__init__(*args, **kwargs)
# Fetch latest reviewer comment if the addon has a pending info
# request, so that the template in which the form is used can display
# that comment.
if self.instance and self.instance.addon.pending_info_request:
try:
self.pending_info_request_comment = (
ActivityLog.objects.for_addons(self.instance.addon)
.filter(action=amo.LOG.REQUEST_INFORMATION.id)
.latest('pk')).details['comments']
except (ActivityLog.DoesNotExist, KeyError):
self.pending_info_request_comment = ''
def save(self, *args, **kwargs):
super(VersionForm, self).save(*args, **kwargs)
# Clear pending info request on the addon if requested.
if self.cleaned_data.get('clear_pending_info_request'):
AddonReviewerFlags.objects.update_or_create(
addon=self.instance.addon,
defaults={'pending_info_request': None})
class AppVersionChoiceField(forms.ModelChoiceField):

1
src/olympia/devhub/templates/devhub/addons/submit/describe_minimal.html
Просмотреть файл

@ -23,6 +23,7 @@
{{ reviewer_form.approvalnotes }}
<p>{{ _('These notes will only be visible to you and our reviewers.') }}</p>
</div>
{% include "devhub/includes/clear_pending_info_request.html" %}
<div class="submission-buttons addon-submission-field">
<button class="delete-button" type="sumbit"
formaction="{{ url('devhub.addons.cancel', addon.slug) }}">

8
src/olympia/devhub/templates/devhub/includes/clear_pending_info_request.html Normal file
Просмотреть файл

@ -0,0 +1,8 @@
{% if addon.pending_info_request %}
<div class="addon-submission-field clear-info-request">
<strong>{{ _('Pending information request:') }}</strong>
<blockquote><p> {{ reviewer_form.pending_info_request_comment }} </p></blockquote>
{{ reviewer_form.clear_pending_info_request }} <label for="{{ reviewer_form.clear_pending_info_request.auto_id }}">
{{ _('This request for information has been resolved') }}</label>
</div>
{% endif %}

3
src/olympia/devhub/templates/devhub/versions/edit.html
Просмотреть файл

@ -119,6 +119,9 @@
<td>
{{ field.errors }}
{{ field }}
{% with reviewer_form = version_form %}
{% include "devhub/includes/clear_pending_info_request.html" %}
{% endwith %}
</td>
{% endwith %}
</tr>

51
src/olympia/devhub/tests/test_views_submit.py
Просмотреть файл

@ -14,7 +14,8 @@ from waffle.testutils import override_switch
from olympia import amo
from olympia.activity.models import ActivityLog
from olympia.addons.models import Addon, AddonCategory, Category
from olympia.addons.models import (
Addon, AddonCategory, AddonReviewerFlags, Category)
from olympia.amo.tests import (
TestCase, addon_factory, formset, initial, version_factory)
from olympia.amo.tests.test_helpers import get_image_path
@ -1548,6 +1549,54 @@ class TestVersionSubmitDetails(TestSubmitBase):
response, reverse('devhub.submit.version.finish',
args=[self.addon.slug, self.version.pk]))
def test_show_request_for_information(self):
AddonReviewerFlags.objects.create(
addon=self.addon, pending_info_request=self.days_ago(2))
ActivityLog.create(
amo.LOG.REVIEWER_REPLY_VERSION, self.addon, self.version,
user=self.user, details={'comments': 'this should not be shown'})
ActivityLog.create(
amo.LOG.REQUEST_INFORMATION, self.addon, self.version,
user=self.user, details={'comments': 'this is an info request'})
response = self.client.get(self.url)
assert response.status_code == 200
assert 'this should not be shown' not in response.content
assert 'this is an info request' in response.content
def test_dont_show_request_for_information_if_none_pending(self):
ActivityLog.create(
amo.LOG.REVIEWER_REPLY_VERSION, self.addon, self.version,
user=self.user, details={'comments': 'this should not be shown'})
ActivityLog.create(
amo.LOG.REQUEST_INFORMATION, self.addon, self.version,
user=self.user, details={'comments': 'this is an info request'})
response = self.client.get(self.url)
assert response.status_code == 200
assert 'this should not be shown' not in response.content
assert 'this is an info request' not in response.content
def test_clear_request_for_information(self):
AddonReviewerFlags.objects.create(
addon=self.addon, pending_info_request=self.days_ago(2))
response = self.client.post(
self.url, {'clear_pending_info_request': True})
self.assert3xx(
response, reverse('devhub.submit.version.finish',
args=[self.addon.slug, self.version.pk]))
flags = AddonReviewerFlags.objects.get(addon=self.addon)
assert flags.pending_info_request is None
def test_dont_clear_request_for_information(self):
past_date = self.days_ago(2)
AddonReviewerFlags.objects.create(
addon=self.addon, pending_info_request=past_date)
response = self.client.post(self.url)
self.assert3xx(
response, reverse('devhub.submit.version.finish',
args=[self.addon.slug, self.version.pk]))
flags = AddonReviewerFlags.objects.get(addon=self.addon)
assert flags.pending_info_request == past_date
def test_can_cancel_review(self):
addon = self.get_addon()
addon_status = addon.status

69
src/olympia/devhub/tests/test_views_versions.py
Просмотреть файл

@ -735,34 +735,51 @@ class TestVersionEditDetails(TestVersionEditBase):
assert version.source
assert not version.addon.needs_admin_code_review
def test_update_source_file_should_drop_info_request_flag(self):
version = Version.objects.get(pk=self.version.pk)
version.has_info_request = True
version.save()
tdir = temp.gettempdir()
tmp_file = temp.NamedTemporaryFile
with tmp_file(suffix=".zip", dir=tdir) as source_file:
source_file.write('a' * (2 ** 21))
source_file.seek(0)
data = self.formset(source=source_file)
response = self.client.post(self.url, data)
version = Version.objects.get(pk=self.version.pk)
assert response.status_code == 302
assert not version.has_info_request
def test_show_request_for_information(self):
self.user = UserProfile.objects.latest('pk')
AddonReviewerFlags.objects.create(
addon=self.addon, pending_info_request=self.days_ago(2))
ActivityLog.create(
amo.LOG.REVIEWER_REPLY_VERSION, self.addon, self.version,
user=self.user, details={'comments': 'this should not be shown'})
ActivityLog.create(
amo.LOG.REQUEST_INFORMATION, self.addon, self.version,
user=self.user, details={'comments': 'this is an info request'})
response = self.client.get(self.url)
assert response.status_code == 200
assert 'this should not be shown' not in response.content
assert 'this is an info request' in response.content
def test_update_approvalnotes_should_drop_info_request_flag(self):
version = Version.objects.get(pk=self.version.pk)
version.has_info_request = True
version.save()
data = self.formset(approvalnotes=u'Néw nót€s.')
response = self.client.post(self.url, data)
version = Version.objects.get(pk=self.version.pk)
assert response.status_code == 302
assert not version.has_info_request
def test_dont_show_request_for_information_if_none_pending(self):
self.user = UserProfile.objects.latest('pk')
ActivityLog.create(
amo.LOG.REVIEWER_REPLY_VERSION, self.addon, self.version,
user=self.user, details={'comments': 'this should not be shown'})
ActivityLog.create(
amo.LOG.REQUEST_INFORMATION, self.addon, self.version,
user=self.user, details={'comments': 'this is an info request'})
response = self.client.get(self.url)
assert response.status_code == 200
assert 'this should not be shown' not in response.content
assert 'this is an info request' not in response.content
# Check that the corresponding automatic activity log has been created.
log = ActivityLog.objects.get(action=amo.LOG.APPROVAL_NOTES_CHANGED.id)
assert log
def test_clear_request_for_information(self):
AddonReviewerFlags.objects.create(
addon=self.addon, pending_info_request=self.days_ago(2))
response = self.client.post(
self.url, self.formset(clear_pending_info_request=True))
assert response.status_code == 302
flags = AddonReviewerFlags.objects.get(addon=self.addon)
assert flags.pending_info_request is None
def test_dont_clear_request_for_information(self):
past_date = self.days_ago(2)
AddonReviewerFlags.objects.create(
addon=self.addon, pending_info_request=past_date)
response = self.client.post(self.url, self.formset())
assert response.status_code == 302
flags = AddonReviewerFlags.objects.get(addon=self.addon)
assert flags.pending_info_request == past_date
class TestVersionEditSearchEngine(TestVersionEditMixin, TestCase):

11
src/olympia/devhub/views.py
Просмотреть файл

@ -1110,10 +1110,14 @@ def version_edit(request, addon_id, addon, version_id):
_log_max_version_change(addon, version, form.instance)
if 'version_form' in data:
# VersionForm.save() clear the pending info request if the
# developer specifically asked for it, but we've got additional
# things to do here that depend on it.
had_pending_info_request = bool(addon.pending_info_request)
data['version_form'].save()
if 'approvalnotes' in version_form.changed_data:
if version.has_info_request:
version.update(has_info_request=False)
if had_pending_info_request:
log_and_notify(amo.LOG.APPROVAL_NOTES_CHANGED, None,
request.user, version)
else:
@ -1124,8 +1128,7 @@ def version_edit(request, addon_id, addon, version_id):
version_form.cleaned_data['source']):
AddonReviewerFlags.objects.update_or_create(
addon=addon, defaults={'needs_admin_code_review': True})
if version.has_info_request:
version.update(has_info_request=False)
if had_pending_info_request:
log_and_notify(amo.LOG.SOURCE_CODE_UPLOADED, None,
request.user, version)
else:

5
src/olympia/migrations/998-add-pending-info-request-flag.sql Normal file
Просмотреть файл

@ -0,0 +1,5 @@
-- Note: that first line is not necessary on dev/stage/prod, which already have a default.
-- Uncomment it in your local environement if you have an old database that you don't want
-- to reset.
-- ALTER TABLE `versions` MODIFY COLUMN `has_info_request` tinyint(1) unsigned DEFAULT NULL
ALTER TABLE `addons_addonreviewerflags` ADD COLUMN `pending_info_request` datetime(6), ADD COLUMN `notified_about_expiring_info_request` bool NOT NULL DEFAULT false;

25
src/olympia/reviewers/forms.py
Просмотреть файл

@ -1,3 +1,4 @@
# -*- coding: utf-8 -*-
import datetime
from datetime import timedelta
@ -12,6 +13,7 @@ from django.utils.translation import get_language, ugettext, ugettext_lazy as _
import olympia.core.logger
from olympia import amo, ratings
from olympia.access import acl
from olympia.activity.models import ActivityLog
from olympia.addons.models import Persona
from olympia.amo.urlresolvers import reverse
@ -286,6 +288,10 @@ class NonValidatingChoiceField(forms.ChoiceField):
pass
class NumberInput(widgets.Input):
input_type = 'number'
class ReviewForm(happyforms.Form):
comments = forms.CharField(required=True, widget=forms.Textarea(),
label=_(u'Comments:'))
@ -305,7 +311,10 @@ class ReviewForm(happyforms.Form):
applications = forms.CharField(required=False,
label=_(u'Applications:'))
info_request = forms.BooleanField(
required=False, label=_(u'Is more info requested?'))
required=False, label=_(u'Require developer to respond in less than…'))
info_request_deadline = forms.IntegerField(
required=False, widget=NumberInput, initial=7, label=_(u'days'),
min_value=1, max_value=99)
def is_valid(self):
# Some actions do not require comments.
@ -325,6 +334,20 @@ class ReviewForm(happyforms.Form):
self.type = kw.pop('type', amo.CANNED_RESPONSE_ADDON)
super(ReviewForm, self).__init__(*args, **kw)
# Info request deadline needs to be readonly unless we're an admin.
user = self.helper.handler.user
deadline_widget_attributes = {}
info_request_deadline = self.fields['info_request_deadline']
if not acl.action_allowed_user(user, amo.permissions.REVIEWS_ADMIN):
info_request_deadline.min_value = info_request_deadline.initial
info_request_deadline.max_value = info_request_deadline.initial
deadline_widget_attributes['readonly'] = 'readonly'
deadline_widget_attributes.update({
'min': info_request_deadline.min_value,
'max': info_request_deadline.max_value,
})
info_request_deadline.widget.attrs.update(deadline_widget_attributes)
# With the helper, we now have the add-on and can set queryset on the
# versions field correctly. Small optimization: we only need to do this
# if the reject_multiple_versions action is available, otherwise we

44
src/olympia/reviewers/management/commands/send_info_request_last_warning_notifications.py Normal file
Просмотреть файл

@ -0,0 +1,44 @@
# -*- coding: utf-8 -*-
from datetime import datetime, timedelta
from django.core.management.base import BaseCommand
import olympia.core.logger
from olympia import amo
from olympia.activity.models import ActivityLog
from olympia.activity.utils import notify_about_activity_log
from olympia.addons.models import Addon
from olympia.users.notifications import reviewer_reviewed
log = olympia.core.logger.getLogger('z.reviewers.send_info_request')
class Command(BaseCommand):
help = 'Notify developers with pending info requests about to expire'
def handle(self, *args, **options):
# Fetch addons with request for information expiring in one day.
one_day_in_the_future = datetime.now() + timedelta(days=1)
qs = Addon.objects.filter(
addonreviewerflags__notified_about_expiring_info_request=False,
addonreviewerflags__pending_info_request__lt=one_day_in_the_future)
for addon in qs:
# The note we need to send the mail should always going to be the
# last information request, as making a new one extends the
# deadline.
note = ActivityLog.objects.for_addons(addon).filter(
action=amo.LOG.REQUEST_INFORMATION.id).latest('pk')
version = note.versionlog_set.latest('pk').version
log.info(
'Notifying developers of %s about expiring info request',
addon.pk)
# This re-sends the notification sent when the information was
# requested, but with the new delay in the body of the email now
# that the notification is about to expire.
notify_about_activity_log(
addon, version, note, perm_setting=reviewer_reviewed.short,
send_to_reviewers=False, send_to_staff=False)
addon.addonreviewerflags.update(
notified_about_expiring_info_request=True)

44
src/olympia/reviewers/models.py
Просмотреть файл

@ -19,8 +19,7 @@ from olympia.access import acl
from olympia.access.models import Group
from olympia.activity.models import ActivityLog
from olympia.addons.models import Addon, Persona
from olympia.amo.models import (
ManagerBase, ModelBase, OnChangeMixin, skip_cache)
from olympia.amo.models import ManagerBase, ModelBase, skip_cache
from olympia.amo.templatetags.jinja_helpers import absolutify
from olympia.amo.urlresolvers import reverse
from olympia.amo.utils import cache_ns_key, send_mail
@ -43,7 +42,8 @@ VIEW_QUEUE_FLAGS = (
_('Needs Admin Content Review')),
('is_jetpack', 'jetpack', _('Jetpack Add-on')),
('is_restart_required', 'is_restart_required', _('Requires Restart')),
('has_info_request', 'info', _('More Information Requested')),
('pending_info_request', 'info', _('More Information Requested')),
('expired_info_request', 'expired-info', _('Expired Information Request')),
('has_reviewer_comment', 'reviewer', _('Contains Reviewer Comment')),
('sources_provided', 'sources-provided', _('Sources provided')),
('is_webextension', 'webextension', _('WebExtension')),
@ -134,9 +134,16 @@ class EventLog(models.Model):
name__startswith='Reviewers: ').exists()]
def get_flags(record):
def get_flags(addon, version):
"""Return a list of tuples (indicating which flags should be displayed for
a particular add-on."""
return [(cls, title) for (prop, cls, title) in VIEW_QUEUE_FLAGS
if getattr(version, prop, getattr(addon, prop, None))]
def get_flags_for_row(record):
"""Like get_flags(), but for the queue pages, using fields directly
returned by the queues SQL query."""
return [(cls, title) for (prop, cls, title) in VIEW_QUEUE_FLAGS
if getattr(record, prop)]
@ -154,7 +161,8 @@ class ViewQueue(RawSQLModel):
source = models.CharField(max_length=100)
is_webextension = models.BooleanField()
latest_version = models.CharField(max_length=255)
has_info_request = models.BooleanField()
pending_info_request = models.DateTimeField()
expired_info_request = models.NullBooleanField()
has_reviewer_comment = models.BooleanField()
waiting_time_days = models.IntegerField()
waiting_time_hours = models.IntegerField()
@ -174,7 +182,11 @@ class ViewQueue(RawSQLModel):
'addons_addonreviewerflags.needs_admin_content_review'),
('latest_version', 'versions.version'),
('has_reviewer_comment', 'versions.has_editor_comment'),
('has_info_request', 'versions.has_info_request'),
('pending_info_request',
'addons_addonreviewerflags.pending_info_request'),
('expired_info_request', (
'TIMEDIFF(addons_addonreviewerflags.pending_info_request,'
'NOW()) < 0')),
('is_jetpack', 'MAX(files.jetpack_version IS NOT NULL)'),
('is_restart_required', 'MAX(files.is_restart_required)'),
('source', 'versions.source'),
@ -212,7 +224,7 @@ class ViewQueue(RawSQLModel):
@property
def flags(self):
return get_flags(self)
return get_flags_for_row(self)
class ViewFullReviewQueue(ViewQueue):
@ -1185,7 +1197,7 @@ class ThemeLock(ModelBase):
db_table = 'theme_locks'
class Whiteboard(OnChangeMixin, ModelBase):
class Whiteboard(ModelBase):
addon = models.OneToOneField(
Addon, on_delete=models.CASCADE, primary_key=True)
private = models.TextField(blank=True)
@ -1197,19 +1209,3 @@ class Whiteboard(OnChangeMixin, ModelBase):
def __unicode__(self):
return u'[%s] private: |%s| public: |%s|' % (
self.addon.name, self.private, self.public)
@Whiteboard.on_change
def watch_public_whiteboard_changes(old_attr=None, new_attr=None,
instance=None, sender=None, **kwargs):
if old_attr is None:
old_attr = {}
if new_attr is None:
new_attr = {}
whiteboard_changed = (
new_attr.get('public') and
old_attr.get('public') != new_attr.get('public'))
if whiteboard_changed:
instance.addon.versions.update(has_info_request=False)

45
src/olympia/reviewers/templates/reviewers/review.html
Просмотреть файл

@ -227,12 +227,18 @@
{{ form.applications.errors }}
</div>
<div class="review-actions-section data-toggle review-info-request"
data-value="{{ actions_info_request|join("|") }}|">
data-value="reply|">
{{ form.info_request }}
<label for="{{ form.info_request.auto_id }}">
{{ form.info_request.label }}
</label>
{{ form.info_request.errors }}
{{ form.info_request_deadline }}
<label for="{{ form.info_request_deadline.auto_id }}">
{{ form.info_request_deadline.label }}
</label>
{{ form.info_request_deadline.errors }}
</div>
<div class="review-actions-section review-actions-save">
<span class="currently_viewing_warning">
@ -288,32 +294,43 @@
{% if is_admin %}
<ul class="admin_only">
<li {% if addon.status == amo.STATUS_DISABLED %}class="hidden"{% endif %}>
<button id="force_disable_addon" data-api-url="{{ url('reviewers-addon-disable', addon.pk) }}"
data-toggle-button-selector="#force_enable_addon">{{ _('Force-disable add-on') }}</button>
<button data-api-url="{{ url('reviewers-addon-disable', addon.pk) }}"
data-toggle-button-selector="#force_enable_addon"
id="force_disable_addon" type="button">{{ _('Force-disable add-on') }}</button>
</li>
<li {% if addon.status != amo.STATUS_DISABLED %}class="hidden"{% endif %}>
<button id="force_enable_addon" data-api-url="{{ url('reviewers-addon-enable', addon.pk) }}"
data-toggle-button-selector="#force_disable_addon">{{ _('Force-enable add-on') }}</button>
<button data-api-url="{{ url('reviewers-addon-enable', addon.pk) }}"
data-toggle-button-selector="#force_disable_addon"
id="force_enable_addon" type="button">{{ _('Force-enable add-on') }}</button>
</li>
<li {% if addon.auto_approval_disabled %}class="hidden"{% endif %}>
<button id="disable_auto_approval" data-api-url="{{ url('reviewers-addon-disable-auto-approval', addon.pk) }}"
data-toggle-button-selector="#enable_auto_approval">{{ _('Disable Auto-Approval') }}</button>
<button data-api-url="{{ url('reviewers-addon-disable-auto-approval', addon.pk) }}"
data-toggle-button-selector="#enable_auto_approval"
id="disable_auto_approval" type="button">{{ _('Disable Auto-Approval') }}</button>
</li>
<li {% if not addon.auto_approval_disabled %}class="hidden"{% endif %}>
<button id="enable_auto_approval" data-api-url="{{ url('reviewers-addon-enable-auto-approval', addon.pk) }}"
data-toggle-button-selector="#disable_auto_approval">{{ _('Enable Auto-Approval') }}</button>
<button data-api-url="{{ url('reviewers-addon-enable-auto-approval', addon.pk) }}"
data-toggle-button-selector="#disable_auto_approval"
id="enable_auto_approval" type="button">{{ _('Enable Auto-Approval') }}</button>
</li>
{% if addon.pending_info_request %}
<li>
<button data-api-url="{{ url('reviewers-addon-clear-pending-info-request', addon.pk) }}"
id="clear_pending_info_request" type="button">{{ _('Clear information request') }}</button>
</li>
{% endif %}
{% if addon.needs_admin_code_review %}
<li>
<button id="clear_admin_code_review" data-api-url="{{ url('reviewers-addon-clear-admin-review-flag', addon.pk) }}"
data-api-flag="code"> {{ _('Clear Admin Code Review Flag') }}</button>
<button data-api-url="{{ url('reviewers-addon-clear-admin-review-flag', addon.pk) }}"
data-api-flag="code"
id="clear_admin_code_review" type="button">{{ _('Clear Admin Code Review Flag') }}</button>
</li>
{% endif %}
{% if addon.needs_admin_content_review %}
<li>
<button id="clear_admin_content_review"
data-api-url="{{ url('reviewers-addon-clear-admin-review-flag', addon.pk) }}"
data-api-flag="content">{{ _('Clear Admin Content Review Flag') }}</button>
<button data-api-url="{{ url('reviewers-addon-clear-admin-review-flag', addon.pk) }}"
data-api-flag="content"
id="clear_admin_content_review" type="button">{{ _('Clear Admin Content Review Flag') }}</button>
</li>
{% endif %}
</ul>

68
src/olympia/reviewers/tests/test_commands.py
Просмотреть файл

@ -1,4 +1,6 @@
# -*- coding: utf-8 -*-
from datetime import datetime, timedelta
from django.conf import settings
from django.core import mail
from django.core.management import call_command
@ -7,7 +9,9 @@ import mock
from olympia import amo
from olympia.activity.models import ActivityLog
from olympia.addons.models import AddonApprovalsCounter, AddonReviewerFlags
from olympia.activity.utils import ACTIVITY_MAIL_GROUP
from olympia.addons.models import (
AddonApprovalsCounter, AddonReviewerFlags, AddonUser)
from olympia.amo.tests import (
TestCase, addon_factory, file_factory, user_factory, version_factory)
from olympia.files.models import FileValidation
@ -411,3 +415,65 @@ class TestRecalculatePostReviewWeightsCommand(TestCase):
summary.reload()
assert summary.weight == 200 # Because of no validation results found.
assert summary.modified != old_modified_date
class TestSendInfoRequestLastWarningNotification(TestCase):
@mock.patch('olympia.reviewers.management.commands.'
'send_info_request_last_warning_notifications.'
'notify_about_activity_log')
def test_non_expired(self, notify_about_activity_log_mock):
addon_factory() # Normal add-on, no pending info request.
addon_not_expired = addon_factory()
flags = AddonReviewerFlags.objects.create(
addon=addon_not_expired,
pending_info_request=datetime.now() + timedelta(days=1, hours=3))
call_command('send_info_request_last_warning_notifications')
assert notify_about_activity_log_mock.call_count == 0
assert flags.notified_about_expiring_info_request is False
@mock.patch('olympia.reviewers.management.commands.'
'send_info_request_last_warning_notifications.'
'notify_about_activity_log')
def test_already_notified(self, notify_about_activity_log_mock):
addon_factory()
addon_already_notified = addon_factory()
flags = AddonReviewerFlags.objects.create(
addon=addon_already_notified,
pending_info_request=datetime.now() + timedelta(hours=23),
notified_about_expiring_info_request=True)
call_command('send_info_request_last_warning_notifications')
assert notify_about_activity_log_mock.call_count == 0
assert flags.notified_about_expiring_info_request is True
def test_normal(self):
addon = addon_factory()
author = user_factory(name=u'Authør')
AddonUser.objects.create(addon=addon, user=author)
# Add a pending info request expiring soon.
flags = AddonReviewerFlags.objects.create(
addon=addon,
pending_info_request=datetime.now() + timedelta(hours=23),
notified_about_expiring_info_request=False)
# Create reviewer and staff users, and create the request for info
# activity. Neither the reviewer nor the staff user should be cc'ed.
reviewer = user_factory(name=u'Revièwer')
self.grant_permission(reviewer, 'Addons:Review')
ActivityLog.create(
amo.LOG.REQUEST_INFORMATION, addon, addon.current_version,
user=reviewer, details={'comments': u'Fly you fôöls!'})
staff = user_factory(name=u'Staff Ûser')
self.grant_permission(staff, 'Some:Perm', name=ACTIVITY_MAIL_GROUP)
# Fire the command.
call_command('send_info_request_last_warning_notifications')
assert len(mail.outbox) == 1
msg = mail.outbox[0]
assert msg.to == [author.email]
assert msg.subject == u'Mozilla Add-ons: Action Required for %s %s' % (
addon.name, addon.current_version.version)
assert 'an issue when reviewing ' in msg.body
assert 'within one (1) day' in msg.body
flags.reload()
assert flags.notified_about_expiring_info_request is True

83
src/olympia/reviewers/tests/test_models.py
Просмотреть файл

@ -23,7 +23,7 @@ from olympia.reviewers.models import (
AutoApprovalNotEnoughFilesError, AutoApprovalNoValidationResultError,
AutoApprovalSummary, RereviewQueueTheme, ReviewerScore,
ReviewerSubscription, ViewFullReviewQueue, ViewPendingQueue,
ViewUnlistedAllList, Whiteboard, send_notifications, set_reviewing_cache)
ViewUnlistedAllList, send_notifications, set_reviewing_cache)
from olympia.users.models import UserProfile
from olympia.versions.models import Version, version_uploaded
@ -125,43 +125,44 @@ class TestPendingQueue(TestQueue):
AddonReviewerFlags.objects.create(
addon=self.new_addon(), needs_admin_code_review=True)
q = self.Queue.objects.get()
assert q.flags == [
queue = self.Queue.objects.get()
assert queue.flags == [
('needs-admin-code-review', 'Needs Admin Code Review')]
def test_flags_info_request(self):
self.new_addon().find_latest_version(self.channel).update(
has_info_request=True)
q = self.Queue.objects.get()
assert q.flags == [('info', 'More Information Requested')]
AddonReviewerFlags.objects.create(
addon=self.new_addon(),
pending_info_request=datetime.now() + timedelta(days=6))
queue = self.Queue.objects.get()
assert queue.flags == [('info', 'More Information Requested')]
def test_flags_reviewer_comment(self):
self.new_addon().find_latest_version(self.channel).update(
has_reviewer_comment=True)
q = self.Queue.objects.get()
assert q.flags == [('reviewer', 'Contains Reviewer Comment')]
queue = self.Queue.objects.get()
assert queue.flags == [('reviewer', 'Contains Reviewer Comment')]
def test_flags_jetpack(self):
self.new_addon().find_latest_version(self.channel).all_files[0].update(
jetpack_version='1.8')
q = self.Queue.objects.get()
assert q.flags == [('jetpack', 'Jetpack Add-on')]
queue = self.Queue.objects.get()
assert queue.flags == [('jetpack', 'Jetpack Add-on')]
def test_flags_is_restart_required(self):
self.new_addon().find_latest_version(self.channel).all_files[0].update(
is_restart_required=True)
q = self.Queue.objects.get()
assert q.flags == [('is_restart_required', 'Requires Restart')]
queue = self.Queue.objects.get()
assert queue.flags == [('is_restart_required', 'Requires Restart')]
def test_flags_sources_provided(self):
self.new_addon().find_latest_version(self.channel).update(
source='/some/source/file')
q = self.Queue.objects.get()
assert q.flags == [('sources-provided', 'Sources provided')]
queue = self.Queue.objects.get()
assert queue.flags == [('sources-provided', 'Sources provided')]
def test_flags_webextension(self):
self.new_addon().find_latest_version(self.channel).all_files[0].update(
@ -173,8 +174,8 @@ class TestPendingQueue(TestQueue):
def test_no_flags(self):
self.new_addon()
q = self.Queue.objects.get()
assert q.flags == []
queue = self.Queue.objects.get()
assert queue.flags == []
class TestFullReviewQueue(TestQueue):
@ -1567,51 +1568,3 @@ class TestAutoApprovalSummary(TestCase):
result = list(AutoApprovalSummary.verdict_info_prettifier({}))
assert result == []
class TestWhiteboardWatchChange(TestCase):
def make_addon(self, whiteboard='', **kwargs):
addon = Addon(type=amo.ADDON_EXTENSION, status=amo.STATUS_PUBLIC,
**kwargs)
addon.save()
addon.versions.create(has_info_request=True)
addon.versions.create(has_info_request=False)
addon.versions.create(has_info_request=True)
whiteboard = Whiteboard(pk=addon.pk, public=whiteboard)
whiteboard.save()
return addon
def assert_has_info_set(self, addon):
assert any([v.has_info_request for v in addon.versions.all()])
def assert_has_info_not_set(self, addon):
assert all([not v.has_info_request for v in addon.versions.all()])
def test_has_info_update_whiteboard(self):
"""Test saving with a change to whiteboard clears has_info_request."""
addon = self.make_addon()
self.assert_has_info_set(addon)
addon.whiteboard.public = 'Info about things.'
addon.whiteboard.save()
self.assert_has_info_not_set(addon)
def test_has_info_update_whiteboard_no_change(self):
"""Test saving without a change to whiteboard doesn't clear
has_info_request."""
addon = self.make_addon(whiteboard='Info about things.')
self.assert_has_info_set(addon)
addon.whiteboard.public = 'Info about things.'
addon.whiteboard.save()
self.assert_has_info_set(addon)
def test_has_info_whiteboard_removed(self):
"""Test saving with an empty whiteboard doesn't clear
has_info_request."""
addon = self.make_addon(whiteboard='Info about things.')
self.assert_has_info_set(addon)
addon.whiteboard.public = ''
addon.whiteboard.save()
self.assert_has_info_set(addon)

86
src/olympia/reviewers/tests/test_utils.py
Просмотреть файл

@ -13,7 +13,8 @@ from pyquery import PyQuery as pq
from olympia import amo
from olympia.activity.models import ActivityLog, ActivityLogToken
from olympia.addons.models import Addon, AddonApprovalsCounter
from olympia.addons.models import (
Addon, AddonApprovalsCounter, AddonReviewerFlags)
from olympia.amo.templatetags.jinja_helpers import absolutify
from olympia.amo.tests import TestCase, file_factory, version_factory
from olympia.amo.urlresolvers import reverse
@ -198,7 +199,7 @@ class TestReviewHelper(TestCase):
return {'comments': 'foo', 'addon_files': self.version.files.all(),
'action': 'public', 'operating_systems': 'osx',
'applications': 'Firefox',
'info_request': self.version.has_info_request}
'info_request': self.addon.pending_info_request}
def get_helper(self, content_review_only=False):
return ReviewHelper(
@ -259,20 +260,6 @@ class TestReviewHelper(TestCase):
self.helper.process()
assert len(mail.outbox) == 1
def test_comment_clear_has_info_request(self):
self.version.update(has_info_request=True)
self.helper.set_data({'action': 'comment', 'comments': 'foo',
'info_request': False})
self.helper.process()
assert not self.version.has_info_request
def test_comment_cant_set_has_info_request(self):
self.version.update(has_info_request=False)
self.helper.set_data({'action': 'comment', 'comments': 'foo',
'info_request': True})
self.helper.process()
assert not self.version.has_info_request
def test_action_details(self):
for status in Addon.STATUS_CHOICES:
self.addon.update(status=status)
@ -346,23 +333,6 @@ class TestReviewHelper(TestCase):
addon_status=amo.STATUS_PUBLIC,
file_status=amo.STATUS_PUBLIC).keys() == expected
def test_info_request_for_comments(self):
# Info request flag not set on the version
assert not self.version.has_info_request
actions = self.get_review_actions(
addon_status=amo.STATUS_PUBLIC,
file_status=amo.STATUS_PUBLIC)
assert actions['reply']['info_request']
assert not actions['comment']['info_request']
# Now set the info request flag
self.version.update(has_info_request=True)
actions = self.get_review_actions(
addon_status=amo.STATUS_PUBLIC,
file_status=amo.STATUS_PUBLIC)
assert actions['reply']['info_request']
assert actions['comment']['info_request']
def test_set_files(self):
self.file.update(datestatuschanged=yesterday)
self.helper.set_data({'addon_files': self.version.files.all()})
@ -437,11 +407,11 @@ class TestReviewHelper(TestCase):
self.helper.set_data(data)
def test_send_reviewer_reply(self):
assert not self.version.has_info_request
assert not self.addon.pending_info_request
self.setup_data(amo.STATUS_PUBLIC, ['addon_files'])
self.helper.handler.reviewer_reply()
assert not self.version.has_info_request
assert not self.addon.pending_info_request
assert len(mail.outbox) == 1
assert mail.outbox[0].subject == self.preamble
@ -453,7 +423,51 @@ class TestReviewHelper(TestCase):
self.helper.handler.data['info_request'] = True
self.helper.handler.reviewer_reply()
assert self.version.has_info_request
self.assertCloseToNow(
self.addon.pending_info_request,
now=datetime.now() + timedelta(days=7))
assert len(mail.outbox) == 1
assert (
mail.outbox[0].subject ==
'Mozilla Add-ons: Action Required for Delicious Bookmarks 2.1.072')
assert self.check_log_count(amo.LOG.REQUEST_INFORMATION.id) == 1
def test_request_more_information_custom_deadline(self):
self.setup_data(amo.STATUS_PUBLIC, ['addon_files'])
self.helper.handler.data['info_request'] = True
self.helper.handler.data['info_request_deadline'] = 42
self.helper.handler.reviewer_reply()
self.assertCloseToNow(
self.addon.pending_info_request,
now=datetime.now() + timedelta(days=42))
assert len(mail.outbox) == 1
assert (
mail.outbox[0].subject ==
'Mozilla Add-ons: Action Required for Delicious Bookmarks 2.1.072')
assert self.check_log_count(amo.LOG.REQUEST_INFORMATION.id) == 1
def test_request_more_information_reset_notified_flag(self):
self.setup_data(amo.STATUS_PUBLIC, ['addon_files'])
flags = AddonReviewerFlags.objects.create(
addon=self.addon,
pending_info_request=datetime.now() - timedelta(days=1),
notified_about_expiring_info_request=True)
self.helper.handler.data['info_request'] = True
self.helper.handler.reviewer_reply()
flags.reload()
self.assertCloseToNow(
flags.pending_info_request,
now=datetime.now() + timedelta(days=7))
assert not flags.notified_about_expiring_info_request
assert len(mail.outbox) == 1
assert (

177
src/olympia/reviewers/tests/test_views.py
Просмотреть файл

@ -534,7 +534,7 @@ class TestDashboard(TestCase):
response = self.client.get(self.url)
assert response.status_code == 200
doc = pq(response.content)
assert len(doc('.dashboard h3')) == 7 # All 7 sections are present.
assert len(doc('.dashboard h3')) == 8 # All 8 sections are present.
expected_links = [
reverse('reviewers.queue_nominated'),
reverse('reviewers.queue_pending'),
@ -560,6 +560,7 @@ class TestDashboard(TestCase):
reverse('reviewers.unlisted_queue_all'),
'https://wiki.mozilla.org/Add-ons/Reviewers/Guide',
reverse('reviewers.motd'),
reverse('reviewers.queue_expired_info_requests'),
]
links = [link.attrib['href'] for link in doc('.dashboard a')]
assert links == expected_links
@ -573,7 +574,7 @@ class TestDashboard(TestCase):
response = self.client.get(self.url)
assert response.status_code == 200
doc = pq(response.content)
assert len(doc('.dashboard h3')) == 7 # All 7 sections are present.
assert len(doc('.dashboard h3')) == 8 # All 7 sections are present.
expected_links = [
reverse('reviewers.queue_nominated'),
reverse('reviewers.queue_pending'),
@ -599,6 +600,7 @@ class TestDashboard(TestCase):
reverse('reviewers.unlisted_queue_all'),
'https://wiki.mozilla.org/Add-ons/Reviewers/Guide',
reverse('reviewers.motd'),
reverse('reviewers.queue_expired_info_requests'),
]
links = [link.attrib['href'] for link in doc('.dashboard a')]
assert links == expected_links
@ -1690,6 +1692,53 @@ class TestAutoApprovedQueue(QueueTest):
u'Results 1\u20131 of 4')
class TestExpiredInfoRequestsQueue(QueueTest):
def setUp(self):
super(TestExpiredInfoRequestsQueue, self).setUp()
self.url = reverse('reviewers.queue_expired_info_requests')
def generate_files(self):
# Extra add-on with no pending info request.
addon_factory(name=u'Extra Addôn 1')
# Extra add-on with a non-expired pending info request.
extra_addon = addon_factory(name=u'Extra Addôn 2')
AddonReviewerFlags.objects.create(
addon=extra_addon,
pending_info_request=datetime.now() + timedelta(days=1))
# Addon with expired info request 1.
addon1 = addon_factory(name=u'Addön 1')
AddonReviewerFlags.objects.create(
addon=addon1,
pending_info_request=self.days_ago(2))
# Addon with expired info request 2.
addon2 = addon_factory(name=u'Addön 2')
AddonReviewerFlags.objects.create(
addon=addon2,
pending_info_request=self.days_ago(42))
self.expected_addons = [addon2, addon1]
def test_results_no_permission(self):
# Addon reviewer doesn't have access.
response = self.client.get(self.url)
assert response.status_code == 403
# Regular user doesn't have access.
self.client.logout()
assert self.client.login(email='regular@mozilla.com')
response = self.client.get(self.url)
assert response.status_code == 403
def test_results(self):
self.grant_permission(self.user, 'Reviews:Admin')
self.generate_files()
self._test_results()
class TestContentReviewQueue(QueueTest):
def setUp(self):
@ -2792,13 +2841,18 @@ class TestReview(ReviewBase):
token = doc('#extra-review-actions').attr('data-api-token')
assert token == 'youdidntsaythemagicword'
def test_extra_actions_admin_disable_enable_not_for_reviewers(self):
def test_extra_actions_not_for_reviewers(self):
self.login_as_reviewer()
response = self.client.get(self.url)
assert response.status_code == 200
doc = pq(response.content)
assert not doc('#force_disable_addon')
assert not doc('#force_enable_addon')
assert not doc('#clear_admin_code_review')
assert not doc('#clear_admin_content_review')
assert not doc('#disable_auto_approval')
assert not doc('#enable_auto_approval')
assert not doc('#clear_pending_info_request')
def test_extra_actions_admin_disable_enable(self):
self.login_as_admin()
@ -2823,16 +2877,6 @@ class TestReview(ReviewBase):
assert doc('#clear_admin_code_review').length == 1
assert doc('#clear_admin_content_review').length == 0
def test_unflag_option_forflagged_as_reviewer(self):
self.login_as_reviewer()
AddonReviewerFlags.objects.create(
addon=self.addon, needs_admin_code_review=True)
response = self.client.get(self.url)
assert response.status_code == 200
doc = pq(response.content)
assert doc('#clear_admin_code_review').length == 0
assert doc('#clear_admin_content_review').length == 0
def test_unflag_content_option_forflagged_as_admin(self):
self.login_as_admin()
AddonReviewerFlags.objects.create(
@ -2845,14 +2889,6 @@ class TestReview(ReviewBase):
assert doc('#clear_admin_code_review').length == 0
assert doc('#clear_admin_content_review').length == 1
def test_disable_auto_approvals_as_reviewer(self):
self.login_as_reviewer()
response = self.client.get(self.url)
assert response.status_code == 200
doc = pq(response.content)
assert not doc('#disable_auto_approval')
assert not doc('#enable_auto_approval')
def test_disable_auto_approvals_as_admin(self):
self.login_as_admin()
response = self.client.get(self.url)
@ -2881,20 +2917,54 @@ class TestReview(ReviewBase):
elem = doc('#enable_auto_approval')[0]
assert 'hidden' not in elem.getparent().attrib.get('class', '')
def test_clear_pending_info_request_as_admin(self):
self.login_as_admin()
response = self.client.get(self.url)
assert response.status_code == 200
doc = pq(response.content)
assert not doc('#clear_pending_info_request')
AddonReviewerFlags.objects.create(
addon=self.addon, pending_info_request=self.days_ago(1))
response = self.client.get(self.url)
assert response.status_code == 200
doc = pq(response.content)
assert doc('#clear_pending_info_request')
def test_info_request_checkbox(self):
self.login_as_reviewer()
assert not self.version.has_info_request
assert not self.addon.pending_info_request
response = self.client.get(self.url)
assert response.status_code == 200
doc = pq(response.content)
assert 'checked' not in doc('#id_info_request')[0].attrib
elm = doc('#id_info_request_deadline')[0]
assert elm.attrib['readonly'] == 'readonly'
assert elm.attrib['min'] == '7'
assert elm.attrib['max'] == '7'
assert elm.attrib['value'] == '7'
self.version.update(has_info_request=True)
AddonReviewerFlags.objects.create(
addon=self.addon,
pending_info_request=datetime.now() + timedelta(days=7))
response = self.client.get(self.url)
assert response.status_code == 200
doc = pq(response.content)
assert doc('#id_info_request')[0].attrib['checked'] == 'checked'
def test_info_request_checkbox_admin(self):
self.login_as_admin()
assert not self.addon.pending_info_request
response = self.client.get(self.url)
assert response.status_code == 200
doc = pq(response.content)
assert 'checked' not in doc('#id_info_request')[0].attrib
elm = doc('#id_info_request_deadline')[0]
assert 'readonly' not in elm.attrib
assert elm.attrib['min'] == '1'
assert elm.attrib['max'] == '99'
assert elm.attrib['value'] == '7'
def test_no_public(self):
has_public = self.version.files.filter(
status=amo.STATUS_PUBLIC).exists()
@ -3678,15 +3748,6 @@ class TestReview(ReviewBase):
doc('.data-toggle.review-info-request')[0].attrib['data-value'] ==
'reply|')
# If we set info request checkbox should be available on comment too.
self.version.update(has_info_request=True)
response = self.client.get(self.url)
assert response.status_code == 200
doc = pq(response.content)
assert (
doc('.data-toggle.review-info-request')[0].attrib['data-value'] ==
'reply|comment|')
def test_data_value_attributes_unreviewed(self):
self.file.update(status=amo.STATUS_AWAITING_REVIEW)
self.grant_permission(self.reviewer, 'Addons:PostReview')
@ -4138,6 +4199,9 @@ class TestAddonReviewerViewSet(TestCase):
self.disable_auto_approval_url = reverse(
'reviewers-addon-disable-auto-approval',
kwargs={'pk': self.addon.pk})
self.clear_pending_info_request_url = reverse(
'reviewers-addon-clear-pending-info-request',
kwargs={'pk': self.addon.pk})
def test_subscribe_not_logged_in(self):
response = self.client.post(self.subscribe_url)
@ -4501,3 +4565,52 @@ class TestAddonReviewerViewSet(TestCase):
assert response.status_code == 202
reviewer_flags.reload()
assert not reviewer_flags.auto_approval_disabled
def test_clear_pending_info_request_not_logged_in(self):
response = self.client.post(self.clear_pending_info_request_url)
assert response.status_code == 401
def test_clear_pending_info_request_no_rights(self):
self.client.login_api(self.user)
response = self.client.post(self.clear_pending_info_request_url)
assert response.status_code == 403
# Being a reviewer is not enough.
self.grant_permission(self.user, 'Addons:Review')
response = self.client.post(self.clear_pending_info_request_url)
assert response.status_code == 403
def test_clear_pending_info_request_addon_does_not_exist(self):
self.grant_permission(self.user, 'Reviews:Admin')
self.client.login_api(self.user)
self.clear_pending_info_request_url = reverse(
'reviewers-addon-clear-pending-info-request',
kwargs={'pk': self.addon.pk + 42})
response = self.client.post(self.clear_pending_info_request_url)
assert response.status_code == 404
def test_clear_pending_info_request_no_flags_yet(self):
assert not AddonReviewerFlags.objects.filter(addon=self.addon).exists()
self.grant_permission(self.user, 'Reviews:Admin')
self.client.login_api(self.user)
response = self.client.post(self.clear_pending_info_request_url)
assert response.status_code == 202
assert not AddonReviewerFlags.objects.filter(addon=self.addon).exists()
assert ActivityLog.objects.count() == 0
def test_clear_pending_info_request(self):
reviewer_flags = AddonReviewerFlags.objects.create(
addon=self.addon,
needs_admin_code_review=True,
pending_info_request=self.days_ago(1))
self.grant_permission(self.user, 'Reviews:Admin')
self.client.login_api(self.user)
response = self.client.post(self.clear_pending_info_request_url)
assert response.status_code == 202
reviewer_flags.reload()
assert reviewer_flags.pending_info_request is None
assert reviewer_flags.needs_admin_code_review # Not touched.
assert ActivityLog.objects.count() == 1
activity_log = ActivityLog.objects.latest('pk')
assert activity_log.action == amo.LOG.CLEAR_INFO_REQUEST.id
assert activity_log.arguments[0] == self.addon

2
src/olympia/reviewers/urls.py
Просмотреть файл

@ -23,6 +23,8 @@ urlpatterns = (
name='reviewers.queue_auto_approved'),
url(r'^queue/content_review', views.queue_content_review,
name='reviewers.queue_content_review'),
url(r'^queue/expired_info_requests', views.queue_expired_info_requests,
name='reviewers.queue_expired_info_requests'),
url(r'^unlisted_queue$', views.unlisted_queue,
name='reviewers.unlisted_queue'),
url(r'^unlisted_queue/all$', views.unlisted_list,

85
src/olympia/reviewers/utils.py
Просмотреть файл

@ -1,7 +1,7 @@
import datetime
import random
from collections import OrderedDict
from datetime import datetime, timedelta
from django.conf import settings
from django.contrib.humanize.templatetags.humanize import naturaltime
@ -28,7 +28,7 @@ from olympia.amo.utils import to_language
from olympia.lib.crypto.packaged import sign_file
from olympia.reviewers.models import (
ReviewerScore, ViewFullReviewQueue, ViewPendingQueue, ViewUnlistedAllList,
get_flags)
get_flags, get_flags_for_row)
from olympia.tags.models import Tag
from olympia.users.models import UserProfile
@ -83,7 +83,7 @@ class ReviewerQueueTable(tables.Table, ItemStateTable):
def render_flags(self, record):
if not hasattr(record, 'flags'):
record.flags = get_flags(record)
record.flags = get_flags_for_row(record)
return ''.join(u'<div class="app-icon ed-sprite-%s" '
u'title="%s"></div>' % flag
for flag in record.flags)
@ -187,7 +187,7 @@ class ViewFullReviewQueueTable(ReviewerQueueTable):
model = ViewFullReviewQueue
class AutoApprovedTable(ReviewerQueueTable):
class ModernAddonQueueTable(ReviewerQueueTable):
addon_name = tables.Column(verbose_name=_(u'Add-on'), accessor='name')
# Override empty_values for flags so that they can be displayed even if the
# model does not have a flags attribute.
@ -207,8 +207,9 @@ class AutoApprovedTable(ReviewerQueueTable):
orderable = False
def render_flags(self, record):
return super(AutoApprovedTable, self).render_flags(
record.current_version)
if not hasattr(record, 'flags'):
record.flags = get_flags(record, record.current_version)
return super(ModernAddonQueueTable, self).render_flags(record)
def _get_addon_name_url(self, record):
return reverse('reviewers.review', args=[record.slug])
@ -237,6 +238,23 @@ class AutoApprovedTable(ReviewerQueueTable):
render_last_content_review = render_last_human_review
class ExpiredInfoRequestsTable(ModernAddonQueueTable):
deadline = tables.Column(
verbose_name=_(u'Information Request Deadline'),
accessor='addonreviewerflags.pending_info_request')
class Meta(ModernAddonQueueTable.Meta):
fields = ('addon_name', 'flags', 'last_human_review', 'weight',
'deadline')
def render_deadline(self, value):
return naturaltime(value) if value else ''
class AutoApprovedTable(ModernAddonQueueTable):
pass
class ContentReviewTable(AutoApprovedTable):
last_content_review = tables.DateTimeColumn(
verbose_name=_(u'Last Content Review'),
@ -318,8 +336,8 @@ class ReviewHelper(object):
not is_limited_reviewer(request) or
(self.version and
self.version.nomination is not None and
(datetime.datetime.now() - self.version.nomination >=
datetime.timedelta(hours=amo.REVIEW_LIMITED_DELAY_HOURS))))
(datetime.now() - self.version.nomination >=
timedelta(hours=amo.REVIEW_LIMITED_DELAY_HOURS))))
reviewable_because_pending = (
self.version and
len(self.version.is_unreviewed) > 0)
@ -409,7 +427,6 @@ class ReviewHelper(object):
'details': _('This will send a message to the developer. '
'You will be notified when they reply.'),
'minimal': True,
'info_request': True,
'available': self.version is not None,
}
actions['super'] = {
@ -428,7 +445,6 @@ class ReviewHelper(object):
'details': _('Make a comment on this version. The developer '
'won\'t be able to see this.'),
'minimal': True,
'info_request': self.version and self.version.has_info_request,
'available': True,
}
@ -473,7 +489,7 @@ class ReviewBase(object):
def set_addon(self, **kw):
"""Alters addon and sets reviewed timestamp on version."""
self.addon.update(**kw)
self.version.update(reviewed=datetime.datetime.now())
self.version.update(reviewed=datetime.now())
def set_data(self, data):
self.data = data
@ -483,8 +499,8 @@ class ReviewBase(object):
def set_files(self, status, files, hide_disabled_file=False):
"""Change the files to be the new status."""
for file in files:
file.datestatuschanged = datetime.datetime.now()
file.reviewed = datetime.datetime.now()
file.datestatuschanged = datetime.now()
file.reviewed = datetime.now()
if hide_disabled_file:
file.hide_disabled_file()
file.status = status
@ -505,7 +521,7 @@ class ReviewBase(object):
else:
args = (self.addon,)
kwargs = {'user': self.user, 'created': datetime.datetime.now(),
kwargs = {'user': self.user, 'created': datetime.now(),
'details': details}
self.log_entry = ActivityLog.create(action, *args, **kwargs)
@ -570,20 +586,35 @@ class ReviewBase(object):
# Default to reviewer reply action.
action = amo.LOG.REVIEWER_REPLY_VERSION
if self.version:
kw = {}
info_request = self.data.get('info_request')
if info_request is not None:
# Update info request flag.
kw['has_info_request'] = info_request
if info_request:
# And change action to request info if set
action = amo.LOG.REQUEST_INFORMATION
if (self.version.channel == amo.RELEASE_CHANNEL_UNLISTED and
not self.version.reviewed):
kw['reviewed'] = datetime.datetime.now()
self.version.update(**kw)
self.version.update(reviewed=datetime.now())
if self.data.get('info_request'):
# It's an information request and not just a simple reply.
# The ActivityLog will be different...
action = amo.LOG.REQUEST_INFORMATION
# And the deadline for the info request will be created or
# updated x days in the future.
info_request_deadline_days = int(
self.data.get('info_request_deadline', 7))
info_request_deadline = (
datetime.now() + timedelta(days=info_request_deadline_days)
)
# Update or create the reviewer flags, overwriting
# self.addon.addonreviewerflags with the one we
# create/update so that we don't use an older version of it
# later when notifying. Also, since this is a new request,
# clear out the notified_about_expiring_info_request field.
self.addon.addonreviewerflags = (
AddonReviewerFlags.objects.update_or_create(
addon=self.addon, defaults={
'pending_info_request': info_request_deadline,
'notified_about_expiring_info_request': False,
}
)[0]
)
log.info(u'Sending request for information for %s to authors and other'
log.info(u'Sending reviewer reply for %s to authors and other'
u'recipients' % self.addon)
log_and_notify(action, self.data['comments'],
self.user, self.version,
@ -593,11 +624,9 @@ class ReviewBase(object):
def process_comment(self):
if self.version:
kw = {'has_reviewer_comment': True}
if not self.data.get('info_request'):
kw['has_info_request'] = False
if (self.version.channel == amo.RELEASE_CHANNEL_UNLISTED and
not self.version.reviewed):
kw['reviewed'] = datetime.datetime.now()
kw['reviewed'] = datetime.now()
self.version.update(**kw)
self.log_action(amo.LOG.COMMENT_VERSION)

48
src/olympia/reviewers/views.py
Просмотреть файл

@ -45,9 +45,9 @@ from olympia.reviewers.models import (
clear_reviewing_cache, get_flags, get_reviewing_cache,
get_reviewing_cache_key, set_reviewing_cache)
from olympia.reviewers.utils import (
AutoApprovedTable, ContentReviewTable, ReviewHelper,
ViewFullReviewQueueTable, ViewPendingQueueTable, ViewUnlistedAllListTable,
is_limited_reviewer)
AutoApprovedTable, ContentReviewTable, ExpiredInfoRequestsTable,
ReviewHelper, ViewFullReviewQueueTable, ViewPendingQueueTable,
ViewUnlistedAllListTable, is_limited_reviewer)
from olympia.users.models import UserProfile
from olympia.versions.models import Version
from olympia.zadmin.models import get_config, set_config
@ -270,6 +270,12 @@ def dashboard(request):
ugettext('Update message of the day'),
reverse('reviewers.motd')
)]
if view_all or acl.action_allowed(
request, amo.permissions.REVIEWS_ADMIN):
sections[ugettext('Admin Tools')] = [(
ugettext('Add-ons with expired information requests'),
reverse('reviewers.queue_expired_info_requests')
)]
return render(request, 'reviewers/dashboard.html', base_context(**{
# base_context includes motd.
'sections': sections
@ -647,6 +653,16 @@ def queue_auto_approved(request):
qs=qs, SearchForm=None)
@permission_required(amo.permissions.REVIEWS_ADMIN)
def queue_expired_info_requests(request):
qs = (
Addon.objects.filter(
addonreviewerflags__pending_info_request__lt=datetime.now()
).order_by('addonreviewerflags__pending_info_request'))
return _queue(request, ExpiredInfoRequestsTable, 'expired_info_requests',
qs=qs, SearchForm=None)
def _get_comments_for_hard_deleted_versions(addon):
"""Versions are soft-deleted now but we need to grab review history for
older deleted versions that were hard-deleted so the only record we have
@ -779,7 +795,7 @@ def review(request, addon, channel=None):
return redirect(reverse('reviewers.queue'))
# Get the current info request state to set as the default.
form_initial = {'info_request': version and version.has_info_request}
form_initial = {'info_request': addon.pending_info_request}
form_helper = ReviewHelper(
request=request, addon=addon, version=version,
@ -862,10 +878,6 @@ def review(request, addon, channel=None):
# the comments form).
actions_comments = [k for (k, a) in actions if a.get('comments', True)]
# The actions we should show the 'info request' checkbox for.
actions_info_request = [k for (k, a) in actions
if a.get('info_request', False)]
versions = (Version.unfiltered.filter(addon=addon, channel=channel)
.select_related('autoapprovalsummary')
.exclude(files__status=amo.STATUS_BETA)
@ -901,10 +913,7 @@ def review(request, addon, channel=None):
verdict_info = summary.calculate_verdict(pretty=True)
auto_approval_info[a_version.pk] = verdict_info
if version:
flags = get_flags(version)
else:
flags = []
flags = get_flags(addon, version) if version else []
try:
whiteboard = Whiteboard.objects.get(pk=addon.pk)
@ -923,7 +932,6 @@ def review(request, addon, channel=None):
addon=addon).order_by('id')
ctx = context(
request, actions=actions, actions_comments=actions_comments,
actions_info_request=actions_info_request,
actions_minimal=actions_minimal, addon=addon,
api_token=request.COOKIES.get(API_TOKEN_COOKIE, None),
approvals_info=approvals_info, auto_approval_info=auto_approval_info,
@ -1194,3 +1202,17 @@ class AddonReviewerViewSet(GenericViewSet):
# If it does not exist, there is nothing to unflag.
pass
return Response(status=status.HTTP_202_ACCEPTED)
@detail_route(
methods=['post'],
permission_classes=[GroupPermission(amo.permissions.REVIEWS_ADMIN)])
def clear_pending_info_request(self, request, **kwargs):
addon = get_object_or_404(Addon, pk=kwargs['pk'])
try:
reviewerflags = addon.addonreviewerflags
reviewerflags.update(pending_info_request=None)
ActivityLog.create(amo.LOG.CLEAR_INFO_REQUEST, addon)
except AddonReviewerFlags.DoesNotExist:
# If it does not exist, there is nothing to clear.
pass
return Response(status=status.HTTP_202_ACCEPTED)

9
src/olympia/versions/models.py
Просмотреть файл

@ -96,7 +96,6 @@ class Version(OnChangeMixin, ModelBase):
nomination = models.DateTimeField(null=True)
reviewed = models.DateTimeField(null=True)
has_info_request = models.BooleanField(default=False)
has_reviewer_comment = models.BooleanField(
db_column='has_editor_comment', default=False)
@ -518,14 +517,6 @@ class Version(OnChangeMixin, ModelBase):
def sources_provided(self):
return bool(self.source)
@property
def needs_admin_code_review(self):
return self.addon.needs_admin_code_review
@property
def needs_admin_content_review(self):
return self.addon.needs_admin_content_review
@classmethod
def _compat_map(cls, avs):
apps = {}

17
static/css/zamboni/developers.css
Просмотреть файл

@ -399,6 +399,23 @@ form .char-count b {
margin-bottom: 1em;
}
.clear-info-request label {
font-weight: normal;
}
.clear-info-request blockquote {
font-style: italic;
}
.clear-info-request blockquote p {
margin-bottom: 0;
}
.clear-info-request input[type="checkbox"] {
margin-left: 0;
}
form.new-addon-file label,
#edit-addon-basic .addon-app-cats label {
display: block;

7
static/css/zamboni/reviewers.styl
Просмотреть файл

@ -13,7 +13,7 @@
.app-icon, .platform-icon {
float: left;
margin-right: 4px;
background: url(../../img/developers/reviewer-sprite.png?9) no-repeat top left;
background: url(../../img/developers/reviewer-sprite.png?10) no-repeat top left;
width: 16px; height: 16px;
position: relative;
top: 4px;
@ -33,6 +33,7 @@
.ed-sprite-info { background-position: 0 -160px; }
.ed-sprite-sources-provided { background-position: 0 -192px; }
.ed-sprite-webextension { background-position: 0 -208px; }
.ed-sprite-expired-info { background-position: 0 -272px; }
.platform-icon {
background: url(../../img/developers/platforms.png?8) no-repeat top left;
@ -916,6 +917,10 @@ form.review-form .data-toggle {
padding-left: 20px;
}
#id_info_request_deadline {
width: 2.5em;
}
.history-comment {
white-space: pre-wrap;
}

Двоичные данные
static/img/developers/reviewer-sprite.png
Просмотреть файл

Двоичный файл не отображается.
Режим "рядом" Свайп Оверлей

До

Ширина:  |  Высота:  |  Размер: 7.0 KiB

После

Ширина:  |  Высота:  |  Размер: 7.5 KiB

21
static/js/zamboni/reviewers.js
Просмотреть файл

@ -199,8 +199,7 @@ function initExtraReviewActions() {
*/
// Checkbox-style actions.
$('#notify_new_listed_versions').click(_pd(function() {
var $input = $(this);
$input.prop('disabled', true); // Prevent double-send.
var $input = $(this).prop('disabled', true); // Prevent double-send.
var checked = !$input.prop('checked'); // It's already changed.
var apiUrl;
if (checked) {
@ -225,17 +224,20 @@ function initExtraReviewActions() {
}));
// One-off-style buttons
$('#clear_admin_code_review, #clear_admin_content_review').click(_pd(function() {
var $button = $(this);
$button.prop('disabled', true); // Prevent double-send.
$('#clear_admin_code_review, #clear_admin_content_review, #clear_pending_info_request').click(_pd(function() {
var $button = $(this).prop('disabled', true); // Prevent double-send.
var apiUrl = $button.data('api-url');
var token = $button.parents('form.more-actions').data('api-token');
var flagType = $button.data('api-flag');
var data = null;
if (flagType) {
data = JSON.stringify({
flag_type: flagType,
});
}
$.ajax({
url: apiUrl,
data: JSON.stringify({
flag_type: flagType,
}),
data: data,
type: 'post',
beforeSend: function (xhr) {
xhr.setRequestHeader ("Authorization", 'Bearer ' + token);
@ -250,9 +252,8 @@ function initExtraReviewActions() {
// Toggle-style buttons.
$('#force_disable_addon, #force_enable_addon, #disable_auto_approval, #enable_auto_approval').click(_pd(function() {
var $button = $(this);
var $button = $(this).prop('disabled', true); // Prevent double-send.
var $other_button = $($button.data('toggle-button-selector'));
$button.prop('disabled', true); // Prevent double-send.
var apiUrl = $button.data('api-url');
var token = $button.parents('form.more-actions').data('api-token');
$.ajax({