create thread/notes from reviewer tools (Bug 879532)
This commit is contained in:
Ashish Dubey
Родитель
dbbad94bac
Коммит
201c627a39
|
|
@ -861,7 +861,7 @@ button.search, .log-filter-outside button {
|
|||
}
|
||||
}
|
||||
}
|
||||
.review-actions-features-override ul {
|
||||
.review-actions-options ul {
|
||||
columns(2, 1em);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -79,6 +79,7 @@ function initReviewActions() {
|
|||
$data_toggle.filter('[data-value*="' + value + '"]').show();
|
||||
|
||||
toggle_input();
|
||||
togglePermissions(value);
|
||||
|
||||
/* Fade out canned responses */
|
||||
var label = $element.text().trim();
|
||||
|
|
@ -107,6 +108,22 @@ function initReviewActions() {
|
|||
$files_input.after($('<input>', {'type': 'checkbox', 'checked': true, 'disabled': true}));
|
||||
}
|
||||
|
||||
function togglePermissions(action) {
|
||||
// Check/Uncheck/Disable default permissions associated with the action.
|
||||
var $reviewerActions = $('.review-actions-visibility');
|
||||
var $permissions = $('input[name="action_visibility"]');
|
||||
$permissions.prop('checked', true).prop('disabled', false);
|
||||
|
||||
var disable_list = $reviewerActions.data('default-visibility')[action];
|
||||
if (disable_list) {
|
||||
_.each(disable_list.disabled, function(v) {
|
||||
$permissions.filter('[value="' + v + '"]')
|
||||
.prop('disabled', true)
|
||||
.prop('checked', false);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
function toggle_input(){
|
||||
var $files_input = $('#review-actions .review-actions-files').find('input:enabled'),
|
||||
$files_checked = $files_input.filter(':checked'),
|
||||
|
|
|
|||
|
|
@ -0,0 +1,2 @@
|
|||
INSERT INTO waffle_switch_mkt (name, active, created, modified, note)
|
||||
VALUES ('comm-dashboard', 0, NOW(), NOW(), 'Communication dashboard for reviewer tools.');
|
||||
|
|
@ -29,6 +29,17 @@ class ReviewAppAttachmentForm(happyforms.Form):
|
|||
AttachmentFormSet = forms.formsets.formset_factory(ReviewAppAttachmentForm,
|
||||
extra=1)
|
||||
|
||||
# This contains default values for action visibility.
|
||||
# `disabled` will be disabled (not allowed to check).
|
||||
DEFAULT_ACTION_VISIBILITY = {
|
||||
'escalate': {
|
||||
'disabled': ['developer']
|
||||
},
|
||||
'comment': {
|
||||
'disabled': ['developer']
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
class ReviewAppForm(happyforms.Form):
|
||||
|
||||
|
|
@ -44,6 +55,17 @@ class ReviewAppForm(happyforms.Form):
|
|||
choices=[(k, v.name) for k, v in amo.DEVICE_TYPES.items()],
|
||||
coerce=int, label=_lazy(u'Device Type Override:'),
|
||||
widget=forms.CheckboxSelectMultiple, required=False)
|
||||
|
||||
thread_perms = [('developer', _lazy('Developers')),
|
||||
('reviewer', _lazy('Reviewers')),
|
||||
('senior_reviewer', _lazy('Senior Reviewers')),
|
||||
('staff', _lazy('Staff')),
|
||||
('mozilla_contact', _lazy('Mozilla Contact'))]
|
||||
action_visibility = forms.TypedMultipleChoiceField(
|
||||
choices=thread_perms,
|
||||
coerce=unicode, label=_lazy('Action Visibility to Users:'),
|
||||
widget=forms.CheckboxSelectMultiple, required=False)
|
||||
|
||||
notify = forms.BooleanField(
|
||||
required=False, label=_lazy(u'Notify me the next time the manifest is '
|
||||
u'updated. (Subsequent updates will not '
|
||||
|
|
|
|||
|
|
@ -254,7 +254,7 @@
|
|||
{{ form.device_override.errors}}
|
||||
</div>
|
||||
{% if waffle.switch('buchets') %}
|
||||
<div class="review-actions-section review-actions-features-override data-toggle" data-value="public">
|
||||
<div class="review-actions-section review-actions-options review-actions-features-override data-toggle" data-value="public">
|
||||
<strong>{{ _('Minimum Requirements Override:') }}</strong>
|
||||
<ul>
|
||||
{% for field in appfeatures_form.required_api_fields() %}
|
||||
|
|
@ -263,6 +263,13 @@
|
|||
</ul>
|
||||
</div>
|
||||
{% endif %}
|
||||
{% if waffle.switch('comm-dashboard') %}
|
||||
<div data-default-visibility="{{ default_visibility|json }}" class="review-actions-section review-actions-options review-actions-visibility">
|
||||
<strong>{{ _('Make this action visible to') }}</strong>
|
||||
{{ form.action_visibility }}
|
||||
{{ form.action_visibility.errors }}
|
||||
</div>
|
||||
{% endif %}
|
||||
<div class="review-actions-section">
|
||||
{{ form.notify }}
|
||||
<label for="id_notify">
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ from django.utils import translation
|
|||
|
||||
import mock
|
||||
import requests
|
||||
import waffle
|
||||
from nose import SkipTest
|
||||
from nose.tools import eq_, ok_
|
||||
from pyquery import PyQuery as pq
|
||||
|
|
@ -29,6 +30,7 @@ from amo.tests import (app_factory, check_links, days_ago,
|
|||
formset, initial, version_factory)
|
||||
from amo.urlresolvers import reverse
|
||||
from amo.utils import isotime
|
||||
from comm.models import CommunicationThread, CommunicationThreadCC
|
||||
from devhub.models import ActivityLog, ActivityLogAttachment, AppLog
|
||||
from devhub.tests.test_models import ATTACHMENTS_DIR
|
||||
from editors.models import (CannedResponse, EscalationQueue, RereviewQueue,
|
||||
|
|
@ -946,6 +948,10 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
|
|||
return Webapp.objects.get(id=337141)
|
||||
|
||||
def post(self, data, queue='pending'):
|
||||
# Set some action visibility form values.
|
||||
data['action_visibility'] = ('senior_reviewer', 'reviewer', 'staff',
|
||||
'mozilla_contact')
|
||||
self.create_switch(name='comm-dashboard')
|
||||
res = self.client.post(self.url, data)
|
||||
self.assert3xx(res, reverse('reviewers.apps.queue_%s' % queue))
|
||||
|
||||
|
|
@ -1000,6 +1006,17 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
|
|||
eq_(msg.from_email, settings.NOBODY_EMAIL)
|
||||
eq_(msg.extra_headers['Reply-To'], settings.MKT_REVIEWERS_EMAIL)
|
||||
|
||||
def _check_thread(self):
|
||||
thread = self.app.threads
|
||||
eq_(thread.count(), 1)
|
||||
|
||||
thread = thread.get()
|
||||
perms = ('senior_reviewer', 'reviewer', 'staff', 'mozilla_contact')
|
||||
|
||||
for key in perms:
|
||||
perm = 'read_permission_%s' % key
|
||||
assert getattr(thread, 'read_permission_%s' % key)
|
||||
|
||||
def _check_admin_email(self, msg, subject):
|
||||
eq_(msg.to, [settings.MKT_SENIOR_EDITORS_EMAIL])
|
||||
eq_(msg.subject, '%s: %s' % (subject, self.app.name))
|
||||
|
|
@ -1052,6 +1069,7 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
|
|||
msg = mail.outbox[0]
|
||||
self._check_email(msg, 'App Approved but waiting')
|
||||
self._check_email_body(msg)
|
||||
self._check_thread()
|
||||
|
||||
def test_pending_to_reject_w_device_overrides(self):
|
||||
# This shouldn't be possible unless there's form hacking.
|
||||
|
|
@ -1075,6 +1093,7 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
|
|||
msg = mail.outbox[0]
|
||||
self._check_email(msg, 'Submission Update')
|
||||
self._check_email_body(msg)
|
||||
self._check_thread()
|
||||
|
||||
def test_pending_to_public_w_requirements_overrides(self):
|
||||
self.create_switch(name='buchets')
|
||||
|
|
@ -1139,6 +1158,7 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
|
|||
msg = mail.outbox[0]
|
||||
self._check_email(msg, 'App Approved')
|
||||
self._check_email_body(msg)
|
||||
self._check_thread()
|
||||
self._check_score(amo.REVIEWED_WEBAPP_HOSTED)
|
||||
|
||||
assert update_name.called
|
||||
|
|
@ -1210,6 +1230,7 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
|
|||
msg = mail.outbox[0]
|
||||
self._check_email(msg, 'App Approved', with_mozilla_contact=False)
|
||||
self._check_email_body(msg)
|
||||
self._check_thread()
|
||||
self._check_score(amo.REVIEWED_WEBAPP_HOSTED)
|
||||
|
||||
@mock.patch('addons.tasks.index_addons')
|
||||
|
|
@ -1234,6 +1255,7 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
|
|||
msg = mail.outbox[0]
|
||||
self._check_email(msg, 'App Approved but waiting')
|
||||
self._check_email_body(msg)
|
||||
self._check_thread()
|
||||
self._check_score(amo.REVIEWED_WEBAPP_HOSTED)
|
||||
|
||||
assert not update_name.called
|
||||
|
|
@ -1267,6 +1289,7 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
|
|||
msg = mail.outbox[0]
|
||||
self._check_email(msg, 'Submission Update')
|
||||
self._check_email_body(msg)
|
||||
self._check_thread()
|
||||
self._check_score(amo.REVIEWED_WEBAPP_HOSTED)
|
||||
|
||||
def test_multiple_versions_reject_hosted(self):
|
||||
|
|
@ -1320,6 +1343,7 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
|
|||
self._check_email(dev_msg, 'Submission Update')
|
||||
adm_msg = mail.outbox[1]
|
||||
self._check_admin_email(adm_msg, 'Escalated Review Requested')
|
||||
self._check_thread()
|
||||
|
||||
def test_pending_to_disable_senior_reviewer(self):
|
||||
self.login_as_senior_reviewer()
|
||||
|
|
@ -1334,6 +1358,7 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
|
|||
self._check_log(amo.LOG.APP_DISABLED)
|
||||
eq_(len(mail.outbox), 1)
|
||||
self._check_email(mail.outbox[0], 'App disabled by reviewer')
|
||||
self._check_thread()
|
||||
|
||||
def test_pending_to_disable(self):
|
||||
self.app.update(status=amo.STATUS_PUBLIC)
|
||||
|
|
@ -1362,6 +1387,7 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
|
|||
msg = mail.outbox[0]
|
||||
self._check_email(msg, 'App Approved')
|
||||
self._check_email_body(msg)
|
||||
self._check_thread()
|
||||
|
||||
def test_escalation_to_reject(self):
|
||||
EscalationQueue.objects.create(addon=self.app)
|
||||
|
|
@ -1380,6 +1406,7 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
|
|||
eq_(len(mail.outbox), 1)
|
||||
msg = mail.outbox[0]
|
||||
self._check_email(msg, 'Submission Update')
|
||||
self._check_thread()
|
||||
self._check_email_body(msg)
|
||||
self._check_score(amo.REVIEWED_WEBAPP_HOSTED)
|
||||
|
||||
|
|
@ -1398,6 +1425,7 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
|
|||
eq_(EscalationQueue.objects.count(), 0)
|
||||
eq_(len(mail.outbox), 1)
|
||||
self._check_email(mail.outbox[0], 'App disabled by reviewer')
|
||||
self._check_thread()
|
||||
|
||||
def test_escalation_to_disable(self):
|
||||
EscalationQueue.objects.create(addon=self.app)
|
||||
|
|
@ -1438,6 +1466,7 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
|
|||
self._check_email(msg, 'Submission Update')
|
||||
self._check_email_body(msg)
|
||||
self._check_score(amo.REVIEWED_WEBAPP_REREVIEW)
|
||||
self._check_thread()
|
||||
|
||||
def test_rereview_to_disable_senior_reviewer(self):
|
||||
self.login_as_senior_reviewer()
|
||||
|
|
@ -1453,6 +1482,7 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
|
|||
eq_(RereviewQueue.objects.filter(addon=self.app).count(), 0)
|
||||
eq_(len(mail.outbox), 1)
|
||||
self._check_email(mail.outbox[0], 'App disabled by reviewer')
|
||||
self._check_thread()
|
||||
|
||||
def test_rereview_to_disable(self):
|
||||
RereviewQueue.objects.create(addon=self.app)
|
||||
|
|
@ -1491,6 +1521,7 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
|
|||
self._check_email(dev_msg, 'Submission Update')
|
||||
adm_msg = mail.outbox[1]
|
||||
self._check_admin_email(adm_msg, 'Escalated Review Requested')
|
||||
self._check_thread()
|
||||
|
||||
def test_more_information(self):
|
||||
# Test the same for all queues.
|
||||
|
|
@ -1525,6 +1556,7 @@ class TestReviewApp(AppReviewerTest, AccessMixin, AttachmentManagementMixin,
|
|||
self.post(data)
|
||||
eq_(len(mail.outbox), 0)
|
||||
self._check_log(amo.LOG.COMMENT_VERSION)
|
||||
self._check_thread()
|
||||
|
||||
def test_receipt_no_node(self):
|
||||
res = self.client.get(self.url)
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ from django.utils import translation
|
|||
from django.utils.datastructures import SortedDict
|
||||
|
||||
import commonware.log
|
||||
import waffle
|
||||
from tower import ugettext_lazy as _lazy
|
||||
|
||||
import amo
|
||||
|
|
@ -15,12 +16,16 @@ from access import acl
|
|||
from amo.helpers import absolutify
|
||||
from amo.urlresolvers import reverse
|
||||
from amo.utils import JSONEncoder, send_mail_jinja, to_language
|
||||
from comm.models import (CommunicationNote, CommunicationThread,
|
||||
CommunicationThreadCC, CommunicationThreadToken)
|
||||
from editors.models import EscalationQueue, RereviewQueue, ReviewerScore
|
||||
from files.models import File
|
||||
|
||||
from mkt.constants import comm
|
||||
from mkt.constants.features import FeatureProfile
|
||||
from mkt.site.helpers import product_as_dict
|
||||
from mkt.webapps.models import Webapp
|
||||
from users.models import UserProfile
|
||||
|
||||
|
||||
log = commonware.log.getLogger('z.mailer')
|
||||
|
|
@ -150,6 +155,10 @@ class ReviewBase(object):
|
|||
log.info(u'Sending request for information for %s to %s' %
|
||||
(self.addon, emails))
|
||||
data = self.get_context_data()
|
||||
|
||||
# Create thread.
|
||||
self.create_comm_thread(action='info')
|
||||
|
||||
send_mail(u'Submission Update: %s' % data['name'],
|
||||
'reviewers/emails/decisions/info.txt',
|
||||
data, emails,
|
||||
|
|
@ -172,6 +181,52 @@ class ReviewApp(ReviewBase):
|
|||
self.data = data
|
||||
self.files = self.version.files.all()
|
||||
|
||||
def create_comm_thread(self, **kwargs):
|
||||
"""Create a thread/note objects for communication."""
|
||||
if not waffle.switch_is_active('comm-dashboard'):
|
||||
return
|
||||
|
||||
action = kwargs['action']
|
||||
action_note_types = {
|
||||
'approve': comm.APPROVAL,
|
||||
'disable': comm.DISABLED,
|
||||
'escalate': comm.ESCALATION,
|
||||
'info': comm.MORE_INFO_REQUIRED,
|
||||
'comment': comm.REVIEWER_COMMENT,
|
||||
'reject': comm.REJECTION
|
||||
}
|
||||
|
||||
thread = CommunicationThread.objects.filter(addon=self.addon,
|
||||
version=self.version)
|
||||
|
||||
if thread.exists():
|
||||
thread = thread[0]
|
||||
else:
|
||||
thread = CommunicationThread(addon=self.addon,
|
||||
version=self.version)
|
||||
|
||||
# Set permissions from the form.
|
||||
for key in self.data['action_visibility']:
|
||||
setattr(thread, 'read_permission_%s' % key, True)
|
||||
thread.save()
|
||||
|
||||
CommunicationNote.objects.create(
|
||||
note_type=action_note_types[action],
|
||||
body=self.data['comments'], author=self.request.amo_user,
|
||||
thread=thread)
|
||||
|
||||
moz_email = self.addon.mozilla_contact
|
||||
# CC mozilla contact.
|
||||
try:
|
||||
moz_contact = UserProfile.objects.get(email=moz_email)
|
||||
except UserProfile.DoesNotExist:
|
||||
moz_contact = None
|
||||
pass
|
||||
|
||||
if moz_contact and moz_email:
|
||||
CommunicationThreadCC.objects.create(thread=thread,
|
||||
user=moz_contact)
|
||||
|
||||
def process_public(self):
|
||||
# Hold onto the status before we change it.
|
||||
status = self.addon.status
|
||||
|
|
@ -186,6 +241,7 @@ class ReviewApp(ReviewBase):
|
|||
|
||||
# Assign reviewer incentive scores.
|
||||
ReviewerScore.award_points(self.request.amo_user, self.addon, status)
|
||||
self.create_comm_thread(action='approve')
|
||||
|
||||
def process_public_waiting(self):
|
||||
"""Make an app pending."""
|
||||
|
|
@ -256,6 +312,7 @@ class ReviewApp(ReviewBase):
|
|||
# Assign reviewer incentive scores.
|
||||
ReviewerScore.award_points(self.request.amo_user, self.addon, status,
|
||||
in_rereview=self.in_rereview)
|
||||
self.create_comm_thread(action='reject')
|
||||
|
||||
def process_escalate(self):
|
||||
"""Ask for escalation for an app."""
|
||||
|
|
@ -263,10 +320,12 @@ class ReviewApp(ReviewBase):
|
|||
self.notify_email('author_super_review', u'Submission Update: %s')
|
||||
|
||||
self.send_escalate_mail()
|
||||
self.create_comm_thread(action='escalate')
|
||||
|
||||
def process_comment(self):
|
||||
self.version.update(has_editor_comment=True)
|
||||
self.log_action(amo.LOG.COMMENT_VERSION)
|
||||
self.create_comm_thread(action='comment')
|
||||
|
||||
def process_clear_escalation(self):
|
||||
"""Clear app from escalation queue."""
|
||||
|
|
@ -299,6 +358,7 @@ class ReviewApp(ReviewBase):
|
|||
RereviewQueue.objects.filter(addon=self.addon).delete()
|
||||
emails = list(self.addon.authors.values_list('email', flat=True))
|
||||
cc_email = self.addon.mozilla_contact or None
|
||||
self.create_comm_thread(action='disable')
|
||||
send_mail(u'App disabled by reviewer: %s' % self.addon.name,
|
||||
'reviewers/emails/decisions/disabled.txt',
|
||||
self.get_context_data(), emails,
|
||||
|
|
|
|||
|
|
@ -44,6 +44,7 @@ from translations.query import order_by_translation
|
|||
from users.models import UserProfile
|
||||
from zadmin.models import set_config, unmemoized_get_config
|
||||
|
||||
from mkt.reviewers.forms import DEFAULT_ACTION_VISIBILITY
|
||||
from mkt.reviewers.utils import (AppsReviewing, clean_sort_param,
|
||||
device_queue_search)
|
||||
from mkt.search.forms import ApiSearchForm
|
||||
|
|
@ -315,7 +316,8 @@ def _review(request, addon, version):
|
|||
actions=actions, actions_minimal=actions_minimal,
|
||||
tab=queue_type, product_attrs=product_attrs,
|
||||
attachment_formset=attachment_formset,
|
||||
appfeatures_form=appfeatures_form)
|
||||
appfeatures_form=appfeatures_form,
|
||||
default_visibility=DEFAULT_ACTION_VISIBILITY)
|
||||
|
||||
if waffle.switch_is_active('buchets'):
|
||||
ctx['feature_list'] = features_list
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче