Show & Allow removing Unlisted Auto-Approval Extra Delay in review tools (#20297)

* Show & Allow removing Unlisted Auto-Approval Extra Delay in review tools

Also refactor how flags are displayed, adding a couple new ones and hiding the ones that are not relevant to the version shown in the review page because of the channel.
This commit is contained in:
Mathieu Pillard 2023-02-09 15:01:01 +01:00 коммит произвёл GitHub
Родитель b54e6b4cae
Коммит dcfe5e87e0
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
14 изменённых файлов: 506 добавлений и 118 удалений

Просмотреть файл

@ -91,9 +91,12 @@ add-on.
.. http:patch:: /api/v5/reviewers/addon/(int:addon_id)/flags/
:>json boolean auto_approval_disabled: Boolean indicating whether auto approval are disabled on an add-on or not. When it's ``true``, new versions for this add-on will make it appear in the regular reviewer queues instead of being auto-approved.
:>json boolean auto_approval_disabled_until_next_approval: Boolean indicating whether auto approval are disabled on an add-on until the next version is approved or not. Has the same effect as ``auto_approval_disabled`` but is automatically reset to ``false`` when the latest version of the add-on is manually approved by a human reviewer.
:>json string|null auto_approval_delayed_until: Date until the add-on auto-approval is delayed.
:>json boolean auto_approval_disabled: Boolean indicating whether auto approval of listed versions is disabled on an add-on or not. When it's ``true``, new listed versions for this add-on will make it appear in the regular reviewer queues instead of being auto-approved.
:>json boolean auto_approval_disabled_until_next_approval: Boolean indicating whether auto approval of listed versions is disabled on an add-on until the next listed version is approved or not. Has the same effect as ``auto_approval_disabled`` but is automatically reset to ``false`` when the latest listed version of the add-on is manually approved by a human reviewer.
:>json string|null auto_approval_delayed_until: Date until the add-on auto-approval is delayed for listed versions.
:>json boolean auto_approval_disabled_unlisted: Boolean indicating whether auto approval of unlisted versions is disabled on an add-on or not. When it's ``true``, new unlisted versions for this add-on will make it appear in the regular reviewer queues instead of being auto-approved.
:>json boolean auto_approval_disabled_until_next_approval_unlisted: Boolean indicating whether auto approval of unlisted versions is disabled on an add-on until the next unlisted version is approved or not. Has the same effect as ``auto_approval_disabled_unlisted`` but is automatically reset to ``false`` when the latest unlisted version of the add-on is manually approved by a human reviewer.
:>json string|null auto_approval_delayed_until_unlisted: Date until the add-on auto-approval is delayed for unlisted versions.
:>json boolean needs_admin_code_review: Boolean indicating whether the add-on needs its code to be reviewed by an admin or not.
:>json boolean needs_admin_content_review: Boolean indicating whether the add-on needs its content to be reviewed by an admin or not.
:>json boolean needs_admin_theme_review: Boolean indicating whether the theme needs to be reviewed by an admin or not.

Просмотреть файл

@ -12,8 +12,15 @@ from django.conf import settings
from django.contrib.staticfiles.storage import staticfiles_storage
from django.core.exceptions import ObjectDoesNotExist
from django.db import models, transaction
from django.db.models import F, Max, Min, Q, signals as dbsignals
from django.db.models.expressions import Func
from django.db.models import (
F,
Max,
Min,
OuterRef,
Q,
Subquery,
signals as dbsignals,
)
from django.db.models.functions import Coalesce, Greatest
from django.dispatch import receiver
from django.urls import reverse
@ -298,7 +305,7 @@ class AddonManager(ManagerBase):
return qs
def get_queryset_for_pending_queues(
self, *, admin_reviewer=False, theme_review=False
self, *, admin_reviewer=False, theme_review=False, show_temporarily_delayed=True
):
if theme_review:
filters = {
@ -319,23 +326,47 @@ class AddonManager(ManagerBase):
# select_related() for listed fields don't make sense.
select_related_fields_for_listed=False,
)
return (
versions_due_qs = (
Version.unfiltered.filter(due_date__isnull=False)
.no_transforms()
.order_by('due_date')
)
if not show_temporarily_delayed:
# If we were asked not to show temporarily delayed, we need to
# exclude versions from the channel of the corresponding addon auto
# approval delay flag.This way, we keep showing the add-on if it
# has other versions that would not be in that channel.
unlisted_delay_flag_field = (
'addon__reviewerflags__auto_approval_delayed_until_unlisted'
)
listed_delay_flag_field = (
'addon__reviewerflags__auto_approval_delayed_until'
)
versions_due_qs = versions_due_qs.exclude(
Q(
Q(channel=amo.CHANNEL_UNLISTED)
& Q(**{f'{unlisted_delay_flag_field}__isnull': False})
& ~Q(**{unlisted_delay_flag_field: datetime.max})
)
| Q(
Q(channel=amo.CHANNEL_LISTED)
& Q(**{f'{listed_delay_flag_field}__isnull': False})
& ~Q(**{listed_delay_flag_field: datetime.max})
)
)
qs = (
qs.filter(**filters)
.exclude(**excludes)
.annotate(
first_version_due_date=Min('versions__due_date'),
# Because of the Min(), a GROUP BY addon.id is created, and the
# versions join will pick the first by due date. Unfortunately
# if we were to annotate with just F('versions__<something>')
# Django would add version.<something> to the GROUP BY, ruining
# it. To prevent that, we wrap it into a harmless Func() - we
# need a no-op function to do that, hence the ANY_VALUE().
# We'll then grab the id in our transformer to fetch all
# related versions.
first_version_id=Func(F('versions__id'), function='ANY_VALUE'),
first_version_id=Subquery(
versions_due_qs.filter(addon=OuterRef('pk')).values('pk')[:1]
),
)
.filter(first_version_id__isnull=False)
.transform(first_pending_version_transformer)
)
return qs
def get_content_review_queue(self, admin_reviewer=False):
"""Return a queryset of Addon objects that need content review."""
@ -422,6 +453,11 @@ class AddonManager(ManagerBase):
)
def get_pending_rejection_queue(self, admin_reviewer=False):
versions_pending_rejection_qs = (
Version.unfiltered.filter(reviewerflags__pending_rejection__isnull=False)
.no_transforms()
.order_by('reviewerflags__pending_rejection')
)
return (
self.get_base_queryset_for_queue(
select_related_fields_for_listed=False, admin_reviewer=admin_reviewer
@ -431,16 +467,13 @@ class AddonManager(ManagerBase):
first_version_pending_rejection_date=Min(
'versions__reviewerflags__pending_rejection'
),
# Because of the Min(), a GROUP BY addon.id is created, and the
# versions join will pick the first by due date. Unfortunately
# if we were to annotate with just F('versions__<something>')
# Django would add version.<something> to the GROUP BY, ruining
# it. To prevent that, we wrap it into a harmless Func() - we
# need a no-op function to do that, hence the ANY_VALUE().
# We'll then grab the id in our transformer to fetch all
# related versions.
first_version_id=Func(F('versions__id'), function='ANY_VALUE'),
first_version_id=Subquery(
versions_pending_rejection_qs.filter(addon=OuterRef('pk')).values(
'pk'
)[:1]
),
)
.filter(first_version_id__isnull=False)
.transform(first_pending_version_transformer)
)
@ -1656,10 +1689,7 @@ class Addon(OnChangeMixin, ModelBase):
@property
def auto_approval_delayed_indefinitely(self):
return (
self.auto_approval_delayed_until == datetime.max
or self.auto_approval_delayed_until_unlisted == datetime.max
)
return self.auto_approval_delayed_until == datetime.max
@property
def auto_approval_delayed_temporarily(self):
@ -1667,7 +1697,15 @@ class Addon(OnChangeMixin, ModelBase):
bool(self.auto_approval_delayed_until)
and self.auto_approval_delayed_until != datetime.max
and self.auto_approval_delayed_until > datetime.now()
) or (
)
@property
def auto_approval_delayed_indefinitely_unlisted(self):
return self.auto_approval_delayed_until_unlisted == datetime.max
@property
def auto_approval_delayed_temporarily_unlisted(self):
return (
bool(self.auto_approval_delayed_until_unlisted)
and self.auto_approval_delayed_until_unlisted != datetime.max
and self.auto_approval_delayed_until_unlisted > datetime.now()

Просмотреть файл

@ -1566,35 +1566,35 @@ class TestAddonModels(TestCase):
flags.update(auto_approval_delayed_until=datetime.max)
assert addon.auto_approval_delayed_indefinitely is True
def test_auto_approval_delayed_indefinitely_property_with_unlisted(self):
def test_auto_approval_delayed_indefinitely_unlisted_property(self):
addon = Addon.objects.get(pk=3615)
# No flags: None
assert addon.auto_approval_delayed_indefinitely is False
assert addon.auto_approval_delayed_indefinitely_unlisted is False
# Flag present, value is None (default): None.
flags = AddonReviewerFlags.objects.create(addon=addon)
assert addon.auto_approval_delayed_indefinitely is False
assert addon.auto_approval_delayed_indefinitely_unlisted is False
# Flag present, value is a date.
in_the_past = self.days_ago(1)
flags.update(auto_approval_delayed_until_unlisted=in_the_past)
assert addon.auto_approval_delayed_indefinitely is False
assert addon.auto_approval_delayed_indefinitely_unlisted is False
# In the future, but not far enough.
in_the_future = datetime.now() + timedelta(hours=24)
flags.update(auto_approval_delayed_until_unlisted=in_the_future)
assert addon.auto_approval_delayed_indefinitely is False
assert addon.auto_approval_delayed_indefinitely_unlisted is False
# This time it's truly delayed indefinitely.
flags.update(auto_approval_delayed_until_unlisted=datetime.max)
assert addon.auto_approval_delayed_indefinitely is True
# If both unlisted or listed are set, we only need one to match.
assert addon.auto_approval_delayed_indefinitely_unlisted is True
# We only consider the unlisted flag.
flags.update(
auto_approval_delayed_until_unlisted=datetime.now(),
auto_approval_delayed_until=datetime.max,
)
assert addon.auto_approval_delayed_indefinitely is True
assert addon.auto_approval_delayed_indefinitely_unlisted is False
flags.update(
auto_approval_delayed_until_unlisted=datetime.max,
auto_approval_delayed_until=datetime.now(),
)
assert addon.auto_approval_delayed_indefinitely is True
assert addon.auto_approval_delayed_indefinitely_unlisted is True
def test_auto_approval_delayed_temporarily_property(self):
addon = Addon.objects.get(pk=3615)
@ -1614,15 +1614,31 @@ class TestAddonModels(TestCase):
# Not considered temporary any more if it's until the end of time!
flags.update(auto_approval_delayed_until=datetime.max)
assert addon.auto_approval_delayed_temporarily is False
# But if the unlisted flag is set in the future, then yes.
# Not even if the unlisted flag is set since it's a separate property.
flags.update(auto_approval_delayed_until_unlisted=in_the_future)
assert addon.auto_approval_delayed_temporarily is True
# Unless it's also at max.
flags.update(auto_approval_delayed_until_unlisted=datetime.max)
assert addon.auto_approval_delayed_temporarily is False
# But not if it's in the past.
def test_auto_approval_delayed_temporarily_unlisted_property(self):
addon = Addon.objects.get(pk=3615)
# No flags: None
assert addon.auto_approval_delayed_temporarily_unlisted is False
# Flag present, value is None (default): None.
flags = AddonReviewerFlags.objects.create(addon=addon)
assert addon.auto_approval_delayed_temporarily_unlisted is False
# Flag present, value is a date.
in_the_past = self.days_ago(1)
flags.update(auto_approval_delayed_until_unlisted=in_the_past)
assert addon.auto_approval_delayed_temporarily is False
assert addon.auto_approval_delayed_temporarily_unlisted is False
# Flag present, now properly in the future.
in_the_future = datetime.now() + timedelta(hours=24)
flags.update(auto_approval_delayed_until_unlisted=in_the_future)
assert addon.auto_approval_delayed_temporarily_unlisted is True
# Not considered temporary any more if it's until the end of time!
flags.update(auto_approval_delayed_until_unlisted=datetime.max)
assert addon.auto_approval_delayed_temporarily_unlisted is False
# Not even if the listed flag is set since it's a separate property.
flags.update(auto_approval_delayed_until=in_the_future)
assert addon.auto_approval_delayed_temporarily_unlisted is False
def test_needs_admin_code_review_property(self):
addon = Addon.objects.get(pk=3615)
@ -3157,8 +3173,10 @@ class TestExtensionsQueues(TestCase):
),
file_kw={'status': amo.STATUS_AWAITING_REVIEW},
channel=amo.CHANNEL_UNLISTED,
due_date=datetime.now() + timedelta(hours=24),
).addon,
file_kw={'status': amo.STATUS_AWAITING_REVIEW},
due_date=datetime.now() + timedelta(hours=48),
).addon,
version_factory(
addon=addon_factory(
@ -3167,6 +3185,40 @@ class TestExtensionsQueues(TestCase):
),
file_kw={'status': amo.STATUS_AWAITING_REVIEW},
).addon,
version_factory(
addon=version_factory(
addon=addon_factory(
name='Auto-approval delayed for listed, disabled for unlisted',
reviewer_flags={
'auto_approval_delayed_until': datetime.now()
+ timedelta(hours=24),
'auto_approval_disabled_unlisted': True,
},
),
file_kw={'status': amo.STATUS_AWAITING_REVIEW},
channel=amo.CHANNEL_UNLISTED,
due_date=datetime.now() + timedelta(hours=24),
).addon,
file_kw={'status': amo.STATUS_AWAITING_REVIEW},
due_date=datetime.now() + timedelta(hours=48),
).addon,
version_factory(
addon=version_factory(
addon=addon_factory(
name='Auto-approval delayed for unlisted, disabled for listed',
reviewer_flags={
'auto_approval_delayed_until_unlisted': datetime.now()
+ timedelta(hours=24),
'auto_approval_disabled': True,
},
),
file_kw={'status': amo.STATUS_AWAITING_REVIEW},
due_date=datetime.now() + timedelta(hours=24),
channel=amo.CHANNEL_UNLISTED,
).addon,
file_kw={'status': amo.STATUS_AWAITING_REVIEW},
due_date=datetime.now() + timedelta(hours=48),
).addon,
]
deleted_addon_human_review = addon_factory(
name='Deleted add-on - human review',
@ -3276,6 +3328,69 @@ class TestExtensionsQueues(TestCase):
addons = Addon.unfiltered.get_queryset_for_pending_queues(admin_reviewer=True)
assert set(addons) == set(expected_addons)
# If we pass show_temporarily_delayed=False, versions in a channel that
# is temporarily delayed should not be considered. We already have a
# couple add-ons with versions delayed, but they should show up since
# they also have a version non-delayed in another channel. Let's add
# some that shouldn't show up.
version_factory(
addon=version_factory(
addon=addon_factory(
name='Auto-approval delayed for unlisted and listed',
reviewer_flags={
'auto_approval_delayed_until_unlisted': datetime.now()
+ timedelta(hours=24),
'auto_approval_delayed_until': datetime.now()
+ timedelta(hours=24),
},
),
file_kw={'status': amo.STATUS_AWAITING_REVIEW},
channel=amo.CHANNEL_UNLISTED,
).addon,
file_kw={'status': amo.STATUS_AWAITING_REVIEW},
)
addon_factory(
name='Pure unlisted with auto-approval delayed',
reviewer_flags={
'auto_approval_delayed_until_unlisted': datetime.now()
+ timedelta(hours=24),
},
file_kw={'status': amo.STATUS_AWAITING_REVIEW},
version_kw={'channel': amo.CHANNEL_UNLISTED},
)
addons = Addon.unfiltered.get_queryset_for_pending_queues(
show_temporarily_delayed=False
)
expected_addons.remove(addon_needing_admin_code_review)
assert set(addons) == set(expected_addons)
def test_get_pending_rejection_queue(self):
expected_addons = [
version_review_flags_factory(
version=version_factory(
addon=version_review_flags_factory(
version=version_factory(addon=addon_factory()),
pending_rejection=datetime.now() + timedelta(hours=24),
).version.addon,
),
pending_rejection=datetime.now() + timedelta(hours=48),
).version.addon,
]
addon_factory()
addons = Addon.objects.get_pending_rejection_queue()
assert set(addons) == set(expected_addons)
# Test that we picked the version with the oldest due date and that we
# added the first_pending_version property.
for addon in addons:
expected_version = (
addon.versions(manager='unfiltered_for_relations')
.filter(reviewerflags__pending_rejection__isnull=False)
.order_by('reviewerflags__pending_rejection')
.first()
)
assert expected_version
assert addon.first_pending_version == expected_version
class TestThemesPendingManualApprovalQueue(TestCase):
def test_basic(self):

Просмотреть файл

@ -45,6 +45,8 @@ ADDON_REVIEWER_MOTD_EDIT = AclPermission('AddonReviewerMOTD', 'Edit')
STATIC_THEMES_REVIEW = AclPermission('Addons', 'ThemeReview')
# Can review recommend(ed|able) add-ons
ADDONS_RECOMMENDED_REVIEW = AclPermission('Addons', 'RecommendedReview')
# Can triage (and therefore see in the queues) add-ons with a temporary delay
ADDONS_TRIAGE_DELAYED = AclPermission('Addons', 'TriageDelayed')
# Can edit all collections.
COLLECTIONS_EDIT = AclPermission('Collections', 'Edit')

Просмотреть файл

@ -31,32 +31,47 @@ log = olympia.core.logger.getLogger('z.reviewers')
VIEW_QUEUE_FLAGS = (
(
'needs_human_review',
'Needs Human Review',
),
(
'needs_admin_code_review',
'needs-admin-code-review',
'Needs Admin Code Review',
),
(
'needs_admin_content_review',
'needs-admin-content-review',
'Needs Admin Content Review',
),
(
'needs_admin_theme_review',
'needs-admin-theme-review',
'Needs Admin Static Theme Review',
),
('sources_provided', 'sources-provided', 'Sources provided'),
('sources_provided', 'Source Code Provided'),
(
'auto_approval_disabled',
'Auto-approval disabled',
),
(
'auto_approval_delayed_temporarily',
'auto-approval-delayed-temporarily',
'Auto-approval delayed temporarily',
),
(
'auto_approval_delayed_indefinitely',
'auto-approval-delayed-indefinitely',
'Auto-approval delayed indefinitely',
),
(
'auto_approval_disabled_unlisted',
'Unlisted Auto-approval disabled',
),
(
'auto_approval_delayed_temporarily_unlisted',
'Unlisted Auto-approval delayed temporarily',
),
(
'auto_approval_delayed_indefinitely_unlisted',
'Unlisted Auto-approval delayed indefinitely',
),
)
@ -105,10 +120,24 @@ class CannedResponse(ModelBase):
def get_flags(addon, version):
"""Return a list of tuples (indicating which flags should be displayed for
a particular add-on."""
flag_filters_by_channel = {
amo.CHANNEL_UNLISTED: (
'auto_approval_disabled',
'auto_approval_delayed_temporarily',
'auto_approval_delayed_indefinitely',
),
amo.CHANNEL_LISTED: (
'auto_approval_disabled_unlisted',
'auto_approval_delayed_temporarily_unlisted',
'auto_approval_delayed_indefinitely_unlisted',
),
}
flags = [
(cls, title)
for (prop, cls, title) in VIEW_QUEUE_FLAGS
(prop.replace('_', '-'), title)
for (prop, title) in VIEW_QUEUE_FLAGS
if getattr(version, prop, getattr(addon, prop, None))
and prop
not in flag_filters_by_channel.get(getattr(version, 'channel', None), ())
]
# add in the promoted group flag and return
if promoted := addon.promoted_group(currently_approved=False):

Просмотреть файл

@ -43,6 +43,7 @@ class AddonReviewerFlagsSerializer(AMOModelSerializer):
model = AddonReviewerFlags
fields = (
'auto_approval_delayed_until',
'auto_approval_delayed_until_unlisted',
'auto_approval_disabled',
'auto_approval_disabled_unlisted',
'auto_approval_disabled_until_next_approval',

Просмотреть файл

@ -112,7 +112,7 @@
</thead>
<tbody>
{% for row in page.object_list %}
<tr data-addon="{{ row.record.addon_id or row.record.id }}" class="addon-row" id="addon-{{ row.record.addon_id or row.record.id }}" data-review-log="{{ row.record.review_log_id }}">
<tr data-addon="{{ row.record.addon_id or row.record.id }}" class="addon-row {{ table.render_flags_classes(row.record) }}" id="addon-{{ row.record.addon_id or row.record.id }}" data-review-log="{{ row.record.review_log_id }}">
<td><div class="addon-locked"></div></td>
{% for value in row %}
<td>{{ value }}</td>

Просмотреть файл

@ -363,6 +363,16 @@
id="clear_auto_approval_delayed_until" class="oneoff" type="button">Clear Auto-Approval Extra Delay</button>
</li>
{% endif %}
{% if addon.auto_approval_delayed_until_unlisted and addon.auto_approval_delayed_until_unlisted > now %}
{# This flag is only meant to be enabled by scanners/new submissions automatically, but admins can still unset it. #}
<li>
<button data-api-url="{{ drf_url('reviewers-addon-flags', addon.pk) }}"
data-api-method="patch"
data-api-data="{&quot;auto_approval_delayed_until_unlisted&quot;: null}"
title="Unlisted Auto-Approval currently disabled until {{ addon.auto_approval_delayed_until_unlisted|datetime }}"
id="clear_auto_approval_delayed_until_unlisted" class="oneoff" type="button">Clear Unlisted Auto-Approval Extra Delay</button>
</li>
{% endif %}
{% endif %}
{% if has_versions_pending_rejection %}
@ -471,7 +481,7 @@
{% endfor %}
</ul>
{% if flags: %}
{% if flags %}
<strong>Flags</strong>
<ul>
{% for cls, title in flags %}

Просмотреть файл

@ -30,6 +30,7 @@ from olympia.reviewers.models import (
CannedResponse,
ReviewActionReason,
ReviewerSubscription,
get_flags,
send_notifications,
set_reviewing_cache,
)
@ -1579,3 +1580,144 @@ class TestReviewActionReason(TestCase):
assert reason.name == name
assert reason.__str__() == name
assert not reason.is_active
class TestGetFlags(TestCase):
def setUp(self):
self.addon = addon_factory()
def test_none(self):
assert get_flags(self.addon, self.addon.current_version) == []
AddonReviewerFlags.objects.create(addon=self.addon)
assert get_flags(self.addon, self.addon.current_version) == []
def test_listed_version_single_flag(self):
AddonReviewerFlags.objects.create(
addon=self.addon,
auto_approval_disabled=True,
)
expected_flags = [
('auto-approval-disabled', 'Auto-approval disabled'),
]
assert get_flags(self.addon, self.addon.current_version) == expected_flags
def test_listed_version(self):
AddonReviewerFlags.objects.create(
addon=self.addon,
auto_approval_disabled=True,
auto_approval_delayed_until=datetime.now() + timedelta(hours=2),
needs_admin_content_review=True,
needs_admin_code_review=True,
needs_admin_theme_review=True,
)
self.addon.current_version.update(
source='something.zip', needs_human_review=True
)
expected_flags = [
('needs-human-review', 'Needs Human Review'),
('needs-admin-code-review', 'Needs Admin Code Review'),
('needs-admin-content-review', 'Needs Admin Content Review'),
('needs-admin-theme-review', 'Needs Admin Static Theme Review'),
('sources-provided', 'Source Code Provided'),
('auto-approval-disabled', 'Auto-approval disabled'),
('auto-approval-delayed-temporarily', 'Auto-approval delayed temporarily'),
]
assert get_flags(self.addon, self.addon.current_version) == expected_flags
# With infinite delay.
self.addon.reviewerflags.update(auto_approval_delayed_until=datetime.max)
expected_flags = [
('needs-human-review', 'Needs Human Review'),
('needs-admin-code-review', 'Needs Admin Code Review'),
('needs-admin-content-review', 'Needs Admin Content Review'),
('needs-admin-theme-review', 'Needs Admin Static Theme Review'),
('sources-provided', 'Source Code Provided'),
('auto-approval-disabled', 'Auto-approval disabled'),
(
'auto-approval-delayed-indefinitely',
'Auto-approval delayed indefinitely',
),
]
assert get_flags(self.addon, self.addon.current_version) == expected_flags
# Adding unlisted flags doesn't matter.
self.addon.reviewerflags.update(
auto_approval_disabled_unlisted=True,
auto_approval_delayed_until_unlisted=datetime.now() + timedelta(hours=2),
)
assert get_flags(self.addon, self.addon.current_version) == expected_flags
def test_unlisted_version(self):
version = self.addon.current_version
version.update(
channel=amo.CHANNEL_UNLISTED,
source='something.zip',
needs_human_review=True,
)
AddonReviewerFlags.objects.create(
addon=self.addon,
auto_approval_disabled_unlisted=True,
auto_approval_delayed_until_unlisted=datetime.now() + timedelta(hours=2),
needs_admin_content_review=True,
needs_admin_code_review=True,
needs_admin_theme_review=True,
)
expected_flags = [
('needs-human-review', 'Needs Human Review'),
('needs-admin-code-review', 'Needs Admin Code Review'),
('needs-admin-content-review', 'Needs Admin Content Review'),
('needs-admin-theme-review', 'Needs Admin Static Theme Review'),
('sources-provided', 'Source Code Provided'),
('auto-approval-disabled-unlisted', 'Unlisted Auto-approval disabled'),
(
'auto-approval-delayed-temporarily-unlisted',
'Unlisted Auto-approval delayed temporarily',
),
]
assert get_flags(self.addon, version) == expected_flags
# With infinite delay.
self.addon.reviewerflags.update(
auto_approval_delayed_until_unlisted=datetime.max
)
expected_flags = [
('needs-human-review', 'Needs Human Review'),
('needs-admin-code-review', 'Needs Admin Code Review'),
('needs-admin-content-review', 'Needs Admin Content Review'),
('needs-admin-theme-review', 'Needs Admin Static Theme Review'),
('sources-provided', 'Source Code Provided'),
('auto-approval-disabled-unlisted', 'Unlisted Auto-approval disabled'),
(
'auto-approval-delayed-indefinitely-unlisted',
'Unlisted Auto-approval delayed indefinitely',
),
]
assert get_flags(self.addon, version) == expected_flags
# Adding listed flags doesn't matter.
self.addon.reviewerflags.update(
auto_approval_disabled=True,
auto_approval_delayed_until=datetime.now() + timedelta(hours=2),
)
assert get_flags(self.addon, version) == expected_flags
def test_listed_version_no_flags(self):
AddonReviewerFlags.objects.create(
addon=self.addon,
auto_approval_disabled_unlisted=True,
auto_approval_delayed_until_unlisted=datetime.now() + timedelta(hours=2),
)
assert get_flags(self.addon, self.addon.current_version) == []
def test_unlisted_version_no_flags(self):
version = self.addon.current_version
version.update(channel=amo.CHANNEL_UNLISTED)
AddonReviewerFlags.objects.create(
addon=self.addon,
auto_approval_disabled=True,
auto_approval_delayed_until=datetime.now() + timedelta(hours=2),
)
assert get_flags(self.addon, version) == []
def test_version_none(self):
assert get_flags(self.addon, None) == []

Просмотреть файл

@ -1571,7 +1571,35 @@ class TestExtensionQueue(QueueTest):
self.expected_versions = self.get_expected_versions(self.expected_addons)
self._test_results()
def test_webextension_with_auto_approval_delayed(self):
def test_webextension_with_auto_approval_delayed_and_no_triage_permission(self):
self.generate_files()
AddonReviewerFlags.objects.create(
addon=self.addons['Pending One'],
auto_approval_delayed_until=datetime.now() + timedelta(hours=24),
)
AddonReviewerFlags.objects.create(
addon=self.addons['Nominated One'],
auto_approval_delayed_until=datetime.now() + timedelta(hours=24),
)
AddonReviewerFlags.objects.create(
addon=self.addons['Pending Two'],
auto_approval_delayed_until=None,
auto_approval_disabled=True,
)
AddonReviewerFlags.objects.create(
addon=self.addons['Nominated Two'],
auto_approval_delayed_until=None,
auto_approval_disabled=True,
)
self.expected_addons = [
self.addons['Nominated Two'],
self.addons['Pending Two'],
]
self.expected_versions = self.get_expected_versions(self.expected_addons)
self._test_results()
def test_webextension_with_auto_approval_delayed_with_triage_permission(self):
self.grant_permission(self.user, 'Addons:TriageDelayed')
self.generate_files()
AddonReviewerFlags.objects.create(
addon=self.addons['Pending One'],
@ -3282,6 +3310,7 @@ class TestReview(ReviewBase):
assert not doc('#disable_auto_approval_until_next_approval_unlisted')
assert not doc('#enable_auto_approval_until_next_approval_unlisted')
assert not doc('#clear_auto_approval_delayed_until')
assert not doc('#clear_auto_approval_delayed_until_unlisted')
assert not doc('#clear_pending_rejections')
assert not doc('#deny_resubmission')
assert not doc('#allow_resubmission')
@ -3301,6 +3330,7 @@ class TestReview(ReviewBase):
# Not present because it hasn't been set yet
assert not doc('#clear_auto_approval_delayed_until')
assert not doc('#clear_auto_approval_delayed_until_unlisted')
flags = AddonReviewerFlags.objects.create(
addon=self.addon, auto_approval_delayed_until=self.days_ago(1)
@ -3317,6 +3347,25 @@ class TestReview(ReviewBase):
assert response.status_code == 200
doc = pq(response.content)
assert doc('#clear_auto_approval_delayed_until')
assert not doc('#clear_auto_approval_delayed_until_unlisted')
flags.update(auto_approval_delayed_until_unlisted=self.days_ago(42))
response = self.client.get(self.url)
assert response.status_code == 200
doc = pq(response.content)
# Still not present because it's in the past.
assert not doc('#clear_auto_approval_delayed_until_unlisted')
# Listed flag should still be there however, that delay has changed.
assert doc('#clear_auto_approval_delayed_until')
flags.update(
auto_approval_delayed_until_unlisted=datetime.now() + timedelta(hours=24)
)
response = self.client.get(self.url)
assert response.status_code == 200
doc = pq(response.content)
assert doc('#clear_auto_approval_delayed_until') # Is still there.
assert doc('#clear_auto_approval_delayed_until_unlisted') # Was added.
def test_no_resubmission_buttons_when_addon_is_not_deleted(self):
self.login_as_admin()
@ -6486,6 +6535,7 @@ class TestAddonReviewerViewSet(TestCase):
auto_approval_disabled=True,
auto_approval_disabled_unlisted=True,
auto_approval_delayed_until=self.days_ago(42),
auto_approval_delayed_until_unlisted=self.days_ago(0),
)
self.grant_permission(self.user, 'Reviews:Admin')
self.client.login_api(self.user)
@ -6495,6 +6545,7 @@ class TestAddonReviewerViewSet(TestCase):
'auto_approval_disabled_until_next_approval': True,
'auto_approval_disabled_until_next_approval_unlisted': True,
'auto_approval_delayed_until': None,
'auto_approval_delayed_until_unlisted': None,
'needs_admin_code_review': True,
'needs_admin_content_review': True,
'needs_admin_theme_review': True,
@ -6510,6 +6561,7 @@ class TestAddonReviewerViewSet(TestCase):
reviewer_flags.auto_approval_disabled_until_next_approval_unlisted is True
)
assert reviewer_flags.auto_approval_delayed_until is None
assert reviewer_flags.auto_approval_delayed_until_unlisted is None
assert reviewer_flags.needs_admin_code_review is True
assert reviewer_flags.needs_admin_content_review is True
assert reviewer_flags.needs_admin_theme_review is True

Просмотреть файл

@ -35,19 +35,11 @@ import markupsafe
log = olympia.core.logger.getLogger('z.mailer')
class ItemStateTable:
def increment_item(self):
self.item_number += 1
def set_page(self, page):
self.item_number = page.start_index()
def is_admin_reviewer(user):
return acl.action_allowed_for(user, amo.permissions.REVIEWS_ADMIN)
def safe_substitute(string, *args):
return string % tuple(markupsafe.escape(arg) for arg in args)
class AddonQueueTable(tables.Table, ItemStateTable):
class AddonQueueTable(tables.Table):
addon_name = tables.Column(verbose_name='Add-on', accessor='name', orderable=False)
# Override empty_values for flags so that they can be displayed even if the
# model does not have a flags attribute.
@ -89,6 +81,11 @@ class AddonQueueTable(tables.Table, ItemStateTable):
def get_version(self, record):
return record.current_version
def render_flags_classes(self, record):
if not hasattr(record, 'flags'):
record.flags = get_flags(record, self.get_version(record))
return ' '.join(flag[0] for flag in record.flags)
def render_flags(self, record):
if not hasattr(record, 'flags'):
record.flags = get_flags(record, self.get_version(record))
@ -107,7 +104,6 @@ class AddonQueueTable(tables.Table, ItemStateTable):
def render_addon_name(self, record):
url = self._get_addon_name_url(record)
self.increment_item()
return markupsafe.Markup(
'<a href="%s">%s <em>%s</em></a>'
% (
@ -157,10 +153,14 @@ class PendingManualApprovalQueueTable(AddonQueueTable):
)
@classmethod
def get_queryset(cls, admin_reviewer=False):
return Addon.unfiltered.get_queryset_for_pending_queues(
admin_reviewer=admin_reviewer
def get_queryset(self, request):
qs = Addon.unfiltered.get_queryset_for_pending_queues(
admin_reviewer=is_admin_reviewer(request.user),
show_temporarily_delayed=acl.action_allowed_for(
request.user, amo.permissions.ADDONS_TRIAGE_DELAYED
),
)
return qs
def get_version(self, record):
# Use the property set by get_queryset_for_pending_queues() to display
@ -196,17 +196,17 @@ class NewThemesQueueTable(PendingManualApprovalQueueTable):
)
@classmethod
def get_queryset(cls, admin_reviewer=False):
def get_queryset(cls, request):
return Addon.objects.get_queryset_for_pending_queues(
admin_reviewer=admin_reviewer, theme_review=True
admin_reviewer=is_admin_reviewer(request.user), theme_review=True
).filter(status__in=(amo.STATUS_NOMINATED,))
class UpdatedThemesQueueTable(NewThemesQueueTable):
@classmethod
def get_queryset(cls, admin_reviewer=False):
def get_queryset(cls, request):
return Addon.objects.get_queryset_for_pending_queues(
admin_reviewer=admin_reviewer, theme_review=True
admin_reviewer=is_admin_reviewer(request.user), theme_review=True
).filter(status__in=(amo.STATUS_APPROVED,))
@ -246,8 +246,10 @@ class PendingRejectionTable(AddonQueueTable):
exclude = ('due_date',)
@classmethod
def get_queryset(cls, admin_reviewer=False):
return Addon.objects.get_pending_rejection_queue(admin_reviewer=admin_reviewer)
def get_queryset(cls, request):
return Addon.objects.get_pending_rejection_queue(
admin_reviewer=is_admin_reviewer(request.user)
)
def get_version(self, record):
# Use the property set by get_pending_rejection_queue() to display
@ -273,8 +275,10 @@ class ContentReviewTable(AddonQueueTable):
orderable = False
@classmethod
def get_queryset(cls, admin_reviewer=False):
return Addon.objects.get_content_review_queue(admin_reviewer=admin_reviewer)
def get_queryset(cls, request):
return Addon.objects.get_content_review_queue(
admin_reviewer=is_admin_reviewer(request.user)
)
def render_last_updated(self, value):
return naturaltime(value) if value else ''
@ -289,9 +293,9 @@ class HumanReviewTable(AddonQueueTable):
show_count_in_dashboard = False
@classmethod
def get_queryset(cls, admin_reviewer=False):
def get_queryset(cls, request):
return Addon.objects.get_human_review_queue(
admin_reviewer=admin_reviewer
admin_reviewer=is_admin_reviewer(request.user)
).annotate(
unlisted_versions_that_need_human_review=Count(
'versions',
@ -345,8 +349,10 @@ class MadReviewTable(HumanReviewTable):
show_count_in_dashboard = False
@classmethod
def get_queryset(cls, admin_reviewer=False):
return Addon.objects.get_mad_queue(admin_reviewer=admin_reviewer).annotate(
def get_queryset(cls, request):
return Addon.objects.get_mad_queue(
admin_reviewer=is_admin_reviewer(request.user)
).annotate(
unlisted_versions_that_need_human_review=Count(
'versions',
filter=Q(
@ -486,8 +492,8 @@ class ReviewHelper:
is_appropriate_reviewer_post_review = acl.action_allowed_for(
self.user, permission_post_review
)
is_admin_reviewer = is_appropriate_reviewer and acl.action_allowed_for(
self.user, amo.permissions.REVIEWS_ADMIN
is_appropriate_admin_reviewer = is_appropriate_reviewer and is_admin_reviewer(
self.user
)
addon_is_not_disabled_or_deleted = self.addon.status not in (
@ -661,7 +667,9 @@ class ReviewHelper:
'developer.'
),
'comments': False,
'available': (addon_is_not_disabled_or_deleted and is_admin_reviewer),
'available': (
addon_is_not_disabled_or_deleted and is_appropriate_admin_reviewer
),
}
actions['block_multiple_versions'] = {
'method': self.handler.block_multiple_versions,

Просмотреть файл

@ -187,8 +187,7 @@ def dashboard(request):
view_all = any(
acl.action_allowed_for(request.user, perm) for perm in view_all_permissions
)
admin_reviewer = is_admin_reviewer(request)
queue_counts = fetch_queue_counts(admin_reviewer=admin_reviewer)
queue_counts = fetch_queue_counts(request)
if view_all or acl.action_allowed_for(request.user, amo.permissions.ADDONS_REVIEW):
sections['Manual Review'] = [
@ -308,30 +307,9 @@ def save_motd(request):
return TemplateResponse(request, 'reviewers/motd.html', context=data)
def is_admin_reviewer(request):
return acl.action_allowed_for(request.user, amo.permissions.REVIEWS_ADMIN)
def filter_admin_review_for_legacy_queue(qs):
return qs.filter(
Q(needs_admin_code_review=None) | Q(needs_admin_code_review=False),
Q(needs_admin_theme_review=None) | Q(needs_admin_theme_review=False),
)
def _queue(request, tab, unlisted=False):
admin_reviewer = is_admin_reviewer(request)
TableObj = reviewer_tables_registry[tab]
qs = TableObj.get_queryset(admin_reviewer=admin_reviewer)
admin_reviewer = is_admin_reviewer(request)
# Those restrictions will only work with our RawSQLModel, so we need to
# make sure we're not dealing with a regular Django ORM queryset first.
if hasattr(qs, 'sql_model'):
if not admin_reviewer:
qs = filter_admin_review_for_legacy_queue(qs)
qs = TableObj.get_queryset(request=request)
params = {}
order_by = request.GET.get('sort')
if order_by is None and hasattr(TableObj, 'default_order_by'):
@ -348,7 +326,6 @@ def _queue(request, tab, unlisted=False):
per_page = REVIEWS_PER_PAGE
count = construct_count_queryset_from_queryset(qs)()
page = paginate(request, table.rows, per_page=per_page, count=count)
table.set_page(page)
return TemplateResponse(
request,
@ -382,14 +359,12 @@ def construct_count_queryset_from_queryset(qs):
return qs.values('pk').order_by().count
def fetch_queue_counts(admin_reviewer):
def count_from_registered_table(table, *, admin_reviewer):
return construct_count_queryset_from_queryset(
table.get_queryset(admin_reviewer=admin_reviewer)
)
def fetch_queue_counts(request):
def count_from_registered_table(table, *, request):
return construct_count_queryset_from_queryset(table.get_queryset(request))
counts = {
key: count_from_registered_table(table, admin_reviewer=admin_reviewer)
key: count_from_registered_table(table, request=request)
for key, table in reviewer_tables_registry.items()
if table.show_count_in_dashboard
}

Просмотреть файл

@ -18,7 +18,7 @@
/* sprites for data grid icons */
.app-icon, .platform-icon {
background: url(../../img/developers/reviewer-sprite.png?12) no-repeat top left;
background: url(../../img/developers/reviewer-sprite.png) no-repeat top left;
display: inline-block;
text-decoration: none;
vertical-align: middle;
@ -47,6 +47,11 @@
.ed-sprite-promoted-line { background-position: 0 -368px; }
.ed-sprite-promoted-strategic { background-position: 0 -320px; }
.ed-sprite-promoted-spotlight { background-position: 0 -320px; }
.ed-sprite-auto-approval-delayed-temporarily-unlisted { background-position: 0 -386px; }
.ed-sprite-auto-approval-delayed-indefinitely-unlisted { background-position: 0 -402px; }
.ed-sprite-needs-human-review { background-position: 0 -418px; }
.ed-sprite-auto-approval-disabled { background-position: 0 -434px }
.ed-sprite-auto-approval-disabled-unlisted { background-position: 0 -450px }
.platform-icon {
background: url(../../img/developers/platforms.png?8) no-repeat top left;
@ -199,6 +204,14 @@ table.data-grid tbody tr:nth-child(odd) {
table.data-grid tbody tr:nth-child(even) {
background-color: #FFFFFF;
}
table.data-grid tbody tr.auto-approval-delayed-temporarily:nth-child(odd),
table.data-grid tbody tr.auto-approval-delayed-temporarily-unlisted:nth-child(odd) {
background-color: #FAF5E4;
}
table.data-grid tbody tr.auto-approval-delayed-temporarily:nth-child(even),
table.data-grid tbody tr.auto-approval-delayed-temporarily-unlisted:nth-child(even) {
background-color: #FFFDF5;
}
table.data-grid .addon-locked {
width: 16px;
height: 16px;

Двоичные данные
static/img/developers/reviewer-sprite.png

Двоичный файл не отображается.

До

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

После

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