From e791dfb61d7bd2c2727b1cf39fd0530cab081784 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Mon, 1 Jul 2024 18:40:10 +0200 Subject: [PATCH 01/38] can_upload_more_files property --- extensions/models.py | 17 +++++++++++++++-- extensions/tests/test_models.py | 33 ++++++++++++++++++++++++++------- files/models.py | 14 ++++++++------ 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/extensions/models.py b/extensions/models.py index 0c02b5c8..cfd079d6 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -677,10 +677,23 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): @cached_property def file(self): files = list(self.files.all()) - if files: + if len(files) == 1: return files[0] - else: + elif len(files) == 0: return None + else: + raise Exception('FIXME: multiple files accessed via .file property') + + @property + def can_upload_more_files(self): + all_platforms = set(p.slug for p in Platform.objects.all()) + for file in self.files.all(): + platforms = file.platforms() + if not platforms: + # no platforms means any platform + return False + all_platforms -= set(platforms) + return len(all_platforms) > 0 @property def is_listed(self): diff --git a/extensions/tests/test_models.py b/extensions/tests/test_models.py index c2f17e5e..ec5cd654 100644 --- a/extensions/tests/test_models.py +++ b/extensions/tests/test_models.py @@ -5,7 +5,9 @@ from django.test import TestCase from common.admin import get_admin_change_path from common.log_entries import entries_for from common.tests.factories.extensions import create_version +from common.tests.factories.files import FileFactory from common.tests.factories.users import UserFactory +from extensions.models import Platform class ExtensionTest(TestCase): @@ -70,19 +72,16 @@ class VersionTest(TestCase): maxDiff = None fixtures = ['dev', 'licenses'] - def setUp(self): - super().setUp() - self.version = create_version( + def test_admin_change_view(self): + version = create_version( metadata__blender_version_min='2.83.1', metadata__name='Extension name', metadata__support='https://example.com/', metadata__version='1.1.2', metadata__website='https://example.com/', ) - self.assertEqual(entries_for(self.version).count(), 0) - - def test_admin_change_view(self): - path = get_admin_change_path(obj=self.version) + self.assertEqual(entries_for(version).count(), 0) + path = get_admin_change_path(obj=version) self.assertEqual(path, '/admin/extensions/version/1/change/') admin_user = UserFactory(is_staff=True, is_superuser=True) @@ -91,6 +90,26 @@ class VersionTest(TestCase): self.assertEqual(response.status_code, 200, path) + def test_can_upload_more_files(self): + version_platforms_none = create_version(metadata__platforms=None) + self.assertFalse(version_platforms_none.can_upload_more_files) + + version_platforms_empty = create_version(metadata__platforms=[]) + self.assertFalse(version_platforms_empty.can_upload_more_files) + + version_platforms_some = create_version(metadata__platforms=['linux-x64', 'windows-x64']) + self.assertTrue(version_platforms_some.can_upload_more_files) + + version_two_files = create_version(metadata__platforms=['linux-x64']) + file = FileFactory(metadata__platforms=['windows-x64']) + version_two_files.files.add(file) + self.assertTrue(version_two_files.can_upload_more_files) + + version_platforms_all = create_version( + metadata__platforms=[p.slug for p in Platform.objects.all()] + ) + self.assertFalse(version_platforms_all.can_upload_more_files) + class UpdateMetadataTest(TestCase): fixtures = ['dev', 'licenses'] diff --git a/files/models.py b/files/models.py index 2c6f47c1..7947ab42 100644 --- a/files/models.py +++ b/files/models.py @@ -177,15 +177,17 @@ class File(CreatedModifiedMixin, TrackChangesMixin, models.Model): 'website': data.get('website'), } - @property - def parsed_version_fields(self) -> Dict[str, Any]: - """Return Version-related data that was parsed from file's content.""" - # Currently, the content of the manifest file is the only - # kind of file metadata that is supported. + def platforms(self): data = self.metadata build_generated_platforms = None if 'build' in data and 'generated' in data['build']: build_generated_platforms = data['build']['generated'].get('platforms') + return build_generated_platforms or data.get('platforms') + + @property + def parsed_version_fields(self) -> Dict[str, Any]: + """Return Version-related data that was parsed from file's content.""" + data = self.metadata return { 'version': data.get('version'), 'tagline': data.get('tagline'), @@ -193,7 +195,7 @@ class File(CreatedModifiedMixin, TrackChangesMixin, models.Model): 'schema_version': data.get('schema_version'), 'licenses': data.get('license'), 'permissions': data.get('permissions'), - 'platforms': build_generated_platforms or data.get('platforms'), + 'platforms': self.platforms(), 'tags': data.get('tags'), } -- 2.30.2 From 3bf3d9678b181b7aaaa1b2db3ba7045bf6f90f22 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Tue, 2 Jul 2024 14:20:06 +0200 Subject: [PATCH 02/38] UploadVersionFileView to upload more files per version --- extensions/models.py | 20 +++++- .../extensions/new_version_finalise.html | 24 +++++-- extensions/templates/extensions/submit.html | 14 +++- extensions/urls.py | 5 ++ extensions/views/manage.py | 40 ++++++++++++ extensions/views/mixins.py | 7 +- files/forms.py | 13 +++- files/validators.py | 64 +++++++++++-------- 8 files changed, 147 insertions(+), 40 deletions(-) diff --git a/extensions/models.py b/extensions/models.py index cfd079d6..748f614b 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -682,18 +682,22 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): elif len(files) == 0: return None else: - raise Exception('FIXME: multiple files accessed via .file property') + log.warning('FIXME: multiple files accessed via .file property') + return files[0] @property def can_upload_more_files(self): + return len(self.get_available_platforms()) > 0 + + def get_available_platforms(self): all_platforms = set(p.slug for p in Platform.objects.all()) for file in self.files.all(): platforms = file.platforms() if not platforms: # no platforms means any platform - return False + return set() all_platforms -= set(platforms) - return len(all_platforms) > 0 + return all_platforms @property def is_listed(self): @@ -767,6 +771,16 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): }, ) + def get_version_upload_url(self) -> str: + return reverse( + 'extensions:version-upload', + kwargs={ + 'type_slug': self.extension.type_slug, + 'slug': self.extension.slug, + 'pk': self.pk, + }, + ) + @property def update_url(self) -> str: return reverse( diff --git a/extensions/templates/extensions/new_version_finalise.html b/extensions/templates/extensions/new_version_finalise.html index 13881e18..eaab347f 100644 --- a/extensions/templates/extensions/new_version_finalise.html +++ b/extensions/templates/extensions/new_version_finalise.html @@ -16,11 +16,7 @@ {% csrf_token %} {% with form=form|add_form_classes %} -

- {% blocktranslate with version=form.instance.version %} - v{{ version }} - {% endblocktranslate %} -

+

v{{ form.instance.version }}

@@ -39,6 +35,24 @@
{% endif %}
+
+ {% with version_files=form.instance.files.all %} + {% trans 'Files' %}: +
    + {% for file in version_files %} +
  • {{ file }} {% trans 'for platforms:' %} {{ file.platforms|join:", " }}
    • + {% endfor %} +
+ {% endwith %} + {% if form.instance.can_upload_more_files %} +
+ + + {% trans 'Upload files for other platforms' %} + +
+
+ {% endif %}
diff --git a/extensions/templates/extensions/submit.html b/extensions/templates/extensions/submit.html index e896a7bd..f993718d 100644 --- a/extensions/templates/extensions/submit.html +++ b/extensions/templates/extensions/submit.html @@ -17,15 +17,19 @@

- {% if extension %} + {% if version %} + {% trans 'Upload New File for' %} {{ version }} + {% elif extension %} {% trans 'Upload New' %} {{ extension.get_type_display }} {% trans 'Version' %} {% else %} {% trans 'Upload Extension' %} {% endif %}

+ {% if extension %}

{{ extension.name }}

+ {% endif %}
{% if not extension and drafts %}
@@ -75,7 +79,9 @@
{% trans 'Size' %}
-
{{ version.file.size_bytes|filesizeformat }}
+ {% if version.single_file %} +
{{ version.files.first.size_bytes|filesizeformat }}
+ {% else %} +
TODO multiple files
+ {% endif %}
diff --git a/extensions/templates/extensions/detail.html b/extensions/templates/extensions/detail.html index 2c0e9936..ffbd890b 100644 --- a/extensions/templates/extensions/detail.html +++ b/extensions/templates/extensions/detail.html @@ -187,7 +187,11 @@
{% trans 'Size' %}
-
{{ latest.file.size_bytes|filesizeformat }}
+ {% if latest.single_file %} +
{{ latest.files.first.size_bytes|filesizeformat }}
+ {% else %} +
TODO multiple files
+ {% endif %}
diff --git a/extensions/templates/extensions/manage/update.html b/extensions/templates/extensions/manage/update.html index 640ff645..69dbf2b4 100644 --- a/extensions/templates/extensions/manage/update.html +++ b/extensions/templates/extensions/manage/update.html @@ -3,7 +3,7 @@ {% block page_title %}{{ extension.name }}{% endblock page_title %} {% block content %} -{% with author=extension.latest_version.file.user form=form|add_form_classes %} +{% with form=form|add_form_classes %}

{{ extension.get_type_display }} {% trans 'details' %}

diff --git a/extensions/templates/extensions/version_list.html b/extensions/templates/extensions/version_list.html index 2182aba1..c2f586af 100644 --- a/extensions/templates/extensions/version_list.html +++ b/extensions/templates/extensions/version_list.html @@ -30,7 +30,11 @@ {% include "extensions/components/blender_version.html" with version=version %}
    -
  • {{ version.file.size_bytes|filesizeformat }}
    • + {% if version.single_file %} +
  • {{ version.files.first.size_bytes|filesizeformat }}
    • + {% else %} +
  • TODO multiple files
    • + {% endif %}
  • {{ version.download_count }}
  • @@ -69,7 +73,11 @@
