Multi-platform: support multiple files per version #201

Merged
Oleg-Komarov merged 43 commits from multi-os into main 2024-07-09 16:27:46 +02:00
33 changed files with 782 additions and 265 deletions

View File

@ -95,20 +95,19 @@
// Create function copyInstallUrl // Create function copyInstallUrl
function copyInstallUrl() { function copyInstallUrl() {
function init() { function init() {
// Create variables // Create variables single
const btnInstall = document.querySelector('.js-btn-install'); const btnInstall = document.querySelector('.js-btn-install');
const btnInstallAction = document.querySelector('.js-btn-install-action'); const btnInstallAction = document.querySelector('.js-btn-install-action');
const btnInstallGroup = document.querySelector('.js-btn-install-group'); 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) { if (btnInstall == null) {
return; return;
} }
// Get data install URL // Show btnInstallAction
const btnInstallUrl = btnInstall.getAttribute('data-install-url');
btnInstall.addEventListener('click', function() { btnInstall.addEventListener('click', function() {
// Hide btnInstallGroup // Hide btnInstallGroup
btnInstallGroup.classList.add('d-none'); btnInstallGroup.classList.add('d-none');
@ -117,6 +116,14 @@
btnInstallAction.classList.add('show'); btnInstallAction.classList.add('show');
}); });
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');
// Get data install URL
const btnInstallUrl = item.getAttribute('data-install-url');
// Drag btnInstallUrl // Drag btnInstallUrl
btnInstallDrag.addEventListener('dragstart', function(e) { btnInstallDrag.addEventListener('dragstart', function(e) {
// Set data install URL to be transferred during drag // Set data install URL to be transferred during drag
@ -131,6 +138,7 @@
// Set drag area inactive // Set drag area inactive
btnInstallDragGroup.classList.remove('opacity-50'); btnInstallDragGroup.classList.remove('opacity-50');
}); });
});
} }
init(); init();

View File

@ -164,6 +164,12 @@
.btn-accent .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) 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,
.btn-install-drag:active .btn-install-drag:active
transform: initial transform: initial

View File

@ -7,7 +7,6 @@ from django.core.exceptions import ObjectDoesNotExist, BadRequest
from django.db import models, transaction from django.db import models, transaction
from django.db.models import Q, Count from django.db.models import Q, Count
from django.urls import reverse from django.urls import reverse
from django.utils.functional import cached_property
from common.fields import FilterableManyToManyField from common.fields import FilterableManyToManyField
from common.model_mixins import CreatedModifiedMixin, RecordDeletionMixin, TrackChangesMixin 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): def update_metadata_from_version(self, version):
update_fields = set() 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 # check if we can also update name
# if we are uploading a new version, we have just validated and don't expect a clash, # 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, # but if a version is being deleted, we want to rollback to a name from an older version,
@ -276,7 +277,7 @@ class Extension(CreatedModifiedMixin, TrackChangesMixin, models.Model):
# TODO: check that latest version is currently in review (whatever that means in data?) # TODO: check that latest version is currently in review (whatever that means in data?)
latest_version = self.latest_version latest_version = self.latest_version
file = latest_version.file for file in latest_version.files.all():
file.status = FILE_STATUS_CHOICES.APPROVED file.status = FILE_STATUS_CHOICES.APPROVED
file.save() file.save()
@ -422,8 +423,12 @@ class Extension(CreatedModifiedMixin, TrackChangesMixin, models.Model):
) )
def suspicious_files(self): def suspicious_files(self):
versions = self.versions.all() files = []
return [v.file for v in versions if not v.file.validation.is_ok] for version in self.versions.all():
for file in version.files.all():
if not file.validation.is_ok:
files.append(file)
return files
@classmethod @classmethod
def get_lookup_field(cls, identifier): def get_lookup_field(cls, identifier):
@ -439,7 +444,8 @@ class Extension(CreatedModifiedMixin, TrackChangesMixin, models.Model):
for version in versions: for version in versions:
if skip_version and version == skip_version: if skip_version and version == skip_version:
continue 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 continue
latest_version = version latest_version = version
break break
@ -667,25 +673,71 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model):
def __str__(self) -> str: def __str__(self) -> str:
return f'{self.extension} v{self.version}' return f'{self.extension} v{self.version}'
# TODO get rid of this once all places are aware of multi-file reality @property
# def has_single_file(self):
# the fact that it is cached is important, this allows to use version.file as if it still was a return len(self.files.all()) == 1
# model field, i.e. write things like
# version.file.status = ... def add_file(self, file: File):
# version.file.save() current_platforms = set([p.slug for p in self.platforms.all()])
# if it isn't cached, then the save call is applied to a different, newly fetched File instance if not current_platforms:
Oleg-Komarov marked this conversation as resolved Outdated

s/get_available_platforms/get_missing_platforms/?

`s/get_available_platforms/get_missing_platforms/`?
@cached_property raise ValueError(
def file(self): f'add_file failed: Version pk={self.pk} is not platform-specific, '
files = list(self.files.all()) f'no more files allowed'
if files: )
return files[0] 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: 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
Oleg-Komarov marked this conversation as resolved Outdated

or s/get_available_platforms/get_remaining_platforms/

in the sense that these are the platforms which don't yet have files uploaded for them, if any platform-specific file were uploaded

or `s/get_available_platforms/get_remaining_platforms/` in the sense that these are the platforms which don't yet have files uploaded for them, if any platform-specific file were uploaded
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 return None
@property @property
def is_listed(self): def is_listed(self):
# To be public, version file must have a public status. # To be public, at least one version file must have a public status.
return self.file.status == self.file.STATUSES.APPROVED return any([file.status == File.STATUSES.APPROVED for file in self.files.all()])
@property @property
def cannot_be_deleted_reasons(self) -> List[str]: def cannot_be_deleted_reasons(self) -> List[str]:
@ -699,35 +751,41 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model):
@property @property
def permissions_with_reasons(self) -> List[dict]: def permissions_with_reasons(self) -> List[dict]:
"""Returns permissions with reasons, slugs, and names to be shown in templates.""" """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]})
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.
Oleg-Komarov marked this conversation as resolved Outdated

The filename below
filename = f'{self.extension.type_slug_singular}-{self.extension.slug}-v{self.version}.zip'

looks like a better name: doesn't rely on self.__str__.

It also looks like these file names should be the same in both places.

The `filename` below `filename = f'{self.extension.type_slug_singular}-{self.extension.slug}-v{self.version}.zip'` looks like a better name: doesn't rely on `self.__str__`. It also looks like these file names should be the same in both places.
"""
slug2reason = {}
Oleg-Komarov marked this conversation as resolved Outdated

to avoid making platform into file ext, maybe -?

to avoid making platform into file ext, maybe `-`?
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 return permissions
@property def _get_download_name(self, file) -> str:
def download_name(self) -> str:
"""Return a file name for downloads.""" """Return a file name for downloads."""
replace_char = f'{self}'.replace('.', '-') parts = [self.extension.type_slug_singular, self.extension.slug, f'v{self.version}']
return f'{utils.slugify(replace_char)}{self.file.suffix}' if platforms := file.get_platforms():
parts.extend(platforms)
return f'{"-".join(parts)}.zip'
@property def get_download_url(self, file, append_repository_and_compatibility=True) -> str:
def downloadable_signed_url(self) -> str: filename = self._get_download_name(file)
# 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'
download_url = reverse( download_url = reverse(
'extensions:version-download', 'extensions:download',
kwargs={ kwargs={
'type_slug': self.extension.type_slug, 'filehash': file.hash,
'slug': self.extension.slug,
'version': self.version,
'filename': filename, 'filename': filename,
}, },
) )
@ -738,12 +796,57 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model):
} }
if self.blender_version_max: if self.blender_version_max:
params['blender_version_max'] = self.blender_version_max params['blender_version_max'] = self.blender_version_max
if platforms := self.platforms.all(): if platforms := file.get_platforms():
params['platforms'] = ','.join([p.slug for p in platforms]) params['platforms'] = ','.join(platforms)
Oleg-Komarov marked this conversation as resolved Outdated

