Reuse existing files as previews, icons or featured images #161

Merged
Anna Sirota merged 11 commits from files-allow-reuse-between-ext into main 2024-06-04 12:23:26 +02:00
16 changed files with 190 additions and 123 deletions
Showing only changes of commit 12bda6c418 - Show all commits

View File

@ -65,10 +65,17 @@ def link_to(field_name, title=None):
@admin.display(description=title, ordering=field_name)
def _raw(obj):
target_obj = getattr(obj, field_name)
if isinstance(target_obj, models.Manager):
admin_urls = []
for _obj in target_obj.all():
if related_field_name:
_obj = getattr(_obj, related_field_name)
admin_urls.append(get_admin_change_url(_obj))
return format_html('<br>'.join(admin_urls))
if related_field_name:
target_obj = getattr(getattr(obj, field_name), related_field_name)
else:
target_obj = getattr(obj, field_name)
target_obj = getattr(target_obj, related_field_name)
admin_url = get_admin_change_url(target_obj)
return admin_url

View File

@ -108,10 +108,7 @@ class VersionFactory(DjangoModelFactory):
def create_version(**kwargs) -> 'Version':
version = VersionFactory(**kwargs)
file = version.file
file.extension = version.extension
file.save(update_fields={'extension'})
file.extension.authors.add(version.file.user)
version.extension.authors.add(version.file.user)
return version

View File

@ -65,17 +65,22 @@ class AddPreviewFileForm(files.forms.BaseMediaFileForm):
instance = super().save(*args, **kwargs)
# Create extension preview and save caption to it
extensions.models.Preview.objects.create(
file=instance,
caption=self.cleaned_data['caption'],
extension=self.extension,
extensions.models.Preview.objects.bulk_create(
[
extensions.models.Preview(
file=instance,
caption=self.cleaned_data['caption'],
extension=self.extension,
)
],
ignore_conflicts=True,
update_conflicts=False,
)
return instance
class AddPreviewModelFormSet(forms.BaseModelFormSet):
msg_duplicate_file = _('Please select another file instead of the duplicate')
def __init__(self, *args, **kwargs):
self.request = kwargs.pop('request')
self.extension = kwargs.pop('extension')
@ -89,14 +94,6 @@ class AddPreviewModelFormSet(forms.BaseModelFormSet):
form_kwargs['extension'] = self.extension
return form_kwargs
def get_unique_error_message(self, unique_check):
"""Replace duplicate `original_hash`/`hash` message with a more meaningful one."""
if len(unique_check) == 1:
field = unique_check[0]
if field in ('original_hash', 'hash'):
return self.msg_duplicate_file
return super().get_unique_error_message(unique_check)
AddPreviewFormSet = forms.modelformset_factory(
files.models.File,
@ -116,7 +113,6 @@ class ExtensionUpdateForm(forms.ModelForm):
'An extension can be converted to draft only while it is Awating Review'
)
msg_need_previews = _('Please add at least one preview.')
msg_duplicate_file = _('Please select another file instead of the duplicate.')
class Meta:
model = extensions.models.Extension
@ -198,16 +194,17 @@ class ExtensionUpdateForm(forms.ModelForm):
self.featured_image_form,
self.icon_form,
]
seen_hashes = set()
seen_hashes = {}
for f in new_file_forms:
hash = f.instance.original_hash
if hash and hash not in seen_hashes:
seen_hashes[hash] = f.instance
# Ignore duplicate files in the formsets: point all forms sharing the hash to the same
# file instance, when forms call .save() it'll be on the same instance.
for f in new_file_forms:
hash = f.instance.original_hash
if hash:
if hash in seen_hashes:
f.add_error('source', self.msg_duplicate_file)
is_valid_flags.append(False)
break
seen_hashes.add(hash)
f.instance = seen_hashes[hash]
return all(is_valid_flags)
def clean_team(self):
@ -352,9 +349,9 @@ class IconForm(files.forms.BaseMediaFileForm):
}
expected_size_px = 256
def clean_source(self):
def clean_source(self, *args, **kwargs):
"""Check image resolution."""
source = self.cleaned_data.get('source')
source = super().clean_source(*args, **kwargs)
if not source:
return
image = getattr(source, 'image', None)

