infrastructure/extensions-website
4
8
Fork 10
Code Issues 20 Pull Requests 2 Projects Releases Wiki Activity

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
Conversation 0 Commits 11 Files Changed 17 +322 -166
17 changed files with 322 additions and 166 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
common/admin.py
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

5
common/tests/factories/extensions.py
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

38
extensions/forms.py
View File

@ -64,18 +64,16 @@ class AddPreviewFileForm(files.forms.BaseMediaFileForm):
"""Save Preview from the cleaned form data."""
instance = super().save(*args, **kwargs)
# Create extension preview and save caption to it
extensions.models.Preview.objects.create(
# Create extension preview and save caption to it, ignore duplicate records
extensions.models.Preview.objects.get_or_create(
file=instance,
caption=self.cleaned_data['caption'],
extension=self.extension,
defaults={'caption': self.cleaned_data['caption']},
)
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 +87,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 +106,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 +187,17 @@ class ExtensionUpdateForm(forms.ModelForm):
self.featured_image_form,
self.icon_form,
]
seen_hashes = set()
seen_hashes = {}
# 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)
if not hash:
continue
if hash in seen_hashes:
f.instance = seen_hashes[hash]
else:
seen_hashes[hash] = f.instance
return all(is_valid_flags)
def clean_team(self):
@ -352,9 +342,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)

30
extensions/migrations/0034_alter_extension_featured_image_alter_extension_icon_and_more.py Normal file
View File

@ -0,0 +1,30 @@
# Generated by Django 4.2.11 on 2024-06-04 08:54
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_extensions_fts_20240603_1918'),
]
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'),
),
]

12
extensions/models.py
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

32
extensions/signals.py
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,20 @@ 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:
#
# 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
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,

35
extensions/tests/test_delete.py
View File

@ -21,23 +21,38 @@ class DeleteTest(TestCase):
def test_unlisted_unrated_extension_can_be_deleted_by_author(self):
self.maxDiff = None
reused_image = FileFactory(
type=files.models.File.TYPES.IMAGE,
original_name='extension_feature_image.png',
source='images/b0/b03fa981527593fbe15b28cf37c020220c3d83021999eab036b87f3bca9c9168.png',
)
version = create_version(
file__status=files.models.File.STATUSES.AWAITING_REVIEW,
ratings=[],
extension__icon=FileFactory(
type=files.models.File.TYPES.IMAGE,
original_name='extension_icon_final.png',
source='images/8a/8a01102de8573d50bbc90033f55f232b7cacc4f1eb3e3c3d851615841d2956e1.png',
),
extension__featured_image=reused_image,
extension__previews=[
reused_image,
FileFactory(
type=files.models.File.TYPES.IMAGE,
original_name='extension_preview_001.png',
source='images/b0/b03fa981527593fbe15b28cf37c020220c3d83021999eab036b87f3bca9c9168.png',
)
),
],
)
extension = version.extension
version_file = version.file
icon = extension.icon
featured_image = extension.featured_image
self.assertEqual(version_file.get_status_display(), 'Awaiting Review')
self.assertEqual(extension.get_status_display(), 'Draft')
self.assertFalse(extension.is_listed)
self.assertEqual(extension.cannot_be_deleted_reasons, [])
preview_file = extension.previews.first()
preview_file = extension.previews.last()
self.assertIsNotNone(preview_file)
# Create some ApprovalActivity as well
moderator = create_moderator()
@ -55,11 +70,11 @@ class DeleteTest(TestCase):
repr,
[
version_file,
preview_file,
file_validation,
extension,
approval_activity,
file_validation,
preview_file.preview,
preview_file.preview_set.first(),
reused_image.preview_set.first(),
version,
],
)
@ -78,12 +93,16 @@ class DeleteTest(TestCase):
version.refresh_from_db()
with self.assertRaises(files.models.File.DoesNotExist):
version_file.refresh_from_db()
with self.assertRaises(files.models.File.DoesNotExist):
preview_file.refresh_from_db()
# Preview files aren't deleted: they might be re-uploaded shortly and should be looked up by hash
preview_file.refresh_from_db()
icon.refresh_from_db()
featured_image.refresh_from_db()
self.assertIsNone(extensions.models.Extension.objects.filter(pk=extension.pk).first())
self.assertIsNone(extensions.models.Version.objects.filter(pk=version.pk).first())
self.assertIsNone(files.models.File.objects.filter(pk=version_file.pk).first())
self.assertIsNone(files.models.File.objects.filter(pk=preview_file.pk).first())
self.assertIsNotNone(files.models.File.objects.filter(pk=preview_file.pk).first())
self.assertIsNotNone(files.models.File.objects.filter(pk=icon.pk).first())
self.assertIsNotNone(files.models.File.objects.filter(pk=featured_image.pk).first())
# Check that each of the deleted records was logged
deletion_log_entries_q = LogEntry.objects.filter(action_flag=DELETION)