Size
-
{{ version.file.size_bytes|filesizeformat }}
+ {% if version.single_file %} +
{{ version.files.first.size_bytes|filesizeformat }}
+ {% else %} +
TODO multiple files
+ {% endif %}
@@ -91,7 +99,11 @@
Status
-
{% include "common/components/status.html" with object=version.file %}
+ {% if version.single_file %} +
{% include "common/components/status.html" with object=version.files.first %}
+ {% else %} +
TODO multiple files
+ {% endif %}
{% endif %} diff --git a/extensions/tests/test_api.py b/extensions/tests/test_api.py index da38b708..8e0c4b4b 100644 --- a/extensions/tests/test_api.py +++ b/extensions/tests/test_api.py @@ -48,8 +48,9 @@ class ListedExtensionsTest(APITestCase): self.assertEqual(self._listed_extensions_count(), 0) def test_moderate_only_version(self): - self.version.file.status = File.STATUSES.DISABLED - self.version.file.save() + for file in self.version.files.all(): + file.status = File.STATUSES.DISABLED + file.save() self.assertEqual(self._listed_extensions_count(), 0) diff --git a/extensions/tests/test_approve.py b/extensions/tests/test_approve.py index f438f316..2bd00de9 100644 --- a/extensions/tests/test_approve.py +++ b/extensions/tests/test_approve.py @@ -26,7 +26,7 @@ class ApproveExtensionTest(TestCase): # latest_version of approved extension must be listed # check that we rollback latest_version when file is not approved - file = new_version.file + file = new_version.files.first() file.status = File.STATUSES.AWAITING_REVIEW file.save(update_fields={'status'}) new_version.refresh_from_db() @@ -36,8 +36,9 @@ class ApproveExtensionTest(TestCase): self.assertTrue(extension.is_listed) # break the first_version, check that nothing is listed anymore - first_version.file.status = File.STATUSES.AWAITING_REVIEW - first_version.file.save() + file = first_version.files.first() + file.status = File.STATUSES.AWAITING_REVIEW + file.save() self.assertFalse(first_version.is_listed) extension.refresh_from_db() self.assertFalse(extension.is_listed) diff --git a/extensions/tests/test_delete.py b/extensions/tests/test_delete.py index fda39d5e..dfaf2f1f 100644 --- a/extensions/tests/test_delete.py +++ b/extensions/tests/test_delete.py @@ -43,7 +43,7 @@ class DeleteTest(TestCase): source='images/b0/b03fa981527593fbe15b28cf37c020220c3d83021999eab036b87f3bca9c9168.png', ) ) - version_file = version.file + version_file = version.files.first() icon = extension.icon featured_image = extension.featured_image self.assertEqual(version_file.get_status_display(), 'Awaiting Review') diff --git a/extensions/tests/test_manifest.py b/extensions/tests/test_manifest.py index 57a48cc4..5686ef73 100644 --- a/extensions/tests/test_manifest.py +++ b/extensions/tests/test_manifest.py @@ -185,7 +185,7 @@ class ValidateManifestTest(CreateFileTest): self.assertEqual(Extension.objects.count(), 1) # The same author is to send a new version to thte same extension - self.client.force_login(version.file.user) + self.client.force_login(version.files.first().user) non_kitsu_1_6 = { "name": "Blender Kitsu with a different ID", @@ -212,7 +212,7 @@ class ValidateManifestTest(CreateFileTest): self.assertEqual(Extension.objects.count(), 1) # The same author is to send a new version to thte same extension - self.client.force_login(version.file.user) + self.client.force_login(version.files.first().user) kitsu_version_clash = { "name": "Change the name for lols", @@ -253,7 +253,7 @@ class ValidateManifestTest(CreateFileTest): ) # The same author is to send a new version to thte same extension - self.client.force_login(version.file.user) + self.client.force_login(version.files.first().user) updated_kitsu = { "name": version.extension.name, @@ -692,7 +692,7 @@ class VersionPermissionsTest(CreateFileTest): self.assertEqual(version_original, extension.latest_version) # The same author is to send a new version to the same extension but with a new permission - self.client.force_login(version_original.file.user) + self.client.force_login(version_original.files.first().user) new_kitsu = { "id": "blender_kitsu", diff --git a/extensions/tests/test_submit.py b/extensions/tests/test_submit.py index 687726bd..a63a2506 100644 --- a/extensions/tests/test_submit.py +++ b/extensions/tests/test_submit.py @@ -118,12 +118,14 @@ EXPECTED_VALIDATION_ERRORS = { 'source': ['The manifest file is missing.'], }, 'invalid-manifest-toml.zip': {'source': ['Manifest file contains invalid code.']}, - 'invalid-theme-multiple-xmls.zip': {'source': ['Themes can only contain one XML file.']}, - 'invalid-theme-multiple-xmls-macosx.zip': {'source': ['Archive contains forbidden files or directories: __MACOSX/']}, + 'invalid-theme-multiple-xmls.zip': { + 'source': ['Themes can only contain one XML file.'] + }, + 'invalid-theme-multiple-xmls-macosx.zip': { + 'source': ['Archive contains forbidden files or directories: __MACOSX/'] + }, 'invalid-missing-wheels.zip': { - 'source': [ - 'Python wheel missing: addon/wheels/test-wheel-whatever.whl' - ] + 'source': ['Python wheel missing: addon/wheels/test-wheel-whatever.whl'] }, } POST_DATA = { @@ -435,11 +437,11 @@ class SubmitFinaliseTest(CheckFilePropertiesMixin, TestCase): self.assertEqual(version.release_notes, data['release_notes']) # Check version file properties self._test_file_properties( - version.file, + version.files.first(), content_type='application/zip', get_status_display='Awaiting Review', get_type_display='Add-on', - hash=version.file.original_hash, + hash=version.files.first().original_hash, original_hash='sha256:2831385', ) # We cannot check for the ManyToMany yet (tags, licences, permissions) @@ -515,7 +517,7 @@ class NewVersionTest(TestCase): self.assertTrue(response['Location'].startswith('/oauth/login')) def test_validation_errors_agreed_with_terms_required(self): - self.client.force_login(self.version.file.user) + self.client.force_login(self.version.files.first().user) with open(TEST_FILES_DIR / 'amaranth-1.0.8.zip', 'rb') as fp: response = self.client.post(self.url, {'source': fp}) @@ -527,7 +529,7 @@ class NewVersionTest(TestCase): ) def test_validation_errors_invalid_extension(self): - self.client.force_login(self.version.file.user) + self.client.force_login(self.version.files.first().user) with open(TEST_FILES_DIR / 'empty.txt', 'rb') as fp: response = self.client.post(self.url, {'source': fp, 'agreed_with_terms': True}) @@ -539,7 +541,7 @@ class NewVersionTest(TestCase): ) def test_validation_errors_empty_file(self): - self.client.force_login(self.version.file.user) + self.client.force_login(self.version.files.first().user) with open(TEST_FILES_DIR / 'empty.zip', 'rb') as fp: response = self.client.post(self.url, {'source': fp, 'agreed_with_terms': True}) @@ -551,7 +553,7 @@ class NewVersionTest(TestCase): ) def test_upload_new_file_and_finalise_new_version(self): - self.client.force_login(self.version.file.user) + self.client.force_login(self.version.files.first().user) self.extension.approve() # Check step 1: upload a new file @@ -570,7 +572,7 @@ class NewVersionTest(TestCase): self.assertEqual(new_version.version, '1.0.8') self.assertEqual(new_version.blender_version_min, '4.2.0') self.assertEqual(new_version.schema_version, '1.0.0') - self.assertEqual(new_version.file.get_status_display(), 'Approved') + self.assertEqual(new_version.files.first().get_status_display(), 'Approved') self.assertEqual(new_version.release_notes, '') self.assertEqual( ApprovalActivity.objects.filter( diff --git a/extensions/tests/test_views.py b/extensions/tests/test_views.py index 67168277..c0cf1a75 100644 --- a/extensions/tests/test_views.py +++ b/extensions/tests/test_views.py @@ -216,7 +216,7 @@ class ExtensionManageViewTest(_BaseTestCase): class UpdateVersionViewTest(_BaseTestCase): def test_update_view_access(self): extension = _create_extension() - extension_owner = extension.latest_version.file.user + extension_owner = extension.latest_version.files.first().user extension.authors.add(extension_owner) random_user = UserFactory() @@ -246,7 +246,7 @@ class UpdateVersionViewTest(_BaseTestCase): def test_blender_max_version(self): extension = _create_extension() - extension_owner = extension.latest_version.file.user + extension_owner = extension.latest_version.files.first().user extension.authors.add(extension_owner) self.client.force_login(extension_owner) url = reverse( diff --git a/extensions/views/api.py b/extensions/views/api.py index 0ce5ecd3..6abccdd7 100644 --- a/extensions/views/api.py +++ b/extensions/views/api.py @@ -55,13 +55,10 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): self.platform = self.UNKNOWN_PLATFORM def to_representation(self, instance): + matching_file = None matching_version = None # avoid triggering additional db queries, reuse the prefetched queryset - versions = [ - v - for v in instance.versions.all() - if v.file and v.file.status in instance.valid_file_statuses - ] + versions = [v for v in instance.versions.all() if v.is_listed] if not versions: return None versions = sorted(versions, key=lambda v: v.date_created, reverse=True) @@ -72,15 +69,17 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): v.blender_version_max, ): continue - platform_slugs = set(p.slug for p in v.platforms.all()) - # empty platforms field matches any platform filter - # UNKNOWN_PLATFORM matches only empty platforms field - if self.platform and (platform_slugs and self.platform not in platform_slugs): - continue - matching_version = v + for file in v.files.all(): + # empty platforms field matches any platform filter + # UNKNOWN_PLATFORM matches only empty platforms field + platforms = file.platforms() + if self.platform and (platforms and self.platform not in platforms): + continue + matching_file = file + matching_version = v break - if not matching_version: + if not matching_file: return None data = { @@ -89,9 +88,10 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): 'name': instance.name, 'version': matching_version.version, 'tagline': matching_version.tagline, - 'archive_hash': matching_version.file.original_hash, - 'archive_size': matching_version.file.size_bytes, + 'archive_hash': matching_file.original_hash, + 'archive_size': matching_file.size_bytes, 'archive_url': self.request.build_absolute_uri( + # FIXME download_url is per file matching_version.download_url(append_repository_and_compatibility=False) ), 'type': EXTENSION_TYPE_SLUGS_SINGULAR.get(instance.type), @@ -101,7 +101,8 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): # avoid triggering additional db queries, reuse the prefetched queryset 'maintainer': instance.team and instance.team.name or str(instance.authors.all()[0]), 'license': [license_iter.slug for license_iter in matching_version.licenses.all()], - 'permissions': matching_version.file.metadata.get('permissions'), + 'permissions': matching_file.metadata.get('permissions'), + # FIXME? should we instead list platforms of matching_file? 'platforms': [platform.slug for platform in matching_version.platforms.all()], # TODO: handle copyright 'tags': [str(tag) for tag in matching_version.tags.all()], diff --git a/extensions/views/manage.py b/extensions/views/manage.py index 0b3abf55..cf848232 100644 --- a/extensions/views/manage.py +++ b/extensions/views/manage.py @@ -350,8 +350,10 @@ class DraftExtensionView( """Return initial values for the version, based on the file.""" initial = super().get_initial() if self.version: - initial['file'] = self.version.file - initial.update(**self.version.file.parsed_version_fields) + # FIXME? double-check that looking at the first file makes sense + file = self.version.files.all()[0] + initial['file'] = file + initial.update(**file.parsed_version_fields) return initial def get_context_data(self, form=None, extension_form=None, **kwargs): diff --git a/ratings/templates/ratings/rating_list.html b/ratings/templates/ratings/rating_list.html index b2e70db7..67d3922e 100644 --- a/ratings/templates/ratings/rating_list.html +++ b/ratings/templates/ratings/rating_list.html @@ -4,7 +4,7 @@ {% block page_title %}{{ extension.name }}{% endblock page_title %} {% block content %} - {% with latest=extension.latest_version author=extension.latest_version.file.user %} + {% with latest=extension.latest_version %}

