Fix reviewer subscription (#15691)
This commit is contained in:
Andreas Wagner
GitHub
Родитель
44a3652eaa
Коммит
87e82fa110
|
|
@ -427,16 +427,26 @@ def send_notifications(sender=None, instance=None, signal=None, **kw):
|
|||
if not subscribers:
|
||||
return
|
||||
|
||||
listed_perms = [
|
||||
amo.permissions.ADDONS_REVIEW,
|
||||
amo.permissions.ADDONS_CONTENT_REVIEW,
|
||||
amo.permissions.ADDONS_RECOMMENDED_REVIEW,
|
||||
amo.permissions.STATIC_THEMES_REVIEW,
|
||||
amo.permissions.REVIEWER_TOOLS_VIEW
|
||||
]
|
||||
|
||||
for subscriber in subscribers:
|
||||
user = subscriber.user
|
||||
is_active_user = user and not user.deleted and user.email
|
||||
is_reviewer_and_listed_submission = (
|
||||
acl.is_user_any_kind_of_reviewer(user, allow_viewers=True) and
|
||||
instance.channel == amo.RELEASE_CHANNEL_LISTED)
|
||||
subscriber.channel == amo.RELEASE_CHANNEL_LISTED and
|
||||
instance.channel == amo.RELEASE_CHANNEL_LISTED and
|
||||
any(acl.action_allowed_user(user, perm) for perm in listed_perms))
|
||||
is_unlisted_reviewer_and_unlisted_submission = (
|
||||
subscriber.channel == amo.RELEASE_CHANNEL_UNLISTED and
|
||||
instance.channel == amo.RELEASE_CHANNEL_UNLISTED and
|
||||
acl.action_allowed_user(user,
|
||||
amo.permissions.ADDONS_REVIEW_UNLISTED) and
|
||||
instance.channel == amo.RELEASE_CHANNEL_UNLISTED)
|
||||
amo.permissions.ADDONS_REVIEW_UNLISTED))
|
||||
if is_active_user and (
|
||||
is_reviewer_and_listed_submission or
|
||||
is_unlisted_reviewer_and_unlisted_submission
|
||||
|
|
|
|||
|
|
@ -223,21 +223,23 @@
|
|||
<div class="more-actions-inner">
|
||||
<ul>
|
||||
{% if not addon.is_deleted %}
|
||||
<li>
|
||||
<input type="checkbox" id="notify_new_listed_versions"
|
||||
data-api-url-subscribe-listed="{{ drf_url('reviewers-addon-subscribe-listed', addon.pk) }}"
|
||||
data-api-url-unsubscribe-listed="{{ drf_url('reviewers-addon-unsubscribe-listed', addon.pk) }}"
|
||||
{% if subscribed_listed %}checked="checked"{% endif %} autocomplete="off" />
|
||||
<label for="notify_new_listed_versions">{{ _('Notify me about new listed versions') }}</label>
|
||||
</li>
|
||||
{% if action_allowed(amo.permissions.ADDONS_REVIEW_UNLISTED) %}
|
||||
<li>
|
||||
<input type="checkbox" id="notify_new_unlisted_versions"
|
||||
data-api-url-subscribe-unlisted="{{ drf_url('reviewers-addon-subscribe-unlisted', addon.pk) }}"
|
||||
data-api-url-unsubscribe-unlisted="{{ drf_url('reviewers-addon-unsubscribe-unlisted', addon.pk) }}"
|
||||
{% if subscribed_unlisted %}checked="checked"{% endif %} autocomplete="off" />
|
||||
<label for="notify_new_unlisted_versions">{{ _('Notify me about new unlisted versions') }}</label>
|
||||
</li>
|
||||
{% if acl_is_reviewer or action_allowed(amo.permissions.REVIEWER_TOOLS_VIEW) %}
|
||||
<li>
|
||||
<input type="checkbox" id="notify_new_listed_versions"
|
||||
data-api-url-subscribe-listed="{{ drf_url('reviewers-addon-subscribe-listed', addon.pk) }}"
|
||||
data-api-url-unsubscribe-listed="{{ drf_url('reviewers-addon-unsubscribe-listed', addon.pk) }}"
|
||||
{% if subscribed_listed %}checked="checked"{% endif %} autocomplete="off" />
|
||||
<label for="notify_new_listed_versions">{{ _('Notify me about new listed versions') }}</label>
|
||||
</li>
|
||||
{% endif %}
|
||||
{% if action_allowed(amo.permissions.ADDONS_REVIEW_UNLISTED) %}
|
||||
<li>
|
||||
<input type="checkbox" id="notify_new_unlisted_versions"
|
||||
data-api-url-subscribe-unlisted="{{ drf_url('reviewers-addon-subscribe-unlisted', addon.pk) }}"
|
||||
data-api-url-unsubscribe-unlisted="{{ drf_url('reviewers-addon-unsubscribe-unlisted', addon.pk) }}"
|
||||
{% if subscribed_unlisted %}checked="checked"{% endif %} autocomplete="off" />
|
||||
<label for="notify_new_unlisted_versions">{{ _('Notify me about new unlisted versions') }}</label>
|
||||
</li>
|
||||
{% endif %}
|
||||
{% endif %}
|
||||
</ul>
|
||||
|
|
|
|||
|
|
@ -386,98 +386,176 @@ class TestReviewerSubscription(TestCase):
|
|||
|
||||
def setUp(self):
|
||||
super(TestReviewerSubscription, self).setUp()
|
||||
self.addon = Addon.objects.get(pk=3615)
|
||||
self.version = self.addon.current_version
|
||||
self.user_one = UserProfile.objects.get(pk=55021)
|
||||
self.user_two = UserProfile.objects.get(pk=999)
|
||||
self.reviewer_group = Group.objects.create(
|
||||
name='Reviewers: Legacy', rules='Addons:Review')
|
||||
self.addon = addon_factory(name='SubscribingTest')
|
||||
self.listed_version = version_factory(addon=self.addon)
|
||||
self.unlisted_version = version_factory(
|
||||
addon=self.addon,
|
||||
channel=amo.RELEASE_CHANNEL_UNLISTED)
|
||||
|
||||
self.listed_reviewer = user_factory(email='listed@reviewer')
|
||||
self.listed_reviewer_group = Group.objects.create(
|
||||
name='Listed Reviewers', rules='Addons:Review')
|
||||
GroupUser.objects.create(
|
||||
group=self.reviewer_group, user=self.user_one)
|
||||
GroupUser.objects.create(
|
||||
group=self.reviewer_group, user=self.user_two)
|
||||
group=self.listed_reviewer_group, user=self.listed_reviewer)
|
||||
ReviewerSubscription.objects.create(
|
||||
addon=self.addon, user=self.user_one,
|
||||
channel=amo.RELEASE_CHANNEL_LISTED)
|
||||
ReviewerSubscription.objects.create(
|
||||
addon=self.addon, user=self.user_two,
|
||||
addon=self.addon, user=self.listed_reviewer,
|
||||
channel=amo.RELEASE_CHANNEL_LISTED)
|
||||
|
||||
def test_email(self):
|
||||
es = ReviewerSubscription.objects.get(user=self.user_one)
|
||||
es.send_notification(self.version)
|
||||
self.unlisted_reviewer = user_factory(email='unlisted@reviewer')
|
||||
self.unlisted_reviewer_group = Group.objects.create(
|
||||
name='Unlisted Reviewers', rules='Addons:ReviewUnlisted')
|
||||
GroupUser.objects.create(
|
||||
group=self.unlisted_reviewer_group, user=self.unlisted_reviewer)
|
||||
ReviewerSubscription.objects.create(
|
||||
addon=self.addon, user=self.unlisted_reviewer,
|
||||
channel=amo.RELEASE_CHANNEL_UNLISTED)
|
||||
|
||||
self.admin_reviewer = user_factory(email='admin@reviewer')
|
||||
GroupUser.objects.create(
|
||||
group=self.listed_reviewer_group, user=self.admin_reviewer)
|
||||
GroupUser.objects.create(
|
||||
group=self.unlisted_reviewer_group, user=self.admin_reviewer)
|
||||
# Don't subscribe admin to updates yet, will be done in tests.
|
||||
|
||||
def test_send_notification(self):
|
||||
subscription = ReviewerSubscription.objects.get(
|
||||
user=self.listed_reviewer)
|
||||
subscription.send_notification(self.listed_version)
|
||||
assert len(mail.outbox) == 1
|
||||
assert mail.outbox[0].to == [u'del@icio.us']
|
||||
assert mail.outbox[0].to == ['listed@reviewer']
|
||||
assert mail.outbox[0].subject == (
|
||||
'Mozilla Add-ons: Delicious Bookmarks Updated')
|
||||
'Mozilla Add-ons: SubscribingTest Updated')
|
||||
|
||||
def test_notifications(self):
|
||||
send_notifications(sender=Version, instance=self.version)
|
||||
def test_send_notifications(self):
|
||||
another_listed_reviewer = user_factory(email='listed2@reviewer')
|
||||
GroupUser.objects.create(
|
||||
group=self.listed_reviewer_group,
|
||||
user=another_listed_reviewer)
|
||||
ReviewerSubscription.objects.create(
|
||||
addon=self.addon, user=another_listed_reviewer,
|
||||
channel=amo.RELEASE_CHANNEL_LISTED)
|
||||
|
||||
send_notifications(sender=Version, instance=self.listed_version)
|
||||
assert len(mail.outbox) == 2
|
||||
emails = sorted([o.to for o in mail.outbox])
|
||||
assert emails == [[u'del@icio.us'], [u'regular@mozilla.com']]
|
||||
assert emails == [['listed2@reviewer'], ['listed@reviewer']]
|
||||
|
||||
def test_notifications_setting_persists(self):
|
||||
send_notifications(Version, self.version)
|
||||
send_notifications(Version, self.listed_version)
|
||||
assert ReviewerSubscription.objects.count() == 2
|
||||
mail.outbox = []
|
||||
send_notifications(Version, self.version)
|
||||
assert len(mail.outbox) == 2
|
||||
send_notifications(Version, self.listed_version)
|
||||
assert len(mail.outbox) == 1
|
||||
mail.outbox = []
|
||||
send_notifications(Version, self.unlisted_version)
|
||||
assert ReviewerSubscription.objects.count() == 2
|
||||
mail.outbox = []
|
||||
send_notifications(Version, self.unlisted_version)
|
||||
assert len(mail.outbox) == 1
|
||||
|
||||
def test_send_notifications_unlisted(self):
|
||||
self.reviewer_group = Group.objects.create(
|
||||
name='Reviewers: Unlisted', rules='Addons:ReviewUnlisted')
|
||||
GroupUser.objects.create(
|
||||
group=self.reviewer_group, user=self.user_one)
|
||||
version_uploaded.send(sender=Version, instance=self.version)
|
||||
assert len(mail.outbox) == 2
|
||||
assert mail.outbox[0].to == [u'del@icio.us']
|
||||
def test_listed_subscription(self):
|
||||
version_uploaded.send(sender=Version, instance=self.listed_version)
|
||||
assert len(mail.outbox) == 1
|
||||
assert mail.outbox[0].to == ['listed@reviewer']
|
||||
assert mail.outbox[0].subject == (
|
||||
'Mozilla Add-ons: Delicious Bookmarks Updated')
|
||||
'Mozilla Add-ons: SubscribingTest Updated')
|
||||
|
||||
def test_unlisted_subscription(self):
|
||||
version_uploaded.send(sender=Version, instance=self.unlisted_version)
|
||||
assert len(mail.outbox) == 1
|
||||
assert mail.outbox[0].to == ['unlisted@reviewer']
|
||||
assert mail.outbox[0].subject == (
|
||||
'Mozilla Add-ons: SubscribingTest Updated')
|
||||
|
||||
def test_unlisted_subscription_listed_reviewer(self):
|
||||
ReviewerSubscription.objects.create(
|
||||
addon=self.addon, user=self.user_one,
|
||||
addon=self.addon, user=self.listed_reviewer,
|
||||
channel=amo.RELEASE_CHANNEL_UNLISTED)
|
||||
self.version.update(channel=amo.RELEASE_CHANNEL_UNLISTED)
|
||||
version_uploaded.send(sender=Version, instance=self.version)
|
||||
|
||||
version_uploaded.send(sender=Version, instance=self.unlisted_version)
|
||||
# No email should be sent since the reviewer does not have access
|
||||
# to unlisted.
|
||||
assert len(mail.outbox) == 0
|
||||
assert len(mail.outbox) == 1
|
||||
# Only unlisted@reviewer
|
||||
assert mail.outbox[0].to != ['listed@reviewer']
|
||||
|
||||
def test_admin_reviewer_listed_subscription(self):
|
||||
ReviewerSubscription.objects.create(
|
||||
addon=self.addon, user=self.admin_reviewer,
|
||||
channel=amo.RELEASE_CHANNEL_LISTED)
|
||||
version_uploaded.send(sender=Version, instance=self.listed_version)
|
||||
assert len(mail.outbox) == 2
|
||||
emails = sorted([o.to for o in mail.outbox])
|
||||
assert emails == [['admin@reviewer'], ['listed@reviewer']]
|
||||
|
||||
mail.outbox = []
|
||||
version_uploaded.send(sender=Version, instance=self.unlisted_version)
|
||||
assert len(mail.outbox) == 1
|
||||
# Only unlisted@reviewer
|
||||
assert mail.outbox[0].to != ['admin@®reviewer']
|
||||
|
||||
def test_admin_reviewer_unlisted_subscription(self):
|
||||
ReviewerSubscription.objects.create(
|
||||
addon=self.addon, user=self.admin_reviewer,
|
||||
channel=amo.RELEASE_CHANNEL_UNLISTED)
|
||||
version_uploaded.send(sender=Version, instance=self.unlisted_version)
|
||||
assert len(mail.outbox) == 2
|
||||
emails = sorted([o.to for o in mail.outbox])
|
||||
assert emails == [['admin@reviewer'], ['unlisted@reviewer']]
|
||||
|
||||
mail.outbox = []
|
||||
version_uploaded.send(sender=Version, instance=self.listed_version)
|
||||
assert len(mail.outbox) == 1
|
||||
# Only listed@reviewer
|
||||
assert mail.outbox[0].to != ['admin@®reviewer']
|
||||
|
||||
def test_admin_reviewer_both_subscriptions(self):
|
||||
ReviewerSubscription.objects.create(
|
||||
addon=self.addon, user=self.admin_reviewer,
|
||||
channel=amo.RELEASE_CHANNEL_LISTED)
|
||||
ReviewerSubscription.objects.create(
|
||||
addon=self.addon, user=self.admin_reviewer,
|
||||
channel=amo.RELEASE_CHANNEL_UNLISTED)
|
||||
version_uploaded.send(sender=Version, instance=self.listed_version)
|
||||
version_uploaded.send(sender=Version, instance=self.unlisted_version)
|
||||
assert len(mail.outbox) == 4
|
||||
emails = sorted([o.to for o in mail.outbox])
|
||||
assert emails == [['admin@reviewer'], ['admin@reviewer'],
|
||||
['listed@reviewer'], ['unlisted@reviewer']]
|
||||
|
||||
def test_signal_edit(self):
|
||||
self.version.save()
|
||||
self.listed_version.save()
|
||||
self.unlisted_version.save()
|
||||
assert len(mail.outbox) == 0
|
||||
|
||||
def test_signal_create(self):
|
||||
version = Version.objects.create(addon=self.addon)
|
||||
version = version_factory(addon=self.addon)
|
||||
version_uploaded.send(sender=Version, instance=version)
|
||||
assert len(mail.outbox) == 2
|
||||
assert len(mail.outbox) == 1
|
||||
assert mail.outbox[0].subject == (
|
||||
'Mozilla Add-ons: Delicious Bookmarks Updated')
|
||||
'Mozilla Add-ons: SubscribingTest Updated')
|
||||
|
||||
def test_signal_create_twice(self):
|
||||
version = Version.objects.create(addon=self.addon)
|
||||
version = version_factory(addon=self.addon)
|
||||
version_uploaded.send(sender=Version, instance=version)
|
||||
mail.outbox = []
|
||||
version = Version.objects.create(addon=self.addon)
|
||||
version = version_factory(addon=self.addon)
|
||||
version_uploaded.send(sender=Version, instance=version)
|
||||
assert len(mail.outbox) == 2
|
||||
assert len(mail.outbox) == 1
|
||||
|
||||
def test_no_email_for_ex_reviewers(self):
|
||||
self.user_one.delete()
|
||||
self.listed_reviewer.delete()
|
||||
mail.outbox = [] # deleting the user sends an email for the addon
|
||||
# Remove user_one from reviewers.
|
||||
GroupUser.objects.get(
|
||||
group=self.reviewer_group, user=self.user_one).delete()
|
||||
send_notifications(sender=Version, instance=self.version)
|
||||
assert len(mail.outbox) == 1 # Only notification for user_two remains.
|
||||
group=self.listed_reviewer_group,
|
||||
user=self.listed_reviewer).delete()
|
||||
send_notifications(sender=Version, instance=self.listed_version)
|
||||
assert len(mail.outbox) == 0
|
||||
|
||||
def test_no_email_address_for_reviewer(self):
|
||||
self.user_one.update(email=None)
|
||||
send_notifications(sender=Version, instance=self.version)
|
||||
assert len(mail.outbox) == 1 # Only notification for user_two remains.
|
||||
self.listed_reviewer.update(email=None)
|
||||
send_notifications(sender=Version, instance=self.listed_version)
|
||||
assert len(mail.outbox) == 0
|
||||
|
||||
|
||||
class TestReviewerScore(TestCase):
|
||||
|
|
|
|||
|
|
@ -891,6 +891,9 @@ def review(request, addon, channel=None):
|
|||
)
|
||||
|
||||
ctx = context(
|
||||
# Used for reviewer subscription check, don't use global `is_reviewer`
|
||||
# since that actually is `is_user_any_kind_of_reviewer`.
|
||||
acl_is_reviewer=acl.is_reviewer(request, addon),
|
||||
actions=actions,
|
||||
actions_comments=actions_comments,
|
||||
actions_delayable=actions_delayable,
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче