Only disable auto approval until next manual one on human rejections, not automatic (#22397)

This commit is contained in:
Mathieu Pillard 2024-06-24 11:36:05 +02:00 коммит произвёл GitHub
Родитель c2c053909b
Коммит ebf179242b
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: B5690EEEBB952194
5 изменённых файлов: 89 добавлений и 31 удалений

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

@ -1170,6 +1170,16 @@ class AutoRejectTestsMixin:
def days_ago(self, days):
return days_ago(days)
def _ensure_auto_approval_until_next_approval_is_not_set(self):
# We shouldn't have disabled auto-approval until next approval when
# performing automatic rejections.
try:
self.addon.reviewerflags.reload()
except AddonReviewerFlags.DoesNotExist:
pass
assert not self.addon.auto_approval_disabled_until_next_approval
assert not self.addon.auto_approval_disabled_until_next_approval_unlisted
class TestAutoReject(AutoRejectTestsMixin, TestCase):
def test_prevent_multiple_runs_in_parallel(self):
@ -1277,6 +1287,7 @@ class TestAutoReject(AutoRejectTestsMixin, TestCase):
assert not VersionReviewerFlags.objects.filter(
pending_rejection__isnull=False
).exists()
self._ensure_auto_approval_until_next_approval_is_not_set()
def _test_reject_versions(self, *, activity_logs_to_keep=None):
if activity_logs_to_keep is None:
@ -1340,6 +1351,7 @@ class TestAutoReject(AutoRejectTestsMixin, TestCase):
def test_reject_versions(self):
self._test_reject_versions()
self._ensure_auto_approval_until_next_approval_is_not_set()
assert len(mail.outbox) == 1
assert 'right to appeal' in mail.outbox[0].body

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

@ -2157,13 +2157,17 @@ class TestReviewHelper(TestReviewHelperBase):
self.addon.update(status=amo.STATUS_NOMINATED)
assert self.get_helper()
def _test_reject_multiple_versions(self, extra_data):
def _test_reject_multiple_versions(self, extra_data, human_review=True):
old_version = self.review_version
self.review_version = version_factory(addon=self.addon, version='3.0')
AutoApprovalSummary.objects.create(
version=self.review_version, verdict=amo.AUTO_APPROVED, weight=101
)
self.setup_data(amo.STATUS_APPROVED, file_status=amo.STATUS_APPROVED)
self.setup_data(
amo.STATUS_APPROVED,
file_status=amo.STATUS_APPROVED,
human_review=human_review,
)
# Safeguards.
assert isinstance(self.helper.handler, ReviewFiles)
@ -2187,8 +2191,11 @@ class TestReviewHelper(TestReviewHelperBase):
for version in self.addon.versions.all():
assert version.pending_rejection is None
assert version.pending_rejection_by is None
assert version.reviewerflags.pending_content_rejection is None
assert version.pending_content_rejection is None
if human_review:
assert version.reload().human_review_date
else:
assert version.reload().human_review_date is None
assert len(mail.outbox) == 1
message = mail.outbox[0]
@ -2196,6 +2203,7 @@ class TestReviewHelper(TestReviewHelperBase):
log_token = ActivityLogToken.objects.get()
assert log_token.uuid.hex in message.reply_to[0]
if human_review:
assert self.check_log_count(amo.LOG.REJECT_VERSION.id) == 1
assert self.check_log_count(amo.LOG.REJECT_CONTENT.id) == 0
@ -2206,11 +2214,18 @@ class TestReviewHelper(TestReviewHelperBase):
)
assert log.arguments == [self.addon, self.review_version, old_version]
# listed auto approvals should be disabled until the next manual approval.
# listed auto approvals should be disabled until the next manual
# approval.
flags = self.addon.reviewerflags
flags.reload()
assert not flags.auto_approval_disabled_until_next_approval_unlisted
assert flags.auto_approval_disabled_until_next_approval
else:
# For non-human, automatic rejections auto approvals should _not_
# be disabled until the next manual approval.
assert not AddonReviewerFlags.objects.filter(addon=self.addon).exists()
assert not self.addon.auto_approval_disabled_until_next_approval_unlisted
assert not self.addon.auto_approval_disabled_until_next_approval
def test_reject_multiple_versions(self):
self._test_reject_multiple_versions({})
@ -2219,6 +2234,9 @@ class TestReviewHelper(TestReviewHelperBase):
assert 'versions of your Extension have been disabled' in message.body
assert 'received from a third party' not in message.body
def test_reject_multiple_versions_non_human(self):
self._test_reject_multiple_versions({}, human_review=False)
def test_reject_multiple_versions_resolving_abuse_report(self):
cinder_job = CinderJob.objects.create(job_id='1')
NeedsHumanReview.objects.create(

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

@ -1365,22 +1365,23 @@ class ReviewBase:
extra_details=extra_details,
)
# A rejection (delayed or not) implies the next version should be
# manually reviewed.
addonreviewerflags = {}
# A human rejection (delayed or not) implies the next version in the
# same channel should be manually reviewed.
if self.human_review:
auto_approval_disabled_until_next_approval_flag = (
'auto_approval_disabled_until_next_approval'
if channel == amo.CHANNEL_LISTED
else 'auto_approval_disabled_until_next_approval_unlisted'
)
addonreviewerflags = {
auto_approval_disabled_until_next_approval_flag: True,
}
addonreviewerflags[auto_approval_disabled_until_next_approval_flag] = True
if pending_rejection_deadline:
# Developers should be notified again once the deadline is close.
addonreviewerflags['notified_about_expiring_delayed_rejections'] = False
else:
# An immediate rejection might require the add-on status to change.
self.addon.update_status()
if addonreviewerflags:
AddonReviewerFlags.objects.update_or_create(
addon=self.addon,
defaults=addonreviewerflags,

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

@ -1085,6 +1085,13 @@ class Version(OnChangeMixin, ModelBase):
except VersionReviewerFlags.DoesNotExist:
return None
@property
def pending_content_rejection(self):
try:
return self.reviewerflags.pending_content_rejection
except VersionReviewerFlags.DoesNotExist:
return None
@property
def pending_rejection_by(self):
try:

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

@ -1330,6 +1330,26 @@ class TestVersion(AMOPaths, TestCase):
pending_content_rejection=False,
)
assert version.pending_rejection == in_the_past
assert not version.pending_content_rejection
def test_pending_content_rejection_property(self):
addon = Addon.objects.get(id=3615)
version = addon.current_version
# No flags: None
assert version.pending_content_rejection is None
# Flag present, value is None (default): None.
flags = version_review_flags_factory(version=version)
assert flags.pending_content_rejection is None
assert version.pending_content_rejection is None
# Flag present, value is a date.
in_the_past = self.days_ago(1)
flags.update(
pending_rejection=in_the_past,
pending_rejection_by=user_factory(),
pending_content_rejection=True,
)
assert version.pending_rejection == in_the_past
assert version.pending_content_rejection
def test_pending_rejection_by_property(self):
addon = Addon.objects.get(id=3615)