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/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/models.py b/extensions/models.py index 0c02b5c8..b91390cd 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -7,7 +7,6 @@ from django.core.exceptions import ObjectDoesNotExist, BadRequest from django.db import models, transaction from django.db.models import Q, Count from django.urls import reverse -from django.utils.functional import cached_property from common.fields import FilterableManyToManyField from common.model_mixins import CreatedModifiedMixin, RecordDeletionMixin, TrackChangesMixin @@ -253,7 +252,9 @@ class Extension(CreatedModifiedMixin, TrackChangesMixin, models.Model): def update_metadata_from_version(self, version): update_fields = set() - metadata = version.file.metadata + files = list(version.files.all()) + # picking the last uploaded file + metadata = files[-1].metadata # check if we can also update name # if we are uploading a new version, we have just validated and don't expect a clash, # but if a version is being deleted, we want to rollback to a name from an older version, @@ -276,9 +277,9 @@ class Extension(CreatedModifiedMixin, TrackChangesMixin, models.Model): # TODO: check that latest version is currently in review (whatever that means in data?) latest_version = self.latest_version - file = latest_version.file - file.status = FILE_STATUS_CHOICES.APPROVED - file.save() + for file in latest_version.files.all(): + file.status = FILE_STATUS_CHOICES.APPROVED + file.save() # TODO: check that this extension is currently in review (whatever that means in data?) self.previews.update(status=FILE_STATUS_CHOICES.APPROVED) @@ -422,8 +423,12 @@ class Extension(CreatedModifiedMixin, TrackChangesMixin, models.Model): ) def suspicious_files(self): - versions = self.versions.all() - return [v.file for v in versions if not v.file.validation.is_ok] + files = [] + for version in self.versions.all(): + for file in version.files.all(): + if not file.validation.is_ok: + files.append(file) + return files @classmethod def get_lookup_field(cls, identifier): @@ -439,7 +444,8 @@ class Extension(CreatedModifiedMixin, TrackChangesMixin, models.Model): for version in versions: if skip_version and version == skip_version: continue - if version.file.status not in self.valid_file_statuses: + files = version.files.all() + if not any([file.status in self.valid_file_statuses for file in files]): continue latest_version = version break @@ -667,25 +673,71 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): def __str__(self) -> str: return f'{self.extension} v{self.version}' - # TODO get rid of this once all places are aware of multi-file reality - # - # the fact that it is cached is important, this allows to use version.file as if it still was a - # model field, i.e. write things like - # version.file.status = ... - # version.file.save() - # if it isn't cached, then the save call is applied to a different, newly fetched File instance - @cached_property - def file(self): - files = list(self.files.all()) - if files: - return files[0] - else: - return None + @property + def has_single_file(self): + return len(self.files.all()) == 1 + + 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( + 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() + + @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.get_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_remaining_platforms(self): + all_platforms = set(p.slug for p in Platform.objects.all()) + for file in self.files.all(): + platforms = file.get_platforms() + if not platforms: + # no platforms means any platform + return set() + 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, version file must have a public status. - return self.file.status == self.file.STATUSES.APPROVED + # To be public, at least one version file must have a public status. + return any([file.status == File.STATUSES.APPROVED for file in self.files.all()]) @property def cannot_be_deleted_reasons(self) -> List[str]: @@ -699,35 +751,41 @@ 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.""" - if 'permissions' not in self.file.metadata: - return [] - permissions = [] - all_permission_names = {p.slug: p.name for p in VersionPermission.objects.all()} - for slug, reason in self.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 - @property - def download_name(self) -> str: + def _get_download_name(self, file) -> str: """Return a file name for downloads.""" - replace_char = f'{self}'.replace('.', '-') - return f'{utils.slugify(replace_char)}{self.file.suffix}' + parts = [self.extension.type_slug_singular, self.extension.slug, f'v{self.version}'] + if platforms := file.get_platforms(): + parts.extend(platforms) + return f'{"-".join(parts)}.zip' - @property - def downloadable_signed_url(self) -> str: - # TODO: actual signed URLs? - return self.file.source.url - - def download_url(self, append_repository_and_compatibility=True) -> str: - filename = f'{self.extension.type_slug_singular}-{self.extension.slug}-v{self.version}.zip' + def get_download_url(self, file, append_repository_and_compatibility=True) -> str: + filename = self._get_download_name(file) download_url = reverse( - 'extensions:version-download', + 'extensions:download', kwargs={ - 'type_slug': self.extension.type_slug, - 'slug': self.extension.slug, - 'version': self.version, + 'filehash': file.hash, 'filename': filename, }, ) @@ -738,12 +796,57 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model): } 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]) + if platforms := file.get_platforms(): + params['platforms'] = ','.join(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': self._get_download_name(file), + 'size': file.size_bytes, + 'url': self.get_download_url(file), + } + ] + platform2file = {} + for file in files: + platforms = file.get_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 [ + { + 'name': self._get_download_name(file), + 'platform': p, + 'size': file.size_bytes, + 'url': self.get_download_url(file), + } + for p, file in platform2file.items() + ] + + def get_build_list(self) -> List[dict]: + build_list = [] + for file in self.files.all(): + platforms = file.get_platforms() or [] + build_list.append( + { + 'name': self._get_download_name(file), + 'platforms': platforms, + 'size': file.size_bytes, + 'url': self.get_download_url(file), + } + ) + return build_list + def get_delete_url(self) -> str: return reverse( 'extensions:version-delete', diff --git a/extensions/signals.py b/extensions/signals.py index f088cfbd..4840604d 100644 --- a/extensions/signals.py +++ b/extensions/signals.py @@ -43,10 +43,19 @@ def _delete_versionfiles_file( logger.info('Deleting File pk=%s of VersionFile pk=%s', file.pk, instance.pk) file.delete() - if instance.version.files.count() == 0: - # this was the last file, clean up the version - logger.info('Deleting Version pk=%s because its last file was deleted', instance.version.pk) - instance.version.delete() + # 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( + 'Deleting Version pk=%s because its last file was deleted', + instance.version.pk, + ) + instance.version.delete() + except extensions.models.Version.DoesNotExist: + pass @receiver(pre_save, sender=extensions.models.Extension) diff --git a/extensions/templates/extensions/components/extension_edit_detail_card.html b/extensions/templates/extensions/components/extension_edit_detail_card.html index 89b3b009..928b8a24 100644 --- a/extensions/templates/extensions/components/extension_edit_detail_card.html +++ b/extensions/templates/extensions/components/extension_edit_detail_card.html @@ -69,7 +69,13 @@
This {{ extension.get_type_display|lower }} is currently under review.
@@ -290,7 +312,7 @@ {% trans 'Review Extension' %}