Multi-platform support: add Version.files field #189

Merged
Oleg-Komarov merged 1 commits from version-add-files into main 2024-06-17 15:01:44 +02:00
10 changed files with 122 additions and 21 deletions

View File

@ -84,6 +84,18 @@ class VersionFactory(DjangoModelFactory):
RatingFactory, size=lambda: random.randint(0, 5), factory_related_name='version' RatingFactory, size=lambda: random.randint(0, 5), factory_related_name='version'
) )
@factory.post_generation
def files(self, create, extracted, **kwargs):
if not create:
return
if not extracted:
self.files.add(self.file)
return
for file in extracted:
self.files.add(file)
@factory.post_generation @factory.post_generation
def platforms(self, create, extracted, **kwargs): def platforms(self, create, extracted, **kwargs):
if not create: if not create:

View File

@ -5,7 +5,7 @@ from django.contrib import admin
from . import models from . import models
from common.admin import NoAddDeleteMixin from common.admin import NoAddDeleteMixin
from extensions.models import Extension, Maintainer, Version, Tag from extensions.models import Extension, Maintainer, Tag, Version, VersionFiles
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
@ -241,6 +241,12 @@ class TagAdmin(admin.ModelAdmin):
return () return ()
class VersionFilesAdmin(admin.ModelAdmin):
model = VersionFiles
list_display = 'file'
readonly_fields = 'file'
class VersionPermissionAdmin(admin.ModelAdmin): class VersionPermissionAdmin(admin.ModelAdmin):
list_display = ('name', 'slug') list_display = ('name', 'slug')

View File

@ -0,0 +1,45 @@
# Generated by Django 4.2.11 on 2024-06-17 10:13
from django.db import migrations, models
import django.db.models.deletion
def populate_files(apps, schema_editor):
Version = apps.get_model('extensions', 'Version')
for v in Version.objects.all():
v.files.add(v.file)
class Migration(migrations.Migration):
dependencies = [
('files', '0010_remove_file_extension_alter_file_hash_and_more'),
('extensions', '0039_alter_extension_status'),
]
operations = [
migrations.AlterField(
model_name='version',
name='file',
field=models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='legacy_version', to='files.file'),
),
migrations.CreateModel(
name='VersionFiles',
fields=[
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('date_created', models.DateTimeField(auto_now_add=True)),
('date_modified', models.DateTimeField(auto_now=True)),
('file', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to='files.file')),
('version', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='extensions.version')),
],
options={
'abstract': False,
},
),
migrations.AddField(
model_name='version',
name='files',
field=models.ManyToManyField(related_name='version', through='extensions.VersionFiles', to='files.file'),
),
migrations.RunPython(populate_files, migrations.RunPython.noop)
]

View File

@ -496,10 +496,12 @@ class VersionManager(models.Manager):
permissions = list(permissions.keys()) permissions = list(permissions.keys())
platforms = kwargs.pop('platforms', []) platforms = kwargs.pop('platforms', [])
tags = kwargs.pop('tags', []) tags = kwargs.pop('tags', [])
file = kwargs.get('file') # FIXME legacy_version: replace get with pop
version, result = super().update_or_create(*args, **kwargs) version, result = super().update_or_create(*args, **kwargs)
# Add the ManyToMany to the already initialized Version # Add the ManyToMany to the already initialized Version
version.files.add(file)
version.set_initial_licenses(licenses) version.set_initial_licenses(licenses)
version.set_initial_permissions(permissions) version.set_initial_permissions(permissions)
version.set_initial_platforms(platforms) version.set_initial_platforms(platforms)
@ -515,14 +517,25 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model):
'version', 'version',
'licenses', 'licenses',
'file', 'file',
'files',
'tagline', 'tagline',
} }
extension = models.ForeignKey( extension = models.ForeignKey(
'extensions.Extension', related_name='versions', on_delete=models.CASCADE 'extensions.Extension', related_name='versions', on_delete=models.CASCADE
) )
# TODO drop once data is migrated to `files`
file = models.OneToOneField( file = models.OneToOneField(
'files.File', related_name='version', on_delete=models.CASCADE, null=False, blank=False 'files.File',
related_name='legacy_version',
on_delete=models.CASCADE,
null=False,
blank=False,
)
files = models.ManyToManyField(
'files.File',
through='VersionFiles',
related_name='version',
) )
version = extensions.fields.VersionStringField( version = extensions.fields.VersionStringField(
@ -746,6 +759,11 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model):
super().delete(*args, **kwargs) super().delete(*args, **kwargs)
class VersionFiles(CreatedModifiedMixin, models.Model):
version = models.ForeignKey(Version, on_delete=models.CASCADE)
file = models.OneToOneField(File, on_delete=models.CASCADE)
class Maintainer(CreatedModifiedMixin, models.Model): class Maintainer(CreatedModifiedMixin, models.Model):
extension = models.ForeignKey(Extension, on_delete=models.CASCADE) extension = models.ForeignKey(Extension, on_delete=models.CASCADE)
user = models.ForeignKey(User, on_delete=models.CASCADE) user = models.ForeignKey(User, on_delete=models.CASCADE)

View File

@ -43,6 +43,23 @@ def _delete_version_file(
version_file.delete() version_file.delete()
@receiver(post_delete, sender=extensions.models.VersionFiles)
def _delete_versionfiles_file(
sender: object, instance: extensions.models.VersionFiles, **kwargs: object
) -> None:
# **N.B.**: this isn't part of an overloaded `VersionFiles.delete()` method because
# that method isn't called when `Extension.delete()` cascades to deleting the versions:
#
# delete() method for an object is not necessarily called ... as a result of a cascading delete
# https://docs.djangoproject.com/en/4.2/topics/db/models/#overriding-predefined-model-methods
# FIXME: uncomment when the signal above is cleaned up
return
version_file = instance.file
logger.info('Deleting file pk=%s of VersionFiles pk=%s', version_file.pk, instance.pk)
version_file.delete()
@receiver(pre_save, sender=extensions.models.Extension) @receiver(pre_save, sender=extensions.models.Extension)
def _record_changes( def _record_changes(
sender: object, sender: object,
@ -69,8 +86,8 @@ def _update_version(
if raw: if raw:
return return
if hasattr(instance, 'version'): if hasattr(instance, 'legacy_version'):
extension = instance.version.extension extension = instance.legacy_version.extension
with transaction.atomic(): with transaction.atomic():
# it's important to update is_listed before computing latest_version # it's important to update is_listed before computing latest_version
# because latest_version for listed and unlisted extensions are defined differently # because latest_version for listed and unlisted extensions are defined differently

View File

@ -73,10 +73,10 @@ class DeleteTest(TestCase):
version_file, version_file,
file_validation, file_validation,
extension, extension,
version,
approval_activity, approval_activity,
preview_file.preview_set.first(), preview_file.preview_set.first(),
reused_image.preview_set.first(), reused_image.preview_set.first(),
version,
], ],
) )
) )