Would it make sense to use file hash in the URL?
This would allow skipping the file-selecting logic in the public view altogether by using the hash to retrieve the file directly.

Would it make sense to use file hash in the URL? This would allow skipping the file-selecting logic in the `public` view altogether by using the hash to retrieve the file directly.
query_string = urlencode(params) query_string = urlencode(params)
download_url += f'?{query_string}' download_url += f'?{query_string}'
return download_url 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: def get_delete_url(self) -> str:
return reverse( return reverse(
'extensions:version-delete', 'extensions:version-delete',

View File

@ -43,10 +43,19 @@ def _delete_versionfiles_file(
logger.info('Deleting File pk=%s of VersionFile pk=%s', file.pk, instance.pk) logger.info('Deleting File pk=%s of VersionFile pk=%s', file.pk, instance.pk)
file.delete() file.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: if instance.version.files.count() == 0:
# this was the last file, clean up the version # this was the last file, clean up the version
logger.info('Deleting Version pk=%s because its last file was deleted', instance.version.pk) logger.info(
'Deleting Version pk=%s because its last file was deleted',
instance.version.pk,
)
instance.version.delete() instance.version.delete()
except extensions.models.Version.DoesNotExist:
pass
@receiver(pre_save, sender=extensions.models.Extension) @receiver(pre_save, sender=extensions.models.Extension)

View File

@ -69,7 +69,13 @@
</div> </div>
<div class="dl-col"> <div class="dl-col">
<dt>{% trans 'Size' %}</dt> <dt>{% trans 'Size' %}</dt>
<dd>{{ version.file.size_bytes|filesizeformat }}</dd> {% if version.has_single_file %}
<dd>{{ version.files.first.size_bytes|filesizeformat }}</dd>
{% else %}
{% for file in version.files.all %}
<dd>{{ file.size_bytes|filesizeformat }} ({{ file.get_platforms|join:", " }})</dd>
{% endfor %}
{% endif %}
</div> </div>
</div> </div>

View File

@ -1,10 +1,16 @@
{% load i18n %}
{% if version.platforms.all %} {% if version.platforms.all %}
<div> <div class="dl-row">
Supported platforms: <div class="dl-col">
<ul> <dt>{% trans "Supported Platforms" %}</dt>
<dd>
<ul class="mb-0 ps-3">
{% for p in version.platforms.all %} {% for p in version.platforms.all %}
<li>{{p.name}}</li> <li>{{p.name}}</li>
{% endfor %} {% endfor %}
</ul> </ul>
</dd>
</div>
</div> </div>
{% endif %} {% endif %}

View File

@ -187,7 +187,20 @@
</div> </div>
<div class="dl-col"> <div class="dl-col">
<dt>{% trans 'Size' %}</dt> <dt>{% trans 'Size' %}</dt>
<dd>{{ latest.file.size_bytes|filesizeformat }}</dd> {% if latest.has_single_file %}
<dd>{{ latest.files.first.size_bytes|filesizeformat }}</dd>
{% else %}
<dd>
<ul class="ps-3">
{% for file in latest.files.all %}
<li class="lh-sm">
{{ file.size_bytes|filesizeformat }}<br>
<span class="fs-sm text-muted">({{ file.get_platforms|join:", " }})</span>
</li>
{% endfor %}
</ul>
</dd>
{% endif %}
</div> </div>
</div> </div>
@ -196,11 +209,12 @@
<dt>{% trans 'Compatibility' %}</dt> <dt>{% trans 'Compatibility' %}</dt>
<dd> <dd>
{% include "extensions/components/blender_version.html" with version=latest %} {% include "extensions/components/blender_version.html" with version=latest %}
{% include "extensions/components/platforms.html" with version=latest %}
</dd> </dd>
</div> </div>
</div> </div>
{% include "extensions/components/platforms.html" with version=latest %}
{% if extension.website %} {% if extension.website %}
<div class="dl-row"> <div class="dl-row">
<div class="dl-col"> <div class="dl-col">
@ -250,36 +264,44 @@
{% block extension_download %} {% block extension_download %}
<section class="ext-detail-download mt-3"> <section class="ext-detail-download mt-3">
{% if extension.is_approved %} {% if extension.is_approved %}
{% with download_list=latest.get_download_list %}
<div class="btn-group js-btn-install-group"> <div class="btn-group js-btn-install-group">
<button class="btn btn-flex btn-accent js-btn-install" data-install-url="{{ request.scheme }}://{{ request.get_host }}{{ latest.download_url }}"> <button class="btn btn-flex btn-accent js-btn-install">
<span>{% trans 'Get' %} {{ extension.get_type_display }}</span> <span>{% trans 'Get' %} {{ extension.get_type_display }}</span>
</button> </button>
</div> </div>
<div class="fade js-btn-install-action"> <div class="fade js-btn-install-action">
{% for download_item in download_list %}
<div class="btn-install-action-item js-btn-install-action-item" data-install-url="{{ request.scheme }}://{{ request.get_host }}{{ download_item.url }}" download="{{ download_item.name }}">
<div class="btn-install-drag-group js-btn-install-drag-group mb-2 rounded"> <div class="btn-install-drag-group js-btn-install-drag-group mb-2 rounded">
<button class="btn btn-flex btn-primary btn-install-drag cursor-move js-btn-install-drag w-100" draggable="true"> <button class="btn btn-flex btn-primary btn-install-drag cursor-move js-btn-install-drag w-100" draggable="true">
<i class="i-move"></i> <i class="i-move"></i>
<span>{% trans 'Drag and Drop into Blender' %}</span> <span>{% trans 'Drag and Drop into Blender' %} <span class="fs-sm">{{ download_item.platform }}</span></span>
</button> </button>
</div> </div>
<small class="d-block text-center w-100"> <small class="d-block text-center w-100">
{# TODO @front-end: Replace URL of the manual /dev/ with /latest/. #} {# TODO @front-end: Replace URL of the manual /dev/ with /latest/. #}
...or <a class="text-underline text-primary" href="{{ request.scheme }}://{{ request.get_host }}{{ latest.download_url }}" download="{{ latest.download_name }}">download</a> ...or <a class="text-underline text-primary" href="{{ request.scheme }}://{{ request.get_host }}{{ download_item.url }}" download="{{ download_item.name }}">download</a>
and <a class="text-underline text-primary" href="https://docs.blender.org/manual/en/dev/editors/preferences/extensions.html#install" target="_blank">Install from Disk</a> and <a class="text-underline text-primary" href="https://docs.blender.org/manual/en/dev/editors/preferences/extensions.html#install" target="_blank">Install from Disk</a>
</small> </small>
</div> </div>
{% endfor %}
</div>
{# If JavaScript is disabled. #} {# If JavaScript is disabled. #}
<noscript> <noscript>
<style>.js-btn-install-group { display: none;}</style> <style>.js-btn-install-group { display: none;}</style>
<div class="btn-col text-center"> <div class="btn-col text-center">
<a class="btn btn-flex btn-accent" href="{{ request.scheme }}://{{ request.get_host }}{{ latest.download_url }}" download="{{ latest.download_name }}"> {% for download_item in download_list %}
<i class="i-download"></i><span>{% trans 'Download' %} {{ extension.get_type_display }}</span> <a class="btn btn-flex btn-accent" href="{{ request.scheme }}://{{ request.get_host }}{{ download_item.url }}" download="{{ download_item.name }}">
<i class="i-download"></i><span>{% trans 'Download' %} {{ extension.get_type_display }} {{ download_item.platform }}</span>
</a> </a>
<small class="mt-3">...and <a class="text-underline text-primary text-center" href="https://docs.blender.org/manual/en/dev/editors/preferences/extensions.html#install" target="_blank">Install from Disk</a></small> <small class="mt-3">...and <a class="text-underline text-primary text-center" href="https://docs.blender.org/manual/en/dev/editors/preferences/extensions.html#install" target="_blank">Install from Disk</a></small>
{% endfor %}
</div> </div>
</noscript> </noscript>
{% endwith %}
{% else %} {% else %}
<div class="card p-3 mt-3"> <div class="card p-3 mt-3">
<p class="text-info">This {{ extension.get_type_display|lower }} is currently under review.</p> <p class="text-info">This {{ extension.get_type_display|lower }} is currently under review.</p>

View File

@ -3,7 +3,7 @@
{% block page_title %}{{ extension.name }}{% endblock page_title %} {% block page_title %}{{ extension.name }}{% endblock page_title %}
{% block content %} {% block content %}
{% with author=extension.latest_version.file.user form=form|add_form_classes %} {% with form=form|add_form_classes %}
<div class="row"> <div class="row">
<div class="col-md-8"> <div class="col-md-8">
<h2>{{ extension.get_type_display }} {% trans 'details' %}</h2> <h2>{{ extension.get_type_display }} {% trans 'details' %}</h2>

View File

@ -16,11 +16,7 @@
{% csrf_token %} {% csrf_token %}
{% with form=form|add_form_classes %} {% with form=form|add_form_classes %}
<h2> <h2>v{{ form.instance.version }}</h2>
{% blocktranslate with version=form.instance.version %}
v{{ version }}
{% endblocktranslate %}
</h2>
<section class="card p-3"> <section class="card p-3">
<div class="row"> <div class="row">
@ -39,6 +35,21 @@
</div> </div>
{% endif %} {% endif %}
</section> </section>
<section class="card p-3 mt-4">
{% trans 'Builds' %}:
{% with build_list=form.instance.get_build_list %}
<div class="btn-col">
{% for build in build_list %}
<a href="{{ build.url }}" download="{{ build.name }}" class="btn btn-danger btn-block">
<i class="i-download"></i>
<span>
{% trans 'Download' %} v{{ form.instance.version }} {{ build.platforms|join:", " }}
</span>
</a>
{% endfor %}
{% endwith %}
</div>
</section>
</div> </div>
<div class="col-md-4"> <div class="col-md-4">
<div class="is-sticky"> <div class="is-sticky">

View File

@ -23,9 +23,11 @@
{% trans 'Upload Extension' %} {% trans 'Upload Extension' %}
{% endif %} {% endif %}
</h1> </h1>
{% if extension %}
<p> <p>
<a href="{{ extension.get_absolute_url }}"><strong>{{ extension.name }}</strong></a> <a href="{{ extension.get_absolute_url }}"><strong>{{ extension.name }}</strong></a>
</p> </p>
{% endif %}
<hr> <hr>
{% if not extension and drafts %} {% if not extension and drafts %}
<div> <div>

View File

@ -21,7 +21,7 @@
<div class="row ext-version-history pb-3"> <div class="row ext-version-history pb-3">
<div class="col"> <div class="col">
{% for version in extension.versions.all %} {% for version in object_list %}
{% if version.is_listed or is_maintainer %} {% if version.is_listed or is_maintainer %}
<details {% if forloop.counter == 1 %}open{% endif %} id="v{{ version.version|slugify }}"> <details {% if forloop.counter == 1 %}open{% endif %} id="v{{ version.version|slugify }}">
<summary> <summary>
@ -30,7 +30,13 @@
{% include "extensions/components/blender_version.html" with version=version %} {% include "extensions/components/blender_version.html" with version=version %}
</span> </span>
<ul class="ms-auto"> <ul class="ms-auto">
<li class="show-on-collapse">{{ version.file.size_bytes|filesizeformat }}</li> {% if version.has_single_file %}
<li class="show-on-collapse">{{ version.files.first.size_bytes|filesizeformat }}</li>
{% else %}
{% for file in version.files.all %}
<li class="show-on-collapse">{{ file.size_bytes|filesizeformat }} ({{ file.get_platforms|join:", " }})</li>
{% endfor %}
{% endif %}
<li class="show-on-collapse"><i class="i-download"></i> {{ version.download_count }}</li> <li class="show-on-collapse"><i class="i-download"></i> {{ version.download_count }}</li>
<li> <li>
<a href="#v{{ version.version|slugify }}" title="{% firstof version.date_approved|date:'l jS, F Y - H:i' version.date_created|date:'l jS, F Y - H:i' %}"> <a href="#v{{ version.version|slugify }}" title="{% firstof version.date_approved|date:'l jS, F Y - H:i' version.date_created|date:'l jS, F Y - H:i' %}">
@ -58,10 +64,12 @@
<dt>{% trans 'Compatibility' %}</dt> <dt>{% trans 'Compatibility' %}</dt>
<dd> <dd>
{% include "extensions/components/blender_version.html" with version=version %} {% include "extensions/components/blender_version.html" with version=version %}
{% include "extensions/components/platforms.html" with version=version %}
</dd> </dd>
</div> </div>
</div> </div>
{% include "extensions/components/platforms.html" with version=version %}
<div class="dl-row"> <div class="dl-row">
<div class="dl-col"> <div class="dl-col">
<dt>Downloads</dt> <dt>Downloads</dt>
@ -69,7 +77,13 @@
</div> </div>
<div class="dl-col"> <div class="dl-col">
<dt>Size</dt> <dt>Size</dt>
<dd>{{ version.file.size_bytes|filesizeformat }}</dd> {% if version.has_single_file %}
<dd>{{ version.files.first.size_bytes|filesizeformat }}</dd>
{% else %}
{% for file in version.files.all %}
<dd>{{ file.size_bytes|filesizeformat }} ({{ file.get_platforms|join:", " }})</dd>
{% endfor %}
{% endif %}
</div> </div>
</div> </div>
<div class="dl-row"> <div class="dl-row">
@ -91,17 +105,22 @@
<div class="dl-row"> <div class="dl-row">
<div class="dl-col"> <div class="dl-col">
<dt>Status</dt> <dt>Status</dt>
<dd>{% include "common/components/status.html" with object=version.file %}</dd> {# all files of a version get approved together #}
<dd>{% include "common/components/status.html" with object=version.files.first %}</dd>
</div> </div>
</div> </div>
{% endif %} {% endif %}
</section> </section>
<div class="btn-col"> <div class="btn-col">
<a href="{{ version.download_url }}" download="{{ version.download_name }}" class="btn btn-primary btn-block"> {% with download_list=version.get_download_list %}
{% for download_item in download_list %}
<a href="{{ download_item.url }}" download="{{ download_item.name }}" class="btn btn-primary btn-block">
<i class="i-download"></i> <i class="i-download"></i>
<span>{% trans 'Download' %} v{{ version.version }}</span> <span>{% trans 'Download' %} v{{ version.version }} {{ download_item.platform }}</span>
</a> </a>
{% endfor %}
{% endwith %}
{% if is_maintainer %} {% if is_maintainer %}
<a href="{{ version.update_url }}" class="btn"> <a href="{{ version.update_url }}" class="btn">
<i class="i-edit"></i><span>Edit</span> <i class="i-edit"></i><span>Edit</span>

View File

@ -5,8 +5,9 @@ from django.urls import reverse
from rest_framework import status from rest_framework import status
from rest_framework.test import APITestCase, APIClient 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.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 common.tests.utils import create_user_token
from extensions.models import Extension, Version from extensions.models import Extension, Version
@ -48,8 +49,9 @@ class ListedExtensionsTest(APITestCase):
self.assertEqual(self._listed_extensions_count(), 0) self.assertEqual(self._listed_extensions_count(), 0)
def test_moderate_only_version(self): def test_moderate_only_version(self):
self.version.file.status = File.STATUSES.DISABLED for file in self.version.files.all():
self.version.file.save() file.status = File.STATUSES.DISABLED
file.save()
self.assertEqual(self._listed_extensions_count(), 0) self.assertEqual(self._listed_extensions_count(), 0)
@ -150,6 +152,66 @@ class FiltersTest(APITestCase):
).json() ).json()
self.assertEqual(len(json['data']), 1) 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_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)
# 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): def test_blender_version_filter_latest_not_max_version(self):
version = create_approved_version(metadata__blender_version_min='4.0.1') version = create_approved_version(metadata__blender_version_min='4.0.1')
date_created = version.date_created date_created = version.date_created
@ -238,7 +300,8 @@ class VersionUploadAPITest(APITestCase):
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual( self.assertEqual(
response.data['message'], response.data['message'],
f'Extension "{other_extension.extension_id}" not maintained by user "{self.user.username}"', f'Extension "{other_extension.extension_id}" not maintained by user '
f'"{self.user.username}"',
) )
def test_version_upload_extension_does_not_exist(self): def test_version_upload_extension_does_not_exist(self):
@ -288,3 +351,59 @@ class VersionUploadAPITest(APITestCase):
self.assertEqual(response.status_code, status.HTTP_201_CREATED) self.assertEqual(response.status_code, status.HTTP_201_CREATED)
self.token.refresh_from_db() self.token.refresh_from_db()
self.assertIsNotNone(self.token.date_last_access) self.assertIsNotNone(self.token.date_last_access)
def test_multiplatform_upload(self):
extension = create_version(
metadata__id="some_addon",
metadata__version="0.0.1",
user=self.user,
).extension
file_linux = TEST_FILES_DIR / 'addon-with-split-platforms-linux.zip'
file_windows = TEST_FILES_DIR / 'addon-with-split-platforms-windows.zip'
with open(file_linux, 'rb') as version_file:
response = self.client.post(
self._get_upload_url('some_addon'),
{
'version_file': version_file,
'release_notes': 'only linux',
},
format='multipart',
HTTP_AUTHORIZATION=f'Bearer {self.token_key}',
)
self.assertEqual(response.status_code, status.HTTP_201_CREATED, response.json())
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 version_file:
response = self.client.post(
self._get_upload_url('some_addon'),
{
'version_file': version_file,
'release_notes': 'linux and windows',
},
format='multipart',
HTTP_AUTHORIZATION=f'Bearer {self.token_key}',
)
self.assertEqual(response.status_code, status.HTTP_201_CREATED, response.json())
extension.refresh_from_db()
self.assertEqual(extension.latest_version.release_notes, 'linux and windows')
self.assertEqual(extension.latest_version.files.count(), 2)
self.assertEqual(extension.latest_version.platforms.count(), 2)
with open(file_windows, 'rb') as version_file:
response = self.client.post(
self._get_upload_url('some_addon'),
{
'version_file': version_file,
'release_notes': 'linux and windows',
},
format='multipart',
HTTP_AUTHORIZATION=f'Bearer {self.token_key}',
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST, response.json())
self.assertEqual(
response.data['message']['version_file'],
[f'{extension.latest_version} already has files for windows-x64'],
)

View File

@ -26,7 +26,7 @@ class ApproveExtensionTest(TestCase):
# latest_version of approved extension must be listed # latest_version of approved extension must be listed
# check that we rollback latest_version when file is not approved # 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.status = File.STATUSES.AWAITING_REVIEW
file.save(update_fields={'status'}) file.save(update_fields={'status'})
new_version.refresh_from_db() new_version.refresh_from_db()
@ -36,8 +36,9 @@ class ApproveExtensionTest(TestCase):
self.assertTrue(extension.is_listed) self.assertTrue(extension.is_listed)
# break the first_version, check that nothing is listed anymore # break the first_version, check that nothing is listed anymore
first_version.file.status = File.STATUSES.AWAITING_REVIEW file = first_version.files.first()
first_version.file.save() file.status = File.STATUSES.AWAITING_REVIEW
file.save()
self.assertFalse(first_version.is_listed) self.assertFalse(first_version.is_listed)
extension.refresh_from_db() extension.refresh_from_db()
self.assertFalse(extension.is_listed) self.assertFalse(extension.is_listed)

View File

@ -43,7 +43,7 @@ class DeleteTest(TestCase):
source='images/b0/b03fa981527593fbe15b28cf37c020220c3d83021999eab036b87f3bca9c9168.png', source='images/b0/b03fa981527593fbe15b28cf37c020220c3d83021999eab036b87f3bca9c9168.png',
) )
) )
version_file = version.file version_file = version.files.first()
icon = extension.icon icon = extension.icon
featured_image = extension.featured_image featured_image = extension.featured_image
self.assertEqual(version_file.get_status_display(), 'Awaiting Review') self.assertEqual(version_file.get_status_display(), 'Awaiting Review')
@ -216,3 +216,14 @@ class DeleteTest(TestCase):
self.assertEqual(response.status_code, 403) self.assertEqual(response.status_code, 403)
extension.refresh_from_db() 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()