{% with text_ratings_count=extension.text_ratings_count %} diff --git a/reviewers/tests/test_views.py b/reviewers/tests/test_views.py index 1fb91fca..d2028a1d 100644 --- a/reviewers/tests/test_views.py +++ b/reviewers/tests/test_views.py @@ -14,7 +14,7 @@ class CommentsViewTest(TestCase): self.default_version = version ApprovalActivity( type=ApprovalActivity.ActivityType.COMMENT, - user=version.file.user, + user=version.files.first().user, extension=version.extension, message='test comment', ).save() diff --git a/users/tests/test_tasks.py b/users/tests/test_tasks.py index 12567b39..9f65100b 100644 --- a/users/tests/test_tasks.py +++ b/users/tests/test_tasks.py @@ -10,7 +10,6 @@ from common.tests.factories.extensions import RatingFactory, create_approved_ver from common.tests.factories.reviewers import ApprovalActivityFactory from common.tests.factories.users import OAuthUserFactory, create_moderator, UserFactory import extensions.models -import files.models import users.tasks as tasks User = get_user_model() @@ -87,7 +86,7 @@ class TestTasks(TestCase): self.assertEqual(response.status_code, 302) self.assertEqual( response['Location'], - f'/media/{version.file.original_name}' + f'/media/{version.files.first().original_name}' f'?filename=add-on-{version.extension.slug}-v{version.version}.zip', ) # Check that ratings remained publicly accessible @@ -146,10 +145,9 @@ class TestTasks(TestCase): user.refresh_from_db() # Check that unlisted extension file and version were deleted - with self.assertRaises(files.models.File.DoesNotExist): - self.authored_unlisted_extension.latest_version.file.refresh_from_db() with self.assertRaises(extensions.models.Version.DoesNotExist): self.authored_unlisted_extension.latest_version.refresh_from_db() + self.assertFalse(self.authored_unlisted_extension.latest_version.files.all()) # Check that reports made by this account remained accessible self.report1.refresh_from_db() -- 2.30.2 From ef122d6427aeefe550a3ea13dd4e044985ce617d Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Thu, 4 Jul 2024 15:03:06 +0200 Subject: [PATCH 07/38] version.add_file and update_platforms --- extensions/models.py | 25 +++++++++++++++++++++++++ extensions/signals.py | 2 ++ extensions/tests/test_models.py | 15 +++++++++++++++ extensions/views/api.py | 1 + extensions/views/manage.py | 2 +- 5 files changed, 44 insertions(+), 1 deletion(-) diff --git a/extensions/models.py b/extensions/models.py index 2870dee8..c5bbf92f 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -681,6 +681,31 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): def can_upload_more_files(self): return len(self.get_available_platforms()) > 0 + def add_file(self, file: File): + self.files.add(file) + self.update_platforms() + + @transaction.atomic + def update_platforms(self): + current_platforms = set([p.slug for p in self.platforms.all()]) + files_platforms = set() + for file in self.files.all(): + if file_platforms := file.platforms(): + files_platforms.update(file_platforms) + else: + # we have a cross-platform file - version must not specify platforms + # other files should not exist (validation takes care of that), + # but here we won't double-check it (?) + if len(current_platforms): + self.platforms.delete() + return + if to_create := files_platforms - current_platforms: + for p in to_create: + self.platforms.add(Platform.objects.get(slug=p)) + if to_delete := current_platforms - files_platforms: + for p in to_delete: + self.platforms.remove(Platform.objects.get(slug=p)) + def get_available_platforms(self): all_platforms = set(p.slug for p in Platform.objects.all()) for file in self.files.all(): diff --git a/extensions/signals.py b/extensions/signals.py index 81b096bb..e4644e22 100644 --- a/extensions/signals.py +++ b/extensions/signals.py @@ -43,6 +43,8 @@ def _delete_versionfiles_file( logger.info('Deleting File pk=%s of VersionFile pk=%s', file.pk, instance.pk) file.delete() + instance.version.update_platforms() + # this code is already quite convoluted # TODO? maybe find some way to have a predictable order of deletion try: diff --git a/extensions/tests/test_models.py b/extensions/tests/test_models.py index ec5cd654..02256a21 100644 --- a/extensions/tests/test_models.py +++ b/extensions/tests/test_models.py @@ -110,6 +110,21 @@ class VersionTest(TestCase): ) self.assertFalse(version_platforms_all.can_upload_more_files) + def test_collect_platforms_across_files(self): + version = create_version(metadata__platforms=['linux-x64']) + file = FileFactory(metadata__platforms=['windows-x64']) + version.add_file(file) + self.assertQuerysetEqual( + version.platforms.order_by('slug'), + Platform.objects.filter(slug__in=['linux-x64', 'windows-x64']).order_by('slug'), + ) + file.delete() + version.refresh_from_db() + self.assertQuerysetEqual( + version.platforms.order_by('slug'), + Platform.objects.filter(slug__in=['linux-x64']).order_by('slug'), + ) + class UpdateMetadataTest(TestCase): fixtures = ['dev', 'licenses'] diff --git a/extensions/views/api.py b/extensions/views/api.py index 6abccdd7..27a60950 100644 --- a/extensions/views/api.py +++ b/extensions/views/api.py @@ -218,6 +218,7 @@ class UploadExtensionVersionView(APIView): file_instance.user = user file_instance.save() + # FIXME check if version already exists, then append file and update release_notes version = extension.create_version_from_file( file=file_instance, release_notes=release_notes, diff --git a/extensions/views/manage.py b/extensions/views/manage.py index cf848232..7705a311 100644 --- a/extensions/views/manage.py +++ b/extensions/views/manage.py @@ -307,7 +307,7 @@ class UploadVersionFileView( @transaction.atomic def form_valid(self, form): response = super().form_valid(form) - self.version.files.add(self.object) + self.version.add_file(self.object) return response def get_success_url(self): -- 2.30.2 From 06c939defbe54eaf3cb6d5044b7fcab35b78bab8 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Thu, 4 Jul 2024 15:57:18 +0200 Subject: [PATCH 08/38] upload multi-os files via api --- ...p => addon-with-split-platforms-linux.zip} | Bin .../addon-with-split-platforms-windows.zip | Bin 0 -> 791 bytes extensions/tests/test_api.py | 59 +++++++++++++++++- extensions/tests/test_submit.py | 2 +- extensions/views/api.py | 17 +++-- files/models.py | 5 +- files/validators.py | 3 +- 7 files changed, 77 insertions(+), 9 deletions(-) rename extensions/tests/files/{addon-with-split-platforms.zip => addon-with-split-platforms-linux.zip} (100%) create mode 100644 extensions/tests/files/addon-with-split-platforms-windows.zip diff --git a/extensions/tests/files/addon-with-split-platforms.zip b/extensions/tests/files/addon-with-split-platforms-linux.zip similarity index 100% rename from extensions/tests/files/addon-with-split-platforms.zip rename to extensions/tests/files/addon-with-split-platforms-linux.zip diff --git a/extensions/tests/files/addon-with-split-platforms-windows.zip b/extensions/tests/files/addon-with-split-platforms-windows.zip new file mode 100644 index 0000000000000000000000000000000000000000..37a5a828f9e6643c5db3cebfc039cb9ad311d643 GIT binary patch literal 791 zcmWIWW@h1H0D%WnJ0rjhD8bDj!w?^znU`4-AFo$X85+XLz|2x8ng+t972FJrEH9ZE z7+78bi2%4E69YOUN|*n=BnmVHgn5C66zAur#wVtv=4Q@5})~c#^)t1N98oqPP(lP*O%wdU;Vx7td_jiYGIbGqJGM%bpo4y zRjyOGo@w-As>0hkq4{2mgf`kYT;4uwX55vtTa)G-eo)Nx%G+UA&rzrNtNT-Y_nz2v zuYY>OVMWp0+grRVUw?e1Ze;b*#=7@T<@@H@#uYDL#>{;`{gvM;;YlBtN9>DMG*f12 zH++6%?Ty8)YXTX!I7+23R# Date: Thu, 4 Jul 2024 17:10:20 +0200 Subject: [PATCH 09/38] test for multi-platform upload via website --- extensions/tests/test_submit.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/extensions/tests/test_submit.py b/extensions/tests/test_submit.py index 3309cde5..d0eaa79a 100644 --- a/extensions/tests/test_submit.py +++ b/extensions/tests/test_submit.py @@ -597,6 +597,37 @@ class NewVersionTest(TestCase): self.assertEqual(new_version.release_notes, 'new version') +class MultiPlatformUploadTest(TestCase): + def test_upload_more_files(self): + extension = create_version( + metadata__id="some_addon", + metadata__version="0.0.1", + ).extension + file_linux = TEST_FILES_DIR / 'addon-with-split-platforms-linux.zip' + file_windows = TEST_FILES_DIR / 'addon-with-split-platforms-windows.zip' + self.client.force_login(extension.authors.all()[0]) + + with open(file_linux, 'rb') as fp: + response = self.client.post( + extension.get_new_version_url(), + {'source': fp, 'agreed_with_terms': True}, + ) + self.assertEqual(response.status_code, 302) + extension.refresh_from_db() + self.assertEqual(extension.latest_version.files.count(), 1) + self.assertEqual(extension.latest_version.platforms.count(), 1) + + with open(file_windows, 'rb') as fp: + response = self.client.post( + extension.latest_version.get_version_upload_url(), + {'source': fp, 'agreed_with_terms': True}, + ) + self.assertEqual(response.status_code, 302) + extension.refresh_from_db() + self.assertEqual(extension.latest_version.files.count(), 2) + self.assertEqual(extension.latest_version.platforms.count(), 2) + + class DraftsWarningTest(TestCase): fixtures = ['licenses'] -- 2.30.2 From 486336c960c0ffc954ba0008429dc1fa3279fbba Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Thu, 4 Jul 2024 17:29:56 +0200 Subject: [PATCH 10/38] a hacky fix of deleting a version with multiple files --- extensions/signals.py | 5 ++--- extensions/tests/test_delete.py | 11 +++++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/extensions/signals.py b/extensions/signals.py index e4644e22..4840604d 100644 --- a/extensions/signals.py +++ b/extensions/signals.py @@ -43,11 +43,10 @@ def _delete_versionfiles_file( logger.info('Deleting File pk=%s of VersionFile pk=%s', file.pk, instance.pk) file.delete() - instance.version.update_platforms() - - # this code is already quite convoluted + # this code is already quite convoluted :double-facepalm: # TODO? maybe find some way to have a predictable order of deletion try: + instance.version.update_platforms() if instance.version.files.count() == 0: # this was the last file, clean up the version logger.info( diff --git a/extensions/tests/test_delete.py b/extensions/tests/test_delete.py index dfaf2f1f..815dd6dd 100644 --- a/extensions/tests/test_delete.py +++ b/extensions/tests/test_delete.py @@ -216,3 +216,14 @@ class DeleteTest(TestCase): self.assertEqual(response.status_code, 403) extension.refresh_from_db() + + def test_multi_file_version_delete(self): + version = create_version(metadata__platforms=['linux-x64']) + version.add_file( + FileFactory( + metadata__id=version.extension.extension_id, + metadata__version=version.version, + metadata__platforms=['windows-x64'], + ) + ) + version.extension.delete() -- 2.30.2 From d88f0422eac425b675e92e23d1b66cc10a05f5ae Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Thu, 4 Jul 2024 18:07:20 +0200 Subject: [PATCH 11/38] fix prefetches --- extensions/templates/extensions/version_list.html | 2 +- extensions/views/manage.py | 10 +--------- extensions/views/public.py | 8 +++----- reviewers/views.py | 6 +++++- 4 files changed, 10 insertions(+), 16 deletions(-) diff --git a/extensions/templates/extensions/version_list.html b/extensions/templates/extensions/version_list.html index c2f586af..c5f18004 100644 --- a/extensions/templates/extensions/version_list.html +++ b/extensions/templates/extensions/version_list.html @@ -21,7 +21,7 @@
- {% for version in extension.versions.all %} + {% for version in object_list %} {% if version.is_listed or is_maintainer %}
diff --git a/extensions/views/manage.py b/extensions/views/manage.py index 7705a311..e67ae74b 100644 --- a/extensions/views/manage.py +++ b/extensions/views/manage.py @@ -30,17 +30,9 @@ class VersionsView(ExtensionQuerysetMixin, ListView): paginate_by = 15 def get_queryset(self): - """Allow logged in users to view unlisted versions in certain conditions. - - * maintainers should be able to preview their yet unlisted versions; - * staff should be able to preview yet unlisted versions; - """ self.extension_queryset = self.get_extension_queryset() self.extension = get_object_or_404(self.extension_queryset, slug=self.kwargs['slug']) - queryset = self.extension.versions - if self.request.user.is_staff or self.extension.has_maintainer(self.request.user): - return queryset.all() - return queryset.listed + return self.extension.versions.prefetch_related('files', 'platforms', 'permissions') def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) diff --git a/extensions/views/public.py b/extensions/views/public.py index ef92cdba..28877e09 100644 --- a/extensions/views/public.py +++ b/extensions/views/public.py @@ -227,6 +227,9 @@ class ExtensionDetailView(DetailView): queryset = Extension.objects.listed.prefetch_related( 'authors', 'latest_version__files', + 'latest_version__files__validation', + 'latest_version__permissions', + 'latest_version__platforms', 'latest_version__tags', 'preview_set', 'preview_set__file', @@ -235,11 +238,6 @@ class ExtensionDetailView(DetailView): 'ratings__ratingreply__user', 'ratings__user', 'team', - 'versions', - 'versions__files', - 'versions__files__validation', - 'versions__permissions', - 'versions__platforms', ).distinct() context_object_name = 'extension' template_name = 'extensions/detail.html' diff --git a/reviewers/views.py b/reviewers/views.py index f9ad2ef7..4e6569e6 100644 --- a/reviewers/views.py +++ b/reviewers/views.py @@ -77,9 +77,13 @@ class ExtensionsApprovalDetailView(DetailView): def get_queryset(self): return self.model.objects.prefetch_related( 'authors', + 'latest_version__files', + 'latest_version__files__validation', + 'latest_version__permissions', + 'latest_version__platforms', + 'latest_version__tags', 'preview_set', 'preview_set__file', - 'versions', ).all() def get_context_data(self, **kwargs): -- 2.30.2 From acca5d5404c4d76d29c30487b8891a40a164565a Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Thu, 4 Jul 2024 18:32:15 +0200 Subject: [PATCH 12/38] a temporary solution for file size display --- .../components/extension_edit_detail_card.html | 4 +++- extensions/templates/extensions/detail.html | 4 +++- extensions/templates/extensions/version_list.html | 13 +++++++------ 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/extensions/templates/extensions/components/extension_edit_detail_card.html b/extensions/templates/extensions/components/extension_edit_detail_card.html index 8d758aff..2d6d3ec4 100644 --- a/extensions/templates/extensions/components/extension_edit_detail_card.html +++ b/extensions/templates/extensions/components/extension_edit_detail_card.html @@ -72,7 +72,9 @@ {% if version.single_file %}
{{ version.files.first.size_bytes|filesizeformat }}
{% else %} -
TODO multiple files
+ {% for file in version.files.all %} +
{{ file.size_bytes|filesizeformat }} ({{ file.platforms|join:", " }})
+ {% endfor %} {% endif %}
diff --git a/extensions/templates/extensions/detail.html b/extensions/templates/extensions/detail.html index ffbd890b..cc82d546 100644 --- a/extensions/templates/extensions/detail.html +++ b/extensions/templates/extensions/detail.html @@ -190,7 +190,9 @@ {% if latest.single_file %}
{{ latest.files.first.size_bytes|filesizeformat }}
{% else %} -
TODO multiple files
+ {% for file in latest.files.all %} +
{{ file.size_bytes|filesizeformat }} ({{ file.platforms|join:", " }})
+ {% endfor %} {% endif %}

