Update extension fields from manifest metadata #111 #121
@ -19,6 +19,7 @@ class ExtensionFactory(DjangoModelFactory):
|
||||
|
||||
name = factory.Faker('catch_phrase')
|
||||
extension_id = factory.Faker('slug')
|
||||
slug = factory.Faker('slug')
|
||||
description = factory.LazyAttribute(
|
||||
lambda _: fake_markdown.post(size=random.choice(('medium', 'large')))
|
||||
)
|
||||
@ -68,7 +69,16 @@ class VersionFactory(DjangoModelFactory):
|
||||
download_count = factory.Faker('random_int')
|
||||
tagline = factory.Faker('bs')
|
||||
|
||||
file = factory.SubFactory('common.tests.factories.files.FileFactory')
|
||||
file = factory.SubFactory(
|
||||
'common.tests.factories.files.FileFactory',
|
||||
metadata=factory.Dict(
|
||||
{
|
||||
'name': factory.Faker('name'),
|
||||
'support': factory.Faker('url'),
|
||||
'website': factory.Faker('url'),
|
||||
}
|
||||
),
|
||||
)
|
||||
ratings = factory.RelatedFactoryList(
|
||||
RatingFactory, size=lambda: random.randint(1, 50), factory_related_name='version'
|
||||
)
|
||||
|
@ -25,3 +25,5 @@ class FileFactory(DjangoModelFactory):
|
||||
source = factory.Faker('file_name', extension='zip')
|
||||
|
||||
user = factory.SubFactory('common.tests.factories.users.UserFactory')
|
||||
|
||||
metadata = factory.Dict({})
|
||||
|
@ -201,8 +201,6 @@ class Extension(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Mod
|
||||
return utils.slugify(EXTENSION_STATUS_CHOICES[self.status - 1][1])
|
||||
|
||||
def clean(self) -> None:
|
||||
if not self.slug:
|
||||
self.slug = utils.slugify(self.name)
|
||||
# Require at least one approved version with a file for approved extensions
|
||||
if self.status == self.STATUSES.APPROVED:
|
||||
if not self.latest_version:
|
||||
@ -219,6 +217,24 @@ class Extension(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Mod
|
||||
self.clean()
|
||||
return super().save(*args, **kwargs)
|
||||
|
||||
def update_metadata_from_version(self, version):
|
||||
update_fields = set()
|
||||
metadata = version.file.metadata
|
||||
# check if we can also update name
|
||||
# if we are uploading a new version, we have just validated and don't expect a clash,
|
||||
# but if a version is being deleted, we want to rollback to a name from an older version,
|
||||
# which may by clashing now, and we can't do anything about it
|
||||
name = metadata.get('name')
|
||||
if not self.__class__.objects.filter(name=name).exclude(pk=self.pk).exists():
|
||||
update_fields.add('name')
|
||||
for field in ['support', 'website']:
|
||||
# if a field is missing from manifest, don't reset the corresponding extension field
|
||||
if metadata.get(field):
|
||||
update_fields.add(field)
|
||||
for field in update_fields:
|
||||
setattr(self, field, metadata.get(field))
|
||||
self.save(update_fields=update_fields)
|
||||
|
||||
@transaction.atomic
|
||||
def approve(self, reviewer=None):
|
||||
"""TODO: Approve an extension which is currently in review."""
|
||||
|
@ -4,7 +4,7 @@ import logging
|
||||
from actstream.actions import follow, unfollow
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.contrib.auth.models import Group
|
||||
from django.db.models.signals import m2m_changed, pre_save, post_save, pre_delete
|
||||
from django.db.models.signals import m2m_changed, pre_save, post_save, pre_delete, post_delete
|
||||
from django.dispatch import receiver
|
||||
|
||||
from constants.activity import Flag
|
||||
@ -195,3 +195,22 @@ def _create_approval_activity_for_new_version_if_listed(
|
||||
extension=instance.extension,
|
||||
message=f'uploaded new version: {instance.version}',
|
||||
).save()
|
||||
|
||||
|
||||
@receiver(post_delete, sender=extensions.models.Version)
|
||||
@receiver(post_save, sender=extensions.models.Version)
|
||||
def _update_extension_metadata_from_latest_version(
|
||||
sender: object,
|
||||
instance: extensions.models.Version,
|
||||
**kwargs: object,
|
||||
):
|
||||
# this code will also be triggered when an extension is deleted
|
||||
# and it deletes all related versions
|
||||
extension = instance.extension
|
||||
latest_version = extension.latest_version
|
||||
|
||||
# should check in case we are deleting the latest version, then no need to update anything
|
||||
if not latest_version:
|
||||
return
|
||||
|
||||
extension.update_metadata_from_version(latest_version)
|
||||
|
@ -1,6 +1,7 @@
|
||||
from django.test import TestCase
|
||||
from django.urls import reverse
|
||||
from django.core.exceptions import ValidationError
|
||||
import factory
|
||||
|
||||
from common.tests.factories.extensions import create_approved_version
|
||||
from common.tests.factories.files import FileFactory
|
||||
@ -63,6 +64,13 @@ class CreateFileTest(TestCase):
|
||||
file=FileFactory(
|
||||
type=File.TYPES.BPY,
|
||||
status=File.STATUSES.APPROVED,
|
||||
metadata=factory.Dict(
|
||||
{
|
||||
'name': factory.Faker('name'),
|
||||
'support': factory.Faker('url'),
|
||||
'website': factory.Faker('url'),
|
||||
}
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
@ -240,6 +248,13 @@ class ValidateManifestTest(CreateFileTest):
|
||||
file=FileFactory(
|
||||
type=File.TYPES.BPY,
|
||||
status=File.STATUSES.APPROVED,
|
||||
metadata=factory.Dict(
|
||||
{
|
||||
'name': factory.Faker('name'),
|
||||
'support': factory.Faker('url'),
|
||||
'website': factory.Faker('url'),
|
||||
}
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
|
@ -22,6 +22,11 @@ class ExtensionTest(TestCase):
|
||||
extension__name='Extension name',
|
||||
extension__status=Extension.STATUSES.INCOMPLETE,
|
||||
extension__support='https://example.com/',
|
||||
file__metadata={
|
||||
'name': 'Extension name',
|
||||
'support': 'https://example.com/',
|
||||
'website': 'https://example.com/',
|
||||
},
|
||||
).extension
|
||||
self.assertEqual(entries_for(self.extension).count(), 0)
|
||||
self.assertIsNone(self.extension.date_approved)
|
||||
@ -127,3 +132,76 @@ class VersionTest(TestCase):
|
||||
response = self.client.get(path)
|
||||
|
||||
self.assertEqual(response.status_code, 200, path)
|
||||
|
||||
|
||||
class UpdateMetadataTest(TestCase):
|
||||
fixtures = ['dev', 'licenses']
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
self.first_version = create_version(
|
||||
extension__description='Extension description',
|
||||
extension__name='name',
|
||||
extension__status=Extension.STATUSES.INCOMPLETE,
|
||||
extension__support='https://example.com/',
|
||||
extension__website='https://example.com/',
|
||||
file__metadata={
|
||||
'name': 'name',
|
||||
'support': 'https://example.com/',
|
||||
'website': 'https://example.com/',
|
||||
},
|
||||
)
|
||||
self.extension = self.first_version.extension
|
||||
|
||||
def test_version_create_and_delete(self):
|
||||
second_version = create_version(
|
||||
extension=self.extension,
|
||||
file__metadata={
|
||||
'name': 'new name',
|
||||
'support': 'https://example.com/new',
|
||||
'website': 'https://example.com/new',
|
||||
},
|
||||
)
|
||||
self.extension.refresh_from_db()
|
||||
self.assertEqual(self.extension.name, 'new name')
|
||||
self.assertEqual(self.extension.support, 'https://example.com/new')
|
||||
self.assertEqual(self.extension.website, 'https://example.com/new')
|
||||
|
||||
second_version.delete()
|
||||
self.extension.refresh_from_db()
|
||||
self.assertEqual(self.extension.name, 'name')
|
||||
self.assertEqual(self.extension.support, 'https://example.com/')
|
||||
self.assertEqual(self.extension.website, 'https://example.com/')
|
||||
|
||||
def test_old_name_taken(self):
|
||||
second_version = create_version(
|
||||
extension=self.extension,
|
||||
file__metadata={
|
||||
'name': 'new name',
|
||||
'support': 'https://example.com/new',
|
||||
'website': 'https://example.com/new',
|
||||
},
|
||||
)
|
||||
|
||||
# another extension uses old name
|
||||
create_version(
|
||||
extension__description='Extension description',
|
||||
extension__extension_id='lalalala',
|
||||
extension__name='name',
|
||||
extension__status=Extension.STATUSES.INCOMPLETE,
|
||||
extension__support='https://example.com/',
|
||||
extension__website='https://example.com/',
|
||||
file__metadata={
|
||||
'name': 'name',
|
||||
'support': 'https://example.com/',
|
||||
'website': 'https://example.com/',
|
||||
},
|
||||
)
|
||||
|
||||
second_version.delete()
|
||||
self.extension.refresh_from_db()
|
||||
# couldn't revert the name because it has been taken
|
||||
self.assertEqual(self.extension.name, 'new name')
|
||||
# reverted other fields
|
||||
self.assertEqual(self.extension.support, 'https://example.com/')
|
||||
self.assertEqual(self.extension.website, 'https://example.com/')
|
||||
|
@ -19,6 +19,11 @@ def _create_extension():
|
||||
extension__website='https://example.com/',
|
||||
extension__status=Extension.STATUSES.INCOMPLETE,
|
||||
extension__average_score=2.5,
|
||||
file__metadata={
|
||||
'name': 'Test Add-on',
|
||||
'support': 'https://example.com/issues/',
|
||||
'website': 'https://example.com/',
|
||||
},
|
||||
).extension
|
||||
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user