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

Make it possible to fully delete unlisted/unrated extensions #81

Merged
Anna Sirota merged 24 commits from fully-delete-extension into main 2024-04-19 11:00:19 +02:00
Conversation 0 Commits 24 Files Changed 32 +400 -266
32 changed files with 400 additions and 266 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
abuse/migrations/0006_remove_abusereport_date_deleted.py Normal file
View File

@ -0,0 +1,17 @@
# Generated by Django 4.2.11 on 2024-04-18 08:59
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
('abuse', '0005_alter_abusereport_type'),
]
operations = [
migrations.RemoveField(
model_name='abusereport',
name='date_deleted',
),
]

4
abuse/models.py
View File

@ -9,13 +9,13 @@ from extended_choices import Choices
from geoip2.errors import GeoIP2Error
from constants.base import ABUSE_TYPE, ABUSE_TYPE_EXTENSION, ABUSE_TYPE_RATING
from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin
from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin
import extensions.fields
User = get_user_model()
class AbuseReport(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Model):
class AbuseReport(CreatedModifiedMixin, TrackChangesMixin, models.Model):
TYPE = ABUSE_TYPE
REASONS = Choices(

42
common/model_mixins.py
View File

@ -3,7 +3,7 @@ import copy
import logging
from django.contrib.contenttypes.models import ContentType
from django.db import models, transaction
from django.db import models
from django.shortcuts import reverse
from django.utils import timezone
@ -152,43 +152,3 @@ class TrackChangesMixin(models.Model):
self.name = markdown.sanitize(self.name)
if update_fields is not None:
kwargs['update_fields'] = kwargs['update_fields'].union({'name'})
class SoftDeleteMixin(models.Model):
"""Model with soft-deletion functionality."""
class Meta:
abstract = True
date_deleted = models.DateTimeField(null=True, blank=True, editable=False)
@property
def is_deleted(self) -> bool:
return self.date_deleted is not None
@transaction.atomic
def delete(self, hard=False):
if hard:
super().delete()
else:
if hasattr(self, 'file'):
# TODO(oleg) move to an archived directory, add random suffix
self.file.delete()
self.date_deleted = timezone.now()
self.save()
logger.info('%r pk=%r deleted', self.__class__, self.pk)
def delete_queryset(self, request, queryset):
"""Given a queryset, soft-delete it from the database."""
queryset.update(date_deleted=timezone.now())
def undelete(self, save=True):
if not self.date_deleted:
logger.warning('%r pk=%r is not deleted, cannot undelete', self.__class__, self.pk)
return
self.date_deleted = None
if save:
self.save()
logger.info('%r pk=%r undeleted', self.__class__, self.pk)

1
common/tests/factories/extensions.py
View File

@ -94,4 +94,5 @@ def create_version(**kwargs) -> 'Version':
def create_approved_version(**kwargs) -> 'Version':
version = create_version(**kwargs)
version.extension.approve()
version.refresh_from_db()
return version

9
extensions/admin.py
View File

@ -53,7 +53,6 @@ class ExtensionAdmin(admin.ModelAdmin):
'date_created',
'date_status_changed',
'date_approved',
'date_deleted',
'date_modified',
'average_score',
'text_ratings_count',
@ -76,7 +75,6 @@ class ExtensionAdmin(admin.ModelAdmin):
'date_status_changed',
'date_approved',
'date_modified',
'date_deleted',
),
'name',
'slug',
@ -135,7 +133,6 @@ class VersionAdmin(admin.ModelAdmin):
'tagline',
'date_created',
'date_modified',
'date_deleted',
'average_score',
'download_count',
)
@ -147,7 +144,7 @@ class VersionAdmin(admin.ModelAdmin):
'fields': (
'id',
'tagline',
('date_created', 'date_modified', 'date_deleted'),
('date_created', 'date_modified'),
'extension',
'version',
'blender_version_min',
@ -185,8 +182,8 @@ class VersionAdmin(admin.ModelAdmin):
class MaintainerAdmin(admin.ModelAdmin):
model = Maintainer
list_display = ('extension', 'user', 'date_deleted')
readonly_fields = ('extension', 'user', 'date_deleted')
list_display = ('extension', 'user')
readonly_fields = ('extension', 'user')
class LicenseAdmin(admin.ModelAdmin):

3
extensions/forms.py
View File

@ -76,9 +76,6 @@ class AddPreviewFileForm(forms.ModelForm):
):
logger.warning('Found an existing %s pk=%s', model, existing_image.pk)
self.instance = existing_image
# Undelete the instance, if necessary
if self.instance.is_deleted:
self.instance.undelete(save=False)
# Fill in missing fields from request and the source file
self.instance.user = self.request.user

25
extensions/migrations/0026_remove_extension_date_deleted_and_more.py Normal file
View File

@ -0,0 +1,25 @@
# Generated by Django 4.2.11 on 2024-04-18 08:59
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
('extensions', '0025_alter_tag_type'),
]
operations = [
migrations.RemoveField(
model_name='extension',
name='date_deleted',
),
migrations.RemoveField(
model_name='maintainer',
name='date_deleted',
),
migrations.RemoveField(
model_name='version',
name='date_deleted',
),
]