diff --git a/extensions/templates/extensions/version_list.html b/extensions/templates/extensions/version_list.html index c5f18004..9e9374ed 100644 --- a/extensions/templates/extensions/version_list.html +++ b/extensions/templates/extensions/version_list.html @@ -33,7 +33,9 @@ {% if version.single_file %}
  • {{ version.files.first.size_bytes|filesizeformat }}
    • {% else %} -
  • TODO multiple files
    • + {% for file in version.files.all %} +
  • {{ file.size_bytes|filesizeformat }} ({{ file.platforms|join:", " }})
    • + {% endfor %} {% endif %}
  • {{ version.download_count }}
  • @@ -76,7 +78,9 @@ {% if version.single_file %}
    {{ version.files.first.size_bytes|filesizeformat }}
    {% else %} -
    TODO multiple files
    + {% for file in version.files.all %} +
    {{ file.size_bytes|filesizeformat }} ({{ file.platforms|join:", " }})
    + {% endfor %} {% endif %}
    • @@ -99,11 +103,8 @@
    Status
    - {% if version.single_file %} + {# all files of a version get approved together #}
    {% include "common/components/status.html" with object=version.files.first %}
    - {% else %} -
    TODO multiple files
    - {% endif %}
    {% endif %} -- 2.30.2 From 9f8a984bc11ee4823573948203796331a839d065 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Thu, 4 Jul 2024 18:54:04 +0200 Subject: [PATCH 13/38] collect permissions from all files, just in case --- extensions/models.py | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/extensions/models.py b/extensions/models.py index c5bbf92f..692ba183 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -733,16 +733,26 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): @property def permissions_with_reasons(self) -> List[dict]: - """Returns permissions with reasons, slugs, and names to be shown in templates.""" - # FIXME? is it ok to check just the first file? - file = self.files.all()[0] - if 'permissions' not in file.metadata: - return [] - permissions = [] - all_permission_names = {p.slug: p.name for p in VersionPermission.objects.all()} - for slug, reason in file.metadata['permissions'].items(): - permissions.append({'slug': slug, 'reason': reason, 'name': all_permission_names[slug]}) + """Returns permissions with reasons, slugs, and names to be shown in templates. + It might make sense to not aggregate at the Version level, and query directly from File. + If we ever restructure multi-platform UI, this method should be revisited. + + Normally we can expect that all files have the same permissions, but checking all files, + just in case. + """ + slug2reason = {} + for file in self.files.all(): + if 'permissions' in file.metadata: + slug2reason.update(file.metadata['permissions']) + + if not slug2reason: + return [] + + all_permission_names = {p.slug: p.name for p in VersionPermission.objects.all()} + permissions = [] + for slug, reason in slug2reason.items(): + permissions.append({'slug': slug, 'reason': reason, 'name': all_permission_names[slug]}) return permissions # FIXME make dependent on File or platform -- 2.30.2 From a0f0c61825cdee6112d82130e55a1451a12279f3 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Thu, 4 Jul 2024 19:26:30 +0200 Subject: [PATCH 14/38] url for version-platform-download --- extensions/models.py | 10 ++-------- extensions/urls.py | 5 +++++ extensions/views/public.py | 16 +++++++++++++++- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/extensions/models.py b/extensions/models.py index 692ba183..b5bb154c 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -755,18 +755,12 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): permissions.append({'slug': slug, 'reason': reason, 'name': all_permission_names[slug]}) return permissions - # FIXME make dependent on File or platform + # FIXME? make dependent on File or platform @property def download_name(self) -> str: """Return a file name for downloads.""" replace_char = f'{self}'.replace('.', '-') - return f'{utils.slugify(replace_char)}{self.files.first().suffix}' - - # FIXME make dependent on File or platform - @property - def downloadable_signed_url(self) -> str: - # TODO: actual signed URLs? - return self.files.first().source.url + return f'{utils.slugify(replace_char)}.zip' # FIXME make dependent on File or platform def download_url(self, append_repository_and_compatibility=True) -> str: diff --git a/extensions/urls.py b/extensions/urls.py index 36dac874..5546016e 100644 --- a/extensions/urls.py +++ b/extensions/urls.py @@ -91,6 +91,11 @@ urlpatterns = [ public.extension_version_download, name='version-download', ), + path( + '///download/', + public.extension_version_platform_download, + name='version-platform-download', + ), path( '/manage/versions//upload/', manage.UploadVersionFileView.as_view(), diff --git a/extensions/views/public.py b/extensions/views/public.py index 28877e09..fae00d25 100644 --- a/extensions/views/public.py +++ b/extensions/views/public.py @@ -4,6 +4,7 @@ import logging from django.contrib.auth import get_user_model from django.db import connection from django.db.models import Count, Q +from django.http import Http404 from django.shortcuts import get_object_or_404, redirect from django.utils.translation import gettext_lazy as _ from django.views.generic import DetailView, ListView @@ -52,6 +53,14 @@ class HomeView(ListedExtensionsView): def extension_version_download(request, type_slug, slug, version, filename): + """A backward-compatible shortcut for the method below. + + No platform is specified, assuming only a single file for a given version. + """ + return extension_version_platform_download(request, type_slug, slug, version, None, filename) + + +def extension_version_platform_download(request, type_slug, slug, version, platform, filename): """Download an extension version and count downloads. The `filename` parameter is used to pass a file name ending with `.zip`. @@ -61,7 +70,12 @@ def extension_version_download(request, type_slug, slug, version, filename): extension_version = get_object_or_404(Version, extension__slug=slug, version=version) ExtensionDownload.create_from_request(request, object_id=extension_version.extension_id) VersionDownload.create_from_request(request, object_id=extension_version.pk) - return redirect(extension_version.downloadable_signed_url + f'?filename={filename}') + for file in extension_version.files.all(): + platforms = file.platforms() or [] + if platform is None or platform in platforms: + url = file.source.url + return redirect(f'{url}?filename={filename}') + raise Http404() class SearchView(ListedExtensionsView): -- 2.30.2 From d5ac9826b6592ff2555d4ecf7444f4e7cdafecf2 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Thu, 4 Jul 2024 21:42:47 +0200 Subject: [PATCH 15/38] download info for files, not versions --- extensions/models.py | 60 ++++++++++----------- extensions/templates/extensions/detail.html | 20 ++++--- extensions/views/api.py | 9 ++-- files/models.py | 45 ++++++++++++++++ users/tests/test_tasks.py | 2 +- 5 files changed, 94 insertions(+), 42 deletions(-) diff --git a/extensions/models.py b/extensions/models.py index b5bb154c..ae169103 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -1,5 +1,4 @@ from typing import List -from urllib.parse import urlencode import logging from django.contrib.auth import get_user_model @@ -755,37 +754,36 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): permissions.append({'slug': slug, 'reason': reason, 'name': all_permission_names[slug]}) return permissions - # FIXME? make dependent on File or platform - @property - def download_name(self) -> str: - """Return a file name for downloads.""" - replace_char = f'{self}'.replace('.', '-') - return f'{utils.slugify(replace_char)}.zip' - - # FIXME make dependent on File or platform - def download_url(self, append_repository_and_compatibility=True) -> str: - filename = f'{self.extension.type_slug_singular}-{self.extension.slug}-v{self.version}.zip' - download_url = reverse( - 'extensions:version-download', - kwargs={ - 'type_slug': self.extension.type_slug, - 'slug': self.extension.slug, - 'version': self.version, - 'filename': filename, - }, - ) - if append_repository_and_compatibility: - params = { - 'repository': '/api/v1/extensions/', - 'blender_version_min': self.blender_version_min, + def get_download_list(self) -> List[dict]: + files = list(self.files.all()) + if len(files) == 1: + file = files[0] + return [ + { + 'name': file.download_name(), + 'url': file.download_url(platform=None), + 'size': file.size_bytes, + } + ] + platform2file = {} + for file in files: + platforms = file.platforms() + if not platforms: + log.warning( + f'data error: Version pk={self.pk} has multiple files, but File pk={file.pk} ' + f'is not platform-specific' + ) + for platform in platforms: + platform2file[platform] = file + return [ + { + 'platform': p, + 'name': file.download_name(), + 'url': file.download_url(platform=p), + 'size': file.size_bytes, } - if self.blender_version_max: - params['blender_version_max'] = self.blender_version_max - if platforms := self.platforms.all(): - params['platforms'] = ','.join([p.slug for p in platforms]) - query_string = urlencode(params) - download_url += f'?{query_string}' - return download_url + for p, file in platform2file.items() + ] def get_delete_url(self) -> str: return reverse( diff --git a/extensions/templates/extensions/detail.html b/extensions/templates/extensions/detail.html index cc82d546..5238754c 100644 --- a/extensions/templates/extensions/detail.html +++ b/extensions/templates/extensions/detail.html @@ -256,37 +256,43 @@ {% block extension_download %}
    {% if extension.is_approved %} + {% with download_list=latest.get_download_list %}
    -
    + {% for download_item in download_list %}
    {# TODO @front-end: Replace URL of the manual /dev/ with /latest/. #} - ...or     download + ...or download and Install from Disk + {% endfor %}
    {# If JavaScript is disabled. #} - {% else %} + {% endwith %} + {% else %}

    This {{ extension.get_type_display|lower }} is currently under review.

    @@ -296,7 +302,7 @@ {% trans 'Review Extension' %}

    - {% endif %} + {% endif %}
    {% endblock extension_download %} diff --git a/extensions/views/api.py b/extensions/views/api.py index dee80eac..2116cbbf 100644 --- a/extensions/views/api.py +++ b/extensions/views/api.py @@ -75,6 +75,7 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): platforms = file.platforms() if self.platform and (platforms and self.platform not in platforms): continue + # TODO? return all matching files (when no self.platform is passed)? matching_file = file matching_version = v break @@ -91,8 +92,10 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): 'archive_hash': matching_file.original_hash, 'archive_size': matching_file.size_bytes, 'archive_url': self.request.build_absolute_uri( - # FIXME download_url is per file - matching_version.download_url(append_repository_and_compatibility=False) + matching_file.download_url( + platform=self.platform, + append_repository_and_compatibility=False, + ) ), 'type': EXTENSION_TYPE_SLUGS_SINGULAR.get(instance.type), 'blender_version_min': matching_version.blender_version_min, @@ -102,7 +105,7 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): 'maintainer': instance.team and instance.team.name or str(instance.authors.all()[0]), 'license': [license_iter.slug for license_iter in matching_version.licenses.all()], 'permissions': matching_file.metadata.get('permissions'), - # FIXME? should we instead list platforms of matching_file? + # TODO? if listing all version files (see the note above) use matching_file.platforms() 'platforms': [platform.slug for platform in matching_version.platforms.all()], # TODO: handle copyright 'tags': [str(tag) for tag in matching_version.tags.all()], diff --git a/files/models.py b/files/models.py index 6505dfa5..e6280854 100644 --- a/files/models.py +++ b/files/models.py @@ -1,9 +1,11 @@ from pathlib import Path from typing import Dict, Any +from urllib.parse import urlencode import logging from django.contrib.auth import get_user_model from django.db import models +from django.urls import reverse from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin from files.utils import get_sha256, guess_mimetype_from_ext, get_thumbnail_upload_to @@ -202,6 +204,49 @@ class File(CreatedModifiedMixin, TrackChangesMixin, models.Model): 'tags': data.get('tags'), } + def download_name(self) -> str: + """Return a file name for downloads.""" + version = self.version.first() + replace_char = f'{version}'.replace('.', '-') + return f'{utils.slugify(replace_char)}.zip' + + def download_url(self, platform=None, append_repository_and_compatibility=True) -> str: + filename = self.download_name() + version = self.version.first() + if platform: + download_url = reverse( + 'extensions:version-platform-download', + kwargs={ + 'type_slug': version.extension.type_slug, + 'slug': version.extension.slug, + 'version': version.version, + 'platform': platform, + 'filename': filename, + }, + ) + else: + download_url = reverse( + 'extensions:version-download', + kwargs={ + 'type_slug': version.extension.type_slug, + 'slug': version.extension.slug, + 'version': version.version, + 'filename': filename, + }, + ) + if append_repository_and_compatibility: + params = { + 'repository': '/api/v1/extensions/', + 'blender_version_min': version.blender_version_min, + } + if version.blender_version_max: + params['blender_version_max'] = version.blender_version_max + if platforms := self.platforms(): + params['platforms'] = ','.join(platforms) + query_string = urlencode(params) + download_url += f'?{query_string}' + return download_url + def get_thumbnail_of_size(self, size_key: str) -> str: """Return absolute path portion of the URL of a thumbnail of this file. diff --git a/users/tests/test_tasks.py b/users/tests/test_tasks.py index 9f65100b..40ebd80d 100644 --- a/users/tests/test_tasks.py +++ b/users/tests/test_tasks.py @@ -82,7 +82,7 @@ class TestTasks(TestCase): self.assertEqual(response.status_code, 200) response = self.client.get(version.extension.get_versions_url()) self.assertEqual(response.status_code, 200) - response = self.client.get(version.download_url()) + response = self.client.get(version.files.first().download_url()) self.assertEqual(response.status_code, 302) self.assertEqual( response['Location'], -- 2.30.2 From a3bcd421147993b122e26057e3d66f2441193258 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 5 Jul 2024 10:37:46 +0200 Subject: [PATCH 16/38] previous commit was a bad idea: keep urls at version level for now --- extensions/models.py | 50 +++++++++++++++++++++++++++++++++++--- extensions/views/api.py | 2 +- extensions/views/public.py | 3 +++ files/models.py | 45 ---------------------------------- users/tests/test_tasks.py | 2 +- 5 files changed, 51 insertions(+), 51 deletions(-) diff --git a/extensions/models.py b/extensions/models.py index ae169103..4a67f972 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -1,4 +1,5 @@ from typing import List +from urllib.parse import urlencode import logging from django.contrib.auth import get_user_model @@ -754,14 +755,55 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): permissions.append({'slug': slug, 'reason': reason, 'name': all_permission_names[slug]}) return permissions + def _get_download_name(self) -> str: + """Return a file name for downloads.""" + replace_char = f'{self}'.replace('.', '-') + return f'{utils.slugify(replace_char)}.zip' + + def get_download_url(self, platform=None, append_repository_and_compatibility=True) -> str: + filename = f'{self.extension.type_slug_singular}-{self.extension.slug}-v{self.version}.zip' + if platform: + download_url = reverse( + 'extensions:version-platform-download', + kwargs={ + 'type_slug': self.extension.type_slug, + 'slug': self.extension.slug, + 'version': self.version, + 'platform': platform, + 'filename': filename, + }, + ) + else: + download_url = reverse( + 'extensions:version-download', + kwargs={ + 'type_slug': self.extension.type_slug, + 'slug': self.extension.slug, + 'version': self.version, + 'filename': filename, + }, + ) + if append_repository_and_compatibility: + params = { + 'repository': '/api/v1/extensions/', + 'blender_version_min': self.blender_version_min, + } + if self.blender_version_max: + params['blender_version_max'] = self.blender_version_max + if platforms := self.platforms.all(): + params['platforms'] = ','.join([p.slug for p in platforms]) + query_string = urlencode(params) + download_url += f'?{query_string}' + return download_url + def get_download_list(self) -> List[dict]: files = list(self.files.all()) if len(files) == 1: file = files[0] return [ { - 'name': file.download_name(), - 'url': file.download_url(platform=None), + 'name': self._get_download_name(), + 'url': self.get_download_url(platform=None), 'size': file.size_bytes, } ] @@ -778,8 +820,8 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): return [ { 'platform': p, - 'name': file.download_name(), - 'url': file.download_url(platform=p), + 'name': self._get_download_name(), + 'url': self.get_download_url(platform=p), 'size': file.size_bytes, } for p, file in platform2file.items() diff --git a/extensions/views/api.py b/extensions/views/api.py index 2116cbbf..e1a0db40 100644 --- a/extensions/views/api.py +++ b/extensions/views/api.py @@ -92,7 +92,7 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): 'archive_hash': matching_file.original_hash, 'archive_size': matching_file.size_bytes, 'archive_url': self.request.build_absolute_uri( - matching_file.download_url( + matching_version.download_url( platform=self.platform, append_repository_and_compatibility=False, ) diff --git a/extensions/views/public.py b/extensions/views/public.py index fae00d25..2e92f339 100644 --- a/extensions/views/public.py +++ b/extensions/views/public.py @@ -63,9 +63,12 @@ def extension_version_download(request, type_slug, slug, version, filename): def extension_version_platform_download(request, type_slug, slug, version, platform, filename): """Download an extension version and count downloads. + This method processes urls constructed by Version.get_download_list. + The `filename` parameter is used to pass a file name ending with `.zip`. This is a convention Blender uses to initiate an extension installation on an HTML anchor drag&drop. + Also see $arg_filename usage in playbooks/templates/nginx/http.conf """ extension_version = get_object_or_404(Version, extension__slug=slug, version=version) ExtensionDownload.create_from_request(request, object_id=extension_version.extension_id) diff --git a/files/models.py b/files/models.py index e6280854..6505dfa5 100644 --- a/files/models.py +++ b/files/models.py @@ -1,11 +1,9 @@ from pathlib import Path from typing import Dict, Any -from urllib.parse import urlencode import logging from django.contrib.auth import get_user_model from django.db import models -from django.urls import reverse from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin from files.utils import get_sha256, guess_mimetype_from_ext, get_thumbnail_upload_to @@ -204,49 +202,6 @@ class File(CreatedModifiedMixin, TrackChangesMixin, models.Model): 'tags': data.get('tags'), } - def download_name(self) -> str: - """Return a file name for downloads.""" - version = self.version.first() - replace_char = f'{version}'.replace('.', '-') - return f'{utils.slugify(replace_char)}.zip' - - def download_url(self, platform=None, append_repository_and_compatibility=True) -> str: - filename = self.download_name() - version = self.version.first() - if platform: - download_url = reverse( - 'extensions:version-platform-download', - kwargs={ - 'type_slug': version.extension.type_slug, - 'slug': version.extension.slug, - 'version': version.version, - 'platform': platform, - 'filename': filename, - }, - ) - else: - download_url = reverse( - 'extensions:version-download', - kwargs={ - 'type_slug': version.extension.type_slug, - 'slug': version.extension.slug, - 'version': version.version, - 'filename': filename, - }, - ) - if append_repository_and_compatibility: - params = { - 'repository': '/api/v1/extensions/', - 'blender_version_min': version.blender_version_min, - } - if version.blender_version_max: - params['blender_version_max'] = version.blender_version_max - if platforms := self.platforms(): - params['platforms'] = ','.join(platforms) - query_string = urlencode(params) - download_url += f'?{query_string}' - return download_url - def get_thumbnail_of_size(self, size_key: str) -> str: """Return absolute path portion of the URL of a thumbnail of this file. diff --git a/users/tests/test_tasks.py b/users/tests/test_tasks.py index 40ebd80d..9f65100b 100644 --- a/users/tests/test_tasks.py +++ b/users/tests/test_tasks.py @@ -82,7 +82,7 @@ class TestTasks(TestCase): self.assertEqual(response.status_code, 200) response = self.client.get(version.extension.get_versions_url()) self.assertEqual(response.status_code, 200) - response = self.client.get(version.files.first().download_url()) + response = self.client.get(version.download_url()) self.assertEqual(response.status_code, 302) self.assertEqual( response['Location'], -- 2.30.2 From 19e7e11347c065f70735e5a72d1c03f9425bc2b7 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 5 Jul 2024 11:19:49 +0200 Subject: [PATCH 17/38] get_build_list for author and reviewers --- extensions/models.py | 32 +++++++++++++++---- .../extensions/new_version_finalise.html | 21 ++++++------ .../templates/extensions/version_list.html | 8 +++-- extensions/views/api.py | 2 +- .../reviewers/extensions_review_detail.html | 8 +++-- users/tests/test_tasks.py | 2 +- 6 files changed, 52 insertions(+), 21 deletions(-) diff --git a/extensions/models.py b/extensions/models.py index 4a67f972..e7ebf759 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -755,10 +755,13 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): permissions.append({'slug': slug, 'reason': reason, 'name': all_permission_names[slug]}) return permissions - def _get_download_name(self) -> str: + def _get_download_name(self, platform=None) -> str: """Return a file name for downloads.""" replace_char = f'{self}'.replace('.', '-') - return f'{utils.slugify(replace_char)}.zip' + if platform: + return f'{utils.slugify(replace_char)}.{platform}.zip' + else: + return f'{utils.slugify(replace_char)}.zip' def get_download_url(self, platform=None, append_repository_and_compatibility=True) -> str: filename = f'{self.extension.type_slug_singular}-{self.extension.slug}-v{self.version}.zip' @@ -802,9 +805,9 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): file = files[0] return [ { - 'name': self._get_download_name(), - 'url': self.get_download_url(platform=None), + 'name': self._get_download_name(platform=None), 'size': file.size_bytes, + 'url': self.get_download_url(platform=None), } ] platform2file = {} @@ -819,14 +822,31 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): platform2file[platform] = file return [ { + 'name': self._get_download_name(platform=p), 'platform': p, - 'name': self._get_download_name(), - 'url': self.get_download_url(platform=p), 'size': file.size_bytes, + 'url': self.get_download_url(platform=p), } for p, file in platform2file.items() ] + def get_build_list(self) -> List[dict]: + build_list = [] + for file in self.files.all(): + platforms = file.platforms() or [] + platform = len(platforms) and platforms[0] or None + # if file has multiple platforms, picking the first one should still produce a correct + # download_url + build_list.append( + { + 'name': self._get_download_name(platform=platform), + 'platforms': platforms, + 'size': file.size_bytes, + 'url': self.get_download_url(platform=platform), + } + ) + return build_list + def get_delete_url(self) -> str: return reverse( 'extensions:version-delete', diff --git a/extensions/templates/extensions/new_version_finalise.html b/extensions/templates/extensions/new_version_finalise.html index eaab347f..964bacaf 100644 --- a/extensions/templates/extensions/new_version_finalise.html +++ b/extensions/templates/extensions/new_version_finalise.html @@ -36,23 +36,26 @@ {% endif %}
    - {% with version_files=form.instance.files.all %} - {% trans 'Files' %}: -
      - {% for file in version_files %} -
    • {{ file }} {% trans 'for platforms:' %} {{ file.platforms|join:", " }}
      • - {% endfor %} -
    + {% trans 'Builds' %}: + {% with build_list=form.instance.get_build_list %} +
    + {% for build in build_list %} + + + + {% trans 'Download' %} v{{ form.instance.version }} {{ build.platforms|join:", " }} + + + {% endfor %} {% endwith %} {% if form.instance.can_upload_more_files %} -
    - {% endif %}
    diff --git a/extensions/templates/extensions/version_list.html b/extensions/templates/extensions/version_list.html index 9e9374ed..bd4d1aa6 100644 --- a/extensions/templates/extensions/version_list.html +++ b/extensions/templates/extensions/version_list.html @@ -111,10 +111,14 @@
    - + {% with download_list=version.get_download_list %} + {% for download_item in download_list %} + - {% trans 'Download' %} v{{ version.version }} + {% trans 'Download' %} v{{ version.version }} {{ download_item.platform }} + {% endfor %} + {% endwith %} {% if is_maintainer %} Edit diff --git a/extensions/views/api.py b/extensions/views/api.py index e1a0db40..4808b2fa 100644 --- a/extensions/views/api.py +++ b/extensions/views/api.py @@ -92,7 +92,7 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): 'archive_hash': matching_file.original_hash, 'archive_size': matching_file.size_bytes, 'archive_url': self.request.build_absolute_uri( - matching_version.download_url( + matching_version.get_download_url( platform=self.platform, append_repository_and_compatibility=False, ) diff --git a/reviewers/templates/reviewers/extensions_review_detail.html b/reviewers/templates/reviewers/extensions_review_detail.html index bd6257ef..f62ccc04 100644 --- a/reviewers/templates/reviewers/extensions_review_detail.html +++ b/reviewers/templates/reviewers/extensions_review_detail.html @@ -180,12 +180,16 @@ Try at your own risk.

    - + {% with build_list=extension.latest_version.get_build_list %} + {% for build in build_list %} + - {% trans 'Download' %} v{{ extension.latest_version.version }} + {% trans 'Download' %} v{{ extension.latest_version.version }} {{ build.platforms|join:", " }} + {% endfor %} + {% endwith %}
    {% endif %} diff --git a/users/tests/test_tasks.py b/users/tests/test_tasks.py index 9f65100b..2352528f 100644 --- a/users/tests/test_tasks.py +++ b/users/tests/test_tasks.py @@ -82,7 +82,7 @@ class TestTasks(TestCase): self.assertEqual(response.status_code, 200) response = self.client.get(version.extension.get_versions_url()) self.assertEqual(response.status_code, 200) - response = self.client.get(version.download_url()) + response = self.client.get(version.get_download_url()) self.assertEqual(response.status_code, 302) self.assertEqual( response['Location'], -- 2.30.2 From 3144d0c5942226bf335de1f88af7b6fc016b6d1f Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 5 Jul 2024 15:18:52 +0200 Subject: [PATCH 18/38] rename get_available_platforms to get_remaining_platforms --- extensions/models.py | 4 ++-- files/validators.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/extensions/models.py b/extensions/models.py index e7ebf759..c17333c7 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -679,7 +679,7 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): @property def can_upload_more_files(self): - return len(self.get_available_platforms()) > 0 + return len(self.get_remaining_platforms()) > 0 def add_file(self, file: File): self.files.add(file) @@ -706,7 +706,7 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): for p in to_delete: self.platforms.remove(Platform.objects.get(slug=p)) - def get_available_platforms(self): + def get_remaining_platforms(self): all_platforms = set(p.slug for p in Platform.objects.all()) for file in self.files.all(): platforms = file.platforms() diff --git a/files/validators.py b/files/validators.py index 7f2688b2..2d48c382 100644 --- a/files/validators.py +++ b/files/validators.py @@ -197,9 +197,9 @@ class ExtensionVersionManifestValidator: # check for platforms # TODO test coverage - available_platforms = version.get_available_platforms() + remaining_platforms = version.get_remaining_platforms() if platforms := files.models.File.parse_platforms_from_manifest(manifest): - if diff := set(platforms) - available_platforms: + if diff := set(platforms) - remaining_platforms: raise ValidationError( { 'source': [f'{version} already has files for {", ".join(diff)}'], @@ -207,12 +207,12 @@ class ExtensionVersionManifestValidator: code='invalid', ) else: - if available_platforms: + if remaining_platforms: raise ValidationError( { 'source': [ f'File upload for {version} is allowed only for remaining platforms: ' - f'{", ".join(available_platforms)}' + f'{", ".join(remaining_platforms)}' ], }, code='invalid', -- 2.30.2 From 8b376825c8ed916ff7902665f96be57a02a2ebe5 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 5 Jul 2024 15:27:59 +0200 Subject: [PATCH 19/38] refactor download name --- extensions/models.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/extensions/models.py b/extensions/models.py index c17333c7..73af2b23 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -757,14 +757,13 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): def _get_download_name(self, platform=None) -> str: """Return a file name for downloads.""" - replace_char = f'{self}'.replace('.', '-') + parts = [self.extension.type_slug_singular, self.extension.slug, f'v{self.version}'] if platform: - return f'{utils.slugify(replace_char)}.{platform}.zip' - else: - return f'{utils.slugify(replace_char)}.zip' + parts.append(platform) + return f'{"-".join(parts)}.zip' def get_download_url(self, platform=None, append_repository_and_compatibility=True) -> str: - filename = f'{self.extension.type_slug_singular}-{self.extension.slug}-v{self.version}.zip' + filename = self._get_download_name(platform=platform) if platform: download_url = reverse( 'extensions:version-platform-download', -- 2.30.2 From 3440c2154836db8f6fd91f869a030756eb3d117c Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 5 Jul 2024 15:46:43 +0200 Subject: [PATCH 20/38] fix matching logic in api --- extensions/tests/test_api.py | 25 +++++++++++++++++++++++++ extensions/views/api.py | 26 ++++++++++++++------------ 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/extensions/tests/test_api.py b/extensions/tests/test_api.py index 0d9cbfd8..790d7e68 100644 --- a/extensions/tests/test_api.py +++ b/extensions/tests/test_api.py @@ -151,6 +151,31 @@ class FiltersTest(APITestCase): ).json() self.assertEqual(len(json['data']), 1) + def test_platform_filter_same_extension(self): + version = create_approved_version( + metadata__platforms=['linux-x64'], + metadata__version='1.0.0', + ) + extension = version.extension + version = create_approved_version( + extension=extension, + metadata__platforms=['windows-x64'], + metadata__version='1.0.1', + ) + + url = reverse('extensions:api') + json = self.client.get( + url + '?platform=linux-x64', + HTTP_ACCEPT='application/json', + ).json() + self.assertEqual(len(json['data']), 1) + + json = self.client.get( + url + '?platform=windows-x64', + HTTP_ACCEPT='application/json', + ).json() + self.assertEqual(len(json['data']), 1) + def test_blender_version_filter_latest_not_max_version(self): version = create_approved_version(metadata__blender_version_min='4.0.1') date_created = version.date_created diff --git a/extensions/views/api.py b/extensions/views/api.py index 4808b2fa..93e144e7 100644 --- a/extensions/views/api.py +++ b/extensions/views/api.py @@ -54,14 +54,16 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): except Platform.DoesNotExist: self.platform = self.UNKNOWN_PLATFORM - def to_representation(self, instance): - matching_file = None - matching_version = None + def find_matching_file_and_version(self, instance): # avoid triggering additional db queries, reuse the prefetched queryset - versions = [v for v in instance.versions.all() if v.is_listed] + versions = sorted( + [v for v in instance.versions.all() if v.is_listed], + key=lambda v: v.date_created, + reverse=True, + ) if not versions: - return None - versions = sorted(versions, key=lambda v: v.date_created, reverse=True) + return (None, None) + for v in versions: if self.blender_version and not is_in_version_range( self.blender_version, @@ -75,14 +77,15 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): platforms = file.platforms() if self.platform and (platforms and self.platform not in platforms): continue - # TODO? return all matching files (when no self.platform is passed)? - matching_file = file - matching_version = v - break + return (file, v) + return (None, None) + + def to_representation(self, instance): + # TODO? return all matching files (when no self.platform is passed)? + matching_file, matching_version = self.find_matching_file_and_version(instance) if not matching_file: return None - data = { 'id': instance.extension_id, 'schema_version': matching_version.schema_version, @@ -110,7 +113,6 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): # TODO: handle copyright 'tags': [str(tag) for tag in matching_version.tags.all()], } - return clean_json_dictionary_from_optional_fields(data) -- 2.30.2 From 77dc2096de4f3ba6b8e66cd2ae33772ffadeb84d Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 5 Jul 2024 15:55:02 +0200 Subject: [PATCH 21/38] file.platforms() renamed to file.get_platforms() --- extensions/models.py | 8 ++++---- extensions/views/api.py | 4 ++-- extensions/views/public.py | 2 +- files/models.py | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/extensions/models.py b/extensions/models.py index 73af2b23..b5332c63 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -690,7 +690,7 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): current_platforms = set([p.slug for p in self.platforms.all()]) files_platforms = set() for file in self.files.all(): - if file_platforms := file.platforms(): + if file_platforms := file.get_platforms(): files_platforms.update(file_platforms) else: # we have a cross-platform file - version must not specify platforms @@ -709,7 +709,7 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): def get_remaining_platforms(self): all_platforms = set(p.slug for p in Platform.objects.all()) for file in self.files.all(): - platforms = file.platforms() + platforms = file.get_platforms() if not platforms: # no platforms means any platform return set() @@ -811,7 +811,7 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): ] platform2file = {} for file in files: - platforms = file.platforms() + platforms = file.get_platforms() if not platforms: log.warning( f'data error: Version pk={self.pk} has multiple files, but File pk={file.pk} ' @@ -832,7 +832,7 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): def get_build_list(self) -> List[dict]: build_list = [] for file in self.files.all(): - platforms = file.platforms() or [] + platforms = file.get_platforms() or [] platform = len(platforms) and platforms[0] or None # if file has multiple platforms, picking the first one should still produce a correct # download_url diff --git a/extensions/views/api.py b/extensions/views/api.py index 93e144e7..8133fdf7 100644 --- a/extensions/views/api.py +++ b/extensions/views/api.py @@ -74,7 +74,7 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): for file in v.files.all(): # empty platforms field matches any platform filter # UNKNOWN_PLATFORM matches only empty platforms field - platforms = file.platforms() + platforms = file.get_platforms() if self.platform and (platforms and self.platform not in platforms): continue return (file, v) @@ -108,7 +108,7 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): 'maintainer': instance.team and instance.team.name or str(instance.authors.all()[0]), 'license': [license_iter.slug for license_iter in matching_version.licenses.all()], 'permissions': matching_file.metadata.get('permissions'), - # TODO? if listing all version files (see the note above) use matching_file.platforms() + # listing all platforms for matching_version, not matching_file (see TODO? above) 'platforms': [platform.slug for platform in matching_version.platforms.all()], # TODO: handle copyright 'tags': [str(tag) for tag in matching_version.tags.all()], diff --git a/extensions/views/public.py b/extensions/views/public.py index 2e92f339..8cd7aa4c 100644 --- a/extensions/views/public.py +++ b/extensions/views/public.py @@ -74,7 +74,7 @@ def extension_version_platform_download(request, type_slug, slug, version, platf ExtensionDownload.create_from_request(request, object_id=extension_version.extension_id) VersionDownload.create_from_request(request, object_id=extension_version.pk) for file in extension_version.files.all(): - platforms = file.platforms() or [] + platforms = file.get_platforms() or [] if platform is None or platform in platforms: url = file.source.url return redirect(f'{url}?filename={filename}') diff --git a/files/models.py b/files/models.py index 6505dfa5..048b9be1 100644 --- a/files/models.py +++ b/files/models.py @@ -177,7 +177,7 @@ class File(CreatedModifiedMixin, TrackChangesMixin, models.Model): 'website': data.get('website'), } - def platforms(self): + def get_platforms(self): return self.parse_platforms_from_manifest(self.metadata) @classmethod @@ -198,7 +198,7 @@ class File(CreatedModifiedMixin, TrackChangesMixin, models.Model): 'schema_version': data.get('schema_version'), 'licenses': data.get('license'), 'permissions': data.get('permissions'), - 'platforms': self.platforms(), + 'platforms': self.get_platforms(), 'tags': data.get('tags'), } -- 2.30.2 From 8af96c48c0febf677ec1ee0ff7af2c52516485f7 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 5 Jul 2024 17:40:02 +0200 Subject: [PATCH 22/38] Version.get_file_for_platform --- extensions/models.py | 17 +++++++++++++++++ extensions/tests/test_models.py | 23 +++++++++++++++++++++++ extensions/views/api.py | 7 +------ extensions/views/public.py | 8 +++----- 4 files changed, 44 insertions(+), 11 deletions(-) diff --git a/extensions/models.py b/extensions/models.py index b5332c63..105fa95b 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -682,6 +682,13 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): return len(self.get_remaining_platforms()) > 0 def add_file(self, file: File): + current_platforms = set([p.slug for p in self.platforms.all()]) + file_platforms = set(file.get_platforms() or []) + if overlap := current_platforms & file_platforms: + raise ValueError( + f'add_file failed: File pk={file.pk} and Version pk={self.pk} have ' + f'overlapping platforms: {overlap}' + ) self.files.add(file) self.update_platforms() @@ -716,6 +723,16 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): all_platforms -= set(platforms) return all_platforms + def get_file_for_platform(self, platform): + for file in self.files.all(): + platforms = file.get_platforms() + # not platform-specific, matches all platforms + if not platforms: + return file + if platform is None or platform in platforms: + return file + return None + @property def is_listed(self): # To be public, at least one version file must have a public status. diff --git a/extensions/tests/test_models.py b/extensions/tests/test_models.py index 02256a21..6f8da816 100644 --- a/extensions/tests/test_models.py +++ b/extensions/tests/test_models.py @@ -125,6 +125,29 @@ class VersionTest(TestCase): Platform.objects.filter(slug__in=['linux-x64']).order_by('slug'), ) + def test_add_file(self): + version = create_version(metadata__platforms=['linux-x64']) + file = FileFactory(metadata__platforms=['linux-x64']) + with self.assertRaises(ValueError): + version.add_file(file) + + def test_get_file_for_platform(self): + version = create_version(metadata__platforms=['linux-x64']) + file = FileFactory(metadata__platforms=['windows-x64']) + version.add_file(file) + self.assertIsNotNone(version.get_file_for_platform(None)) + self.assertIsNotNone(version.get_file_for_platform('linux-x64')) + self.assertIsNotNone(version.get_file_for_platform('windows-x64')) + self.assertIsNone(version.get_file_for_platform('windows-arm64')) + + file2 = FileFactory(metadata__platforms=['macos-x64', 'macos-arm64']) + version.add_file(file2) + self.assertIsNotNone(version.get_file_for_platform('macos-x64')) + + version2 = create_version(metadata__platforms=[]) + self.assertIsNotNone(version2.get_file_for_platform(None)) + self.assertIsNotNone(version2.get_file_for_platform('macos-x64')) + class UpdateMetadataTest(TestCase): fixtures = ['dev', 'licenses'] diff --git a/extensions/views/api.py b/extensions/views/api.py index 8133fdf7..cd004146 100644 --- a/extensions/views/api.py +++ b/extensions/views/api.py @@ -71,12 +71,7 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): v.blender_version_max, ): continue - for file in v.files.all(): - # empty platforms field matches any platform filter - # UNKNOWN_PLATFORM matches only empty platforms field - platforms = file.get_platforms() - if self.platform and (platforms and self.platform not in platforms): - continue + if file := v.get_file_for_platform(self.platform): return (file, v) return (None, None) diff --git a/extensions/views/public.py b/extensions/views/public.py index 8cd7aa4c..2a9ae203 100644 --- a/extensions/views/public.py +++ b/extensions/views/public.py @@ -73,11 +73,9 @@ def extension_version_platform_download(request, type_slug, slug, version, platf extension_version = get_object_or_404(Version, extension__slug=slug, version=version) ExtensionDownload.create_from_request(request, object_id=extension_version.extension_id) VersionDownload.create_from_request(request, object_id=extension_version.pk) - for file in extension_version.files.all(): - platforms = file.get_platforms() or [] - if platform is None or platform in platforms: - url = file.source.url - return redirect(f'{url}?filename={filename}') + if file := extension_version.get_file_for_platform(platform): + url = file.source.url + return redirect(f'{url}?filename={filename}') raise Http404() -- 2.30.2 From b922118d9342ba98f8de73316f71d23cf0c27668 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 5 Jul 2024 18:33:25 +0200 Subject: [PATCH 23/38] forgotten get_platforms rename in templates --- .../extensions/components/extension_edit_detail_card.html | 2 +- extensions/templates/extensions/detail.html | 2 +- extensions/templates/extensions/version_list.html | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/extensions/templates/extensions/components/extension_edit_detail_card.html b/extensions/templates/extensions/components/extension_edit_detail_card.html index 2d6d3ec4..bed15ee9 100644 --- a/extensions/templates/extensions/components/extension_edit_detail_card.html +++ b/extensions/templates/extensions/components/extension_edit_detail_card.html @@ -73,7 +73,7 @@
    {{ version.files.first.size_bytes|filesizeformat }}
    {% else %} {% for file in version.files.all %} -
    {{ file.size_bytes|filesizeformat }} ({{ file.platforms|join:", " }})
    +
    {{ file.size_bytes|filesizeformat }} ({{ file.get_platforms|join:", " }})
    {% endfor %} {% endif %}
    diff --git a/extensions/templates/extensions/detail.html b/extensions/templates/extensions/detail.html index 5238754c..2659cd5b 100644 --- a/extensions/templates/extensions/detail.html +++ b/extensions/templates/extensions/detail.html @@ -191,7 +191,7 @@
    {{ latest.files.first.size_bytes|filesizeformat }}
    {% else %} {% for file in latest.files.all %} -
    {{ file.size_bytes|filesizeformat }} ({{ file.platforms|join:", " }})
    +
    {{ file.size_bytes|filesizeformat }} ({{ file.get_platforms|join:", " }})
    {% endfor %} {% endif %}
    diff --git a/extensions/templates/extensions/version_list.html b/extensions/templates/extensions/version_list.html index bd4d1aa6..dbdd0526 100644 --- a/extensions/templates/extensions/version_list.html +++ b/extensions/templates/extensions/version_list.html @@ -34,7 +34,7 @@
  • {{ version.files.first.size_bytes|filesizeformat }}
    • {% else %} {% for file in version.files.all %} -
  • {{ file.size_bytes|filesizeformat }} ({{ file.platforms|join:", " }})
    • +
  • {{ file.size_bytes|filesizeformat }} ({{ file.get_platforms|join:", " }})
    • {% endfor %} {% endif %}
  • {{ version.download_count }}
    • @@ -79,7 +79,7 @@
    {{ version.files.first.size_bytes|filesizeformat }}
    {% else %} {% for file in version.files.all %} -
    {{ file.size_bytes|filesizeformat }} ({{ file.platforms|join:", " }})
    +
    {{ file.size_bytes|filesizeformat }} ({{ file.get_platforms|join:", " }})
    {% endfor %} {% endif %} -- 2.30.2 From 49e0cb0fe5152d6a9d52737b9ecb4e4f7f23e553 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 5 Jul 2024 18:41:37 +0200 Subject: [PATCH 24/38] a stricter add_file check --- extensions/models.py | 5 +++++ extensions/tests/test_models.py | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/extensions/models.py b/extensions/models.py index 105fa95b..18aa302d 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -683,6 +683,11 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): def add_file(self, file: File): current_platforms = set([p.slug for p in self.platforms.all()]) + if not current_platforms: + raise ValueError( + f'add_file failed: Version pk={self.pk} is not platform-specific, ' + f'no more files allowed' + ) file_platforms = set(file.get_platforms() or []) if overlap := current_platforms & file_platforms: raise ValueError( diff --git a/extensions/tests/test_models.py b/extensions/tests/test_models.py index 6f8da816..8a923ffe 100644 --- a/extensions/tests/test_models.py +++ b/extensions/tests/test_models.py @@ -131,6 +131,11 @@ class VersionTest(TestCase): with self.assertRaises(ValueError): version.add_file(file) + all_platforms_version = create_version(metadata__platforms=[]) + file = FileFactory(metadata__platforms=['linux-x64']) + with self.assertRaises(ValueError): + all_platforms_version.add_file(file) + def test_get_file_for_platform(self): version = create_version(metadata__platforms=['linux-x64']) file = FileFactory(metadata__platforms=['windows-x64']) -- 2.30.2 From 050ba772d736109057c2fa66759fd12161874890 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Fri, 5 Jul 2024 18:53:55 +0200 Subject: [PATCH 25/38] use file's platform values for the download url --- extensions/models.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/extensions/models.py b/extensions/models.py index 18aa302d..3d768be9 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -814,7 +814,9 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): } if self.blender_version_max: params['blender_version_max'] = self.blender_version_max - if platforms := self.platforms.all(): + if platform: + params['platforms'] = platform + elif platforms := self.platforms.all(): params['platforms'] = ','.join([p.slug for p in platforms]) query_string = urlencode(params) download_url += f'?{query_string}' -- 2.30.2 From c336009d5b268616e5398d4547330f8bc39792d6 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Mon, 8 Jul 2024 13:41:01 +0200 Subject: [PATCH 26/38] fix typo --- files/validators.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/files/validators.py b/files/validators.py index d197a650..e9f3ac7d 100644 --- a/files/validators.py +++ b/files/validators.py @@ -508,12 +508,12 @@ class BuildValidator: if 'generated' not in value: return if 'platforms' in value['generated']: - if plafroms_error := PlatformsValidator.validate( + if plaforms_error := PlatformsValidator.validate( name=name, value=value['generated']['platforms'], manifest=manifest, ): - return plafroms_error + return plaforms_error if 'wheels' in value['generated']: if wheels_error := WheelsValidator.validate( -- 2.30.2 From 1684ca01f49f451bfb2d2555b78c5475992874bc Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Mon, 8 Jul 2024 13:42:31 +0200 Subject: [PATCH 27/38] handle additional file uploads in NewVersionView remove a separate view for uploading a file for an existing version --- extensions/models.py | 14 ------ .../extensions/new_version_finalise.html | 6 --- extensions/templates/extensions/submit.html | 4 +- extensions/tests/test_models.py | 20 +++++--- extensions/tests/test_submit.py | 13 +++++- extensions/urls.py | 5 -- extensions/views/api.py | 1 - extensions/views/manage.py | 46 ++----------------- files/forms.py | 2 - files/validators.py | 20 +------- 10 files changed, 34 insertions(+), 97 deletions(-) diff --git a/extensions/models.py b/extensions/models.py index 3d768be9..8b24f9ed 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -677,10 +677,6 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): def single_file(self): return len(self.files.all()) == 1 - @property - def can_upload_more_files(self): - return len(self.get_remaining_platforms()) > 0 - def add_file(self, file: File): current_platforms = set([p.slug for p in self.platforms.all()]) if not current_platforms: @@ -880,16 +876,6 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): }, ) - def get_version_upload_url(self) -> str: - return reverse( - 'extensions:version-upload', - kwargs={ - 'type_slug': self.extension.type_slug, - 'slug': self.extension.slug, - 'pk': self.pk, - }, - ) - @property def update_url(self) -> str: return reverse( diff --git a/extensions/templates/extensions/new_version_finalise.html b/extensions/templates/extensions/new_version_finalise.html index 964bacaf..7681164e 100644 --- a/extensions/templates/extensions/new_version_finalise.html +++ b/extensions/templates/extensions/new_version_finalise.html @@ -48,12 +48,6 @@ {% endfor %} {% endwith %} - {% if form.instance.can_upload_more_files %} - - - {% trans 'Upload files for other platforms' %} - - {% endif %} diff --git a/extensions/templates/extensions/submit.html b/extensions/templates/extensions/submit.html index f993718d..99b0ce23 100644 --- a/extensions/templates/extensions/submit.html +++ b/extensions/templates/extensions/submit.html @@ -17,9 +17,7 @@

    - {% if version %} - {% trans 'Upload New File for' %} {{ version }} - {% elif extension %} + {% if extension %} {% trans 'Upload New' %} {{ extension.get_type_display }} {% trans 'Version' %} {% else %} {% trans 'Upload Extension' %} diff --git a/extensions/tests/test_models.py b/extensions/tests/test_models.py index 8a923ffe..84c4e541 100644 --- a/extensions/tests/test_models.py +++ b/extensions/tests/test_models.py @@ -90,25 +90,33 @@ class VersionTest(TestCase): self.assertEqual(response.status_code, 200, path) - def test_can_upload_more_files(self): + def test_get_remaining_platforms(self): version_platforms_none = create_version(metadata__platforms=None) - self.assertFalse(version_platforms_none.can_upload_more_files) + self.assertEqual(version_platforms_none.get_remaining_platforms(), set()) version_platforms_empty = create_version(metadata__platforms=[]) - self.assertFalse(version_platforms_empty.can_upload_more_files) + self.assertEqual(version_platforms_empty.get_remaining_platforms(), set()) version_platforms_some = create_version(metadata__platforms=['linux-x64', 'windows-x64']) - self.assertTrue(version_platforms_some.can_upload_more_files) + self.assertTrue(version_platforms_some.get_remaining_platforms()) version_two_files = create_version(metadata__platforms=['linux-x64']) file = FileFactory(metadata__platforms=['windows-x64']) version_two_files.files.add(file) - self.assertTrue(version_two_files.can_upload_more_files) + self.assertTrue(version_two_files.get_remaining_platforms()) version_platforms_all = create_version( metadata__platforms=[p.slug for p in Platform.objects.all()] ) - self.assertFalse(version_platforms_all.can_upload_more_files) + self.assertEqual(version_platforms_all.get_remaining_platforms(), set()) + + skip_platforms = set(['linux-x64', 'windows-x64']) + version_platforms_without_some = create_version( + metadata__platforms=[ + p.slug for p in Platform.objects.all() if p.slug not in skip_platforms + ] + ) + self.assertEqual(version_platforms_without_some.get_remaining_platforms(), skip_platforms) def test_collect_platforms_across_files(self): version = create_version(metadata__platforms=['linux-x64']) diff --git a/extensions/tests/test_submit.py b/extensions/tests/test_submit.py index d0eaa79a..a484eea3 100644 --- a/extensions/tests/test_submit.py +++ b/extensions/tests/test_submit.py @@ -619,7 +619,7 @@ class MultiPlatformUploadTest(TestCase): with open(file_windows, 'rb') as fp: response = self.client.post( - extension.latest_version.get_version_upload_url(), + extension.get_new_version_url(), {'source': fp, 'agreed_with_terms': True}, ) self.assertEqual(response.status_code, 302) @@ -627,6 +627,17 @@ class MultiPlatformUploadTest(TestCase): self.assertEqual(extension.latest_version.files.count(), 2) self.assertEqual(extension.latest_version.platforms.count(), 2) + # can't upload the same file a second time + with open(file_windows, 'rb') as fp: + response = self.client.post( + extension.get_new_version_url(), + {'source': fp, 'agreed_with_terms': True}, + ) + self.assertEqual(response.status_code, 200) + extension.refresh_from_db() + self.assertEqual(extension.latest_version.files.count(), 2) + self.assertEqual(extension.latest_version.platforms.count(), 2) + class DraftsWarningTest(TestCase): fixtures = ['licenses'] diff --git a/extensions/urls.py b/extensions/urls.py index 5546016e..3d350df7 100644 --- a/extensions/urls.py +++ b/extensions/urls.py @@ -96,11 +96,6 @@ urlpatterns = [ public.extension_version_platform_download, name='version-platform-download', ), - path( - '/manage/versions//upload/', - manage.UploadVersionFileView.as_view(), - name='version-upload', - ), path('/versions/', manage.VersionsView.as_view(), name='versions'), ], ), diff --git a/extensions/views/api.py b/extensions/views/api.py index cd004146..d1a304fe 100644 --- a/extensions/views/api.py +++ b/extensions/views/api.py @@ -202,7 +202,6 @@ class UploadExtensionVersionView(APIView): ) form = FileFormSkipAgreed( - allow_existing_version=True, data={}, extension=extension, request=request, diff --git a/extensions/views/manage.py b/extensions/views/manage.py index e67ae74b..0a8607f7 100644 --- a/extensions/views/manage.py +++ b/extensions/views/manage.py @@ -214,7 +214,11 @@ class NewVersionView( @transaction.atomic def form_valid(self, form): response = super().form_valid(form) - self.extension.create_version_from_file(self.object) + manifest_version = self.object.metadata['version'] + if version := self.extension.versions.filter(version=manifest_version).first(): + version.add_file(self.object) + else: + version = self.extension.create_version_from_file(self.object) return response def get_success_url(self): @@ -273,46 +277,6 @@ class UpdateVersionView(LoginRequiredMixin, UserPassesTestMixin, UpdateView): return self.get_object().extension.has_maintainer(self.request.user) -class UploadVersionFileView( - LoginRequiredMixin, - MaintainedExtensionMixin, - CreateView, -): - """Upload a new file for an existing version. Multi-OS support.""" - - form_class = FileForm - template_name = 'extensions/submit.html' - - def get_context_data(self, **kwargs): - ctx = super().get_context_data(**kwargs) - ctx['extension'] = self.extension - ctx['version'] = self.version - return ctx - - def get_form_kwargs(self): - kwargs = super().get_form_kwargs() - kwargs['request'] = self.request - kwargs['extension'] = self.extension - kwargs['allow_existing_version'] = True - return kwargs - - @transaction.atomic - def form_valid(self, form): - response = super().form_valid(form) - self.version.add_file(self.object) - return response - - def get_success_url(self): - return reverse( - 'extensions:new-version-finalise', - kwargs={ - 'type_slug': self.extension.type_slug, - 'slug': self.extension.slug, - 'pk': self.object.pk, - }, - ) - - class DraftExtensionView( LoginRequiredMixin, MaintainedExtensionMixin, diff --git a/files/forms.py b/files/forms.py index 9a5c5470..44464771 100644 --- a/files/forms.py +++ b/files/forms.py @@ -74,7 +74,6 @@ class FileForm(forms.ModelForm): ) def __init__(self, *args, **kwargs): - self.allow_existing_version = kwargs.pop('allow_existing_version', None) self.extension = kwargs.pop('extension', None) self.request = kwargs.pop('request') for field in self.base_fields: @@ -169,7 +168,6 @@ class FileForm(forms.ModelForm): ExtensionVersionManifestValidator( manifest, self.extension, - self.allow_existing_version, ) self.cleaned_data['metadata'] = manifest diff --git a/files/validators.py b/files/validators.py index e9f3ac7d..1420a4bc 100644 --- a/files/validators.py +++ b/files/validators.py @@ -166,14 +166,9 @@ class ExtensionNameManifestValidator: class ExtensionVersionManifestValidator: - """Validates version existence in db and available platforms. + """Validates version existence in db and available platforms.""" - allow_existing_version is meant to be used from API and UploadVersionFileNew - when a version either may exist or definitely exists. - TODO? create separate flags for these scenarios? - """ - - def __init__(self, manifest, extension_to_be_updated, allow_existing_version=False): + def __init__(self, manifest, extension_to_be_updated): # If the extension wasn't created yet, any version is valid if not extension_to_be_updated: @@ -184,17 +179,6 @@ class ExtensionVersionManifestValidator: version = extension_to_be_updated.versions.filter(version=manifest_version).first() if not version: return - if not allow_existing_version: - raise ValidationError( - { - 'source': [ - f'The version {escape(manifest_version)} was already uploaded for this ' - f'extension ({extension_to_be_updated.name})' - ], - }, - code='invalid', - ) - # check for platforms # TODO test coverage remaining_platforms = version.get_remaining_platforms() -- 2.30.2 From 666e2b0fb88629ed4d6112b1c99c666cdb4ef69c Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Mon, 8 Jul 2024 14:16:51 +0200 Subject: [PATCH 28/38] more tests for version platforms validation --- .../addon-without-platforms-for-split-test.zip | Bin 0 -> 748 bytes extensions/tests/test_submit.py | 12 ++++++++++++ files/validators.py | 4 +--- 3 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 extensions/tests/files/addon-without-platforms-for-split-test.zip diff --git a/extensions/tests/files/addon-without-platforms-for-split-test.zip b/extensions/tests/files/addon-without-platforms-for-split-test.zip new file mode 100644 index 0000000000000000000000000000000000000000..f3df336a08e896a3d1a60d2365ca4d8623edf19b GIT binary patch literal 748 zcmWIWW@h1H0D%WnJ0rjhD8bDj!w?^znU`4-AFo$X85+XLz|2x8ng+t972FJrEH9ZE z7+78bi2%4E69YOUN|*n=BnmVHgn5C66zAur#wVtv*PTh6AI}L;(cDw*N6-4;<8D<|g5NIv`%eC1d- Date: Mon, 8 Jul 2024 14:43:53 +0200 Subject: [PATCH 29/38] API: return multiple files per Version if no platform filter is specified --- extensions/tests/test_api.py | 36 +++++++++++++++- extensions/views/api.py | 83 +++++++++++++++++++----------------- 2 files changed, 80 insertions(+), 39 deletions(-) diff --git a/extensions/tests/test_api.py b/extensions/tests/test_api.py index 790d7e68..29cec6f8 100644 --- a/extensions/tests/test_api.py +++ b/extensions/tests/test_api.py @@ -5,8 +5,9 @@ from django.urls import reverse from rest_framework import status from rest_framework.test import APITestCase, APIClient -from common.tests.factories.users import UserFactory from common.tests.factories.extensions import create_approved_version, create_version +from common.tests.factories.files import FileFactory +from common.tests.factories.users import UserFactory from common.tests.utils import create_user_token from extensions.models import Extension, Version @@ -176,6 +177,39 @@ class FiltersTest(APITestCase): ).json() self.assertEqual(len(json['data']), 1) + def test_platform_filter_same_version(self): + version = create_approved_version( + metadata__id='filter_test', + metadata__platforms=['linux-x64'], + metadata__version='1.0.0', + ) + version = version.add_file( + FileFactory( + metadata__id='filter_test', + metadata__platforms=['windows-x64'], + metadata__version='1.0.0', + ) + ) + + url = reverse('extensions:api') + json = self.client.get( + url + '?platform=linux-x64', + HTTP_ACCEPT='application/json', + ).json() + self.assertEqual(len(json['data']), 1) + + json = self.client.get( + url + '?platform=windows-x64', + HTTP_ACCEPT='application/json', + ).json() + self.assertEqual(len(json['data']), 1) + + json = self.client.get( + url, + HTTP_ACCEPT='application/json', + ).json() + self.assertEqual(len(json['data']), 2) + def test_blender_version_filter_latest_not_max_version(self): version = create_approved_version(metadata__blender_version_min='4.0.1') date_created = version.date_created diff --git a/extensions/views/api.py b/extensions/views/api.py index d1a304fe..4e756eae 100644 --- a/extensions/views/api.py +++ b/extensions/views/api.py @@ -54,7 +54,7 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): except Platform.DoesNotExist: self.platform = self.UNKNOWN_PLATFORM - def find_matching_file_and_version(self, instance): + def find_matching_files_and_version(self, instance): # avoid triggering additional db queries, reuse the prefetched queryset versions = sorted( [v for v in instance.versions.all() if v.is_listed], @@ -62,7 +62,7 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): reverse=True, ) if not versions: - return (None, None) + return ([], None) for v in versions: if self.blender_version and not is_in_version_range( @@ -71,44 +71,48 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): v.blender_version_max, ): continue - if file := v.get_file_for_platform(self.platform): - return (file, v) + if self.platform: + if file := v.get_file_for_platform(self.platform): + return ([file], v) + else: + return (v.files.all(), v) - return (None, None) + return ([], None) def to_representation(self, instance): - # TODO? return all matching files (when no self.platform is passed)? - matching_file, matching_version = self.find_matching_file_and_version(instance) - if not matching_file: - return None - data = { - 'id': instance.extension_id, - 'schema_version': matching_version.schema_version, - 'name': instance.name, - 'version': matching_version.version, - 'tagline': matching_version.tagline, - 'archive_hash': matching_file.original_hash, - 'archive_size': matching_file.size_bytes, - 'archive_url': self.request.build_absolute_uri( - matching_version.get_download_url( - platform=self.platform, - append_repository_and_compatibility=False, - ) - ), - 'type': EXTENSION_TYPE_SLUGS_SINGULAR.get(instance.type), - 'blender_version_min': matching_version.blender_version_min, - 'blender_version_max': matching_version.blender_version_max, - 'website': self.request.build_absolute_uri(instance.get_absolute_url()), - # avoid triggering additional db queries, reuse the prefetched queryset - 'maintainer': instance.team and instance.team.name or str(instance.authors.all()[0]), - 'license': [license_iter.slug for license_iter in matching_version.licenses.all()], - 'permissions': matching_file.metadata.get('permissions'), - # listing all platforms for matching_version, not matching_file (see TODO? above) - 'platforms': [platform.slug for platform in matching_version.platforms.all()], - # TODO: handle copyright - 'tags': [str(tag) for tag in matching_version.tags.all()], - } - return clean_json_dictionary_from_optional_fields(data) + matching_files, matching_version = self.find_matching_files_and_version(instance) + result = [] + for file in matching_files: + data = { + 'id': instance.extension_id, + 'schema_version': matching_version.schema_version, + 'name': instance.name, + 'version': matching_version.version, + 'tagline': matching_version.tagline, + 'archive_hash': file.original_hash, + 'archive_size': file.size_bytes, + 'archive_url': self.request.build_absolute_uri( + matching_version.get_download_url( + platform=self.platform, + append_repository_and_compatibility=False, + ) + ), + 'type': EXTENSION_TYPE_SLUGS_SINGULAR.get(instance.type), + 'blender_version_min': matching_version.blender_version_min, + 'blender_version_max': matching_version.blender_version_max, + 'website': self.request.build_absolute_uri(instance.get_absolute_url()), + # avoid triggering additional db queries, reuse the prefetched queryset + 'maintainer': ( + instance.team and instance.team.name or str(instance.authors.all()[0]) + ), + 'license': [license_iter.slug for license_iter in matching_version.licenses.all()], + 'permissions': file.metadata.get('permissions'), + 'platforms': file.get_platforms(), + # TODO: handle copyright + 'tags': [str(tag) for tag in matching_version.tags.all()], + } + result.append(clean_json_dictionary_from_optional_fields(data)) + return result class ExtensionsAPIView(APIView): @@ -153,7 +157,10 @@ class ExtensionsAPIView(APIView): request=request, many=True, ) - data = [e for e in serializer.data if e is not None] + data = [] + for entry in serializer.data: + data.extend(entry) + return Response( { 'blocklist': Extension.objects.blocklisted.values_list('extension_id', flat=True), -- 2.30.2 From a1ebb51a2a9e297da0d7bbf2937f658c9aa31d22 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Mon, 8 Jul 2024 16:06:20 +0200 Subject: [PATCH 30/38] cleanup --- extensions/views/mixins.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/extensions/views/mixins.py b/extensions/views/mixins.py index 52cb99f4..eeb62183 100644 --- a/extensions/views/mixins.py +++ b/extensions/views/mixins.py @@ -27,18 +27,13 @@ class ExtensionQuerysetMixin: class MaintainedExtensionMixin: - """Fetch an extension by slug if current user is a maintainer. - - If pk is present, also fetch a corresponding version. - """ + """Fetch an extension by slug if current user is a maintainer.""" def dispatch(self, *args, **kwargs): self.extension = get_object_or_404( Extension.objects.authored_by(self.request.user), slug=self.kwargs['slug'], ) - if 'pk' in self.kwargs: - self.version = get_object_or_404(self.extension.versions, pk=self.kwargs['pk']) return super().dispatch(*args, **kwargs) -- 2.30.2 From 49b763d0b80fd742c23b5e1f081fbf491681267a Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Mon, 8 Jul 2024 17:15:25 +0200 Subject: [PATCH 31/38] add a test for archive_url uniqueness --- extensions/tests/test_api.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/extensions/tests/test_api.py b/extensions/tests/test_api.py index 29cec6f8..0ec1fc07 100644 --- a/extensions/tests/test_api.py +++ b/extensions/tests/test_api.py @@ -209,6 +209,8 @@ class FiltersTest(APITestCase): HTTP_ACCEPT='application/json', ).json() self.assertEqual(len(json['data']), 2) + # all archive_url values should be unique + self.assertEqual(len(json['data']), len(set(item['archive_url'] for item in json['data']))) def test_blender_version_filter_latest_not_max_version(self): version = create_approved_version(metadata__blender_version_min='4.0.1') -- 2.30.2 From d55c3c9115573f4ae8c1b097d944d126c696449e Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Mon, 8 Jul 2024 18:40:17 +0200 Subject: [PATCH 32/38] new pattern for download url: include file hash --- extensions/models.py | 59 +++++++++++++------------------------- extensions/urls.py | 6 +--- extensions/views/api.py | 2 +- extensions/views/public.py | 28 +++++++++--------- users/tests/test_tasks.py | 2 +- 5 files changed, 37 insertions(+), 60 deletions(-) diff --git a/extensions/models.py b/extensions/models.py index 8b24f9ed..a82da127 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -773,36 +773,22 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): permissions.append({'slug': slug, 'reason': reason, 'name': all_permission_names[slug]}) return permissions - def _get_download_name(self, platform=None) -> str: + def _get_download_name(self, file) -> str: """Return a file name for downloads.""" parts = [self.extension.type_slug_singular, self.extension.slug, f'v{self.version}'] - if platform: - parts.append(platform) + if platforms := file.get_platforms(): + parts.extend(platforms) return f'{"-".join(parts)}.zip' - def get_download_url(self, platform=None, append_repository_and_compatibility=True) -> str: - filename = self._get_download_name(platform=platform) - if platform: - download_url = reverse( - 'extensions:version-platform-download', - kwargs={ - 'type_slug': self.extension.type_slug, - 'slug': self.extension.slug, - 'version': self.version, - 'platform': platform, - 'filename': filename, - }, - ) - else: - download_url = reverse( - 'extensions:version-download', - kwargs={ - 'type_slug': self.extension.type_slug, - 'slug': self.extension.slug, - 'version': self.version, - 'filename': filename, - }, - ) + def get_download_url(self, file, append_repository_and_compatibility=True) -> str: + filename = self._get_download_name(file) + download_url = reverse( + 'extensions:download', + kwargs={ + 'filehash': file.hash, + 'filename': filename, + }, + ) if append_repository_and_compatibility: params = { 'repository': '/api/v1/extensions/', @@ -810,10 +796,8 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): } if self.blender_version_max: params['blender_version_max'] = self.blender_version_max - if platform: - params['platforms'] = platform - elif platforms := self.platforms.all(): - params['platforms'] = ','.join([p.slug for p in platforms]) + if platforms := file.get_platforms(): + params['platforms'] = ','.join(platforms) query_string = urlencode(params) download_url += f'?{query_string}' return download_url @@ -824,9 +808,9 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): file = files[0] return [ { - 'name': self._get_download_name(platform=None), + 'name': self._get_download_name(file), 'size': file.size_bytes, - 'url': self.get_download_url(platform=None), + 'url': self.get_download_url(file), } ] platform2file = {} @@ -841,10 +825,10 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): platform2file[platform] = file return [ { - 'name': self._get_download_name(platform=p), + 'name': self._get_download_name(file), 'platform': p, 'size': file.size_bytes, - 'url': self.get_download_url(platform=p), + 'url': self.get_download_url(file), } for p, file in platform2file.items() ] @@ -853,15 +837,12 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): build_list = [] for file in self.files.all(): platforms = file.get_platforms() or [] - platform = len(platforms) and platforms[0] or None - # if file has multiple platforms, picking the first one should still produce a correct - # download_url build_list.append( { - 'name': self._get_download_name(platform=platform), + 'name': self._get_download_name(file), 'platforms': platforms, 'size': file.size_bytes, - 'url': self.get_download_url(platform=platform), + 'url': self.get_download_url(file), } ) return build_list diff --git a/extensions/urls.py b/extensions/urls.py index 3d350df7..576a3dd5 100644 --- a/extensions/urls.py +++ b/extensions/urls.py @@ -28,6 +28,7 @@ urlpatterns = [ path('search/', public.SearchView.as_view(), name='search'), path('tag//', public.SearchView.as_view(), name='by-tag'), path('team//', public.SearchView.as_view(), name='by-team'), + path('download//', public.file_download, name='download'), re_path( rf'^(?P{EXTENSION_SLUGS_PATH})/', include( @@ -91,11 +92,6 @@ urlpatterns = [ public.extension_version_download, name='version-download', ), - path( - '///download/', - public.extension_version_platform_download, - name='version-platform-download', - ), path('/versions/', manage.VersionsView.as_view(), name='versions'), ], ), diff --git a/extensions/views/api.py b/extensions/views/api.py index 4e756eae..7e254d05 100644 --- a/extensions/views/api.py +++ b/extensions/views/api.py @@ -93,7 +93,7 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): 'archive_size': file.size_bytes, 'archive_url': self.request.build_absolute_uri( matching_version.get_download_url( - platform=self.platform, + file, append_repository_and_compatibility=False, ) ), diff --git a/extensions/views/public.py b/extensions/views/public.py index 2a9ae203..2209d24d 100644 --- a/extensions/views/public.py +++ b/extensions/views/public.py @@ -4,21 +4,19 @@ import logging from django.contrib.auth import get_user_model from django.db import connection from django.db.models import Count, Q -from django.http import Http404 from django.shortcuts import get_object_or_404, redirect from django.utils.translation import gettext_lazy as _ from django.views.generic import DetailView, ListView -from extensions.models import Extension, Version, Tag from constants.base import ( EXTENSION_TYPE_SLUGS, EXTENSION_TYPE_PLURAL, EXTENSION_TYPE_CHOICES, ) -from stats.models import ExtensionView +from extensions.models import Extension, Version, Tag +from files.models import File +from stats.models import ExtensionDownload, ExtensionView, VersionDownload import ratings.models - -from stats.models import ExtensionDownload, VersionDownload import teams.models User = get_user_model() @@ -53,14 +51,16 @@ class HomeView(ListedExtensionsView): def extension_version_download(request, type_slug, slug, version, filename): - """A backward-compatible shortcut for the method below. + """A backward-compatible url for downloads. - No platform is specified, assuming only a single file for a given version. + Assuming only a single file for a given version. """ - return extension_version_platform_download(request, type_slug, slug, version, None, filename) + extension_version = get_object_or_404(Version, extension__slug=slug, version=version) + file = extension_version.files.first() + return file_download(request, file.hash, filename) -def extension_version_platform_download(request, type_slug, slug, version, platform, filename): +def file_download(request, filehash, filename): """Download an extension version and count downloads. This method processes urls constructed by Version.get_download_list. @@ -70,13 +70,13 @@ def extension_version_platform_download(request, type_slug, slug, version, platf drag&drop. Also see $arg_filename usage in playbooks/templates/nginx/http.conf """ - extension_version = get_object_or_404(Version, extension__slug=slug, version=version) + # TODO check file status + file = get_object_or_404(File, hash=filehash, version__isnull=False) + extension_version = file.version.first() ExtensionDownload.create_from_request(request, object_id=extension_version.extension_id) VersionDownload.create_from_request(request, object_id=extension_version.pk) - if file := extension_version.get_file_for_platform(platform): - url = file.source.url - return redirect(f'{url}?filename={filename}') - raise Http404() + url = file.source.url + return redirect(f'{url}?filename={filename}') class SearchView(ListedExtensionsView): diff --git a/users/tests/test_tasks.py b/users/tests/test_tasks.py index 2352528f..59028ff5 100644 --- a/users/tests/test_tasks.py +++ b/users/tests/test_tasks.py @@ -82,7 +82,7 @@ class TestTasks(TestCase): self.assertEqual(response.status_code, 200) response = self.client.get(version.extension.get_versions_url()) self.assertEqual(response.status_code, 200) - response = self.client.get(version.get_download_url()) + response = self.client.get(version.get_download_url(version.files.first())) self.assertEqual(response.status_code, 302) self.assertEqual( response['Location'], -- 2.30.2 From 2d11c02c3a1f356c63c91114d9d34e41f6c5db6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rton=20Lente?= Date: Mon, 8 Jul 2024 19:27:49 +0200 Subject: [PATCH 33/38] UI: Improve template component platforms support display Part of #194, #201 --- .../extensions/components/platforms.html | 22 ++++++++++++------- extensions/templates/extensions/detail.html | 3 ++- .../templates/extensions/version_list.html | 4 +++- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/extensions/templates/extensions/components/platforms.html b/extensions/templates/extensions/components/platforms.html index 037a84d1..df959763 100644 --- a/extensions/templates/extensions/components/platforms.html +++ b/extensions/templates/extensions/components/platforms.html @@ -1,10 +1,16 @@ +{% load i18n %} + {% if version.platforms.all %} -
    - Supported platforms: -
      - {% for p in version.platforms.all %} -
    • {{p.name}}
      • - {% endfor %} -
    -
    +
    +
    +
    {% trans "Supported Platforms" %}
    +
    +
      + {% for p in version.platforms.all %} +
    • {{p.name}}
      • + {% endfor %} +
    +
    +
    +
    {% endif %} diff --git a/extensions/templates/extensions/detail.html b/extensions/templates/extensions/detail.html index 2659cd5b..cef29890 100644 --- a/extensions/templates/extensions/detail.html +++ b/extensions/templates/extensions/detail.html @@ -202,11 +202,12 @@
    {% trans 'Compatibility' %}
    {% include "extensions/components/blender_version.html" with version=latest %} - {% include "extensions/components/platforms.html" with version=latest %}

    + {% include "extensions/components/platforms.html" with version=latest %} + {% if extension.website %}
    diff --git a/extensions/templates/extensions/version_list.html b/extensions/templates/extensions/version_list.html index dbdd0526..0a5fcc73 100644 --- a/extensions/templates/extensions/version_list.html +++ b/extensions/templates/extensions/version_list.html @@ -64,10 +64,12 @@
    {% trans 'Compatibility' %}
    {% include "extensions/components/blender_version.html" with version=version %} - {% include "extensions/components/platforms.html" with version=version %}
    + + {% include "extensions/components/platforms.html" with version=version %} +
    Downloads
    -- 2.30.2 From f0837804d3cf1f7fb7deea1c2a5be26799861283 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rton=20Lente?= Date: Tue, 9 Jul 2024 12:15:09 +0200 Subject: [PATCH 34/38] UI: Improve template detail sizes display for multiple platforms Part of #194, #201 --- .../extensions/components/platforms.html | 2 +- extensions/templates/extensions/detail.html | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/extensions/templates/extensions/components/platforms.html b/extensions/templates/extensions/components/platforms.html index df959763..ef77985f 100644 --- a/extensions/templates/extensions/components/platforms.html +++ b/extensions/templates/extensions/components/platforms.html @@ -5,7 +5,7 @@
    {% trans "Supported Platforms" %}
    -
      +
        {% for p in version.platforms.all %}
      • {{p.name}}
        • {% endfor %} diff --git a/extensions/templates/extensions/detail.html b/extensions/templates/extensions/detail.html index cef29890..c7c3688e 100644 --- a/extensions/templates/extensions/detail.html +++ b/extensions/templates/extensions/detail.html @@ -188,11 +188,18 @@
        {% trans 'Size' %}
        {% if latest.single_file %} -
        {{ latest.files.first.size_bytes|filesizeformat }}
        +
        {{ latest.files.first.size_bytes|filesizeformat }}
        {% else %} - {% for file in latest.files.all %} -
        {{ file.size_bytes|filesizeformat }} ({{ file.get_platforms|join:", " }})
        - {% endfor %} +
        +
          + {% for file in latest.files.all %} +
        • + {{ file.size_bytes|filesizeformat }}
          + ({{ file.get_platforms|join:", " }}) +
          • + {% endfor %} +
        +
        {% endif %}
    -- 2.30.2 From 0d9fea120c1c6585f2d2513aec9a13f886a0358f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rton=20Lente?= Date: Tue, 9 Jul 2024 12:36:53 +0200 Subject: [PATCH 35/38] UI: Improve style btn-install-action-item display for multiple platforms Improve how install action item buttons are display when extension variants are available for multiple platforms. Part of #194, #201 --- common/static/common/styles/_extension.sass | 6 ++++++ extensions/templates/extensions/detail.html | 24 +++++++++++---------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/common/static/common/styles/_extension.sass b/common/static/common/styles/_extension.sass index 33127a35..07826cca 100644 --- a/common/static/common/styles/_extension.sass +++ b/common/static/common/styles/_extension.sass @@ -164,6 +164,12 @@ .btn-accent box-shadow: .2rem .8rem .8rem rgba(0,82,255,.102), 1.6rem 1.0rem 1.8rem rgba(0,82,255,.102), 3.2rem 1.0rem 3.2rem rgba(0,82,255,.349) + .btn-install-action-item + +margin(3, bottom) + + &:last-child + +margin(0, bottom) + .btn-install-drag, .btn-install-drag:active transform: initial diff --git a/extensions/templates/extensions/detail.html b/extensions/templates/extensions/detail.html index c7c3688e..cb14d95c 100644 --- a/extensions/templates/extensions/detail.html +++ b/extensions/templates/extensions/detail.html @@ -272,17 +272,19 @@
    {% for download_item in download_list %} -
    - -
    - - {# TODO @front-end: Replace URL of the manual /dev/ with /latest/. #} - ...or download - and Install from Disk - +
    +
    + +
    + + {# TODO @front-end: Replace URL of the manual /dev/ with /latest/. #} + ...or download + and Install from Disk + +
    {% endfor %}
    -- 2.30.2 From 50a9ec7be5ade23e500486b3d0acc634a572793c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rton=20Lente?= Date: Tue, 9 Jul 2024 13:13:24 +0200 Subject: [PATCH 36/38] Feat: Add support for multiple install-with-drag-and-drop buttons Add drag and drop install buttons support for extensions with multiple explicit platform versions. Part of #194, #201 --- common/static/common/scripts/app.js | 42 ++++++++++++--------- extensions/templates/extensions/detail.html | 4 +- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/common/static/common/scripts/app.js b/common/static/common/scripts/app.js index e54772e0..67d8865c 100644 --- a/common/static/common/scripts/app.js +++ b/common/static/common/scripts/app.js @@ -95,20 +95,19 @@ // Create function copyInstallUrl function copyInstallUrl() { function init() { - // Create variables + // Create variables single const btnInstall = document.querySelector('.js-btn-install'); const btnInstallAction = document.querySelector('.js-btn-install-action'); const btnInstallGroup = document.querySelector('.js-btn-install-group'); - const btnInstallDrag = document.querySelector('.js-btn-install-drag'); - const btnInstallDragGroup = document.querySelector('.js-btn-install-drag-group'); + + // Create variables multiple + const btnInstallActionItem = document.querySelectorAll('.js-btn-install-action-item'); if (btnInstall == null) { return; } - // Get data install URL - const btnInstallUrl = btnInstall.getAttribute('data-install-url'); - + // Show btnInstallAction btnInstall.addEventListener('click', function() { // Hide btnInstallGroup btnInstallGroup.classList.add('d-none'); @@ -117,19 +116,28 @@ btnInstallAction.classList.add('show'); }); - // Drag btnInstallUrl - btnInstallDrag.addEventListener('dragstart', function(e) { - // Set data install URL to be transferred during drag - e.dataTransfer.setData('text/plain', btnInstallUrl); + btnInstallActionItem.forEach(function(item) { + // Create variables in function scope + const btnInstallDrag = item.querySelector('.js-btn-install-drag'); + const btnInstallDragGroup = item.querySelectorAll('.js-btn-install-drag-group'); - // Set drag area active - btnInstallDragGroup.classList.add('opacity-50'); - }); + // Get data install URL + const btnInstallUrl = item.getAttribute('data-install-url'); - // Undrag btnInstallUrl - btnInstallDrag.addEventListener('dragend', function() { - // Set drag area inactive - btnInstallDragGroup.classList.remove('opacity-50'); + // Drag btnInstallUrl + btnInstallDrag.addEventListener('dragstart', function(e) { + // Set data install URL to be transferred during drag + e.dataTransfer.setData('text/plain', btnInstallUrl); + + // Set drag area active + btnInstallDragGroup.classList.add('opacity-50'); + }); + + // Undrag btnInstallUrl + btnInstallDrag.addEventListener('dragend', function() { + // Set drag area inactive + btnInstallDragGroup.classList.remove('opacity-50'); + }); }); } diff --git a/extensions/templates/extensions/detail.html b/extensions/templates/extensions/detail.html index cb14d95c..74873a7d 100644 --- a/extensions/templates/extensions/detail.html +++ b/extensions/templates/extensions/detail.html @@ -266,13 +266,13 @@ {% if extension.is_approved %} {% with download_list=latest.get_download_list %}
    -
    {% for download_item in download_list %} -
    +
    {% trans 'Size' %}
    - {% if version.single_file %} + {% if version.has_single_file %}
    {{ version.files.first.size_bytes|filesizeformat }}
    {% else %} {% for file in version.files.all %} diff --git a/extensions/templates/extensions/detail.html b/extensions/templates/extensions/detail.html index 74873a7d..6d939420 100644 --- a/extensions/templates/extensions/detail.html +++ b/extensions/templates/extensions/detail.html @@ -187,7 +187,7 @@
    {% trans 'Size' %}
    - {% if latest.single_file %} + {% if latest.has_single_file %}
    {{ latest.files.first.size_bytes|filesizeformat }}
    {% else %}
    diff --git a/extensions/templates/extensions/version_list.html b/extensions/templates/extensions/version_list.html index 0a5fcc73..1cf6a52d 100644 --- a/extensions/templates/extensions/version_list.html +++ b/extensions/templates/extensions/version_list.html @@ -30,7 +30,7 @@ {% include "extensions/components/blender_version.html" with version=version %}
      - {% if version.single_file %} + {% if version.has_single_file %}
    • {{ version.files.first.size_bytes|filesizeformat }}
      • {% else %} {% for file in version.files.all %} @@ -77,7 +77,7 @@
    Size
    - {% if version.single_file %} + {% if version.has_single_file %}
    {{ version.files.first.size_bytes|filesizeformat }}
    {% else %} {% for file in version.files.all %} -- 2.30.2