View File

@ -186,7 +186,7 @@ class ValidateManifestTest(CreateFileTest):
self.assertEqual(Extension.objects.count(), 1) self.assertEqual(Extension.objects.count(), 1)
# The same author is to send a new version to thte same extension # 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 = { non_kitsu_1_6 = {
"name": "Blender Kitsu with a different ID", "name": "Blender Kitsu with a different ID",
@ -213,7 +213,7 @@ class ValidateManifestTest(CreateFileTest):
self.assertEqual(Extension.objects.count(), 1) self.assertEqual(Extension.objects.count(), 1)
# The same author is to send a new version to thte same extension # 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 = { kitsu_version_clash = {
"name": "Change the name for lols", "name": "Change the name for lols",
@ -254,7 +254,7 @@ class ValidateManifestTest(CreateFileTest):
) )
# The same author is to send a new version to thte same extension # 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 = { updated_kitsu = {
"name": version.extension.name, "name": version.extension.name,
@ -692,7 +692,7 @@ class VersionPermissionsTest(CreateFileTest):
self.assertEqual(version_original, extension.latest_version) 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 # 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 = { new_kitsu = {
"id": "blender_kitsu", "id": "blender_kitsu",

View File

@ -5,7 +5,9 @@ from django.test import TestCase
from common.admin import get_admin_change_path from common.admin import get_admin_change_path
from common.log_entries import entries_for from common.log_entries import entries_for
from common.tests.factories.extensions import create_version from common.tests.factories.extensions import create_version
from common.tests.factories.files import FileFactory
from common.tests.factories.users import UserFactory from common.tests.factories.users import UserFactory
from extensions.models import Platform
class ExtensionTest(TestCase): class ExtensionTest(TestCase):
@ -70,19 +72,16 @@ class VersionTest(TestCase):
maxDiff = None maxDiff = None
fixtures = ['dev', 'licenses'] fixtures = ['dev', 'licenses']
def setUp(self): def test_admin_change_view(self):
super().setUp() version = create_version(
self.version = create_version(
metadata__blender_version_min='2.83.1', metadata__blender_version_min='2.83.1',
metadata__name='Extension name', metadata__name='Extension name',
metadata__support='https://example.com/', metadata__support='https://example.com/',
metadata__version='1.1.2', metadata__version='1.1.2',
metadata__website='https://example.com/', metadata__website='https://example.com/',
) )
self.assertEqual(entries_for(self.version).count(), 0) self.assertEqual(entries_for(version).count(), 0)
path = get_admin_change_path(obj=version)
def test_admin_change_view(self):
path = get_admin_change_path(obj=self.version)
self.assertEqual(path, '/admin/extensions/version/1/change/') self.assertEqual(path, '/admin/extensions/version/1/change/')
admin_user = UserFactory(is_staff=True, is_superuser=True) admin_user = UserFactory(is_staff=True, is_superuser=True)
@ -91,6 +90,77 @@ class VersionTest(TestCase):
self.assertEqual(response.status_code, 200, path) self.assertEqual(response.status_code, 200, path)
def test_get_remaining_platforms(self):
version_platforms_none = create_version(metadata__platforms=None)
self.assertEqual(version_platforms_none.get_remaining_platforms(), set())
version_platforms_empty = create_version(metadata__platforms=[])
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.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.get_remaining_platforms())
version_platforms_all = create_version(
metadata__platforms=[p.slug for p in Platform.objects.all()]
)
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'])
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'),
)
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)
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'])
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): class UpdateMetadataTest(TestCase):
fixtures = ['dev', 'licenses'] fixtures = ['dev', 'licenses']

