Update extension fields from manifest metadata #111 #121
@ -19,6 +19,7 @@ class ExtensionFactory(DjangoModelFactory):
|
|||||||
|
|
||||||
name = factory.Faker('catch_phrase')
|
name = factory.Faker('catch_phrase')
|
||||||
extension_id = factory.Faker('slug')
|
extension_id = factory.Faker('slug')
|
||||||
|
slug = factory.Faker('slug')
|
||||||
description = factory.LazyAttribute(
|
description = factory.LazyAttribute(
|
||||||
lambda _: fake_markdown.post(size=random.choice(('medium', 'large')))
|
lambda _: fake_markdown.post(size=random.choice(('medium', 'large')))
|
||||||
)
|
)
|
||||||
@ -68,7 +69,16 @@ class VersionFactory(DjangoModelFactory):
|
|||||||
download_count = factory.Faker('random_int')
|
download_count = factory.Faker('random_int')
|
||||||
tagline = factory.Faker('bs')
|
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(
|
ratings = factory.RelatedFactoryList(
|
||||||
RatingFactory, size=lambda: random.randint(1, 50), factory_related_name='version'
|
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')
|
source = factory.Faker('file_name', extension='zip')
|
||||||
|
|
||||||
user = factory.SubFactory('common.tests.factories.users.UserFactory')
|
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])
|
return utils.slugify(EXTENSION_STATUS_CHOICES[self.status - 1][1])
|
||||||
|
|
||||||
def clean(self) -> None:
|
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
|
# Require at least one approved version with a file for approved extensions
|
||||||
if self.status == self.STATUSES.APPROVED:
|
if self.status == self.STATUSES.APPROVED:
|
||||||
if not self.latest_version:
|
if not self.latest_version:
|
||||||
@ -219,6 +217,24 @@ class Extension(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Mod
|
|||||||
self.clean()
|
self.clean()
|
||||||
return super().save(*args, **kwargs)
|
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
|
@transaction.atomic
|
||||||
def approve(self, reviewer=None):
|
def approve(self, reviewer=None):
|
||||||
"""TODO: Approve an extension which is currently in review."""
|
"""TODO: Approve an extension which is currently in review."""
|
||||||
|
@ -4,7 +4,7 @@ import logging
|
|||||||
from actstream.actions import follow, unfollow
|
from actstream.actions import follow, unfollow
|
||||||
from django.contrib.auth import get_user_model
|
from django.contrib.auth import get_user_model
|
||||||
from django.contrib.auth.models import Group
|
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 django.dispatch import receiver
|
||||||
|
|
||||||
from constants.activity import Flag
|
from constants.activity import Flag
|
||||||
@ -195,3 +195,22 @@ def _create_approval_activity_for_new_version_if_listed(
|
|||||||
extension=instance.extension,
|
extension=instance.extension,
|
||||||
message=f'uploaded new version: {instance.version}',
|
message=f'uploaded new version: {instance.version}',
|
||||||
).save()
|
).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.test import TestCase
|
||||||
from django.urls import reverse
|
from django.urls import reverse
|
||||||
from django.core.exceptions import ValidationError
|
from django.core.exceptions import ValidationError
|
||||||
|
import factory
|
||||||
|
|
||||||
from common.tests.factories.extensions import create_approved_version
|
from common.tests.factories.extensions import create_approved_version
|
||||||
from common.tests.factories.files import FileFactory
|
from common.tests.factories.files import FileFactory
|
||||||
@ -63,6 +64,13 @@ class CreateFileTest(TestCase):
|
|||||||
file=FileFactory(
|
file=FileFactory(
|
||||||
type=File.TYPES.BPY,
|
type=File.TYPES.BPY,
|
||||||
status=File.STATUSES.APPROVED,
|
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(
|
file=FileFactory(
|
||||||
type=File.TYPES.BPY,
|
type=File.TYPES.BPY,
|
||||||
status=File.STATUSES.APPROVED,
|
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__name='Extension name',
|
||||||
extension__status=Extension.STATUSES.INCOMPLETE,
|
extension__status=Extension.STATUSES.INCOMPLETE,
|
||||||
extension__support='https://example.com/',
|
extension__support='https://example.com/',
|
||||||
|
file__metadata={
|
||||||
|
'name': 'Extension name',
|
||||||
|
'support': 'https://example.com/',
|
||||||
|
'website': 'https://example.com/',
|
||||||
|
},
|
||||||
).extension
|
).extension
|
||||||
self.assertEqual(entries_for(self.extension).count(), 0)
|
self.assertEqual(entries_for(self.extension).count(), 0)
|
||||||
self.assertIsNone(self.extension.date_approved)
|
self.assertIsNone(self.extension.date_approved)
|
||||||
@ -127,3 +132,76 @@ class VersionTest(TestCase):
|
|||||||
response = self.client.get(path)
|
response = self.client.get(path)
|
||||||
|
|
||||||
self.assertEqual(response.status_code, 200, 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__website='https://example.com/',
|
||||||
extension__status=Extension.STATUSES.INCOMPLETE,
|
extension__status=Extension.STATUSES.INCOMPLETE,
|
||||||
extension__average_score=2.5,
|
extension__average_score=2.5,
|
||||||
|
file__metadata={
|
||||||
|
'name': 'Test Add-on',
|
||||||
|
'support': 'https://example.com/issues/',
|
||||||
|
'website': 'https://example.com/',
|
||||||
|
},
|
||||||
).extension
|
).extension
|
||||||
|
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user