34
extensions/tests/test_manifest.py
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 Ж')

20
extensions/tests/test_submit.py
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/')

174
extensions/tests/test_update.py
View File

@ -81,7 +81,7 @@ class UpdateTest(CheckFilePropertiesMixin, TestCase):
self.assertEqual(File.objects.filter(type=File.TYPES.IMAGE).count(), 1)
self.assertEqual(extension.previews.count(), 1)
file1 = extension.previews.all()[0]
self.assertEqual(file1.preview.caption, 'First Preview Caption Text')
self.assertEqual(file1.preview_set.first().caption, 'First Preview Caption Text')
self.assertEqual(
file1.original_hash,
'sha256:643e15eb6c4831173bbcf71b8c85efc70cf3437321bf2559b39aa5e9acfd5340',
@ -125,7 +125,7 @@ class UpdateTest(CheckFilePropertiesMixin, TestCase):
extension.refresh_from_db()
self.assertEqual(extension.previews.count(), 1)
video_file = extension.previews.all()[0]
self.assertEqual(video_file.preview.caption, 'First Preview Caption Text')
self.assertEqual(video_file.preview_set.first().caption, 'First Preview Caption Text')
self._test_file_properties(
video_file,
content_type='video/mp4',
@ -169,8 +169,8 @@ class UpdateTest(CheckFilePropertiesMixin, TestCase):
self.assertEqual(extension.previews.count(), 2)
file1 = extension.previews.all()[0]
file2 = extension.previews.all()[1]
self.assertEqual(file1.preview.caption, 'First Preview Caption Text')
self.assertEqual(file2.preview.caption, 'Second Preview Caption Text')
self.assertEqual(file1.preview_set.first().caption, 'First Preview Caption Text')
self.assertEqual(file2.preview_set.first().caption, 'Second Preview Caption Text')
self.assertEqual(
file1.original_hash,
'sha256:643e15eb6c4831173bbcf71b8c85efc70cf3437321bf2559b39aa5e9acfd5340',
@ -222,8 +222,10 @@ class UpdateTest(CheckFilePropertiesMixin, TestCase):
_get_all_form_errors(response),
)
def test_post_upload_validation_error_duplicate_images(self):
def test_post_upload_duplicate_preview_files_ignored(self):
extension = create_approved_version().extension
images_count_before = File.objects.filter(type=File.TYPES.IMAGE).count()
self.assertEqual(extension.previews.count(), 0)
data = {
**POST_DATA,
@ -243,31 +245,24 @@ class UpdateTest(CheckFilePropertiesMixin, TestCase):
}
response = self.client.post(url, {**data, **files})
self.assertEqual(response.status_code, 200)
self.maxDiff = None
self.assertEqual(response.status_code, 302)
# One image was uploaded successfully
self.assertEqual(
[
response.context['add_preview_formset'].forms[0].errors,
response.context['add_preview_formset'].forms[1].errors,
response.context['add_preview_formset'].non_form_errors(),
],
[
{},
{
'__all__': ['Please correct the duplicate values below.'],
'source': ['Please select another file instead of the duplicate.'],
},
['Please select another file instead of the duplicate'],
],
File.objects.filter(type=File.TYPES.IMAGE).count(), images_count_before + 1
)
self.assertEqual(extension.previews.count(), 1)
def test_post_upload_validation_error_image_already_exists(self):
FileFactory(
def test_post_upload_existing_image_file_linked_to_extension(self):
file = FileFactory(
type=File.TYPES.IMAGE,
original_hash='sha256:643e15eb6c4831173bbcf71b8c85efc70cf3437321bf2559b39aa5e9acfd5340',
hash='sha256:643e15eb6c4831173bbcf71b8c85efc70cf3437321bf2559b39aa5e9acfd5340',
source='file/original_image_source.jpg',
)
extension = create_approved_version().extension
images_count_before = File.objects.filter(type=File.TYPES.IMAGE).count()
self.assertEqual(extension.previews.count(), 0)
old_user = file.user
data = {
**POST_DATA,
@ -281,15 +276,56 @@ class UpdateTest(CheckFilePropertiesMixin, TestCase):
files = {'form-0-source': fp1}
response = self.client.post(url, {**data, **files})
self.assertEqual(response.status_code, 200)
self.maxDiff = None
self.assertEqual(
response.context['add_preview_formset'].forms[0].errors,
{'source': ['File with this Original hash already exists.']},
)
self.assertEqual(response.status_code, 302)
# No new files were created: the existing one was linked to the extension instead
self.assertEqual(File.objects.filter(type=File.TYPES.IMAGE).count(), images_count_before)
self.assertEqual(extension.previews.count(), 1)
self.assertEqual(extension.previews.first().original_hash, file.original_hash)
self.assertEqual(extension.previews.first().pk, file.pk)
file.refresh_from_db()
self.assertEqual(file.user, old_user)
def test_post_upload_validation_error_duplicate_across_forms(self):
def test_post_upload_existing_preview_file_linked_to_extension(self):
file = FileFactory(
type=File.TYPES.IMAGE,
original_hash='sha256:643e15eb6c4831173bbcf71b8c85efc70cf3437321bf2559b39aa5e9acfd5340',
hash='sha256:643e15eb6c4831173bbcf71b8c85efc70cf3437321bf2559b39aa5e9acfd5340',
source='file/original_image_source.jpg',
)
extension = create_approved_version().extension
another_extension = create_approved_version(extension__previews=[file]).extension
images_count_before = File.objects.filter(type=File.TYPES.IMAGE).count()
self.assertEqual(extension.previews.count(), 0)
self.assertEqual(another_extension.previews.count(), 1)
old_user = file.user
data = {
**POST_DATA,
'form-TOTAL_FORMS': ['1'],
}
file_name1 = 'test_preview_image_0001.png'
url = extension.get_manage_url()
user = extension.authors.first()
self.client.force_login(user)
with open(TEST_FILES_DIR / file_name1, 'rb') as fp1:
files = {'form-0-source': fp1}
response = self.client.post(url, {**data, **files})
self.assertEqual(response.status_code, 302)
# No new files were created: the existing one was linked to the extension instead
self.assertEqual(File.objects.filter(type=File.TYPES.IMAGE).count(), images_count_before)
self.assertEqual(extension.previews.count(), 1)
self.assertEqual(extension.previews.first().original_hash, file.original_hash)
self.assertEqual(extension.previews.first().pk, file.pk)
# File is referenced as a preview by both extensions
self.assertEqual(file.preview_set.count(), 2)
file.refresh_from_db()
self.assertEqual(file.user, old_user)
def test_post_upload_duplicates_are_ignored_across_forms(self):
extension = create_approved_version().extension
images_count_before = File.objects.filter(type=File.TYPES.IMAGE).count()
self.assertEqual(extension.previews.count(), 0)
data = {
**POST_DATA,
@ -304,12 +340,12 @@ class UpdateTest(CheckFilePropertiesMixin, TestCase):
files = {'form-0-source': fp, 'icon-source': fp1}
response = self.client.post(url, {**data, **files})
self.assertEqual(response.status_code, 200)
self.maxDiff = None
self.assertEqual(response.status_code, 302)
# One image was uploaded successfully
self.assertEqual(
response.context['icon_form'].errors,
{'source': ['Please select another file instead of the duplicate.']},
File.objects.filter(type=File.TYPES.IMAGE).count(), images_count_before + 1
)
self.assertEqual(extension.previews.count(), 1)
def test_post_upload_validation_error_unexpected_preview_format_gif(self):
extension = create_approved_version().extension
@ -419,6 +455,7 @@ class UpdateTest(CheckFilePropertiesMixin, TestCase):
original_name='old_icon.png',
size_bytes=1234,
)
old_icon = extension.icon
self.client.force_login(extension.authors.first())
url = extension.get_manage_url()
@ -427,7 +464,9 @@ class UpdateTest(CheckFilePropertiesMixin, TestCase):
response = self.client.post(url, {**POST_DATA, **files})
self.assertEqual(response.status_code, 302)
extension.icon.refresh_from_db()
old_icon.refresh_from_db()
extension.refresh_from_db()
self.assertNotEqual(extension.icon_id, old_icon.pk)
self._test_file_properties(
extension.icon,
content_type='image/png',
@ -440,6 +479,68 @@ class UpdateTest(CheckFilePropertiesMixin, TestCase):
size_bytes=30177,
)
def test_update_icon_existing_file_linked(self):
file = FileFactory(
type=File.TYPES.IMAGE,
original_hash='sha256:ee3a015c51e35a237755713ec578334efa9ed8870af65b708f591f9254ff4472',
hash='sha256:ee3a015c51e35a237755713ec578334efa9ed8870af65b708f591f9254ff4472',
source='test_icon_0001.png',
)
extension = create_approved_version().extension
images_count_before = File.objects.filter(type=File.TYPES.IMAGE).count()
self.assertIsNone(extension.icon)
self.assertEqual(file.icon_of.count(), 0)
old_user = file.user
url = extension.get_manage_url()
user = extension.authors.first()
self.client.force_login(user)
with open(TEST_FILES_DIR / 'test_icon_0001.png', 'rb') as fp:
files = {'icon-source': fp}
response = self.client.post(url, {**POST_DATA, **files})
self.assertEqual(response.status_code, 302)
# No new files were created: the existing one was linked to the extension instead
self.assertEqual(File.objects.filter(type=File.TYPES.IMAGE).count(), images_count_before)
extension.refresh_from_db()
self.assertEqual(extension.icon, file)
self.assertEqual(file.icon_of.count(), 1)
file.refresh_from_db()
self.assertEqual(file.user, old_user)
def test_update_icon_existing_file_linked_to_multiple_extensions(self):
file = FileFactory(
type=File.TYPES.IMAGE,
original_hash='sha256:ee3a015c51e35a237755713ec578334efa9ed8870af65b708f591f9254ff4472',
hash='sha256:ee3a015c51e35a237755713ec578334efa9ed8870af65b708f591f9254ff4472',
source='test_icon_0001.png',
)
extension = create_approved_version().extension
another_extension = create_approved_version(extension__icon=file).extension
images_count_before = File.objects.filter(type=File.TYPES.IMAGE).count()
self.assertIsNone(extension.icon)
self.assertEqual(another_extension.icon_id, file.pk)
old_user = file.user
url = extension.get_manage_url()
user = extension.authors.first()
self.client.force_login(user)
with open(TEST_FILES_DIR / 'test_icon_0001.png', 'rb') as fp:
files = {'icon-source': fp}
response = self.client.post(url, {**POST_DATA, **files})
self.assertEqual(response.status_code, 302)
# No new files were created: the existing one was linked to the extension instead
self.assertEqual(File.objects.filter(type=File.TYPES.IMAGE).count(), images_count_before)
extension.refresh_from_db()
another_extension.refresh_from_db()
self.assertEqual(extension.icon, file)
self.assertEqual(another_extension.icon, file)
# File is referenced as a preview by both extensions
self.assertEqual(file.icon_of.count(), 2)
file.refresh_from_db()
self.assertEqual(file.user, old_user)
def test_update_featured_image_changes_expected_file_fields(self):
extension = create_approved_version(
extension__featured_image=ImageFactory(
@ -456,6 +557,7 @@ class UpdateTest(CheckFilePropertiesMixin, TestCase):
original_name='old_featured_image.png',
size_bytes=1234,
)
old_featured_image = extension.featured_image
self.client.force_login(extension.authors.first())
url = extension.get_manage_url()
@ -464,7 +566,9 @@ class UpdateTest(CheckFilePropertiesMixin, TestCase):
response = self.client.post(url, {**POST_DATA, **files})
self.assertEqual(response.status_code, 302)
extension.featured_image.refresh_from_db()
old_featured_image.refresh_from_db()
extension.refresh_from_db()
self.assertNotEqual(extension.featured_image_id, old_featured_image.pk)
self._test_file_properties(
extension.featured_image,
content_type='image/png',

1
extensions/views/submit.py
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]

13
files/admin.py
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']

40
files/forms.py
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'):

27
files/migrations/0010_remove_file_extension_alter_file_hash_and_more.py Normal file
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),
),
]

12
files/models.py
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.

2
notifications/tests/test_follow_logic.py
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)

BIN
public/media/images/8a/8a01102de8573d50bbc90033f55f232b7cacc4f1eb3e3c3d851615841d2956e1.png Normal file
View File

Binary file not shown.
Side by Side

After

Width:  |  Height:  |  Size: 28 KiB