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

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
Conversation 16 Commits 43 Files Changed 33 +9 -9
4 changed files with 9 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 77dc2096de - Show all commits

8
extensions/models.py
View File

@ -690,7 +690,7 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model):
current_platforms = set([p.slug for p in self.platforms.all()]) current_platforms = set([p.slug for p in self.platforms.all()])
files_platforms = set() files_platforms = set()
for file in self.files.all(): for file in self.files.all():
if file_platforms := file.platforms(): if file_platforms := file.get_platforms():
files_platforms.update(file_platforms) files_platforms.update(file_platforms)
else: else:
# we have a cross-platform file - version must not specify platforms # 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): def get_remaining_platforms(self):
Oleg-Komarov marked this conversation as resolved Outdated
Anna Sirota commented 2024-07-05 14:01:47 +02:00
Outdated
Review
Copy Link

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
all_platforms = set(p.slug for p in Platform.objects.all()) all_platforms = set(p.slug for p in Platform.objects.all())
for file in self.files.all(): for file in self.files.all():
platforms = file.platforms() platforms = file.get_platforms()
if not platforms: if not platforms:
# no platforms means any platform # no platforms means any platform
return set() return set()
@ -811,7 +811,7 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model):
] ]
platform2file = {} platform2file = {}
for file in files: for file in files:
platforms = file.platforms() platforms = file.get_platforms()
if not platforms: if not platforms:
log.warning( log.warning(
f'data error: Version pk={self.pk} has multiple files, but File pk={file.pk} ' 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]: def get_build_list(self) -> List[dict]:
build_list = [] build_list = []
for file in self.files.all(): for file in self.files.all():
platforms = file.platforms() or [] platforms = file.get_platforms() or []
platform = len(platforms) and platforms[0] or None platform = len(platforms) and platforms[0] or None
# if file has multiple platforms, picking the first one should still produce a correct # if file has multiple platforms, picking the first one should still produce a correct
# download_url # download_url

4
extensions/views/api.py
View File

@ -74,7 +74,7 @@ class ListedExtensionsSerializer(serializers.ModelSerializer):
for file in v.files.all(): for file in v.files.all():
# empty platforms field matches any platform filter # empty platforms field matches any platform filter
# UNKNOWN_PLATFORM matches only empty platforms field # 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): if self.platform and (platforms and self.platform not in platforms):
continue continue
Oleg-Komarov marked this conversation as resolved Outdated
Anna Sirota commented 2024-07-05 14:42:10 +02:00
Outdated
Review
Copy Link

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.
return (file, v) return (file, v)
Oleg-Komarov marked this conversation as resolved Outdated
Anna Sirota commented 2024-07-05 14:23:18 +02:00
Outdated
Review
Copy Link

needs another break here

needs another `break` here
@ -108,7 +108,7 @@ class ListedExtensionsSerializer(serializers.ModelSerializer):
'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_file.metadata.get('permissions'), '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()], 'platforms': [platform.slug for platform in matching_version.platforms.all()],
# 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()],

2
extensions/views/public.py
View File

@ -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) 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)
for file in extension_version.files.all(): for file in extension_version.files.all():
platforms = file.platforms() or [] platforms = file.get_platforms() or []
if platform is None or platform in platforms: if platform is None or platform in platforms:
url = file.source.url url = file.source.url
return redirect(f'{url}?filename={filename}') return redirect(f'{url}?filename={filename}')

4
files/models.py
View File

@ -177,7 +177,7 @@ class File(CreatedModifiedMixin, TrackChangesMixin, models.Model):
'website': data.get('website'), 'website': data.get('website'),
} }
def platforms(self): def get_platforms(self):
return self.parse_platforms_from_manifest(self.metadata) return self.parse_platforms_from_manifest(self.metadata)
@classmethod @classmethod
@ -198,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': self.platforms(), 'platforms': self.get_platforms(),
Oleg-Komarov marked this conversation as resolved Outdated
Anna Sirota commented 2024-07-05 14:29:52 +02:00
Outdated
Review
Copy Link

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'),
} }