support webhook addon decisions (#21408)

* Record different reasons for a DISABLED override of an original status

* restore file statuses on force_enable

* add to test_ban_and_disable_related_content_bulk

* Apply suggestions from code review

Co-authored-by: Mathieu Pillard <diox@users.noreply.github.com>

* restore bulk File disabling

* move activity log inside conditions in is_user_disabled setter

---------

Co-authored-by: Mathieu Pillard <diox@users.noreply.github.com>
This commit is contained in:
Andrew Williamson 2023-11-08 15:23:51 +00:00 коммит произвёл GitHub
Родитель 52c0ec13a8
Коммит 33da485b0a
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
18 изменённых файлов: 226 добавлений и 47 удалений

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

@ -78,7 +78,6 @@ class CinderActionApprove(CinderAction):
target = self.abuse_report.target target = self.abuse_report.target
if isinstance(target, Addon) and target.status == amo.STATUS_DISABLED: if isinstance(target, Addon) and target.status == amo.STATUS_DISABLED:
target.force_enable() target.force_enable()
# TODO: renable versions
self.notify_targets(target.authors.all()) self.notify_targets(target.authors.all())
elif isinstance(target, UserProfile) and target.banned: elif isinstance(target, UserProfile) and target.banned:

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

@ -652,7 +652,7 @@ class Addon(OnChangeMixin, ModelBase):
).update(is_active=False) ).update(is_active=False)
self.update_all_due_dates() self.update_all_due_dates()
# https://github.com/mozilla/addons-server/issues/13194 # https://github.com/mozilla/addons-server/issues/13194
self.disable_all_files() Addon.disable_all_files([self], File.STATUS_DISABLED_REASONS.ADDON_DISABLE)
def force_enable(self, skip_activity_log=False): def force_enable(self, skip_activity_log=False):
if not skip_activity_log: if not skip_activity_log:
@ -660,6 +660,16 @@ class Addon(OnChangeMixin, ModelBase):
log.info( log.info(
'Addon "%s" status force-changed to: %s', self.slug, amo.STATUS_APPROVED 'Addon "%s" status force-changed to: %s', self.slug, amo.STATUS_APPROVED
) )
qs = File.objects.filter(
version__addon=self,
status=amo.STATUS_DISABLED,
status_disabled_reason=File.STATUS_DISABLED_REASONS.ADDON_DISABLE,
).exclude(original_status=amo.STATUS_NULL)
qs.update(status=F('original_status'))
qs.update(
status_disabled_reason=File.STATUS_DISABLED_REASONS.NONE,
original_status=amo.STATUS_NULL,
)
self.update(status=amo.STATUS_APPROVED) self.update(status=amo.STATUS_APPROVED)
# Call update_status() to fix the status if the add-on is not actually # Call update_status() to fix the status if the add-on is not actually
# in a state that allows it to be public. # in a state that allows it to be public.
@ -683,8 +693,16 @@ class Addon(OnChangeMixin, ModelBase):
log.info('Allow resubmission for addon "%s"', self.slug) log.info('Allow resubmission for addon "%s"', self.slug)
DeniedGuid.objects.filter(guid=self.guid).delete() DeniedGuid.objects.filter(guid=self.guid).delete()
def disable_all_files(self): @classmethod
File.objects.filter(version__addon=self).update(status=amo.STATUS_DISABLED) def disable_all_files(cls, addons, reason):
qs = File.objects.filter(version__addon__in=addons).exclude(
status=amo.STATUS_DISABLED
)
qs.update(original_status=F('status'))
qs.update(
status=amo.STATUS_DISABLED,
status_disabled_reason=reason,
)
def set_needs_human_review_on_latest_versions( def set_needs_human_review_on_latest_versions(
self, *, reason, due_date=None, ignore_reviewed=True, unique_reason=False self, *, reason, due_date=None, ignore_reviewed=True, unique_reason=False
@ -843,7 +861,7 @@ class Addon(OnChangeMixin, ModelBase):
rating.delete(skip_activity_log=True) rating.delete(skip_activity_log=True)
# We avoid triggering signals for Version & File on purpose to # We avoid triggering signals for Version & File on purpose to
# avoid extra work. # avoid extra work.
self.disable_all_files() Addon.disable_all_files([self], File.STATUS_DISABLED_REASONS.ADDON_DELETE)
self.versions.all().update(deleted=True) self.versions.all().update(deleted=True)
VersionReviewerFlags.objects.filter(version__addon=self).update( VersionReviewerFlags.objects.filter(version__addon=self).update(
@ -1912,7 +1930,11 @@ def watch_status(old_attr=None, new_attr=None, instance=None, sender=None, **kwa
# review should be disabled right away, we don't want reviewers to look # review should be disabled right away, we don't want reviewers to look
# at it. That might in turn change the add-on status from NOMINATED # at it. That might in turn change the add-on status from NOMINATED
# back to NULL, through update_status(). # back to NULL, through update_status().
latest_version.file.update(status=amo.STATUS_DISABLED) latest_version.file.update(
status=amo.STATUS_DISABLED,
original_status=latest_version.file.status,
status_disabled_reason=File.STATUS_DISABLED_REASONS.DEVELOPER,
)
instance.update_status() instance.update_status()
elif old_status == amo.STATUS_NOMINATED: elif old_status == amo.STATUS_NOMINATED:
# Update latest version due date if necessary for nominated add-ons. # Update latest version due date if necessary for nominated add-ons.

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

@ -422,7 +422,7 @@ def disable_addons(addon_ids, **kw):
for addon in addons: for addon in addons:
activity.log_create(amo.LOG.FORCE_DISABLE, addon, user=get_task_user()) activity.log_create(amo.LOG.FORCE_DISABLE, addon, user=get_task_user())
addons.update(status=amo.STATUS_DISABLED, _current_version=None) addons.update(status=amo.STATUS_DISABLED, _current_version=None)
File.objects.filter(version__addon__in=addons).update(status=amo.STATUS_DISABLED) Addon.disable_all_files(addons, File.STATUS_DISABLED_REASONS.ADDON_DISABLE)
index_addons.delay(addon_ids) index_addons.delay(addon_ids)

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

@ -753,6 +753,10 @@ class TestAddonModels(TestCase):
assert files assert files
for file_ in files: for file_ in files:
assert file_.status == amo.STATUS_DISABLED assert file_.status == amo.STATUS_DISABLED
assert (
file_.status_disabled_reason
== File.STATUS_DISABLED_REASONS.ADDON_DELETE
)
for version in versions: for version in versions:
assert version.deleted assert version.deleted
@ -771,6 +775,13 @@ class TestAddonModels(TestCase):
assert files assert files
for file_ in files: for file_ in files:
assert file_.status != amo.STATUS_DISABLED assert file_.status != amo.STATUS_DISABLED
already_disabled_version = version_factory(
addon=addon,
file_kw={
'status': amo.STATUS_DISABLED,
'status_disabled_reason': File.STATUS_DISABLED_REASONS.DEVELOPER,
},
)
assert version1.due_date assert version1.due_date
assert version2.due_date assert version2.due_date
@ -784,6 +795,20 @@ class TestAddonModels(TestCase):
assert files assert files
for file_ in files: for file_ in files:
assert file_.status == amo.STATUS_DISABLED assert file_.status == amo.STATUS_DISABLED
if file_.version == already_disabled_version:
assert (
file_.status_disabled_reason
== File.STATUS_DISABLED_REASONS.DEVELOPER
)
else:
assert (
file_.status_disabled_reason
== File.STATUS_DISABLED_REASONS.ADDON_DISABLE
)
assert file_.original_status in (
amo.STATUS_APPROVED,
amo.STATUS_AWAITING_REVIEW,
)
assert not file_.version.due_date assert not file_.version.due_date
assert not file_.version.needshumanreview_set.filter( assert not file_.version.needshumanreview_set.filter(
is_active=True is_active=True
@ -801,9 +826,34 @@ class TestAddonModels(TestCase):
def test_force_enable(self): def test_force_enable(self):
core.set_user(UserProfile.objects.get(email='admin@mozilla.com')) core.set_user(UserProfile.objects.get(email='admin@mozilla.com'))
addon = Addon.unfiltered.get(pk=3615) addon = Addon.unfiltered.get(pk=3615)
v1 = addon.current_version
v2 = version_factory(addon=addon)
v3 = version_factory(addon=addon)
addon.update(status=amo.STATUS_DISABLED) addon.update(status=amo.STATUS_DISABLED)
v1.file.update(
status=amo.STATUS_DISABLED,
original_status=amo.STATUS_APPROVED,
# We don't want to re-enable a version the developer disabled
status_disabled_reason=File.STATUS_DISABLED_REASONS.DEVELOPER,
)
v2.file.update(
status=amo.STATUS_DISABLED,
original_status=amo.STATUS_APPROVED,
# we also don't want to re-enable a version we rejected
status_disabled_reason=File.STATUS_DISABLED_REASONS.NONE,
)
v3.file.update(
status=amo.STATUS_DISABLED,
original_status=amo.STATUS_APPROVED,
# but we do want to re-enable a version we only disabled with the addon
status_disabled_reason=File.STATUS_DISABLED_REASONS.ADDON_DISABLE,
)
addon.force_enable() addon.force_enable()
assert addon.status == amo.STATUS_APPROVED assert addon.reload().status == amo.STATUS_APPROVED
assert v1.file.reload().status == amo.STATUS_DISABLED
assert v2.file.reload().status == amo.STATUS_DISABLED
assert v3.file.reload().status == amo.STATUS_APPROVED
log = ActivityLog.objects.latest('pk') log = ActivityLog.objects.latest('pk')
assert log.action == amo.LOG.FORCE_ENABLE.id assert log.action == amo.LOG.FORCE_ENABLE.id
@ -1237,6 +1287,11 @@ class TestAddonModels(TestCase):
file_ = version.file file_ = version.file
file_.reload() file_.reload()
assert version.file.status == amo.STATUS_DISABLED assert version.file.status == amo.STATUS_DISABLED
assert version.file.original_status == amo.STATUS_AWAITING_REVIEW
assert (
version.file.status_disabled_reason
== File.STATUS_DISABLED_REASONS.DEVELOPER
)
assert addon.status == amo.STATUS_NULL assert addon.status == amo.STATUS_NULL
assert addon.is_disabled assert addon.is_disabled

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

@ -522,6 +522,10 @@ def test_disable_addons(index_addons_mock):
assert addon.status == amo.STATUS_DISABLED assert addon.status == amo.STATUS_DISABLED
assert addon.current_version is None assert addon.current_version is None
assert addon.versions.all()[0].file.status == amo.STATUS_DISABLED assert addon.versions.all()[0].file.status == amo.STATUS_DISABLED
assert (
addon.versions.all()[0].file.status_disabled_reason
== File.STATUS_DISABLED_REASONS.ADDON_DISABLE
)
assert ActivityLog.objects.filter( assert ActivityLog.objects.filter(
action=amo.LOG.FORCE_DISABLE.id, addonlog__addon=addon action=amo.LOG.FORCE_DISABLE.id, addonlog__addon=addon

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

@ -26,6 +26,7 @@ from olympia.amo.tests import (
) )
from olympia.applications.models import AppVersion from olympia.applications.models import AppVersion
from olympia.constants.promoted import RECOMMENDED from olympia.constants.promoted import RECOMMENDED
from olympia.files.models import File
from olympia.reviewers.models import AutoApprovalSummary from olympia.reviewers.models import AutoApprovalSummary
from olympia.users.models import Group, UserProfile from olympia.users.models import Group, UserProfile
from olympia.versions.models import ApplicationsVersions, Version, VersionReviewerFlags from olympia.versions.models import ApplicationsVersions, Version, VersionReviewerFlags
@ -258,7 +259,9 @@ class TestVersion(TestCase):
def test_reenable_version(self): def test_reenable_version(self):
Version.objects.get(pk=81551).file.update( Version.objects.get(pk=81551).file.update(
status=amo.STATUS_DISABLED, original_status=amo.STATUS_APPROVED status=amo.STATUS_DISABLED,
original_status=amo.STATUS_APPROVED,
status_disabled_reason=File.STATUS_DISABLED_REASONS.DEVELOPER,
) )
self.reenable_url = reverse('devhub.versions.reenable', args=['a3615']) self.reenable_url = reverse('devhub.versions.reenable', args=['a3615'])
response = self.client.post(self.reenable_url, self.delete_data, follow=True) response = self.client.post(self.reenable_url, self.delete_data, follow=True)

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

@ -45,7 +45,11 @@ class FileAdmin(AMOModelAdmin):
( (
'Details', 'Details',
{ {
'fields': ('cert_serial_num', 'original_status'), 'fields': (
'cert_serial_num',
'original_status',
'status_disabled_reason',
),
}, },
), ),
( (

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

@ -0,0 +1,18 @@
# Generated by Django 4.2.7 on 2023-11-07 13:54
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('files', '0030_remove-add-guid-to-manifest-waffle-switch'),
]
operations = [
migrations.AddField(
model_name='file',
name='status_disabled_reason',
field=models.PositiveSmallIntegerField(choices=[(0, 'None'), (1, 'Developer disabled'), (2, 'Add-on disabled'), (3, 'Add-on deleted'), (4, 'Version deleted')], default=0),
),
]

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

@ -0,0 +1,26 @@
# Generated by Django 4.2.7 on 2023-11-07 15:46
from django.db import migrations
from olympia import amo
from olympia.files.models import File as FileModel
def set_status_disabled_reason(apps, schema_editor):
File = apps.get_model('files', 'File')
(
File.objects.filter(status=amo.STATUS_DISABLED)
.exclude(original_status=amo.STATUS_NULL)
.update(status_disabled_reason=FileModel.STATUS_DISABLED_REASONS.DEVELOPER)
)
class Migration(migrations.Migration):
dependencies = [
('files', '0031_file_status_disabled_reason'),
]
operations = [
migrations.RunPython(set_status_disabled_reason)
]

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

@ -18,6 +18,7 @@ from django.utils.text import slugify
from django.utils.translation import gettext from django.utils.translation import gettext
from django_statsd.clients import statsd from django_statsd.clients import statsd
from extended_choices import Choices
import olympia.core.logger import olympia.core.logger
from olympia import amo, core from olympia import amo, core
@ -76,6 +77,13 @@ def files_storage():
class File(OnChangeMixin, ModelBase): class File(OnChangeMixin, ModelBase):
id = PositiveAutoField(primary_key=True) id = PositiveAutoField(primary_key=True)
STATUS_CHOICES = amo.STATUS_CHOICES_FILE STATUS_CHOICES = amo.STATUS_CHOICES_FILE
STATUS_DISABLED_REASONS = Choices(
('NONE', 0, 'None'),
('DEVELOPER', 1, 'Developer disabled'),
('ADDON_DISABLE', 2, 'Add-on disabled'),
('ADDON_DELETE', 3, 'Add-on deleted'),
('VERSION_DELETE', 4, 'Version deleted'),
)
SUPPORTED_MANIFEST_VERSIONS = ((2, 'Manifest V2'), (3, 'Manifest V3')) SUPPORTED_MANIFEST_VERSIONS = ((2, 'Manifest V2'), (3, 'Manifest V3'))
version = models.OneToOneField('versions.Version', on_delete=models.CASCADE) version = models.OneToOneField('versions.Version', on_delete=models.CASCADE)
@ -106,9 +114,12 @@ class File(OnChangeMixin, ModelBase):
# Is the file a special "Mozilla Signed Extension" # Is the file a special "Mozilla Signed Extension"
# see https://wiki.mozilla.org/Add-ons/InternalSigning # see https://wiki.mozilla.org/Add-ons/InternalSigning
is_mozilla_signed_extension = models.BooleanField(default=False) is_mozilla_signed_extension = models.BooleanField(default=False)
# The user has disabled this file and this was its status. # The file status has been changed to DISABLED - this was its status before.
# STATUS_NULL means the user didn't disable the File - i.e. Mozilla did. # STATUS_NULL means the status hasn't been changed
original_status = models.PositiveSmallIntegerField(default=amo.STATUS_NULL) original_status = models.PositiveSmallIntegerField(default=amo.STATUS_NULL)
status_disabled_reason = models.PositiveSmallIntegerField(
choices=STATUS_DISABLED_REASONS.choices, default=STATUS_DISABLED_REASONS.NONE
)
# The manifest_version defined in manifest.json # The manifest_version defined in manifest.json
manifest_version = models.SmallIntegerField(choices=SUPPORTED_MANIFEST_VERSIONS) manifest_version = models.SmallIntegerField(choices=SUPPORTED_MANIFEST_VERSIONS)

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

@ -38,6 +38,7 @@ class TestFileAdmin(TestCase):
'original_hash': 'xxx', 'original_hash': 'xxx',
'status': file_.status, 'status': file_.status,
'original_status': file_.original_status, 'original_status': file_.original_status,
'status_disabled_reason': file_.status_disabled_reason,
'manifest_version': 3, 'manifest_version': 3,
} }
response = self.client.post(detail_url, post_data, follow=True) response = self.client.post(detail_url, post_data, follow=True)

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

@ -17,7 +17,7 @@ from olympia.addons.models import Addon, AddonApprovalsCounter
from olympia.amo.models import ModelBase from olympia.amo.models import ModelBase
from olympia.amo.templatetags.jinja_helpers import absolutify from olympia.amo.templatetags.jinja_helpers import absolutify
from olympia.amo.utils import send_mail from olympia.amo.utils import send_mail
from olympia.files.models import FileValidation from olympia.files.models import File, FileValidation
from olympia.ratings.models import Rating from olympia.ratings.models import Rating
from olympia.users.models import UserProfile from olympia.users.models import UserProfile
from olympia.users.utils import get_task_user from olympia.users.utils import get_task_user
@ -313,14 +313,13 @@ class AutoApprovalSummary(ModelBase):
# Average daily users: value divided by 10000 is added to the # Average daily users: value divided by 10000 is added to the
# weight, up to a maximum of 100. # weight, up to a maximum of 100.
'average_daily_users': min(addon.average_daily_users // 10000, 100), 'average_daily_users': min(addon.average_daily_users // 10000, 100),
# Pas rejection history: each "recent" rejected version (disabled # Past rejection history: each "recent" rejected version
# with an original status of null, so not disabled by a developer)
# adds 10 to the weight, up to a maximum of 100. # adds 10 to the weight, up to a maximum of 100.
'past_rejection_history': min( 'past_rejection_history': min(
Version.objects.filter( Version.objects.filter(
addon=addon, addon=addon,
human_review_date__gte=one_year_ago, human_review_date__gte=one_year_ago,
file__original_status=amo.STATUS_NULL, file__status_disabled_reason=File.STATUS_DISABLED_REASONS.NONE,
file__status=amo.STATUS_DISABLED, file__status=amo.STATUS_DISABLED,
).count() ).count()
* 10, * 10,

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

@ -14,6 +14,7 @@ from olympia.amo.tests import (
version_factory, version_factory,
) )
from olympia.constants.reviewers import REVIEWER_DELAYED_REJECTION_PERIOD_DAYS_DEFAULT from olympia.constants.reviewers import REVIEWER_DELAYED_REJECTION_PERIOD_DAYS_DEFAULT
from olympia.files.models import File
from olympia.reviewers.forms import ReviewForm from olympia.reviewers.forms import ReviewForm
from olympia.reviewers.models import ( from olympia.reviewers.models import (
AutoApprovalSummary, AutoApprovalSummary,
@ -627,6 +628,7 @@ class TestReviewForm(TestCase):
file_kw={ file_kw={
'status': amo.STATUS_DISABLED, 'status': amo.STATUS_DISABLED,
'original_status': amo.STATUS_APPROVED, 'original_status': amo.STATUS_APPROVED,
'status_disabled_reason': File.STATUS_DISABLED_REASONS.DEVELOPER,
'is_signed': True, 'is_signed': True,
}, },
) )
@ -635,6 +637,7 @@ class TestReviewForm(TestCase):
file_kw={ file_kw={
'status': amo.STATUS_DISABLED, 'status': amo.STATUS_DISABLED,
'original_status': amo.STATUS_AWAITING_REVIEW, 'original_status': amo.STATUS_AWAITING_REVIEW,
'status_disabled_reason': File.STATUS_DISABLED_REASONS.DEVELOPER,
'is_signed': False, 'is_signed': False,
}, },
) )

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

@ -25,7 +25,7 @@ from olympia.constants.promoted import (
STRATEGIC, STRATEGIC,
) )
from olympia.constants.scanners import CUSTOMS, MAD from olympia.constants.scanners import CUSTOMS, MAD
from olympia.files.models import FileValidation, WebextPermission from olympia.files.models import File, FileValidation, WebextPermission
from olympia.promoted.models import PromotedAddon from olympia.promoted.models import PromotedAddon
from olympia.ratings.models import Rating from olympia.ratings.models import Rating
from olympia.reviewers.models import ( from olympia.reviewers.models import (
@ -485,14 +485,14 @@ class TestAutoApprovalSummary(TestCase):
}, },
) )
# Version disabled by the developer, not Mozilla (original_status # Version disabled by the developer, not Mozilla
# is set to something different than STATUS_NULL), does not count. # (status_disabled_reason is DEVELOPER), does not count.
version_factory( version_factory(
addon=self.addon, addon=self.addon,
human_review_date=self.days_ago(15), human_review_date=self.days_ago(15),
file_kw={ file_kw={
'status': amo.STATUS_DISABLED, 'status': amo.STATUS_DISABLED,
'original_status': amo.STATUS_APPROVED, 'status_disabled_reason': File.STATUS_DISABLED_REASONS.DEVELOPER,
}, },
) )

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

@ -35,6 +35,7 @@ from olympia.amo.fields import CIDRField, PositiveAutoField
from olympia.amo.models import LongNameIndex, ManagerBase, ModelBase, OnChangeMixin from olympia.amo.models import LongNameIndex, ManagerBase, ModelBase, OnChangeMixin
from olympia.amo.utils import id_to_path from olympia.amo.utils import id_to_path
from olympia.amo.validators import OneOrMorePrintableCharacterValidator from olympia.amo.validators import OneOrMorePrintableCharacterValidator
from olympia.files.models import File
from olympia.translations.query import order_by_translation from olympia.translations.query import order_by_translation
from olympia.users.notifications import NOTIFICATIONS_BY_ID from olympia.users.notifications import NOTIFICATIONS_BY_ID
@ -476,7 +477,6 @@ class UserProfile(OnChangeMixin, ModelBase, AbstractBaseUser):
from olympia.addons.models import Addon, AddonUser from olympia.addons.models import Addon, AddonUser
from olympia.addons.tasks import index_addons from olympia.addons.tasks import index_addons
from olympia.bandwagon.models import Collection from olympia.bandwagon.models import Collection
from olympia.files.models import File
from olympia.ratings.models import Rating from olympia.ratings.models import Rating
# collect affected addons # collect affected addons
@ -498,10 +498,8 @@ class UserProfile(OnChangeMixin, ModelBase, AbstractBaseUser):
addons_sole = Addon.unfiltered.filter(id__in=addon_ids - addon_joint_ids) addons_sole = Addon.unfiltered.filter(id__in=addon_ids - addon_joint_ids)
# set the status to disabled - using the manager update() method # set the status to disabled - using the manager update() method
addons_sole.update(status=amo.STATUS_DISABLED) addons_sole.update(status=amo.STATUS_DISABLED)
# collect Files that need to be disabled now the addons are disabled # disable Files in bulk that need to be disabled now the addons are disabled
File.objects.filter(version__addon__in=addons_sole).update( Addon.disable_all_files(addons_sole, File.STATUS_DISABLED_REASONS.ADDON_DISABLE)
status=amo.STATUS_DISABLED
)
# Finally run Addon.force_disable to add the logging; update versions. # Finally run Addon.force_disable to add the logging; update versions.
addons_sole_ids = [] addons_sole_ids = []

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

@ -207,6 +207,7 @@ class TestUserProfile(TestCase):
email='sole@foo.baa', fxa_id='13579', last_login_ip='127.0.0.1' email='sole@foo.baa', fxa_id='13579', last_login_ip='127.0.0.1'
) )
addon_sole = addon_factory(users=[user_sole]) addon_sole = addon_factory(users=[user_sole])
addon_sole_file = addon_sole.current_version.file
self.setup_user_to_be_have_content_disabled(user_sole) self.setup_user_to_be_have_content_disabled(user_sole)
user_multi = user_factory( user_multi = user_factory(
email='multi@foo.baa', fxa_id='24680', last_login_ip='127.0.0.2' email='multi@foo.baa', fxa_id='24680', last_login_ip='127.0.0.2'
@ -215,6 +216,7 @@ class TestUserProfile(TestCase):
addon_multi = addon_factory( addon_multi = addon_factory(
users=UserProfile.objects.filter(id__in=[user_multi.id, innocent_user.id]) users=UserProfile.objects.filter(id__in=[user_multi.id, innocent_user.id])
) )
addon_multi_file = addon_multi.current_version.file
self.setup_user_to_be_have_content_disabled(user_multi) self.setup_user_to_be_have_content_disabled(user_multi)
# Now that everything is set up, disable/delete related content. # Now that everything is set up, disable/delete related content.
@ -230,17 +232,15 @@ class TestUserProfile(TestCase):
assert list(addon_multi.authors.all()) == [innocent_user] assert list(addon_multi.authors.all()) == [innocent_user]
# the File objects have been disabled # the File objects have been disabled
addon_sole_file.reload()
assert addon_sole_file.status == amo.STATUS_DISABLED
assert addon_sole_file.original_status == amo.STATUS_APPROVED
assert ( assert (
not File.objects.filter(version__addon=addon_sole) addon_sole_file.status_disabled_reason
.exclude(status=amo.STATUS_DISABLED) == File.STATUS_DISABLED_REASONS.ADDON_DISABLE
.exists()
) )
# But not for the Add-on that wasn't disabled # But not for the Add-on that wasn't disabled
assert ( assert addon_multi_file.reload().status == amo.STATUS_APPROVED
File.objects.filter(version__addon=addon_multi)
.exclude(status=amo.STATUS_DISABLED)
.exists()
)
assert not user_sole._ratings_all.exists() # Even replies. assert not user_sole._ratings_all.exists() # Even replies.
assert not user_sole.collections.exists() assert not user_sole.collections.exists()

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

@ -638,9 +638,13 @@ class Version(OnChangeMixin, ModelBase):
else: else:
# By default we soft delete so we can keep the files for comparison # By default we soft delete so we can keep the files for comparison
# and a record of the version number. # and a record of the version number.
if hasattr(self, 'file'): if hasattr(self, 'file') and self.file.status != amo.STATUS_DISABLED:
# .file should always exist but we don't want to break delete regardless # .file should always exist but we don't want to break delete regardless
self.file.update(status=amo.STATUS_DISABLED) self.file.update(
status=amo.STATUS_DISABLED,
original_status=self.file.status,
status_disabled_reason=File.STATUS_DISABLED_REASONS.VERSION_DELETE,
)
self.deleted = True self.deleted = True
self.save() self.save()
@ -666,22 +670,33 @@ class Version(OnChangeMixin, ModelBase):
def is_user_disabled(self): def is_user_disabled(self):
return ( return (
self.file.status == amo.STATUS_DISABLED self.file.status == amo.STATUS_DISABLED
and self.file.original_status != amo.STATUS_NULL and self.file.status_disabled_reason
== File.STATUS_DISABLED_REASONS.DEVELOPER
) )
@is_user_disabled.setter @is_user_disabled.setter
def is_user_disabled(self, disable): def is_user_disabled(self, disable):
# User wants to disable (and the File isn't already). # User wants to disable (and the File isn't already).
if disable: if disable:
activity.log_create(amo.LOG.DISABLE_VERSION, self.addon, self)
if (file_ := self.file) and file_.status != amo.STATUS_DISABLED: if (file_ := self.file) and file_.status != amo.STATUS_DISABLED:
file_.update(original_status=file_.status, status=amo.STATUS_DISABLED) activity.log_create(amo.LOG.DISABLE_VERSION, self.addon, self)
file_.update(
original_status=file_.status,
status=amo.STATUS_DISABLED,
status_disabled_reason=File.STATUS_DISABLED_REASONS.DEVELOPER,
)
# User wants to re-enable (and user did the disable, not Mozilla). # User wants to re-enable (and user did the disable, not Mozilla).
else: else:
activity.log_create(amo.LOG.ENABLE_VERSION, self.addon, self) if (
if (file_ := self.file) and file_.original_status != amo.STATUS_NULL: (file_ := self.file)
and file_.status_disabled_reason
== File.STATUS_DISABLED_REASONS.DEVELOPER
):
activity.log_create(amo.LOG.ENABLE_VERSION, self.addon, self)
file_.update( file_.update(
status=file_.original_status, original_status=amo.STATUS_NULL status=file_.original_status,
original_status=amo.STATUS_NULL,
status_disabled_reason=File.STATUS_DISABLED_REASONS.NONE,
) )
@cached_property @cached_property

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

@ -482,12 +482,19 @@ class TestVersion(AMOPaths, TestCase):
version = Version.objects.get(pk=81551) version = Version.objects.get(pk=81551)
version_preview = VersionPreview.objects.create(version=version) version_preview = VersionPreview.objects.create(version=version)
assert version.file assert version.file
version_file = version.file
version.delete() version.delete()
addon = Addon.objects.get(pk=3615) addon = Addon.objects.get(pk=3615)
assert not Version.objects.filter(addon=addon).exists() assert not Version.objects.filter(addon=addon).exists()
assert Version.unfiltered.filter(addon=addon).exists() assert Version.unfiltered.filter(addon=addon).exists()
assert File.objects.filter(version=version).exists() assert File.objects.filter(version=version).exists()
version_file.reload()
assert version_file.original_status == amo.STATUS_APPROVED
assert (
version_file.status_disabled_reason
== File.STATUS_DISABLED_REASONS.VERSION_DELETE
)
delete_preview_files_mock.assert_called_with( delete_preview_files_mock.assert_called_with(
sender=None, instance=version_preview sender=None, instance=version_preview
) )
@ -547,13 +554,18 @@ class TestVersion(AMOPaths, TestCase):
version.file.reload() version.file.reload()
assert version.file.status == amo.STATUS_DISABLED assert version.file.status == amo.STATUS_DISABLED
assert version.file.original_status == amo.STATUS_APPROVED assert version.file.original_status == amo.STATUS_APPROVED
assert (
version.file.status_disabled_reason
== File.STATUS_DISABLED_REASONS.DEVELOPER
)
version.is_user_disabled = False version.is_user_disabled = False
version.file.reload() version.file.reload()
assert version.file.status == amo.STATUS_APPROVED assert version.file.status == amo.STATUS_APPROVED
assert version.file.original_status == amo.STATUS_NULL assert version.file.original_status == amo.STATUS_NULL
assert version.file.status_disabled_reason == File.STATUS_DISABLED_REASONS.NONE
def test_version_disable_after_mozila_disabled(self): def test_version_disable_after_mozilla_disabled(self):
# Check that a user disable doesn't override mozilla disable # Check that a user disable doesn't override mozilla disable
version = Version.objects.get(pk=81551) version = Version.objects.get(pk=81551)
version.file.update(status=amo.STATUS_DISABLED) version.file.update(status=amo.STATUS_DISABLED)
@ -562,11 +574,18 @@ class TestVersion(AMOPaths, TestCase):
version.file.reload() version.file.reload()
assert version.file.status == amo.STATUS_DISABLED assert version.file.status == amo.STATUS_DISABLED
assert version.file.original_status == amo.STATUS_NULL assert version.file.original_status == amo.STATUS_NULL
assert version.file.status_disabled_reason == File.STATUS_DISABLED_REASONS.NONE
version.is_user_disabled = False version.file.update(original_status=amo.STATUS_APPROVED)
version.file.reload() for reason in File.STATUS_DISABLED_REASONS.values:
assert version.file.status == amo.STATUS_DISABLED if reason == File.STATUS_DISABLED_REASONS.DEVELOPER:
assert version.file.original_status == amo.STATUS_NULL # DEVELOPER is the only reason we expect to succeed
continue
version.file.update(status_disabled_reason=reason)
version.is_user_disabled = False
version.file.reload()
assert version.file.status == amo.STATUS_DISABLED
assert version.file.original_status == amo.STATUS_APPROVED
def _reset_version(self, version): def _reset_version(self, version):
version.file.status = amo.STATUS_APPROVED version.file.status = amo.STATUS_APPROVED
@ -1472,7 +1491,9 @@ class TestVersion(AMOPaths, TestCase):
assert self.version.get_review_status_display() == 'Unreviewed' assert self.version.get_review_status_display() == 'Unreviewed'
self.version.update(human_review_date=datetime.now()) self.version.update(human_review_date=datetime.now())
assert self.version.get_review_status_display() == 'Rejected' assert self.version.get_review_status_display() == 'Rejected'
self.version.file.update(original_status=amo.STATUS_APPROVED) self.version.file.update(
status_disabled_reason=File.STATUS_DISABLED_REASONS.DEVELOPER
)
assert self.version.get_review_status_display() == 'Disabled by Developer' assert self.version.get_review_status_display() == 'Disabled by Developer'
self.version.update(deleted=True) self.version.update(deleted=True)
assert self.version.get_review_status_display() == 'Deleted' assert self.version.get_review_status_display() == 'Deleted'