Don't treat permission name as a machine readable field, use slug instead #115
@ -202,8 +202,13 @@ class TagAdmin(admin.ModelAdmin):
|
|||||||
return ()
|
return ()
|
||||||
|
|
||||||
|
|
||||||
|
class VersionPermissionAdmin(admin.ModelAdmin):
|
||||||
|
list_display = ('name', 'slug')
|
||||||
|
|
||||||
|
|
||||||
admin.site.register(models.Extension, ExtensionAdmin)
|
admin.site.register(models.Extension, ExtensionAdmin)
|
||||||
admin.site.register(models.Version, VersionAdmin)
|
admin.site.register(models.Version, VersionAdmin)
|
||||||
admin.site.register(models.Maintainer, MaintainerAdmin)
|
admin.site.register(models.Maintainer, MaintainerAdmin)
|
||||||
admin.site.register(models.License, LicenseAdmin)
|
admin.site.register(models.License, LicenseAdmin)
|
||||||
admin.site.register(models.Tag, TagAdmin)
|
admin.site.register(models.Tag, TagAdmin)
|
||||||
|
admin.site.register(models.VersionPermission, VersionPermissionAdmin)
|
||||||
|
@ -0,0 +1,23 @@
|
|||||||
|
# Generated by Django 4.2.11 on 2024-05-06 12:10
|
||||||
|
|
||||||
|
from django.db import migrations, models
|
||||||
|
|
||||||
|
|
||||||
|
class Migration(migrations.Migration):
|
||||||
|
|
||||||
|
dependencies = [
|
||||||
|
('extensions', '0027_unique_preview_files'),
|
||||||
|
]
|
||||||
|
|
||||||
|
operations = [
|
||||||
|
migrations.AlterField(
|
||||||
|
model_name='license',
|
||||||
|
name='slug',
|
||||||
|
field=models.SlugField(help_text='Should be taken from https://spdx.org/licenses/', unique=True),
|
||||||
|
),
|
||||||
|
migrations.AlterField(
|
||||||
|
model_name='versionpermission',
|
||||||
|
name='slug',
|
||||||
|
field=models.SlugField(help_text='Permissions add-ons are expected to need.', unique=True),
|
||||||
|
),
|
||||||
|
]
|
@ -19,8 +19,6 @@ from constants.base import (
|
|||||||
EXTENSION_TYPE_SLUGS,
|
EXTENSION_TYPE_SLUGS,
|
||||||
FILE_STATUS_CHOICES,
|
FILE_STATUS_CHOICES,
|
||||||
)
|
)
|
||||||
from constants.licenses import ALL_LICENSES
|
|
||||||
from constants.version_permissions import ALL_VERSION_PERMISSIONS
|
|
||||||
import common.help_texts
|
import common.help_texts
|
||||||
import extensions.fields
|
import extensions.fields
|
||||||
|
|
||||||
@ -90,22 +88,13 @@ class License(CreatedModifiedMixin, models.Model):
|
|||||||
blank=False,
|
blank=False,
|
||||||
null=False,
|
null=False,
|
||||||
help_text='Should be taken from https://spdx.org/licenses/',
|
help_text='Should be taken from https://spdx.org/licenses/',
|
||||||
|
unique=True,
|
||||||
)
|
)
|
||||||
url = models.URLField(blank=False, null=False)
|
url = models.URLField(blank=False, null=False)
|
||||||
|
|
||||||
def __str__(self) -> str:
|
def __str__(self) -> str:
|
||||||
return f'{self.name}'
|
return f'{self.name}'
|
||||||
|
|
||||||
@classmethod
|
|
||||||
def generate(cls):
|
|
||||||
"""Generate License records from constants."""
|
|
||||||
licenses = [cls(id=li.id, name=li.name, slug=li.slug, url=li.url) for li in ALL_LICENSES]
|
|
||||||
cls.objects.bulk_create(licenses)
|
|
||||||
|
|
||||||
@classmethod
|
|
||||||
def get_by_name(cls, name: str):
|
|
||||||
return cls.objects.filter(name__startswith=name).first()
|
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def get_by_slug(cls, slug: str):
|
def get_by_slug(cls, slug: str):
|
||||||
return cls.objects.filter(slug__startswith=slug).first()
|
return cls.objects.filter(slug__startswith=slug).first()
|
||||||
@ -386,28 +375,16 @@ class VersionPermission(CreatedModifiedMixin, models.Model):
|
|||||||
blank=False,
|
blank=False,
|
||||||
null=False,
|
null=False,
|
||||||
help_text='Permissions add-ons are expected to need.',
|
help_text='Permissions add-ons are expected to need.',
|
||||||
|
unique=True,
|
||||||
)
|
)
|
||||||
help = models.CharField(max_length=128, null=False, blank=False, unique=True)
|
help = models.CharField(max_length=128, null=False, blank=False, unique=True)
|
||||||
|
|
||||||
def __str__(self) -> str:
|
def __str__(self) -> str:
|
||||||
return f'{self.name}'
|
return f'{self.name}'
|
||||||
|
|
||||||
@classmethod
|
|
||||||
def generate(cls):
|
|
||||||
"""Generate Permission records from constants."""
|
|
||||||
permissions = [
|
|
||||||
cls(id=li.id, name=li.name, slug=li.slug, help=li.help)
|
|
||||||
for li in ALL_VERSION_PERMISSIONS
|
|
||||||
]
|
|
||||||
cls.objects.bulk_create(permissions)
|
|
||||||
|
|
||||||
@classmethod
|
|
||||||
def get_by_name(cls, name: str):
|
|
||||||
return cls.objects.filter(name__startswith=name).first()
|
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def get_by_slug(cls, slug: str):
|
def get_by_slug(cls, slug: str):
|
||||||
return cls.objects.filter(slug__startswith=slug).first()
|
return cls.objects.get(slug=slug)
|
||||||
Oleg-Komarov marked this conversation as resolved
Outdated
|
|||||||
|
|
||||||
|
|
||||||
class Tag(CreatedModifiedMixin, models.Model):
|
class Tag(CreatedModifiedMixin, models.Model):
|
||||||
@ -537,12 +514,7 @@ class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Model
|
|||||||
return
|
return
|
||||||
|
|
||||||
for permission_name in _permissions:
|
for permission_name in _permissions:
|
||||||
permission = VersionPermission.get_by_name(permission_name)
|
permission = VersionPermission.get_by_slug(permission_name)
|
||||||
|
|
||||||
# Just ignore versions that are incompatible.
|
|
||||||
if not permission:
|
|
||||||
Oleg-Komarov marked this conversation as resolved
Outdated
Oleg-Komarov
commented
@dfelinto here we explicitly allow to submit a manifest with incorrect permissions names. is it intentional? shouldn't we return a validation error instead if somebody mistypes a permission slug? @dfelinto here we explicitly allow to submit a manifest with incorrect permissions names. is it intentional? shouldn't we return a validation error instead if somebody mistypes a permission slug?
Dalai Felinto
commented
Right, it should be an error indeed. Maybe already is (caught early on by the validator)? Right, it should be an error indeed. Maybe already is (caught early on by the validator)?
|
|||||||
continue
|
|
||||||
|
|
||||||
self.permissions.add(permission)
|
self.permissions.add(permission)
|
||||||
|
|
||||||
def set_initial_licenses(self, _licenses):
|
def set_initial_licenses(self, _licenses):
|
||||||
|
@ -54,8 +54,7 @@ class FileMIMETypeValidator:
|
|||||||
|
|
||||||
|
|
||||||
class ExtensionIDManifestValidator:
|
class ExtensionIDManifestValidator:
|
||||||
"""
|
"""Make sure the extension id is valid:
|
||||||
Make sure the extension id is valid:
|
|
||||||
* Extension id consists of Unicode letters, numbers or underscores.
|
* Extension id consists of Unicode letters, numbers or underscores.
|
||||||
* Neither hyphens nor spaces are supported.
|
* Neither hyphens nor spaces are supported.
|
||||||
* Each extension id most be unique across all extensions.
|
* Each extension id most be unique across all extensions.
|
||||||
@ -307,8 +306,9 @@ class PermissionsValidator:
|
|||||||
is_error = True
|
is_error = True
|
||||||
else:
|
else:
|
||||||
for permission in value:
|
for permission in value:
|
||||||
if VersionPermission.get_by_slug(permission):
|
try:
|
||||||
continue
|
VersionPermission.get_by_slug(permission)
|
||||||
|
except VersionPermission.DoesNotExist:
|
||||||
is_error = True
|
is_error = True
|
||||||
logger.info(f'Permission unavailable: {permission}')
|
logger.info(f'Permission unavailable: {permission}')
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user
I'm replacing this with an exact match. Was there any usecase to do a prefix match?
I don't recall, if it wasn't explained on the corresponding commit than just assume an exact match should be fine. I think I copied this over from the license and other code snippets