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 +187 -284
23 changed files with 187 additions and 284 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 7949be8e2e - Show all commits

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_REVIEW
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(

7
blender_extensions/settings.py
View File

@ -1,5 +1,4 @@
"""
Django settings for blender_extensions project.
"""Django settings for blender_extensions project.
Generated by 'django-admin startproject' using Django 4.0.6.
@ -192,9 +191,9 @@ LOGGING = {
},
},
'loggers': {
'django': {'level': 'WARNING'},
'django': {'level': 'INFO'},
},
'root': {'level': 'WARNING', 'handlers': ['console']},
'root': {'level': 'INFO', 'handlers': ['console']},
}
PIPELINE = {

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
@ -147,43 +147,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)

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

125
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:
@ -238,32 +226,18 @@ class Extension(
@property
def cannot_be_deleted_reasons(self) -> List[str]:
"""Return a list of reasons why this extension cannot be fully deleted."""
"""Return a list of reasons why this extension cannot be deleted."""
reasons = []
if self.abusereport_set.count() > 0:
reasons.append('has_abuse_reports')
if self.ratings.count() > 0:
reasons.append('has_ratings')
if self.is_listed:
reasons.append('is_listed')
for v in self.versions.all():
reasons.extend(v.cannot_be_deleted_reasons)
return reasons
@transaction.atomic
def delete(self):
hard = self.cannot_be_deleted_reasons == []
versions = self.versions.all()
previews = self.previews.all()
# When soft-deleting, filter out already soft-deleted versions and previews
if not hard:
previews = self.previews.filter(date_deleted__isnull=True)
versions = self.versions.filter(date_deleted__isnull=True)
mode = not hard and 'soft-' or ''
for p in previews:
p.delete(hard=hard)
log.warning('%(mode)sdeleting preview file pk=%s source=%s', mode, p.pk, p.source.name)
for v in versions:
v.delete(hard=hard)
log.warning('%(mode)sdeleting extension pk=%s', mode, self.pk)
super().delete(hard=hard)
def get_absolute_url(self):
return reverse('extensions:detail', args=[self.type_slug, self.slug])
@ -316,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,
)
@ -333,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
@ -343,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
@ -381,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.
@ -451,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
@ -478,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',
@ -563,21 +524,6 @@ class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, SoftDeleteMi
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
def delete(self, hard=False):
file = self.file
mode = not hard and 'soft-' or ''
args = {
'file_id': file.pk,
'mode': mode,
'source': file.source.name,
'version_id': self.pk,
'version': self.version,
}
log.warning('%(mode)sdeleting file pk=%(file_id)s source=%(source)s', args)
file.delete(hard=hard)
log.warning('%(mode)sdeleting version pk=%(version_id)s "%(version)s"', args)
super().delete(hard=hard)
def set_initial_permissions(self, _permissions):
if not _permissions:
return
@ -618,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.ratings.count() > 0:
reasons.append('version_has_ratings')
if self.is_listed:
reasons.append('version_is_listed')
return reasons
@property
def pending_rejection(self):
@ -692,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)
@ -716,6 +663,10 @@ class Preview(CreatedModifiedMixin, models.Model):
ordering = ('position', 'date_created')
unique_together = [['extension', 'file']]
@property
def cannot_be_deleted_reasons(self) -> List[str]:
return []
class ExtensionReviewerFlags(models.Model):
extension = models.OneToOneField(

32
extensions/signals.py
View File

@ -1,19 +1,43 @@
from typing import Union
import logging
from django.db.models.signals import pre_save, post_save, post_delete
from django.core.exceptions import ValidationError
from django.db.models.signals import pre_save, post_save, pre_delete, post_delete
from django.dispatch import receiver
import django.dispatch
import extensions.models
import files.models
logger = logging.getLogger(__name__)
version_changed = django.dispatch.Signal()
version_uploaded = django.dispatch.Signal()
@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
# FIXME: remove this temporary sanity check:
# this would break the assumption that admin deletion is "normal" deletion,
# which we'd like to stick to.
if cannot_be_deleted_reasons != []:
# This shouldn't happen: prior validation steps should have taken care of this.
raise ValidationError({'__all__': cannot_be_deleted_reasons})
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)
@ -46,7 +70,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
)
@ -81,6 +104,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()

2
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 %}
{% if version.is_listed or is_maintainer %}
<details {% if forloop.counter == 1 %}open{% endif %} id="v{{ version.version|slugify }}">
<summary>

101
extensions/tests/test_delete.py
View File

@ -1,4 +1,4 @@
from django.test import TestCase
from django.test import TransactionTestCase
from common.tests.factories.extensions import create_approved_version, create_version
from common.tests.factories.files import FileFactory
@ -7,37 +7,12 @@ import extensions.models
import files.models
class DeleteTest(TestCase):
class DeleteTest(TransactionTestCase):
fixtures = ['dev', 'licenses']
def test_happy_path(self):
extension = create_approved_version().extension
url = extension.get_delete_url()
user = extension.authors.first()
self.client.force_login(user)
response = self.client.post(url)
self.assertEqual(response.status_code, 302)
extension.refresh_from_db()
self.assertIsNotNone(extension.date_deleted)
self.assertTrue(all(v.date_deleted is not None for v in extension.versions.all()))
def test_random_user_cant_delete(self):
extension = create_approved_version().extension
url = extension.get_delete_url()
user = UserFactory()
self.client.force_login(user)
response = self.client.post(url)
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()))
def test_incomplete_unrated_extension_can_be_fully_deleted_by_author(self):
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(
@ -48,7 +23,10 @@ class DeleteTest(TestCase):
)
extension = version.extension
version_file = version.file
self.assertEqual(extension.status, extension.STATUSES.INCOMPLETE)
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)
@ -71,3 +49,66 @@ class DeleteTest(TestCase):
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
url = extension.get_delete_url()
user = UserFactory()
self.client.force_login(user)
response = self.client.post(url)
self.assertEqual(response.status_code, 403)
extension.refresh_from_db()

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

27
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()
@ -210,12 +189,6 @@ class ListedExtensionsTest(_BaseTestCase):
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)

9
extensions/views/manage.py
View File

@ -179,8 +179,15 @@ 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 cannot_be_deleted_reasons != []:
return False
return True
class VersionDeleteView(

4
extensions/views/mixins.py
View File

@ -51,9 +51,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)

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

9
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
@ -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
@ -30,3 +30,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)

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]

1
ratings/tests/test_views.py
View File

@ -29,7 +29,6 @@ class RatingsViewTest(TestCase):
),
RatingFactory(
version=version,
date_deleted='2022-01-01 01:01:01Z',
text='this rating is deleted',
status=Rating.STATUSES.APPROVED,
),

2
reviewers/tests/test_views.py
View File

@ -18,9 +18,7 @@ class CommentsViewTest(TestCase):
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)

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'