View File

@ -0,0 +1,30 @@
# Generated by Django 4.2.11 on 2024-06-03 15:05
from django.db import migrations, models
import django.db.models.deletion
class Migration(migrations.Migration):
dependencies = [
('files', '0010_remove_file_extension_alter_file_hash_and_more'),
('extensions', '0032_extension_extensions__is_list_765936_idx_and_more'),
]
operations = [
migrations.AlterField(
model_name='extension',
name='featured_image',
field=models.ForeignKey(help_text='Shown by social networks when this extension is shared (used as `og:image` metadata field).Should have resolution of at least 1920 x 1080 and aspect ratio of 16:9.', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='featured_image_of', to='files.file'),
),
migrations.AlterField(
model_name='extension',
name='icon',
field=models.ForeignKey(help_text='A 256 x 256 PNG icon representing this extension.', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='icon_of', to='files.file'),
),
migrations.AlterField(
model_name='preview',
name='file',
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='files.file'),
),
]

View File

@ -0,0 +1,30 @@
# Generated by Django 4.2.11 on 2024-06-03 16:38
from django.db import migrations, models
import django.db.models.deletion
class Migration(migrations.Migration):
dependencies = [
('files', '0010_remove_file_extension_alter_file_hash_and_more'),
('extensions', '0033_alter_extension_featured_image_alter_extension_icon_and_more'),
]
operations = [
migrations.AlterField(
model_name='extension',
name='featured_image',
field=models.ForeignKey(help_text='Shown by social networks when this extension is shared (used as `og:image` metadata field).Should have resolution of at least 1920 x 1080 and aspect ratio of 16:9.', null=True, on_delete=django.db.models.deletion.PROTECT, related_name='featured_image_of', to='files.file'),
),
migrations.AlterField(
model_name='extension',
name='icon',
field=models.ForeignKey(help_text='A 256 x 256 PNG icon representing this extension.', null=True, on_delete=django.db.models.deletion.PROTECT, related_name='icon_of', to='files.file'),
),
migrations.AlterField(
model_name='preview',
name='file',
field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='files.file'),
),
]

View File