View File

@ -330,7 +330,7 @@ class ValidateManifestTest(CreateFileTest):
self.assertEqual(response.status_code, 302) self.assertEqual(response.status_code, 302)
file = File.objects.first() file = File.objects.first()
extension = file.version.extension extension = file.legacy_version.extension
self.assertEqual(extension.slug, 'an-id') self.assertEqual(extension.slug, 'an-id')
self.assertEqual(extension.name, 'Name. - With Extra spaces and other characters Ж') self.assertEqual(extension.name, 'Name. - With Extra spaces and other characters Ж')

View File

@ -98,6 +98,8 @@ class VersionTest(TestCase):
extension__support='https://example.com/', extension__support='https://example.com/',
) )
self.assertEqual(entries_for(self.version).count(), 0) self.assertEqual(entries_for(self.version).count(), 0)
# FIXME remove when dropping Version.file field
self.assertEqual(self.version.file, self.version.files.first())
def _check_change_message(self): def _check_change_message(self):
entries = entries_for(self.version) entries = entries_for(self.version)

View File

@ -154,8 +154,8 @@ class SubmitFileTest(TestCase):
self.assertEqual(response.status_code, 302) self.assertEqual(response.status_code, 302)
self.assertEqual(File.objects.count(), 1) self.assertEqual(File.objects.count(), 1)
file = File.objects.first() file = File.objects.first()
self.assertEqual(response['Location'], file.version.extension.get_draft_url()) self.assertEqual(response['Location'], file.legacy_version.extension.get_draft_url())
extension = file.version.extension extension = file.legacy_version.extension
self.assertEqual(extension.slug, slug) self.assertEqual(extension.slug, slug)
self.assertEqual(extension.name, name) self.assertEqual(extension.name, name)
self.assertEqual(file.original_name, file_name) self.assertEqual(file.original_name, file_name)
@ -166,7 +166,8 @@ class SubmitFileTest(TestCase):
self.assertEqual(file.metadata['version'], version_str) self.assertEqual(file.metadata['version'], version_str)
self.assertEqual(file.metadata.get('permissions'), other_metadata.get('permissions')) self.assertEqual(file.metadata.get('permissions'), other_metadata.get('permissions'))
self.assertEqual( self.assertEqual(
[p.slug for p in file.version.platforms.all()], other_metadata.get('platforms', []) [p.slug for p in file.legacy_version.platforms.all()],
other_metadata.get('platforms', []),
) )
def test_not_allowed_anonymous(self): def test_not_allowed_anonymous(self):
@ -224,7 +225,7 @@ class SubmitFileTest(TestCase):
self.assertEqual(response.status_code, 302, _get_all_form_errors(response)) self.assertEqual(response.status_code, 302, _get_all_form_errors(response))
self.assertEqual(File.objects.count(), 1) self.assertEqual(File.objects.count(), 1)
file = File.objects.first() file = File.objects.first()
self.assertEqual(response['Location'], file.version.extension.get_draft_url()) self.assertEqual(response['Location'], file.legacy_version.extension.get_draft_url())
self.assertEqual(file.user, user) self.assertEqual(file.user, user)
self.assertEqual(file.original_name, 'theme.zip') self.assertEqual(file.original_name, 'theme.zip')
self.assertEqual(file.size_bytes, 5895) self.assertEqual(file.size_bytes, 5895)
@ -305,19 +306,19 @@ class SubmitFinaliseTest(CheckFilePropertiesMixin, TestCase):
) )
def test_get_finalise_addon_redirects_if_anonymous(self): def test_get_finalise_addon_redirects_if_anonymous(self):
response = self.client.post(self.file.version.extension.get_draft_url(), {}) response = self.client.post(self.file.legacy_version.extension.get_draft_url(), {})
self.assertEqual(response.status_code, 302) self.assertEqual(response.status_code, 302)
self.assertEqual( self.assertEqual(
response['Location'], response['Location'],
f'/oauth/login?next=/add-ons/{self.file.version.extension.slug}/draft/', f'/oauth/login?next=/add-ons/{self.file.legacy_version.extension.slug}/draft/',
) )
def test_get_finalise_addon_not_allowed_if_different_user(self): def test_get_finalise_addon_not_allowed_if_different_user(self):
user = UserFactory() user = UserFactory()
self.client.force_login(user) self.client.force_login(user)
response = self.client.post(self.file.version.extension.get_draft_url(), {}) response = self.client.post(self.file.legacy_version.extension.get_draft_url(), {})
# Technically this could (should) be a 403, but changing this means changing # Technically this could (should) be a 403, but changing this means changing
# the MaintainedExtensionMixin which is used in multiple places. # the MaintainedExtensionMixin which is used in multiple places.
@ -326,7 +327,7 @@ class SubmitFinaliseTest(CheckFilePropertiesMixin, TestCase):
def test_post_finalise_addon_validation_errors(self): def test_post_finalise_addon_validation_errors(self):
self.client.force_login(self.file.user) self.client.force_login(self.file.user)
data = {**POST_DATA, 'submit_draft': ''} data = {**POST_DATA, 'submit_draft': ''}
response = self.client.post(self.file.version.extension.get_draft_url(), data) response = self.client.post(self.file.legacy_version.extension.get_draft_url(), data)
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
self.assertDictEqual( self.assertDictEqual(
@ -393,7 +394,7 @@ class SubmitFinaliseTest(CheckFilePropertiesMixin, TestCase):
'featured-image-source': fp4, 'featured-image-source': fp4,
} }
response = self.client.post( response = self.client.post(
self.file.version.extension.get_draft_url(), {**data, **files} self.file.legacy_version.extension.get_draft_url(), {**data, **files}
) )
self.assertEqual(response.status_code, 302, _get_all_form_errors(response)) self.assertEqual(response.status_code, 302, _get_all_form_errors(response))