View File

@ -80,7 +80,7 @@ EXPECTED_EXTENSION_DATA = {
'version_str': '0.1.0', 'version_str': '0.1.0',
'slug': 'some-addon', 'slug': 'some-addon',
}, },
'addon-with-split-platforms.zip': { 'addon-with-split-platforms-linux.zip': {
'metadata': { 'metadata': {
'tagline': 'Some add-on tag line', 'tagline': 'Some add-on tag line',
'name': 'Some Add-on', 'name': 'Some Add-on',
@ -118,12 +118,14 @@ EXPECTED_VALIDATION_ERRORS = {
'source': ['The manifest file is missing.'], 'source': ['The manifest file is missing.'],
}, },
'invalid-manifest-toml.zip': {'source': ['Manifest file contains invalid code.']}, 'invalid-manifest-toml.zip': {'source': ['Manifest file contains invalid code.']},
'invalid-theme-multiple-xmls.zip': {'source': ['Themes can only contain <strong>one XML file</strong>.']}, 'invalid-theme-multiple-xmls.zip': {
'invalid-theme-multiple-xmls-macosx.zip': {'source': ['Archive contains forbidden files or directories: __MACOSX/']}, 'source': ['Themes can only contain <strong>one XML file</strong>.']
},
'invalid-theme-multiple-xmls-macosx.zip': {
'source': ['Archive contains forbidden files or directories: __MACOSX/']
},
'invalid-missing-wheels.zip': { 'invalid-missing-wheels.zip': {
'source': [ 'source': ['Python wheel missing: addon/wheels/test-wheel-whatever.whl']
'Python wheel missing: addon/wheels/test-wheel-whatever.whl'
]
}, },
} }
POST_DATA = { POST_DATA = {
@ -435,11 +437,11 @@ class SubmitFinaliseTest(CheckFilePropertiesMixin, TestCase):
self.assertEqual(version.release_notes, data['release_notes']) self.assertEqual(version.release_notes, data['release_notes'])
# Check version file properties # Check version file properties
self._test_file_properties( self._test_file_properties(
version.file, version.files.first(),
content_type='application/zip', content_type='application/zip',
get_status_display='Awaiting Review', get_status_display='Awaiting Review',
get_type_display='Add-on', get_type_display='Add-on',
hash=version.file.original_hash, hash=version.files.first().original_hash,
original_hash='sha256:2831385', original_hash='sha256:2831385',
) )
# We cannot check for the ManyToMany yet (tags, licences, permissions) # We cannot check for the ManyToMany yet (tags, licences, permissions)
@ -515,7 +517,7 @@ class NewVersionTest(TestCase):
self.assertTrue(response['Location'].startswith('/oauth/login')) self.assertTrue(response['Location'].startswith('/oauth/login'))
def test_validation_errors_agreed_with_terms_required(self): 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: with open(TEST_FILES_DIR / 'amaranth-1.0.8.zip', 'rb') as fp:
response = self.client.post(self.url, {'source': fp}) response = self.client.post(self.url, {'source': fp})
@ -527,7 +529,7 @@ class NewVersionTest(TestCase):
) )
def test_validation_errors_invalid_extension(self): 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: with open(TEST_FILES_DIR / 'empty.txt', 'rb') as fp:
response = self.client.post(self.url, {'source': fp, 'agreed_with_terms': True}) 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): 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: with open(TEST_FILES_DIR / 'empty.zip', 'rb') as fp:
response = self.client.post(self.url, {'source': fp, 'agreed_with_terms': True}) 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): 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() self.extension.approve()
# Check step 1: upload a new file # 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.version, '1.0.8')
self.assertEqual(new_version.blender_version_min, '4.2.0') self.assertEqual(new_version.blender_version_min, '4.2.0')
self.assertEqual(new_version.schema_version, '1.0.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(new_version.release_notes, '')
self.assertEqual( self.assertEqual(
ApprovalActivity.objects.filter( ApprovalActivity.objects.filter(
@ -595,6 +597,60 @@ class NewVersionTest(TestCase):
self.assertEqual(new_version.release_notes, 'new version') 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'
file_no_platforms = TEST_FILES_DIR / 'addon-without-platforms-for-split-test.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.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(), 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)
# can't upload a platform-agnostic file
with open(file_no_platforms, '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): class DraftsWarningTest(TestCase):
fixtures = ['licenses'] fixtures = ['licenses']

View File

@ -216,7 +216,7 @@ class ExtensionManageViewTest(_BaseTestCase):
class UpdateVersionViewTest(_BaseTestCase): class UpdateVersionViewTest(_BaseTestCase):
def test_update_view_access(self): def test_update_view_access(self):
extension = _create_extension() extension = _create_extension()
extension_owner = extension.latest_version.file.user extension_owner = extension.latest_version.files.first().user
extension.authors.add(extension_owner) extension.authors.add(extension_owner)
random_user = UserFactory() random_user = UserFactory()
@ -246,7 +246,7 @@ class UpdateVersionViewTest(_BaseTestCase):
def test_blender_max_version(self): def test_blender_max_version(self):
extension = _create_extension() extension = _create_extension()
extension_owner = extension.latest_version.file.user extension_owner = extension.latest_version.files.first().user
extension.authors.add(extension_owner) extension.authors.add(extension_owner)
self.client.force_login(extension_owner) self.client.force_login(extension_owner)
url = reverse( url = reverse(

View File

@ -17,7 +17,7 @@ urlpatterns = [
# API # API
path('api/v1/extensions/', api.ExtensionsAPIView.as_view(), name='api'), path('api/v1/extensions/', api.ExtensionsAPIView.as_view(), name='api'),
path( path(
'api/v1/extensions/<str:extension_id>/versions/new/', 'api/v1/extensions/<str:extension_id>/versions/upload/',
api.UploadExtensionVersionView.as_view(), api.UploadExtensionVersionView.as_view(),
name='upload-extension-version', name='upload-extension-version',
), ),
@ -28,6 +28,7 @@ urlpatterns = [
path('search/', public.SearchView.as_view(), name='search'), path('search/', public.SearchView.as_view(), name='search'),
path('tag/<slug:tag_slug>/', public.SearchView.as_view(), name='by-tag'), path('tag/<slug:tag_slug>/', public.SearchView.as_view(), name='by-tag'),
path('team/<slug:team_slug>/', public.SearchView.as_view(), name='by-team'), path('team/<slug:team_slug>/', public.SearchView.as_view(), name='by-team'),
path('download/<filehash>/<filename>', public.file_download, name='download'),
re_path( re_path(
rf'^(?P<type_slug>{EXTENSION_SLUGS_PATH})/', rf'^(?P<type_slug>{EXTENSION_SLUGS_PATH})/',
include( include(

View File

@ -12,7 +12,6 @@ from django.db import transaction
from common.compare import is_in_version_range, version from common.compare import is_in_version_range, version
from extensions.models import Extension, Platform from extensions.models import Extension, Platform
from extensions.utils import clean_json_dictionary_from_optional_fields from extensions.utils import clean_json_dictionary_from_optional_fields
from extensions.views.manage import NewVersionView
from files.forms import FileFormSkipAgreed from files.forms import FileFormSkipAgreed
@ -55,17 +54,16 @@ class ListedExtensionsSerializer(serializers.ModelSerializer):
except Platform.DoesNotExist: except Platform.DoesNotExist:
self.platform = self.UNKNOWN_PLATFORM self.platform = self.UNKNOWN_PLATFORM
def to_representation(self, instance): def find_matching_files_and_version(self, instance):
matching_version = None
# avoid triggering additional db queries, reuse the prefetched queryset # avoid triggering additional db queries, reuse the prefetched queryset
versions = [ versions = sorted(
v [v for v in instance.versions.all() if v.is_listed],
for v in instance.versions.all() key=lambda v: v.date_created,
if v.file and v.file.status in instance.valid_file_statuses reverse=True,
] )
if not versions: if not versions:
return None return ([], None)
versions = sorted(versions, key=lambda v: v.date_created, reverse=True)
for v in versions: for v in versions:
if self.blender_version and not is_in_version_range( if self.blender_version and not is_in_version_range(
self.blender_version, self.blender_version,
@ -73,42 +71,48 @@ class ListedExtensionsSerializer(serializers.ModelSerializer):
v.blender_version_max, v.blender_version_max,
): ):
continue continue
platform_slugs = set(p.slug for p in v.platforms.all()) if self.platform:
# empty platforms field matches any platform filter if file := v.get_file_for_platform(self.platform):
# UNKNOWN_PLATFORM matches only empty platforms field return ([file], v)
if self.platform and (platform_slugs and self.platform not in platform_slugs): else:
continue return (v.files.all(), v)
matching_version = v
break
Oleg-Komarov marked this conversation as resolved Outdated

this looks shared with the logic in public view that selects which file to pick from storage, and should probable become a utility or method somewhere.

this looks shared with the logic in `public` view that selects which file to pick from storage, and should probable become a utility or method somewhere.
if not matching_version: return ([], None)
Oleg-Komarov marked this conversation as resolved Outdated

needs another break here

needs another `break` here
return None
def to_representation(self, instance):
matching_files, matching_version = self.find_matching_files_and_version(instance)
result = []
for file in matching_files:
data = { data = {
'id': instance.extension_id, 'id': instance.extension_id,
'schema_version': matching_version.schema_version, 'schema_version': matching_version.schema_version,
'name': instance.name, 'name': instance.name,
'version': matching_version.version, 'version': matching_version.version,
'tagline': matching_version.tagline, 'tagline': matching_version.tagline,
'archive_hash': matching_version.file.original_hash, 'archive_hash': file.original_hash,
'archive_size': matching_version.file.size_bytes, 'archive_size': file.size_bytes,
'archive_url': self.request.build_absolute_uri( 'archive_url': self.request.build_absolute_uri(
matching_version.download_url(append_repository_and_compatibility=False) matching_version.get_download_url(
file,
append_repository_and_compatibility=False,
)
), ),
'type': EXTENSION_TYPE_SLUGS_SINGULAR.get(instance.type), 'type': EXTENSION_TYPE_SLUGS_SINGULAR.get(instance.type),
'blender_version_min': matching_version.blender_version_min, 'blender_version_min': matching_version.blender_version_min,
'blender_version_max': matching_version.blender_version_max, 'blender_version_max': matching_version.blender_version_max,
'website': self.request.build_absolute_uri(instance.get_absolute_url()), 'website': self.request.build_absolute_uri(instance.get_absolute_url()),
# avoid triggering additional db queries, reuse the prefetched queryset # avoid triggering additional db queries, reuse the prefetched queryset
'maintainer': instance.team and instance.team.name or str(instance.authors.all()[0]), '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()], 'license': [license_iter.slug for license_iter in matching_version.licenses.all()],
'permissions': matching_version.file.metadata.get('permissions'), 'permissions': file.metadata.get('permissions'),
'platforms': [platform.slug for platform in matching_version.platforms.all()], 'platforms': file.get_platforms(),
# TODO: handle copyright # TODO: handle copyright
'tags': [str(tag) for tag in matching_version.tags.all()], 'tags': [str(tag) for tag in matching_version.tags.all()],
} }
result.append(clean_json_dictionary_from_optional_fields(data))
return clean_json_dictionary_from_optional_fields(data) return result
class ExtensionsAPIView(APIView): class ExtensionsAPIView(APIView):
@ -153,7 +157,10 @@ class ExtensionsAPIView(APIView):
request=request, request=request,
many=True, 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( return Response(
{ {
'blocklist': Extension.objects.blocklisted.values_list('extension_id', flat=True), 'blocklist': Extension.objects.blocklisted.values_list('extension_id', flat=True),
@ -201,14 +208,16 @@ class UploadExtensionVersionView(APIView):
status=status.HTTP_403_FORBIDDEN, status=status.HTTP_403_FORBIDDEN,
) )
# Create a NewVersionView instance to handle file creation form = FileFormSkipAgreed(
new_version_view = NewVersionView(request=request, extension=extension) data={},
extension=extension,
# Pass the version_file to the form request=request,
form = new_version_view.get_form(FileFormSkipAgreed) )
form.fields['source'].initial = version_file form.fields['source'].initial = version_file
if not form.is_valid(): if not form.is_valid():
if 'source' in form.errors:
form.errors['version_file'] = form.errors.pop('source')
return Response({'message': form.errors}, status=status.HTTP_400_BAD_REQUEST) return Response({'message': form.errors}, status=status.HTTP_400_BAD_REQUEST)
with transaction.atomic(): with transaction.atomic():
@ -217,6 +226,12 @@ class UploadExtensionVersionView(APIView):
file_instance.user = user file_instance.user = user
file_instance.save() file_instance.save()
manifest_version = file_instance.metadata['version']
if version := extension.versions.filter(version=manifest_version).first():
version.add_file(file_instance)
version.release_notes = release_notes
version.save(update_fields={'release_notes'})
else:
version = extension.create_version_from_file( version = extension.create_version_from_file(
file=file_instance, file=file_instance,
release_notes=release_notes, release_notes=release_notes,

View File

@ -30,17 +30,9 @@ class VersionsView(ExtensionQuerysetMixin, ListView):
paginate_by = 15 paginate_by = 15
def get_queryset(self): 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_queryset = self.get_extension_queryset()
self.extension = get_object_or_404(self.extension_queryset, slug=self.kwargs['slug']) self.extension = get_object_or_404(self.extension_queryset, slug=self.kwargs['slug'])
queryset = self.extension.versions return self.extension.versions.prefetch_related('files', 'platforms', 'permissions')
if self.request.user.is_staff or self.extension.has_maintainer(self.request.user):
return queryset.all()
return queryset.listed
def get_context_data(self, **kwargs): def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs) context = super().get_context_data(**kwargs)
@ -222,7 +214,11 @@ class NewVersionView(
@transaction.atomic @transaction.atomic
def form_valid(self, form): def form_valid(self, form):
response = super().form_valid(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 return response
def get_success_url(self): def get_success_url(self):

View File

@ -8,16 +8,15 @@ from django.shortcuts import get_object_or_404, redirect
from django.utils.translation import gettext_lazy as _ from django.utils.translation import gettext_lazy as _
from django.views.generic import DetailView, ListView from django.views.generic import DetailView, ListView
from extensions.models import Extension, Version, Tag
from constants.base import ( from constants.base import (
EXTENSION_TYPE_SLUGS, EXTENSION_TYPE_SLUGS,
EXTENSION_TYPE_PLURAL, EXTENSION_TYPE_PLURAL,
EXTENSION_TYPE_CHOICES, 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 import ratings.models
from stats.models import ExtensionDownload, VersionDownload
import teams.models import teams.models
User = get_user_model() User = get_user_model()
@ -52,16 +51,32 @@ class HomeView(ListedExtensionsView):
def extension_version_download(request, type_slug, slug, version, filename): def extension_version_download(request, type_slug, slug, version, filename):
"""A backward-compatible url for downloads.
Assuming only a single file for a given version.
"""
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 file_download(request, filehash, filename):
"""Download an extension version and count downloads. """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`. 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 This is a convention Blender uses to initiate an extension installation on an HTML anchor
drag&drop. 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) ExtensionDownload.create_from_request(request, object_id=extension_version.extension_id)
VersionDownload.create_from_request(request, object_id=extension_version.pk) VersionDownload.create_from_request(request, object_id=extension_version.pk)
return redirect(extension_version.downloadable_signed_url + f'?filename={filename}') url = file.source.url
return redirect(f'{url}?filename={filename}')
class SearchView(ListedExtensionsView): class SearchView(ListedExtensionsView):
@ -227,6 +242,9 @@ class ExtensionDetailView(DetailView):
queryset = Extension.objects.listed.prefetch_related( queryset = Extension.objects.listed.prefetch_related(
'authors', 'authors',
'latest_version__files', 'latest_version__files',
'latest_version__files__validation',
'latest_version__permissions',
'latest_version__platforms',
'latest_version__tags', 'latest_version__tags',
'preview_set', 'preview_set',
'preview_set__file', 'preview_set__file',
@ -235,11 +253,6 @@ class ExtensionDetailView(DetailView):
'ratings__ratingreply__user', 'ratings__ratingreply__user',
'ratings__user', 'ratings__user',
'team', 'team',
'versions',
'versions__files',
'versions__files__validation',
'versions__permissions',
'versions__platforms',
).distinct() ).distinct()
context_object_name = 'extension' context_object_name = 'extension'
template_name = 'extensions/detail.html' template_name = 'extensions/detail.html'

View File

@ -10,6 +10,7 @@ from django.utils.translation import gettext_lazy as _
from .validators import ( from .validators import (
ExtensionIDManifestValidator, ExtensionIDManifestValidator,
ExtensionNameManifestValidator, ExtensionNameManifestValidator,
ExtensionVersionManifestValidator,
FileMIMETypeValidator, FileMIMETypeValidator,
ManifestValidator, ManifestValidator,
) )
@ -36,11 +37,15 @@ class FileForm(forms.ModelForm):
# TODO: surface TOML parsing errors? # TODO: surface TOML parsing errors?
'invalid_manifest_toml': _('Manifest file contains invalid code.'), 'invalid_manifest_toml': _('Manifest file contains invalid code.'),
'invalid_missing_init': mark_safe(_('Add-on file missing: <strong>__init__.py</strong>.')), 'invalid_missing_init': mark_safe(_('Add-on file missing: <strong>__init__.py</strong>.')),
'missing_or_multiple_theme_xml': mark_safe(_('Themes can only contain <strong>one XML file</strong>.')), 'missing_or_multiple_theme_xml': mark_safe(
_('Themes can only contain <strong>one XML file</strong>.')
),
'invalid_zip_archive': msg_only_zip_files, 'invalid_zip_archive': msg_only_zip_files,
'missing_manifest_toml': _('The manifest file is missing.'), 'missing_manifest_toml': _('The manifest file is missing.'),
'missing_wheel': _('Python wheel missing: %(path)s'), # TODO <strong>%(path)s</strong> 'missing_wheel': _('Python wheel missing: %(path)s'), # TODO <strong>%(path)s</strong>
'forbidden_filepaths': _('Archive contains forbidden files or directories: %(paths)s'), #TODO <strong>%(paths)s</strong> 'forbidden_filepaths': _(
'Archive contains forbidden files or directories: %(paths)s'
), # TODO <strong>%(paths)s</strong>
} }
class Meta: class Meta:
@ -69,8 +74,8 @@ class FileForm(forms.ModelForm):
) )
def __init__(self, *args, **kwargs): def __init__(self, *args, **kwargs):
self.request = kwargs.pop('request')
self.extension = kwargs.pop('extension', None) self.extension = kwargs.pop('extension', None)
self.request = kwargs.pop('request')
for field in self.base_fields: for field in self.base_fields:
if field not in {'source', 'agreed_with_terms'}: if field not in {'source', 'agreed_with_terms'}:
self.base_fields[field].required = False self.base_fields[field].required = False
@ -160,6 +165,10 @@ class FileForm(forms.ModelForm):
ManifestValidator(manifest) ManifestValidator(manifest)
ExtensionIDManifestValidator(manifest, self.extension) ExtensionIDManifestValidator(manifest, self.extension)
ExtensionNameManifestValidator(manifest, self.extension) ExtensionNameManifestValidator(manifest, self.extension)
ExtensionVersionManifestValidator(
manifest,
self.extension,
)
self.cleaned_data['metadata'] = manifest self.cleaned_data['metadata'] = manifest
self.cleaned_data['type'] = EXTENSION_SLUG_TYPES[manifest['type']] self.cleaned_data['type'] = EXTENSION_SLUG_TYPES[manifest['type']]

View File

@ -177,15 +177,20 @@ class File(CreatedModifiedMixin, TrackChangesMixin, models.Model):
'website': data.get('website'), 'website': data.get('website'),
} }
@property def get_platforms(self):
def parsed_version_fields(self) -> Dict[str, Any]: return self.parse_platforms_from_manifest(self.metadata)
"""Return Version-related data that was parsed from file's content."""
# Currently, the content of the manifest file is the only @classmethod
# kind of file metadata that is supported. def parse_platforms_from_manifest(self, data):
data = self.metadata
build_generated_platforms = None build_generated_platforms = None
if 'build' in data and 'generated' in data['build']: if 'build' in data and 'generated' in data['build']:
build_generated_platforms = data['build']['generated'].get('platforms') 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 { return {
'version': data.get('version'), 'version': data.get('version'),
'tagline': data.get('tagline'), 'tagline': data.get('tagline'),
@ -193,7 +198,7 @@ class File(CreatedModifiedMixin, TrackChangesMixin, models.Model):
'schema_version': data.get('schema_version'), 'schema_version': data.get('schema_version'),
'licenses': data.get('license'), 'licenses': data.get('license'),
'permissions': data.get('permissions'), 'permissions': data.get('permissions'),
'platforms': build_generated_platforms or data.get('platforms'), 'platforms': self.get_platforms(),
Oleg-Komarov marked this conversation as resolved Outdated

if keeping it a method is preferable, then get_platforms() might be cleaner,
otherwise making it a platforms property would make sense.

if keeping it a method is preferable, then `get_platforms()` might be cleaner, otherwise making it a `platforms` property would make sense.
'tags': data.get('tags'), 'tags': data.get('tags'),
} }

View File

@ -16,6 +16,7 @@ from extensions.models import (
) )
from constants.base import EXTENSION_TYPE_SLUGS_SINGULAR, EXTENSION_TYPE_CHOICES from constants.base import EXTENSION_TYPE_SLUGS_SINGULAR, EXTENSION_TYPE_CHOICES
from files.utils import guess_mimetype_from_ext, guess_mimetype_from_content from files.utils import guess_mimetype_from_ext, guess_mimetype_from_content
import files.models
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@ -164,6 +165,52 @@ class ExtensionNameManifestValidator:
) )
class ExtensionVersionManifestValidator:
"""Validates version existence in db and available platforms."""
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:
return
manifest_version = manifest.get('version')
version = extension_to_be_updated.versions.filter(version=manifest_version).first()
if not version:
return
# for existing versions only submissions for remaining platfroms are valid
remaining_platforms = version.get_remaining_platforms()
if platforms := files.models.File.parse_platforms_from_manifest(manifest):
if diff := set(platforms) - remaining_platforms:
raise ValidationError(
{
'source': [f'{version} already has files for {", ".join(diff)}'],
},
code='invalid',
)
else:
if remaining_platforms:
raise ValidationError(
{
'source': [
f'File upload for {version} is allowed only for remaining platforms: '
f'{", ".join(remaining_platforms)}'
],
},
code='invalid',
)
else:
raise ValidationError(
{
'source': [
f'The version {escape(manifest_version)} was already uploaded for this '
f'extension ({extension_to_be_updated.name})'
],
},
code='invalid',
)
class ManifestFieldValidator: class ManifestFieldValidator:
@classmethod @classmethod
def validate(cls, *, name: str, value: object, manifest: dict) -> str: def validate(cls, *, name: str, value: object, manifest: dict) -> str:
@ -511,30 +558,6 @@ class SchemaVersionValidator(VersionValidator):
) )
class VersionVersionValidator(VersionValidator):
example = '1.0.0'
@classmethod
def validate(cls, *, name: str, value: str, manifest: dict) -> str:
"""Return error message if cannot validate, otherwise returns nothing"""
if err_message := super().validate(name=name, value=value, manifest=manifest):
return err_message
extension = Extension.objects.filter(extension_id=manifest.get("id")).first()
# If the extension wasn't created yet, any version is valid
if not extension:
return
version = extension.versions.filter(version=value).first()
if version:
return mark_safe(
f'The version {value} was already uploaded for this extension '
f'({extension.name})'
)
class VersionMinValidator(VersionValidator): class VersionMinValidator(VersionValidator):
example = '4.2.0' example = '4.2.0'
@ -639,7 +662,7 @@ class ManifestValidator:
'schema_version': SchemaVersionValidator, 'schema_version': SchemaVersionValidator,
'tagline': TaglineValidator, 'tagline': TaglineValidator,
'type': TypeValidator, 'type': TypeValidator,
'version': VersionVersionValidator, 'version': VersionValidator,
} }
optional_fields = { optional_fields = {
'blender_version_max': VersionMaxValidator, 'blender_version_max': VersionMaxValidator,

View File

@ -4,7 +4,7 @@
{% block page_title %}{{ extension.name }}{% endblock page_title %} {% block page_title %}{{ extension.name }}{% endblock page_title %}
{% block content %} {% block content %}
{% with latest=extension.latest_version author=extension.latest_version.file.user %} {% with latest=extension.latest_version %}
<div class="d-flex mt-4"> <div class="d-flex mt-4">
<h2> <h2>
{% with text_ratings_count=extension.text_ratings_count %} {% with text_ratings_count=extension.text_ratings_count %}

View File

@ -180,12 +180,16 @@
<strong>Try at your own risk.</strong> <strong>Try at your own risk.</strong>
</p> </p>
<div class="btn-col"> <div class="btn-col">
<a href="{{ extension.latest_version.download_url }}" download="{{ extension.latest_version.download_name }}" class="btn btn-danger btn-block"> {% with build_list=extension.latest_version.get_build_list %}
{% for build in build_list %}
<a href="{{ build.url }}" download="{{ build.name }}" class="btn btn-danger btn-block">
<i class="i-download"></i> <i class="i-download"></i>
<span> <span>
{% trans 'Download' %} v{{ extension.latest_version.version }} {% trans 'Download' %} v{{ extension.latest_version.version }} {{ build.platforms|join:", " }}
</span> </span>
</a> </a>
{% endfor %}
{% endwith %}
</div> </div>
</div> </div>
{% endif %} {% endif %}

View File

@ -14,7 +14,7 @@ class CommentsViewTest(TestCase):
self.default_version = version self.default_version = version
ApprovalActivity( ApprovalActivity(
type=ApprovalActivity.ActivityType.COMMENT, type=ApprovalActivity.ActivityType.COMMENT,
user=version.file.user, user=version.files.first().user,
extension=version.extension, extension=version.extension,
message='test comment', message='test comment',
).save() ).save()

View File

@ -77,9 +77,13 @@ class ExtensionsApprovalDetailView(DetailView):
def get_queryset(self): def get_queryset(self):
return self.model.objects.prefetch_related( return self.model.objects.prefetch_related(
'authors', 'authors',
'latest_version__files',
'latest_version__files__validation',
'latest_version__permissions',
'latest_version__platforms',
'latest_version__tags',
'preview_set', 'preview_set',
'preview_set__file', 'preview_set__file',
'versions',
).all() ).all()
def get_context_data(self, **kwargs): def get_context_data(self, **kwargs):

View File

@ -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.reviewers import ApprovalActivityFactory
from common.tests.factories.users import OAuthUserFactory, create_moderator, UserFactory from common.tests.factories.users import OAuthUserFactory, create_moderator, UserFactory
import extensions.models import extensions.models
import files.models
import users.tasks as tasks import users.tasks as tasks
User = get_user_model() User = get_user_model()
@ -83,11 +82,11 @@ class TestTasks(TestCase):
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
response = self.client.get(version.extension.get_versions_url()) response = self.client.get(version.extension.get_versions_url())
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
response = self.client.get(version.download_url()) response = self.client.get(version.get_download_url(version.files.first()))
self.assertEqual(response.status_code, 302) self.assertEqual(response.status_code, 302)
self.assertEqual( self.assertEqual(
response['Location'], 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', f'?filename=add-on-{version.extension.slug}-v{version.version}.zip',
) )
# Check that ratings remained publicly accessible # Check that ratings remained publicly accessible
@ -146,10 +145,9 @@ class TestTasks(TestCase):
user.refresh_from_db() user.refresh_from_db()
# Check that unlisted extension file and version were deleted # 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): with self.assertRaises(extensions.models.Version.DoesNotExist):
self.authored_unlisted_extension.latest_version.refresh_from_db() 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 # Check that reports made by this account remained accessible
self.report1.refresh_from_db() self.report1.refresh_from_db()