@ -178,24 +178,24 @@ class Extension(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Mod
related_name='latest_version_of',
)
featured_image = models.OneToOneField(
featured_image = models.ForeignKey(
'files.File',
related_name='featured_image_of',
null=True,
blank=False,
on_delete=models.SET_NULL,
on_delete=models.PROTECT,
help_text=(
"Shown by social networks when this extension is shared"
" (used as `og:image` metadata field)."
"Should have resolution of at least 1920 x 1080 and aspect ratio of 16:9."
),
)
icon = models.OneToOneField(
icon = models.ForeignKey(
'files.File',
related_name='icon_of',
null=True,
blank=False,
on_delete=models.SET_NULL,
on_delete=models.PROTECT,
help_text="A 256 x 256 PNG icon representing this extension.",
)
previews = FilterableManyToManyField('files.File', through='Preview', related_name='extensions')
@ -748,12 +748,14 @@ class Maintainer(CreatedModifiedMixin, models.Model):
class Preview(CreatedModifiedMixin, RecordDeletionMixin, models.Model):
extension = models.ForeignKey(Extension, on_delete=models.CASCADE)
file = models.OneToOneField('files.File', on_delete=models.CASCADE)
# Files can potentially be referenced by different extensions
file = models.ForeignKey('files.File', on_delete=models.PROTECT)
caption = models.CharField(max_length=255, default='', null=False, blank=True)
position = models.IntegerField(default=0)
class Meta:
ordering = ('position', 'date_created')
# We don't want to have duplicate previews on the same extension
unique_together = [['extension', 'file']]
@property

View File

@ -5,7 +5,7 @@ from actstream.actions import follow, unfollow
from django.contrib.auth import get_user_model
from django.contrib.auth.models import Group
from django.db import transaction
from django.db.models.signals import m2m_changed, pre_save, post_save, pre_delete
from django.db.models.signals import m2m_changed, pre_save, post_save, pre_delete, post_delete
from django.dispatch import receiver
from constants.activity import Flag
@ -16,22 +16,6 @@ logger = logging.getLogger(__name__)
User = get_user_model()
@receiver(pre_save, sender=extensions.models.Preview)
def _set_extension(
sender: object, instance: extensions.models.Preview, raw: bool, **kwargs: object
) -> None:
if raw:
return
file = instance.file
if not file:
return
if not file.extension_id:
file.extension_id = instance.extension_id
file.save(update_fields={'extension_id'})
@receiver(pre_delete, sender=extensions.models.Extension)
@receiver(pre_delete, sender=extensions.models.Preview)
@receiver(pre_delete, sender=extensions.models.Version)
@ -45,6 +29,17 @@ def _log_deletion(
instance.record_deletion()
@receiver(post_delete, sender=extensions.models.Version)
def _delete_version_file(
sender: object, instance: extensions.models.Version, **kwargs: object
) -> None:
# **N.B.**: this isn't part of an overloaded `Version.delete()` method because
# that method isn't called when `Extension.delete()` cascades to deleting the versions.
version_file = instance.file
logger.info('Deleting file pk=%s of Version pk=%s', version_file.pk, instance.pk)
version_file.delete()
@receiver(pre_save, sender=extensions.models.Extension)
def _record_changes(
sender: object,

View File

@ -1,5 +1,5 @@
from django.test import TestCase
from django.urls import reverse
from django.urls import reverse_lazy
from django.core.exceptions import ValidationError
import factory
@ -35,6 +35,8 @@ META_DATA = {
class CreateFileTest(TestCase):
submit_url = reverse_lazy('extensions:submit')
def setUp(self):
super().setUp()
@ -52,10 +54,6 @@ class CreateFileTest(TestCase):
super().tearDown()
shutil.rmtree(self.temp_directory)
@classmethod
def _get_submit_url(cls):
return reverse('extensions:submit')
def _create_valid_extension(self, extension_id):
return create_approved_version(
extension__name='Blender Kitsu',
@ -120,9 +118,7 @@ class ValidateManifestTest(CreateFileTest):
bad_file = self._create_file_from_data("theme.zip", file_data, self.user)
with open(bad_file, 'rb') as fp:
response = self.client.post(
self._get_submit_url(), {'source': fp, 'agreed_with_terms': True}
)
response = self.client.post(self.submit_url, {'source': fp, 'agreed_with_terms': True})
self.assertEqual(response.status_code, 200)
error = response.context['form'].errors.get('source')[0]
@ -140,9 +136,7 @@ class ValidateManifestTest(CreateFileTest):
bad_file = self._create_file_from_data("theme.zip", file_data, self.user)
with open(bad_file, 'rb') as fp:
response = self.client.post(
self._get_submit_url(), {'source': fp, 'agreed_with_terms': True}
)
response = self.client.post(self.submit_url, {'source': fp, 'agreed_with_terms': True})
self.assertEqual(response.status_code, 200)
error = response.context['form'].errors.get('source')[0]
@ -165,9 +159,7 @@ class ValidateManifestTest(CreateFileTest):
extension_file = self._create_file_from_data("theme.zip", kitsu_1_5, self.user)
with open(extension_file, 'rb') as fp:
response = self.client.post(
self._get_submit_url(), {'source': fp, 'agreed_with_terms': True}
)
response = self.client.post(self.submit_url, {'source': fp, 'agreed_with_terms': True})
self.assertEqual(response.status_code, 200)
error = response.context['form'].errors.get('source')[0]
@ -191,9 +183,7 @@ class ValidateManifestTest(CreateFileTest):
extension_file = self._create_file_from_data("theme.zip", kitsu_1_5, self.user)
with open(extension_file, 'rb') as fp:
response = self.client.post(
self._get_submit_url(), {'source': fp, 'agreed_with_terms': True}
)
response = self.client.post(self.submit_url, {'source': fp, 'agreed_with_terms': True})
self.assertEqual(response.status_code, 200)
error = response.context['form'].errors.get('source')[0]
@ -317,9 +307,7 @@ class ValidateManifestTest(CreateFileTest):
bad_file = self._create_file_from_data("theme.zip", file_data, self.user)
with open(bad_file, 'rb') as fp:
response = self.client.post(
self._get_submit_url(), {'source': fp, 'agreed_with_terms': True}
)
response = self.client.post(self.submit_url, {'source': fp, 'agreed_with_terms': True})
self.assertEqual(response.status_code, 200)
error = response.context['form'].errors.get('source')
@ -338,13 +326,11 @@ class ValidateManifestTest(CreateFileTest):
extension_file = self._create_file_from_data("theme.zip", file_data, self.user)
with open(extension_file, 'rb') as fp:
response = self.client.post(
self._get_submit_url(), {'source': fp, 'agreed_with_terms': True}
)
response = self.client.post(self.submit_url, {'source': fp, 'agreed_with_terms': True})
self.assertEqual(response.status_code, 302)
file = File.objects.first()
extension = file.extension
extension = file.version.extension
self.assertEqual(extension.slug, 'an-id')
self.assertEqual(extension.name, 'Name. - With Extra spaces and other characters Ж')

View File

@ -137,8 +137,8 @@ class SubmitFileTest(TestCase):
self.assertEqual(response.status_code, 302)
self.assertEqual(File.objects.count(), 1)
file = File.objects.first()
self.assertEqual(response['Location'], file.get_submit_url())
extension = file.extension
self.assertEqual(response['Location'], file.version.extension.get_draft_url())
extension = file.version.extension
self.assertEqual(extension.slug, slug)
self.assertEqual(extension.name, name)
self.assertEqual(file.original_name, file_name)
@ -203,8 +203,7 @@ class SubmitFileTest(TestCase):
self.assertEqual(response.status_code, 302, _get_all_form_errors(response))
self.assertEqual(File.objects.count(), 1)
file = File.objects.first()
self.assertIsNotNone(file.extension_id)
self.assertEqual(response['Location'], file.get_submit_url())
self.assertEqual(response['Location'], file.version.extension.get_draft_url())
self.assertEqual(file.user, user)
self.assertEqual(file.original_name, 'theme.zip')
self.assertEqual(file.size_bytes, 5895)
@ -285,18 +284,19 @@ class SubmitFinaliseTest(CheckFilePropertiesMixin, TestCase):
)
def test_get_finalise_addon_redirects_if_anonymous(self):
response = self.client.post(self.file.get_submit_url(), {})
response = self.client.post(self.file.version.extension.get_draft_url(), {})
self.assertEqual(response.status_code, 302)
self.assertEqual(
response['Location'], f'/oauth/login?next=/add-ons/{self.file.extension.slug}/draft/'
response['Location'],
f'/oauth/login?next=/add-ons/{self.file.version.extension.slug}/draft/',
)
def test_get_finalise_addon_not_allowed_if_different_user(self):
user = UserFactory()
self.client.force_login(user)
response = self.client.post(self.file.get_submit_url(), {})
response = self.client.post(self.file.version.extension.get_draft_url(), {})
# Technically this could (should) be a 403, but changing this means changing
# the MaintainedExtensionMixin which is used in multiple places.
@ -305,7 +305,7 @@ class SubmitFinaliseTest(CheckFilePropertiesMixin, TestCase):
def test_post_finalise_addon_validation_errors(self):
self.client.force_login(self.file.user)
data = {**POST_DATA, 'submit_draft': ''}
response = self.client.post(self.file.get_submit_url(), data)
response = self.client.post(self.file.version.extension.get_draft_url(), data)
self.assertEqual(response.status_code, 200)
self.assertDictEqual(
@ -371,7 +371,9 @@ class SubmitFinaliseTest(CheckFilePropertiesMixin, TestCase):
'icon-source': fp3,
'featured-image-source': fp4,
}
response = self.client.post(self.file.get_submit_url(), {**data, **files})
response = self.client.post(
self.file.version.extension.get_draft_url(), {**data, **files}
)
self.assertEqual(response.status_code, 302, _get_all_form_errors(response))
self.assertEqual(response['Location'], '/add-ons/edit-breakdown/manage/')

View File

@ -64,7 +64,6 @@ class UploadFileView(LoginRequiredMixin, CreateView):
# Need to save the form to be able to use the file to create the version.
self.object = self.file = form.save()
self.file.extension = self.extension
Version.objects.update_or_create(
extension=self.extension, file=self.file, **self.file.parsed_version_fields
)[0]

View File

@ -41,6 +41,10 @@ class FileAdmin(admin.ModelAdmin):
class Media:
css = {'all': ('files/admin/file.css',)}
def get_queryset(self, *args, **kwargs):
q = super().get_queryset(*args, **kwargs)
return q.prefetch_related(*self.list_prefetch_related)
def thumbnails(self, obj):
if not obj or not (obj.is_image or obj.is_video):
return ''
@ -72,7 +76,6 @@ class FileAdmin(admin.ModelAdmin):
'date_created',
'date_modified',
'date_status_changed',
('extension', admin.EmptyFieldListFilter),
('version', admin.EmptyFieldListFilter),
('icon_of', admin.EmptyFieldListFilter),
('featured_image_of', admin.EmptyFieldListFilter),
@ -80,7 +83,6 @@ class FileAdmin(admin.ModelAdmin):
)
list_display = (
'original_name',
link_to('extension'),
link_to('user'),
'date_created',
'type',
@ -88,19 +90,20 @@ class FileAdmin(admin.ModelAdmin):
link_to('version'),
link_to('icon_of'),
link_to('featured_image_of'),
link_to('preview.extension', 'preview of'),
link_to('preview_set.extension', 'preview of'),
'is_ok',
)
list_select_related = (
'version__extension',
'user',
'extension',
'version',
'validation',
)
list_prefetch_related = (
'icon_of',
'featured_image_of',
'preview__extension',
'preview_set__extension',
)
autocomplete_fields = ['user']

View File

@ -6,7 +6,6 @@ import tempfile
from django import forms
from django.utils.safestring import mark_safe
from django.utils.translation import gettext_lazy as _
import django.core.exceptions
from .validators import (
ExtensionIDManifestValidator,
@ -44,7 +43,7 @@ class FileForm(forms.ModelForm):
class Meta:
model = files.models.File
fields = ('source', 'type', 'metadata', 'agreed_with_terms', 'user', 'extension')
fields = ('source', 'type', 'metadata', 'agreed_with_terms', 'user')
source = forms.FileField(
allow_empty_file=False,
@ -123,7 +122,6 @@ class FileForm(forms.ModelForm):
'size_bytes': source.size,
'original_hash': hash_,
'hash': hash_,
'extension': self.extension,
}
)
@ -180,8 +178,7 @@ class FileFormSkipAgreed(FileForm):
class BaseMediaFileForm(forms.ModelForm):
class Meta:
model = files.models.File
fields = ('source', 'original_hash')
widgets = {'original_hash': forms.HiddenInput()}
fields = ('source',)
source = forms.ImageField(widget=forms.FileInput, allow_empty_file=False)
@ -210,22 +207,23 @@ class BaseMediaFileForm(forms.ModelForm):
super().__init__(*args, **kwargs)
self.instance.user = self.request.user
def clean_original_hash(self, *args, **kwargs):
"""Calculate original hash of the uploaded file."""
def clean_source(self, *args, **kwargs):
"""Calculate original hash of the uploaded file, reuse existing record matching it."""
source = self.cleaned_data.get('source')
if 'source' in self.changed_data and source:
return files.models.File.generate_hash(source)
def add_error(self, field, error):
"""Add hidden `original_hash` errors to the visible `source` field instead."""
if isinstance(error, django.core.exceptions.ValidationError):
if getattr(error, 'error_dict', None):
hash_error = error.error_dict.pop('original_hash', None)
if hash_error:
error.error_dict['source'] = hash_error
super(forms.ModelForm, self).add_error(field, error)
original_hash = files.models.File.generate_hash(source)
instance = files.models.File.objects.filter(original_hash=original_hash).first()
if instance:
# File with this hash exists already, make sure it's reused here
if instance.pk != self.instance.pk:
self.instance = instance
else:
previous_hash = self.instance.hash
if previous_hash and original_hash != previous_hash and self.instance.pk:
# Create a new file instead of changing the existing one
self.instance.pk = None
self.instance.original_hash = original_hash
return source
def save(self, *args, **kwargs):
"""Save as `to_field` on the parent object (Extension)."""
@ -235,7 +233,9 @@ class BaseMediaFileForm(forms.ModelForm):
self.instance.original_name = source.name
self.instance.size_bytes = source.size
self.instance.extension = self.extension
if not self.instance.user_id:
self.instance.user = self.request.user
instance = super().save(*args, **kwargs)
if hasattr(self, 'to_field'):

View File

@ -0,0 +1,27 @@
# Generated by Django 4.2.11 on 2024-06-03 14:16
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('files', '0009_file_extension'),
]
operations = [
migrations.RemoveField(
model_name='file',
name='extension',
),
migrations.AlterField(
model_name='file',
name='hash',
field=models.CharField(blank=True, editable=False, max_length=255, unique=True),
),
migrations.AlterField(
model_name='file',
name='original_hash',
field=models.CharField(blank=True, editable=False, help_text='The original hash of the file before we repackage it any way.', max_length=255, unique=True),
),
]

View File

@ -49,12 +49,6 @@ class File(CreatedModifiedMixin, TrackChangesMixin, models.Model):
date_approved = models.DateTimeField(null=True, blank=True, editable=False)
date_status_changed = models.DateTimeField(null=True, blank=True, editable=False)
extension = models.ForeignKey(
'extensions.Extension',
null=True,
blank=True,
on_delete=models.CASCADE,
)
source = models.FileField(null=False, blank=False, upload_to=file_upload_to)
thumbnail = models.ImageField(
@ -82,12 +76,13 @@ class File(CreatedModifiedMixin, TrackChangesMixin, models.Model):
User, related_name='files', null=False, blank=False, on_delete=models.CASCADE
)
size_bytes = models.PositiveBigIntegerField(default=0, editable=False)
hash = models.CharField(max_length=255, null=False, blank=True, unique=True)
hash = models.CharField(max_length=255, null=False, blank=True, unique=True, editable=False)
original_name = models.CharField(max_length=255, blank=True, null=False)
original_hash = models.CharField(
max_length=255,
null=False,
blank=True,
editable=False,
unique=True,
help_text='The original hash of the file before we repackage it any way.',
)
@ -196,9 +191,6 @@ class File(CreatedModifiedMixin, TrackChangesMixin, models.Model):
'tags': data.get('tags'),
}
def get_submit_url(self) -> str:
return self.extension.get_draft_url()
def get_thumbnail_of_size(self, size_key: str) -> str:
"""Return absolute path portion of the URL of a thumbnail of this file.

View File

@ -159,7 +159,7 @@ class TestTasks(TestCase):
'form-0-source': fp1,
'form-1-source': fp2,
}
self.client.post(file.get_submit_url(), {**data, **files})
self.client.post(file.version.extension.get_draft_url(), {**data, **files})
new_notification_nr = Notification.objects.filter(recipient=moderator).count()
self.assertEqual(new_notification_nr, notification_nr + 1)

Binary file not shown.

After

Width:  |  Height:  |  Size: 28 KiB