View File

@ -76,7 +76,7 @@ class FileAdmin(admin.ModelAdmin):
'date_created', 'date_created',
'date_modified', 'date_modified',
'date_status_changed', 'date_status_changed',
('version', admin.EmptyFieldListFilter), ('legacy_version', admin.EmptyFieldListFilter),
('icon_of', admin.EmptyFieldListFilter), ('icon_of', admin.EmptyFieldListFilter),
('featured_image_of', admin.EmptyFieldListFilter), ('featured_image_of', admin.EmptyFieldListFilter),
('preview', admin.EmptyFieldListFilter), ('preview', admin.EmptyFieldListFilter),
@ -87,7 +87,7 @@ class FileAdmin(admin.ModelAdmin):
'date_created', 'date_created',
'type', 'type',
'status', 'status',
link_to('version'), link_to('legacy_version'),
link_to('icon_of'), link_to('icon_of'),
link_to('featured_image_of'), link_to('featured_image_of'),
link_to('preview_set.extension', 'preview of'), link_to('preview_set.extension', 'preview of'),
@ -95,9 +95,9 @@ class FileAdmin(admin.ModelAdmin):
) )
list_select_related = ( list_select_related = (
'version__extension', 'legacy_version__extension',
'user', 'user',
'version', 'legacy_version',
'validation', 'validation',
) )
list_prefetch_related = ( list_prefetch_related = (
@ -123,8 +123,8 @@ class FileAdmin(admin.ModelAdmin):
'content_type', 'content_type',
) )
search_fields = ( search_fields = (
'version__extension__slug', 'legacy_version__extension__slug',
'version__extension__name', 'legacy_version__extension__name',
'extensions__slug', 'extensions__slug',
'extensions__name', 'extensions__name',
'original_name', 'original_name',