106
extensions/models.py
View File

@ -10,7 +10,7 @@ from django.db.models import F, Q, Count
from django.urls import reverse
from common.fields import FilterableManyToManyField
from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin
from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin
from constants.base import (
AUTHOR_ROLE_CHOICES,
AUTHOR_ROLE_DEV,
@ -106,43 +106,33 @@ class License(CreatedModifiedMixin, models.Model):
class ExtensionManager(models.Manager):
@property
def exclude_deleted(self):
return self.filter(date_deleted__isnull=True)
@property
def listed(self):
return self.exclude_deleted.filter(
return self.filter(
status=self.model.STATUSES.APPROVED,
is_listed=True,
)
@property
def unlisted(self):
return self.exclude_deleted.exclude(status=self.model.STATUSES.APPROVED)
return self.exclude(status=self.model.STATUSES.APPROVED)
def authored_by(self, user_id: int):
return self.exclude_deleted.filter(
maintainer__user_id=user_id, maintainer__date_deleted__isnull=True
)
return self.filter(maintainer__user_id=user_id)
def listed_or_authored_by(self, user_id: int):
return self.exclude_deleted.filter(
Q(status=self.model.STATUSES.APPROVED)
| Q(maintainer__user_id=user_id, maintainer__date_deleted__isnull=True)
return self.filter(
Q(status=self.model.STATUSES.APPROVED) | Q(maintainer__user_id=user_id)
).distinct()
class Extension(
CreatedModifiedMixin, RatingMixin, TrackChangesMixin, SoftDeleteMixin, models.Model
):
class Extension(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Model):
track_changes_to_fields = {
'status',
'name',
'description',
'support',
'website',
'date_deleted',
}
TYPES = EXTENSION_TYPE_CHOICES
STATUSES = EXTENSION_STATUS_CHOICES
@ -176,7 +166,6 @@ class Extension(
User,
through='Maintainer',
related_name='extensions',
q_filter=Q(maintainer__date_deleted__isnull=True),
)
team = models.ForeignKey('teams.Team', null=True, blank=True, on_delete=models.SET_NULL)
@ -190,8 +179,7 @@ class Extension(
ordering = ['-average_score', '-date_created', 'name']
def __str__(self):
label_deleted = f'{self.date_deleted and " (DELETED ❌)" or ""}'
return f'{self.get_type_display()} "{self.name}"{label_deleted}'
return f'{self.get_type_display()} "{self.name}"'
@property
def type_slug(self) -> str:
@ -236,12 +224,19 @@ class Extension(
self.status = self.STATUSES.APPROVED
self.save()
@transaction.atomic
def delete(self, hard=False):
versions = self.versions.filter(date_deleted__isnull=True)
for v in versions:
v.delete(hard=hard)
super().delete(hard=hard)
@property
def cannot_be_deleted_reasons(self) -> List[str]:
"""Return a list of reasons why this extension cannot be deleted."""
reasons = []
if self.is_listed:
reasons.append('is_listed')
if self.ratings.count() > 0:
reasons.append('has_ratings')
if self.abusereport_set.count() > 0:
reasons.append('has_abuse_reports')
for v in self.versions.all():
reasons.extend(v.cannot_be_deleted_reasons)
return reasons
def get_absolute_url(self):
return reverse('extensions:detail', args=[self.type_slug, self.slug])
@ -295,7 +290,6 @@ class Extension(
"""Retrieve the latest version."""
return (
self.versions.filter(
date_deleted__isnull=True,
file__status__in=self.valid_file_statuses,
file__isnull=False,
)
@ -312,7 +306,7 @@ class Extension(
If the add-on has not been created yet or is deleted, it returns None.
"""
if not self.id or self.is_deleted:
if not self.id:
return None
try:
return self.version
@ -322,14 +316,9 @@ class Extension(
def can_request_review(self):
"""Return whether an add-on can request a review or not."""
if (
self.is_deleted
or self.is_disabled
or self.status
in (
if self.is_disabled or self.status in (
self.STATUSES.APPROVED,
self.STATUSES.AWAITING_REVIEW,
)
):
return False
@ -360,9 +349,7 @@ class Extension(
"""Return True if given user is listed as a maintainer."""
if user is None or user.is_anonymous:
return False
return self.authors.filter(
maintainer__user_id=user.pk, maintainer__date_deleted__isnull=True
).exists()
return self.authors.filter(maintainer__user_id=user.pk).exists()
def can_rate(self, user) -> bool:
"""Return True if given user can rate this extension.
@ -430,17 +417,13 @@ class Tag(CreatedModifiedMixin, models.Model):
class VersionManager(models.Manager):
@property
def exclude_deleted(self):
return self.filter(date_deleted__isnull=True)
@property
def listed(self):
return self.exclude_deleted.filter(file__status=FILE_STATUS_CHOICES.APPROVED)
return self.filter(file__status=FILE_STATUS_CHOICES.APPROVED)
@property
def unlisted(self):
return self.exclude_deleted.exclude(file__status=FILE_STATUS_CHOICES.APPROVED)
return self.exclude(file__status=FILE_STATUS_CHOICES.APPROVED)
def update_or_create(self, *args, **kwargs):
# Stash the ManyToMany to be created after the Version has a valid ID already
@ -457,11 +440,10 @@ class VersionManager(models.Manager):
return version, result
class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, SoftDeleteMixin, models.Model):
class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Model):
track_changes_to_fields = {
'blender_version_min',
'blender_version_max',
'date_deleted',
'permissions',
'version',
'licenses',
@ -582,21 +564,22 @@ class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, SoftDeleteMi
self.tags.add(tag)
def __str__(self) -> str:
label_deleted = f'{self.date_deleted and " (DELETED ❌)" or ""}'
return f'{self.extension} v{self.version}{label_deleted}'
return f'{self.extension} v{self.version}'
@property
def is_listed(self):
# To be public, a version must not be deleted, must belong to a public
# extension, and its attached file must have a public status.
try:
return (
not self.is_deleted
and self.extension.is_listed
and self.file is not None
and self.file.status == self.file.STATUSES.APPROVED
)
except models.ObjectDoesNotExist:
return False
# To be public, version file must have a public status.
return self.file is not None and self.file.status == self.file.STATUSES.APPROVED
@property
def cannot_be_deleted_reasons(self) -> List[str]:
"""Return a list of reasons why this version cannot be deleted."""
reasons = []
if self.is_listed:
reasons.append('version_is_listed')
if self.ratings.count() > 0:
reasons.append('version_has_ratings')
return reasons
@property
def pending_rejection(self):
@ -656,7 +639,7 @@ class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, SoftDeleteMi
)
class Maintainer(CreatedModifiedMixin, SoftDeleteMixin, models.Model):
class Maintainer(CreatedModifiedMixin, models.Model):
extension = models.ForeignKey(Extension, on_delete=models.CASCADE)
user = models.ForeignKey(User, on_delete=models.CASCADE)
role = models.SmallIntegerField(default=AUTHOR_ROLE_DEV, choices=AUTHOR_ROLE_CHOICES)
@ -680,6 +663,11 @@ class Preview(CreatedModifiedMixin, models.Model):
ordering = ('position', 'date_created')
unique_together = [['extension', 'file']]
@property
def cannot_be_deleted_reasons(self) -> List[str]:
"""Return a list of reasons why this preview cannot be deleted."""
return []
class ExtensionReviewerFlags(models.Model):
extension = models.OneToOneField(

56
extensions/signals.py
View File

@ -1,21 +1,42 @@
from typing import Union
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, post_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
import extensions.models
import files.models
logger = logging.getLogger(__name__)
User = get_user_model()
@receiver(pre_delete, sender=extensions.models.Extension)
@receiver(pre_delete, sender=extensions.models.Preview)
@receiver(pre_delete, sender=extensions.models.Version)
def _log_extension_delete(sender: object, instance: object, **kwargs: object) -> None:
cannot_be_deleted_reasons = instance.cannot_be_deleted_reasons
if len(cannot_be_deleted_reasons) > 0:
# This shouldn't happen: prior validation steps should have taken care of this.
# raise ValidationError({'__all__': cannot_be_deleted_reasons})
args = {'sender': sender, 'pk': instance.pk, 'reasons': cannot_be_deleted_reasons}
logger.error("%(sender)s pk=%(pk)s is being deleted but it %(reasons)s", args)
logger.info('Deleting %s pk=%s "%s"', sender, instance.pk, str(instance))
@receiver(post_delete, sender=extensions.models.Preview)
def _delete_file(sender: object, instance: extensions.models.Preview, **kwargs: object) -> None:
instance.file.delete()
@receiver(post_delete, sender=extensions.models.Version)
def _delete_file(sender: object, instance: object, **kwargs: object) -> None:
f = instance.file
args = {'f_id': f.pk, 'h': f.hash, 'pk': instance.pk, 'sender': sender, 's': f.source.name}
logger.info('Deleting file pk=%(f_id)s s=%(s)s hash=%(h)s linked to %(sender)s pk=%(pk)s', args)
f.delete()
# TODO: this doesn't mean that the file was deleted from disk
@receiver(pre_save, sender=extensions.models.Extension)
@ -45,7 +66,6 @@ def extension_should_be_listed(extension):
return (
extension.latest_version is not None
and extension.latest_version.is_listed
and extension.latest_version.date_deleted is None
and extension.status == extension.STATUSES.APPROVED
)
@ -80,6 +100,7 @@ def _set_is_listed(
if extension.status == extensions.models.Extension.STATUSES.APPROVED and not new_is_listed:
extension.status = extensions.models.Extension.STATUSES.INCOMPLETE
logger.info('Extension pk=%s becomes listed', extension.pk)
extension.is_listed = new_is_listed
extension.save()
@ -118,3 +139,30 @@ def _update_authors_follow(instance, action, model, reverse, pk_set, **kwargs):
unfollow(user, extension, send_action=False, flag=Flag.AUTHOR)
elif action == 'post_add':
follow(user, extension, send_action=False, flag=Flag.AUTHOR)
@receiver(post_save, sender=extensions.models.Preview)
@receiver(post_save, sender=extensions.models.Version)
def _auto_approve_subsequent_uploads(
sender: object,
instance: Union[extensions.models.Preview, extensions.models.Version],
created: bool,
raw: bool,
**kwargs: object,
):
if raw:
return
if not created:
return
if not instance.file_id:
return
# N.B.: currently, subsequent version and preview uploads get approved automatically,
# if extension is currently listed (meaning, it was approved by a human already).
extension = instance.extension
file = instance.file
if extension.is_listed:
file.status = files.models.File.STATUSES.APPROVED
args = {'f_id': file.pk, 'pk': instance.pk, 'sender': sender, 's': file.source.name}
logger.info('Auto-approving file pk=%(f_id)s of %(sender)s pk=%(pk)s source=%(s)s', args)
file.save(update_fields={'status', 'date_modified'})

3
extensions/templates/extensions/confirm_delete.html
View File

@ -9,8 +9,7 @@
</h2>
<p>
{% blocktranslate with extension_name=extension_name %}
By deleting <strong>{{ extension_name }}</strong> you will lose the files,
download count, reviews as well ratings for the whole extension.
All extension previews and versions files will be deleted.
{% endblocktranslate %}
</p>
<div class="btn-row-fluid">

9
extensions/templates/extensions/draft_finalise.html
View File

@ -90,6 +90,15 @@
<i class="i-send"></i>
<span>{% trans 'Submit for Approval' %}</span>
</button>
{% with cannot_be_deleted_reasons=extension.cannot_be_deleted_reasons %}
{% if not cannot_be_deleted_reasons %}
<a href="{{ extension.get_delete_url }}" class="btn btn-danger">
<i class="i-trash"></i>
<span>{% trans 'Delete Extension' %}</span>
</a>
{% endif %}
{% endwith %}
</div>
</section>
</div>

5
extensions/templates/extensions/version_confirm_delete.html
View File

@ -8,9 +8,8 @@
{% blocktranslate with extension_name=extension_name %} Delete {{ extension_name }} {{ version }}?{% endblocktranslate %}
</h2>
<p>
{% blocktranslate with extension_name=extension_name version=version.version %}
By deleting <strong>{{ extension_name }} {{ version }}</strong> you will lose the files,
download count, reviews as well ratings for this version.
{% blocktranslate with extension_name=extension_name %}
Files belonging to this version will be deleted.
{% endblocktranslate %}
</p>
<div class="btn-row-fluid">

6
extensions/templates/extensions/version_list.html
View File

@ -21,7 +21,7 @@
<div class="row ext-version-history pb-3">
<div class="col">
{% for version in extension.versions.exclude_deleted %}
{% for version in extension.versions.all %}
{% if version.is_listed or is_maintainer %}
<details {% if forloop.counter == 1 %}open{% endif %} id="v{{ version.version|slugify }}">
<summary>
@ -103,11 +103,15 @@
<a href="{{ version.update_url }}" class="btn">
<i class="i-edit"></i><span>Edit</span>
</a>
{% with cannot_be_deleted_reasons=version.cannot_be_deleted_reasons %}
{% if not cannot_be_deleted_reasons %}
<a href="{{ version.get_delete_url }}" class="btn btn-danger">
<i class="i-trash"></i>
<span>{% trans "Delete Version" %}</span>
</a>
{% endif %}
{% endwith %}
{% endif %}
</div>
</div>
</div>

94
extensions/tests/test_delete.py
View File

@ -1,14 +1,34 @@
from django.test import TestCase
from common.tests.factories.extensions import create_approved_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
import extensions.models
import files.models
class DeleteTest(TestCase):
fixtures = ['dev', 'licenses']
def test_happy_path(self):
extension = create_approved_version().extension
def test_unlisted_unrated_extension_can_be_deleted_by_author(self):
version = create_version(
file__status=files.models.File.STATUSES.AWAITING_REVIEW,
ratings=[],
extension__previews=[
FileFactory(
type=files.models.File.TYPES.IMAGE,
source='images/b0/b03fa981527593fbe15b28cf37c020220c3d83021999eab036b87f3bca9c9168.png',
)
],
)
extension = version.extension
version_file = version.file
self.assertEqual(version_file.get_status_display(), 'Awaiting Review')
self.assertEqual(extension.get_status_display(), 'Incomplete')
self.assertFalse(extension.is_listed)
self.assertEqual(extension.cannot_be_deleted_reasons, [])
preview_file = extension.previews.first()
self.assertIsNotNone(preview_file)
url = extension.get_delete_url()
user = extension.authors.first()
@ -16,9 +36,71 @@ class DeleteTest(TestCase):
response = self.client.post(url)
self.assertEqual(response.status_code, 302)
# All relevant records should have been deleted
with self.assertRaises(extensions.models.Extension.DoesNotExist):
extension.refresh_from_db()
self.assertIsNotNone(extension.date_deleted)
self.assertTrue(all(v.date_deleted is not None for v in extension.versions.all()))
with self.assertRaises(extensions.models.Version.DoesNotExist):
version.refresh_from_db()
with self.assertRaises(files.models.File.DoesNotExist):
version_file.refresh_from_db()
with self.assertRaises(files.models.File.DoesNotExist):
preview_file.refresh_from_db()
self.assertIsNone(extensions.models.Extension.objects.filter(pk=extension.pk).first())
self.assertIsNone(extensions.models.Version.objects.filter(pk=version.pk).first())
self.assertIsNone(files.models.File.objects.filter(pk=version_file.pk).first())
self.assertIsNone(files.models.File.objects.filter(pk=preview_file.pk).first())
# TODO: check that files were deleted from storage (create a temp one prior to the check)
def test_publicly_listed_extension_cannot_be_deleted(self):
version = create_approved_version(ratings=[])
self.assertTrue(version.is_listed)
extension = version.extension
self.assertTrue(extension.is_listed)
self.assertEqual(extension.get_status_display(), 'Approved')
self.assertEqual(version.cannot_be_deleted_reasons, ['version_is_listed'])
self.assertEqual(extension.cannot_be_deleted_reasons, ['is_listed', 'version_is_listed'])
url = extension.get_delete_url()
user = extension.authors.first()
self.client.force_login(user)
response = self.client.post(url)
self.assertEqual(response.status_code, 403)
def test_rated_extension_cannot_be_deleted(self):
version = create_version(file__status=files.models.File.STATUSES.AWAITING_REVIEW)
self.assertFalse(version.is_listed)
extension = version.extension
self.assertFalse(extension.is_listed)
self.assertEqual(extension.get_status_display(), 'Incomplete')
self.assertEqual(version.cannot_be_deleted_reasons, ['version_has_ratings'])
self.assertEqual(
extension.cannot_be_deleted_reasons, ['has_ratings', 'version_has_ratings']
)
url = extension.get_delete_url()
user = extension.authors.first()
self.client.force_login(user)
response = self.client.post(url)
self.assertEqual(response.status_code, 403)
def test_reported_extension_cannot_be_deleted(self): # TODO
pass
def test_extension_with_ratings_cannot_be_deleted(self):
version = create_approved_version()
extension = version.extension
self.assertEqual(extension.status, extension.STATUSES.APPROVED)
url = extension.get_delete_url()
user = extension.authors.first()
self.client.force_login(user)
response = self.client.post(url)
self.assertEqual(response.status_code, 403)
def test_random_user_cant_delete(self):
extension = create_approved_version().extension
@ -30,5 +112,3 @@ class DeleteTest(TestCase):
self.assertEqual(response.status_code, 403)
extension.refresh_from_db()
self.assertIsNone(extension.date_deleted)
self.assertTrue(all(v.date_deleted is None for v in extension.versions.all()))

1
extensions/tests/test_models.py
View File

@ -47,7 +47,6 @@ class ExtensionTest(TestCase):
'name': 'Extension name',
'status': 1,
'support': 'https://example.com/',
'date_deleted': None,
},
}
},

2
extensions/tests/test_submit.py
View File

@ -352,7 +352,7 @@ class SubmitFinaliseTest(TestCase):
self.assertEqual(version.blender_version_max, None)
self.assertEqual(version.schema_version, '1.0.0')
self.assertEqual(version.release_notes, data['release_notes'])
self.assertEqual(version.file.get_status_display(), 'Approved')
self.assertEqual(version.file.get_status_display(), 'Awaiting Review')
# We cannot check for the ManyToMany yet (tags, licences, permissions)
# Check that author can access the page they are redirected to

43
extensions/tests/test_views.py
View File

@ -98,27 +98,6 @@ class ExtensionDetailViewTest(_BaseTestCase):
self._check_detail_page(response, extension)
def test_cannot_view_deleted_extension_anonymously(self):
extension = _create_extension()
extension.delete()
self.assertTrue(extension.is_deleted)
url = extension.get_absolute_url()
response = self.client.get(url)
self.assertEqual(response.status_code, 404)
def test_can_view_deleted_extension_if_staff(self):
staff_user = UserFactory(is_staff=True)
extension = _create_extension()
extension.delete()
self.assertTrue(extension.is_deleted)
self.client.force_login(staff_user)
response = self.client.get(extension.get_absolute_url())
self._check_detail_page(response, extension)
def test_can_view_unlisted_extension_if_maintaner(self):
extension = _create_extension()
@ -135,6 +114,14 @@ class ExtensionDetailViewTest(_BaseTestCase):
self._check_detail_page(response, extension)
def test_can_view_publicly_listed_extension_versions_anonymously(self):
extension = _create_extension()
extension.approve()
response = self.client.get(extension.get_versions_url())
self._check_detail_page(response, extension)
def test_can_view_publicly_listed_extension_ratings_anonymously(self):
extension = _create_extension()
extension.approve()
@ -201,25 +188,11 @@ class ListedExtensionsTest(_BaseTestCase):
create_approved_version(extension=self.extension)
self.assertEqual(self._listed_extensions_count(), 1)
def test_delete_extension(self):
self.extension.delete()
self.assertEqual(self._listed_extensions_count(), 0)
def test_moderate_extension(self):
self.extension.status = Extension.STATUSES.DISABLED
self.extension.save()
self.assertEqual(self._listed_extensions_count(), 0)
def test_soft_delete_only_version(self):
self.version.date_deleted = '1994-01-02 0:0:0+00:00'
self.version.save()
self.assertFalse(self.extension.is_listed)
self.assertEqual(self._listed_extensions_count(), 0)
def test_delete_only_version(self):
self.version.delete()
self.assertEqual(self._listed_extensions_count(), 0)
def test_moderate_only_version(self):
self.version.file.status = File.STATUSES.DISABLED
self.version.file.save()

19
extensions/views/manage.py
View File

@ -180,13 +180,21 @@ class DeleteExtensionView(
return context
def test_func(self) -> bool:
obj = self.get_object()
# Only maintainers allowed
return self.get_object().has_maintainer(self.request.user)
if not obj.has_maintainer(self.request.user):
return False
# Unless this extension cannot be deleted anymore
cannot_be_deleted_reasons = obj.cannot_be_deleted_reasons
if len(cannot_be_deleted_reasons) > 0:
return False
return True
class VersionDeleteView(
LoginRequiredMixin,
MaintainedExtensionMixin,
UserPassesTestMixin,
DeleteView,
):
model = Version
@ -226,6 +234,14 @@ class VersionDeleteView(
context['confirm_url'] = version.get_delete_url()
return context
def test_func(self) -> bool:
obj = self.get_object()
# Unless this version cannot be deleted anymore
cannot_be_deleted_reasons = obj.cannot_be_deleted_reasons
if len(cannot_be_deleted_reasons) > 0:
return False
return True
class ManageVersionsView(
LoginRequiredMixin,
@ -355,6 +371,7 @@ class DraftExtensionView(
def get_initial(self):
"""Return initial values for the version, based on the file."""
initial = super().get_initial()
if self.version:
initial['file'] = self.version.file
initial.update(**self.version.file.parsed_version_fields)
return initial

8
extensions/views/mixins.py
View File

@ -1,6 +1,5 @@
from django.contrib.auth.mixins import UserPassesTestMixin
from django.shortcuts import get_object_or_404, redirect
from django.core.exceptions import BadRequest
from extensions.models import Extension
from files.models import File
@ -51,9 +50,7 @@ class ExtensionMixin:
"""Fetch an extension by slug in the URL before dispatching the view."""
def dispatch(self, *args, **kwargs):
self.extension = get_object_or_404(
Extension.objects.exclude_deleted, slug=self.kwargs['slug']
)
self.extension = get_object_or_404(Extension, slug=self.kwargs['slug'])
return super().dispatch(*args, **kwargs)
@ -62,9 +59,6 @@ class DraftVersionMixin:
def dispatch(self, *args, **kwargs):
self.version = self.extension.versions.first()
if self.version is None:
error_message = f'Extension "{self.extension.name}" has no versions.'
raise BadRequest(error_message)
return super().dispatch(*args, **kwargs)

3
extensions/views/public.py
View File

@ -2,7 +2,6 @@ import logging
from django.contrib.auth import get_user_model
from django.db.models import Q
from django.http import Http404
from django.shortcuts import get_object_or_404, redirect
from django.views.generic.list import ListView
@ -54,8 +53,6 @@ class HomeView(ListedExtensionsView):
def extension_version_download(request, type_slug, slug, version):
"""Download an extension version and count downloads."""
extension_version = get_object_or_404(Version, extension__slug=slug, version=version)
if extension_version.date_deleted is not None:
raise Http404("This extension version has been deleted")
ExtensionDownload.create_from_request(request, object_id=extension_version.extension_id)
VersionDownload.create_from_request(request, object_id=extension_version.pk)
return redirect(extension_version.downloadable_signed_url)

3
files/admin.py
View File

@ -36,7 +36,6 @@ class FileAdmin(admin.ModelAdmin):
'status',
'date_status_changed',
'date_approved',
'date_deleted',
)
list_display = ('original_name', 'extension', 'user', 'date_created', 'type', 'status', 'is_ok')
@ -45,7 +44,6 @@ class FileAdmin(admin.ModelAdmin):
readonly_fields = (
'id',
'date_created',
'date_deleted',
'date_modified',
'date_approved',
'date_status_changed',
@ -84,7 +82,6 @@ class FileAdmin(admin.ModelAdmin):
'date_modified',
'date_status_changed',
'date_approved',
'date_deleted',
)
},
),

17
files/migrations/0006_remove_file_date_deleted.py Normal file
View File

@ -0,0 +1,17 @@
# Generated by Django 4.2.11 on 2024-04-18 08:59
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
('files', '0005_rename_validation_filevalidation_results_and_more'),
]
operations = [
migrations.RemoveField(
model_name='file',
name='date_deleted',
),
]

47
files/migrations/0007_alter_file_status.py Normal file
View File

@ -0,0 +1,47 @@
# Generated by Django 4.2.11 on 2024-04-18 10:10
import logging
from django.db import migrations, models
logger = logging.getLogger(__name__)
def change_first_uploads_to_awaiting_review(apps, schema_editor):
from constants.base import FILE_STATUS_CHOICES
File = apps.get_model('files', 'File')
to_update = []
for obj in File.objects.filter(status=FILE_STATUS_CHOICES.APPROVED):
# Unless this is a subsequent upload of a version/preview of an already approved extension,
# it should have a default status (Awaiting Review).
extension = (
obj.extension_preview.first() and obj.extension_preview.first().extension
or (hasattr(obj, 'version') and obj.version.extension)
)
if extension.is_listed:
continue
logger.info(
'Will change file pk=%s status from %s to %s', obj.pk, obj.get_status_display(),
'Awaiting Review',
)
obj.status = FILE_STATUS_CHOICES.AWAITING_REVIEW
to_update.append(obj)
logger.info('Updating %s files', len(to_update))
File.objects.bulk_update(to_update, batch_size=500, fields={'status'})
class Migration(migrations.Migration):
dependencies = [
('files', '0006_remove_file_date_deleted'),
]
operations = [
migrations.AlterField(
model_name='file',
name='status',
field=models.PositiveSmallIntegerField(choices=[(2, 'Awaiting Review'), (3, 'Approved'), (4, 'Disabled by staff'), (5, 'Disabled by author')], default=2),
),
migrations.RunPython(change_first_uploads_to_awaiting_review),
]

11
files/models.py
View File

@ -5,7 +5,7 @@ import logging
from django.contrib.auth import get_user_model
from django.db import models
from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin
from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin
from files.utils import get_sha256, guess_mimetype_from_ext
from constants.base import (
FILE_STATUS_CHOICES,
@ -48,8 +48,8 @@ def thumbnail_upload_to(instance, filename):
return path
class File(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Model):
track_changes_to_fields = {'status', 'size_bytes', 'hash', 'date_deleted'}
class File(CreatedModifiedMixin, TrackChangesMixin, models.Model):
track_changes_to_fields = {'status', 'size_bytes', 'hash'}
TYPES = FILE_TYPE_CHOICES
STATUSES = FILE_STATUS_CHOICES
@ -76,7 +76,7 @@ class File(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Mode
'as guessed from its contents.'
),
)
status = models.PositiveSmallIntegerField(choices=STATUSES, default=STATUSES.APPROVED)
status = models.PositiveSmallIntegerField(choices=STATUSES, default=STATUSES.AWAITING_REVIEW)
user = models.ForeignKey(
User, related_name='files', null=False, blank=False, on_delete=models.CASCADE
@ -103,8 +103,7 @@ class File(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Mode
objects = FileManager()
def __str__(self) -> str:
label_deleted = f'{self.date_deleted and " (DELETED ❌)" or ""}'
return f'{self.original_name} ({self.get_status_display()}){label_deleted}'
return f'{self.original_name} ({self.get_status_display()})'
@property
def has_been_validated(self):

7
files/signals.py
View File

@ -1,6 +1,6 @@
import logging
from django.db.models.signals import pre_save, post_save
from django.db.models.signals import pre_save, post_save, pre_delete
from django.dispatch import receiver
import files.models
@ -32,3 +32,8 @@ def _scan_new_file(
return
schedule_scan(instance)
@receiver(pre_delete, sender=files.models.File)
def _log_file_delete(sender: object, instance: files.models.File, **kwargs: object) -> None:
logger.info('Deleting file pk=%s source=%s', instance.pk, instance.source.name)

1
files/tests/test_models.py
View File

@ -45,7 +45,6 @@ class FileTest(TestCase):
'status': 2,
'hash': 'foobar',
'size_bytes': 7149,
'date_deleted': None,
},
}
},

30
ratings/admin.py
View File

@ -16,12 +16,12 @@ class RatingTypeFilter(admin.SimpleListFilter):
parameter_name = 'type'
def lookups(self, request, model_admin):
"""
Returns a list of tuples. The first element in each
tuple is the coded value for the option that will
appear in the URL query. The second element is the
human-readable name for the option that will appear
in the right sidebar.
"""Return a list of lookup option tuples.
The first element in each tuple is the coded value
for the option that will appear in the URL query.
The second element is the human-readable name for
the option that will appear in the right sidebar.
"""
return (
('rating', 'User Rating'),
@ -29,10 +29,10 @@ class RatingTypeFilter(admin.SimpleListFilter):
)
def queryset(self, request, queryset):
"""
Returns the filtered queryset based on the value
provided in the query string and retrievable via
`self.value()`.
"""Return the filtered queryset.
Filter based on the value provided in the query string
and retrievable via `self.value()`.
"""
if self.value() == 'rating':
return queryset.filter(reply_to__isnull=True)
@ -53,7 +53,6 @@ class RatingAdmin(admin.ModelAdmin):
readonly_fields = (
'date_created',
'date_modified',
'date_deleted',
'extension',
'version',
'text',
@ -63,7 +62,6 @@ class RatingAdmin(admin.ModelAdmin):
fields = ('status',) + readonly_fields
list_display = (
'date_created',
'is_deleted',
'user',
'ip_address',
'score',
@ -71,7 +69,7 @@ class RatingAdmin(admin.ModelAdmin):
'status',
'truncated_text',
)
list_filter = ('status', RatingTypeFilter, 'score', 'date_deleted')
list_filter = ('status', RatingTypeFilter, 'score')
actions = ('delete_selected',)
list_select_related = ('user',) # For extension/reply_to see get_queryset()
@ -94,11 +92,5 @@ class RatingAdmin(admin.ModelAdmin):
is_reply.boolean = True
is_reply.admin_order_field = 'reply_to'
def is_deleted(self, obj):
return bool(obj.date_deleted)
is_deleted.boolean = True
is_deleted.admin_order_field = 'date_deleted'
admin.site.register(Rating, RatingAdmin)

17
ratings/migrations/0005_remove_rating_date_deleted.py Normal file
View File

@ -0,0 +1,17 @@
# Generated by Django 4.2.11 on 2024-04-18 08:59
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
('ratings', '0004_alter_rating_status'),
]
operations = [
migrations.RemoveField(
model_name='rating',
name='date_deleted',
),
]

39
ratings/models.py
View File

@ -1,12 +1,12 @@
import logging
from django.contrib.auth import get_user_model
from django.db import models, transaction
from django.db import models
from django.template.defaultfilters import truncatechars
from django.utils.translation import gettext_lazy as _
from django.urls import reverse
from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin
from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin
from common.templatetags import common
from constants.base import RATING_STATUS_CHOICES, RATING_SCORE_CHOICES
from utils import send_mail
@ -17,28 +17,20 @@ log = logging.getLogger(__name__)
class RatingManager(models.Manager):
# TODO: figure out how to retrieve reviews "annotated" with replies, if any
@property
def exclude_deleted(self):
return self.filter(date_deleted__isnull=True)
@property
def listed(self):
return self.exclude_deleted.filter(
status=self.model.STATUSES.APPROVED, reply_to__isnull=True
)
return self.filter(status=self.model.STATUSES.APPROVED, reply_to__isnull=True)
@property
def unlisted(self):
return self.exclude_deleted.exclude(
status=self.models.STATUSES.APPROVED, reply_to__isnull=True
)
return self.exclude(status=self.models.STATUSES.APPROVED, reply_to__isnull=True)
@property
def listed_texts(self):
return self.listed.filter(text__isnull=False)
class Rating(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Model):
class Rating(CreatedModifiedMixin, TrackChangesMixin, models.Model):
track_changes_to_fields = {'status'}
STATUSES = RATING_STATUS_CHOICES
@ -107,7 +99,7 @@ class Rating(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Mo
@classmethod
def get_for(cls, user_id: int, extension_id: int):
"""Get rating left by a given user for a given extension."""
return cls.objects.exclude_deleted.filter(
return cls.objects.filter(
reply_to=None,
user_id=user_id,
extension_id=extension_id,
@ -141,25 +133,6 @@ class Rating(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Mo
# user_responsible=user.
self.save()
@transaction.atomic
def delete(self, user_responsible=None, send_post_save_signal=True):
if user_responsible is None:
user_responsible = self.user
for flag in self.ratingflag_set.all():
flag.delete()
SoftDeleteMixin.delete(self)
log.warning(
'Rating deleted: user pk=%s (%s) deleted id:%s by pk=%s (%s) ("%s")',
user_responsible.pk,
str(user_responsible),
self.pk,
self.user.pk,
str(self.user),
str(self.text),
)
@classmethod
def get_replies(cls, ratings):
ratings = [r.id for r in ratings]

6
ratings/tests/test_views.py
View File

@ -27,12 +27,6 @@ class RatingsViewTest(TestCase):
text='this rating is also approved',
status=Rating.STATUSES.APPROVED,
),
RatingFactory(
version=version,
date_deleted='2022-01-01 01:01:01Z',
text='this rating is deleted',
status=Rating.STATUSES.APPROVED,
),
]
url = version.extension.get_ratings_url()

8
reviewers/tests/test_views.py
View File

@ -17,14 +17,6 @@ class CommentsViewTest(TestCase):
self.assertEqual(r.status_code, 200)
self.assertEqual(len(r.context['object_list']), 1)
# Deleted extensions don't show up in the approval queue
self.assertIsNone(self.default_version.extension.date_deleted)
self.default_version.extension.delete()
self.assertIsNotNone(self.default_version.extension.date_deleted)
r = self.client.get(reverse('reviewers:approval-queue'))
self.assertEqual(r.status_code, 200)
self.assertEqual(len(r.context['object_list']), 0)
# Page is visible for every extension and does not require authentication
def test_visibility(self):
r = self.client.get(

6
reviewers/views.py
View File

@ -18,10 +18,8 @@ class ApprovalQueueView(ListView):
paginate_by = 100
def get_queryset(self):
return (
Extension.objects.exclude_deleted
.exclude(status=Extension.STATUSES.APPROVED)
.order_by('-date_created')
return Extension.objects.exclude(status=Extension.STATUSES.APPROVED).order_by(
'-date_created'
)
template_name = 'reviewers/extensions_review_list.html'