README.md

@@ -61,53 +61,46 @@ log into http://extensions.local:8111/admin/ with `admin`/`admin`.
 
 ### Blender ID
 
-All the configuration of this web app is done via environment variables
-(there's only one `settings.py` regardless of an environment).
-
 Blender Extensions, as all other Blender web services, uses Blender ID.
-To configure OAuth login, first create a new OAuth2 application in Blender ID with the following settings:
+Blender Extensions can also receive Blender ID account modifications such as badge updates
+via a webhook.
 
-* Redirect URIs: `http://extensions.local:8111/oauth/authorized`
+For development, Blender ID's code contains a fixture with an OAuth app and a webhook
-* Client type: "Confidential";
+that should work without any changes to default configuration.
-* Authorization grant type: "Authorization code";
+To load this fixture, go to your development Blender ID and run the following:
-* Name: "Blender Extensions Dev";
 
-Then copy client ID and secret and save them as `BID_OAUTH_CLIENT` and `BID_OAUTH_SECRET` into a `.env` file:
+    ./manage.py loaddata blender_extensions_devserver
-
-    export BID_OAUTH_CLIENT=<CLIENT ID HERE>
-    export BID_OAUTH_SECRET=<SECRET HERE>
-
-Run the dev server using the following command:
-
-    source .env && ./manage.py runserver 8111
-
-#### Webhook
-
-Blender Extensions can receive account modifications such as badge updates via a webhook,
-which has to be configured in Blender ID admin separately from the OAuth app.
-
-In Admin › Blender-ID API › Webhooks click `Add Webhook` and set the following:
-
-* Name: "Blender Extensions Dev";
-* URL: `http://extensions.local:8111/webhooks/user-modified/`;
-* App: choose the app created in the previous step;
-
-Then copy webhook's secret into the `.env` file as `BID_WEBHOOK_USER_MODIFIED_SECRET`:
-
-    export BID_WEBHOOK_USER_MODIFIED_SECRET=<WEBHOOK SECRET HERE>
 
 **N.B.**: the webhook view delegates the actual updating of the user profile
 to a background task, so in order to see the updates locally, start the processing of
 tasks using the following:
 
-    source .env && ./manage.py process_tasks
+    ./manage.py process_tasks
 
 #### Blender ID and staging/production
 
-The above steps use local development setup as example.
+For staging/production, create an OAuth2 application in Blender ID using
-For staging/production the steps are the same, the only differences being
+Admin › Blender-ID › OAuth2 applications -> Add:
-the names of the app and the webhook,
+
-and `http://extensions.local:8111` being replaced with the appropriate base URL.
+* Redirect URIs: `https://staging.extensions.blender.org/oauth/authorized` (`https://extensions.blender.org` for production);
+* Client type: "Confidential";
+* Authorization grant type: "Authorization code";
+* Name: "Blender Extensions Staging" (or "Blender Extensions" for production);
+
+Copy client ID and secret and save them as `BID_OAUTH_CLIENT` and `BID_OAUTH_SECRET` into a `.env` file:
+
+    export BID_OAUTH_CLIENT=<CLIENT ID HERE>
+    export BID_OAUTH_SECRET=<SECRET HERE>
+
+Create a webhook using Admin › Blender-ID API › Webhooks > Add:
+
+* Name: "Blender Extensions Staging" (or "Blender Extensions" for production)";
+* URL: `https://staging.extensions.blender.org/webhooks/user-modified/` (or `https://extensions.blender.org/webhooks/user-modified/` for production);
+* App: choose the app created in the previous step;
+
+Copy webhook's secret into the `.env` file as `BID_WEBHOOK_USER_MODIFIED_SECRET`:
+
+    export BID_WEBHOOK_USER_MODIFIED_SECRET=<WEBHOOK SECRET HERE>
 
 ## Pre-commit hooks
 

abuse/apps.py

@@ -4,3 +4,9 @@ from django.apps import AppConfig
 class AbuseConfig(AppConfig):
     default_auto_field = 'django.db.models.BigAutoField'
     name = 'abuse'
+
+    def ready(self):
+        from actstream import registry
+        import abuse.signals  # noqa: F401
+
+        registry.register(self.get_model('AbuseReport'))

abuse/migrations/0005_alter_abusereport_type.py

@@ -0,0 +1,18 @@
+# Generated by Django 4.2.11 on 2024-04-18 13:48
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('abuse', '0004_abusereport_rating_abusereport_type'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='abusereport',
+            name='type',
+            field=models.PositiveSmallIntegerField(choices=[(1, 'Extension'), (2, 'User'), (3, 'Rating')], default=1),
+        ),
+    ]

abuse/migrations/0006_remove_abusereport_date_deleted.py

@@ -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',
+        ),
+    ]

abuse/models.py

@@ -8,14 +8,14 @@ from django.urls import reverse
 from extended_choices import Choices
 from geoip2.errors import GeoIP2Error
 
-from constants.base import ABUSE_TYPE, ABUSE_TYPE_EXTENSION, ABUSE_TYPE_REVIEW
+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(
@@ -34,6 +34,7 @@ class AbuseReport(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, mode
     )
 
     # NULL if the reporter is anonymous.
+    # FIXME? make non-null
     reporter = models.ForeignKey(
         User,
         null=True,
@@ -100,7 +101,7 @@ class AbuseReport(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, mode
             reporter_id=user_id,
             extension_id=extension_id,
             rating_id=rating_id,
-            type=ABUSE_TYPE_REVIEW,
+            type=ABUSE_TYPE_RATING,
         ).exists()
 
     def get_absolute_url(self):

abuse/signals.py

@@ -0,0 +1,47 @@
+import logging
+
+from actstream import action
+from django.db.models.signals import post_save
+from django.dispatch import receiver
+
+from abuse.models import AbuseReport
+from constants.activity import Verb
+from constants.base import (
+    ABUSE_TYPE_EXTENSION,
+    ABUSE_TYPE_RATING,
+    ABUSE_TYPE_USER,
+)
+
+logger = logging.getLogger(__name__)
+
+
+@receiver(post_save, sender=AbuseReport)
+def _create_action_from_report(
+    sender: object,
+    instance: AbuseReport,
+    created: bool,
+    raw: bool,
+    **kwargs: object,
+) -> None:
+    if not created:
+        return
+    if raw:
+        return
+
+    if instance.type == ABUSE_TYPE_EXTENSION:
+        verb = Verb.REPORTED_EXTENSION
+    elif instance.type == ABUSE_TYPE_RATING:
+        verb = Verb.REPORTED_RATING
+    elif instance.type == ABUSE_TYPE_USER:
+        # TODO?
+        return
+    else:
+        logger.warning(f'ignoring an unexpected AbuseReport type={instance.type}')
+        return
+
+    action.send(
+        instance.reporter,
+        verb=verb,
+        target=instance.extension,
+        action_object=instance,
+    )

abuse/tests/test_abuse.py

@@ -0,0 +1,21 @@
+from django.test import TestCase
+
+from common.tests.factories.extensions import create_approved_version
+from common.tests.factories.users import UserFactory
+
+POST_DATA = {
+    'message': 'test message',
+    'reason': '127',
+    'version': '',
+}
+
+
+class ReportTest(TestCase):
+    def test_report_twice(self):
+        version = create_approved_version()
+        user = UserFactory()
+        self.client.force_login(user)
+        url = version.extension.get_report_url()
+        _ = self.client.post(url, POST_DATA)
+        response = self.client.get(url, follow=True)
+        self.assertEqual(response.status_code, 200)

abuse/views.py

@@ -5,10 +5,10 @@ from django.http import Http404
 from django.views.generic import DetailView
 from django.views.generic.list import ListView
 from django.views.generic.edit import CreateView
-from django.shortcuts import get_object_or_404
+from django.shortcuts import get_object_or_404, redirect
 
 from .forms import ReportForm
-from constants.base import ABUSE_TYPE_EXTENSION, ABUSE_TYPE_REVIEW
+from constants.base import ABUSE_TYPE_EXTENSION, ABUSE_TYPE_RATING
 from abuse.models import AbuseReport
 from ratings.models import Rating
 from extensions.models import Extension, Version
@@ -37,15 +37,20 @@ class ReportList(
 class ReportExtensionView(
     LoginRequiredMixin,
     extensions.views.mixins.ListedExtensionMixin,
-    UserPassesTestMixin,
     CreateView,
 ):
     model = AbuseReport
     form_class = ReportForm
 
-    def test_func(self) -> bool:
+    def get(self, request, *args, **kwargs):
-        # TODO: best to redirect to existing report or show a friendly message
+        extension = get_object_or_404(Extension.objects.listed, slug=self.kwargs['slug'])
-        return not AbuseReport.exists(user_id=self.request.user.pk, extension_id=self.extension.id)
+        report = AbuseReport.objects.filter(
+            reporter_id=self.request.user.pk, extension_id=extension.id
+        ).first()
+        if report is not None:
+            return redirect('abuse:view-report', pk=report.pk)
+        else:
+            return super().get(request, *args, **kwargs)
 
     def form_valid(self, form):
         """Link newly created rating to latest version and current user."""
@@ -104,7 +109,7 @@ class ReportReviewView(
         form.instance.extension = self.extension
         form.instance.rating = self.rating
         form.instance.extension_version = self.version.version
-        form.instance.type = ABUSE_TYPE_REVIEW
+        form.instance.type = ABUSE_TYPE_RATING
         return super().form_valid(form)
 
     def get_context_data(self, **kwargs):

blender_extensions/settings.py

@@ -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.
 
@@ -55,6 +54,7 @@ INSTALLED_APPS = [
     'common',
     'files',
     'loginas',
+    'notifications',
     'pipeline',
     'ratings',
     'rangefilter',
@@ -74,6 +74,7 @@ INSTALLED_APPS = [
     'django.contrib.staticfiles',
     'django.contrib.flatpages',
     'django.contrib.humanize',
+    'actstream',
 ]
 
 MIDDLEWARE = [
@@ -144,7 +145,7 @@ AUTH_PASSWORD_VALIDATORS = [
 
 LANGUAGE_CODE = 'en-us'
 
-TIME_ZONE = 'UTC'
+TIME_ZONE = 'Europe/Amsterdam'
 
 USE_I18N = True
 
@@ -173,6 +174,30 @@ STATICFILES_STORAGE = 'pipeline.storage.PipelineManifestStorage'
 
 DEFAULT_AUTO_FIELD = 'django.db.models.BigAutoField'
 
+LOGGING = {
+    'version': 1,
+    'disable_existing_loggers': False,
+    'formatters': {
+        'default': {'format': '%(asctime)-15s %(levelname)8s %(name)s %(message)s'},
+        'verbose': {
+            'format': (
+                '%(asctime)s %(levelname)8s [%(filename)s:%(lineno)d '
+                '%(funcName)s] %(name)s %(message)s '
+            ),
+        },
+    },
+    'handlers': {
+        'console': {
+            'class': 'logging.StreamHandler',
+            'formatter': 'verbose',
+        },
+    },
+    'loggers': {
+        'django': {'level': 'INFO'},
+    },
+    'root': {'level': 'INFO', 'handlers': ['console']},
+}
+
 PIPELINE = {
     'JS_COMPRESSOR': 'pipeline.compressors.jsmin.JSMinCompressor',
     'CSS_COMPRESSOR': 'pipeline.compressors.NoopCompressor',
@@ -229,9 +254,13 @@ if SENTRY_DSN:
 BLENDER_ID = {
     # MUST end in a slash:
     'BASE_URL': os.environ.get('BID_BASE_URL', 'http://id.local:8000/'),
-    'OAUTH_CLIENT': os.environ.get('BID_OAUTH_CLIENT'),
+    'OAUTH_CLIENT': os.environ.get('BID_OAUTH_CLIENT', 'BLENDER-EXTENSIONS-DEV'),
-    'OAUTH_SECRET': os.environ.get('BID_OAUTH_SECRET'),
+    'OAUTH_SECRET': os.environ.get(
-    'WEBHOOK_USER_MODIFIED_SECRET': os.environ.get('BID_WEBHOOK_USER_MODIFIED_SECRET'),
+        'BID_OAUTH_SECRET', 'DEVELOPMENT-ONLY NON SECRET NEVER USE IN PRODUCTION'
+    ),
+    'WEBHOOK_USER_MODIFIED_SECRET': os.environ.get(
+        'BID_WEBHOOK_USER_MODIFIED_SECRET', 'DEVELOPMENT-ONLY NON SECRET NEVER USE IN PRODUCTION'
+    ),
 }
 
 TAGGIT_CASE_INSENSITIVE = True
@@ -241,8 +270,6 @@ ACTSTREAM_SETTINGS = {
     'FETCH_RELATIONS': True,
 }
 
-CSRF_FAILURE_VIEW = 'common.views.errors.csrf_failure'
-
 REST_FRAMEWORK = {
     'DEFAULT_SCHEMA_CLASS': 'drf_spectacular.openapi.AutoSchema',
 }
@@ -259,7 +286,7 @@ SPECTACULAR_SETTINGS = {
 }
 
 # Fallback user for logging
-SYSTEM_USER_ID = 1
+SYSTEM_USER_ID = os.environ.get('SYSTEM_USER_ID', 1)
 
 if TESTING:
     # Avoid "ValueError: Missing staticfiles manifest entry for"
@@ -294,3 +321,7 @@ EMAIL_HOST = os.getenv('EMAIL_HOST')
 EMAIL_PORT = os.getenv('EMAIL_PORT', '587')
 EMAIL_HOST_USER = os.getenv('EMAIL_HOST_USER')
 EMAIL_HOST_PASSWORD = os.getenv('EMAIL_HOST_PASSWORD')
+
+ACTSTREAM_SETTINGS = {
+    'MANAGER': 'actstream.managers.ActionManager',
+}

blender_extensions/urls.py

@@ -39,6 +39,7 @@ urlpatterns = [
     path('', include('users.urls')),
     path('', include('teams.urls')),
     path('', include('reviewers.urls')),
+    path('', include('notifications.urls')),
     path('api/swagger/', RedirectView.as_view(url='/api/v1/swagger/')),
     path('api/v1/', SpectacularAPIView.as_view(), name='schema_v1'),
     path('api/v1/swagger/', SpectacularSwaggerView.as_view(url_name='schema_v1'), name='swagger'),
@@ -52,7 +53,7 @@ handler404 = common.views.errors.ErrorView.as_view(status=404)
 handler500 = common.views.errors.ErrorView.as_view(status=500)
 
 if settings.DEBUG:
-    urlpatterns += [
+    urlpatterns = [
         re_path(
             r'^media/(?P<path>.*)$',
             serve,
@@ -61,4 +62,4 @@ if settings.DEBUG:
             },
         ),
         path('__debug__/', include('debug_toolbar.urls')),
-    ]
+    ] + urlpatterns

common/admin.py

@@ -212,9 +212,6 @@ class LogEntryAdmin(admin.ModelAdmin):
     def has_change_permission(self, request, obj=None):
         return False
 
-    def has_delete_permission(self, request, obj=None):
-        return False
-
     def has_view_permission(self, request, obj=None):
         return request.user.is_superuser
 

common/log_entries.py

@@ -2,7 +2,7 @@ from typing import Optional, Dict, List
 import json
 import logging
 
-from django.contrib.admin.models import CHANGE, LogEntry
+from django.contrib.admin.models import CHANGE, ACTION_FLAG_CHOICES, LogEntry
 from django.contrib.contenttypes.models import ContentType
 import django.conf
 import django.db.models
@@ -10,6 +10,7 @@ import django.db.models
 from common.middleware import threadlocal
 
 logger = logging.getLogger(__name__)
+ACTION_FLAG_DISPLAY = {k: v for k, v in ACTION_FLAG_CHOICES}
 
 
 def attach_log_entry(
@@ -32,13 +33,15 @@ def attach_log_entry(
 
     if user_id is None:
         request = threadlocal.get_current_request()
-        if request:
+        # N.B.: request.user.pk can be None after login of a new user
-            user_id = request.user.pk if request and request.user.is_authenticated else None
+        if request and request.user.is_authenticated and request.user.pk:
+            user_id = request.user.pk
         else:
             user_id = django.conf.settings.SYSTEM_USER_ID
 
     object_repr = repr(instance)
-    # Check that user refered to by the entry actually exists
+    logger.info('%s of "%s": %s', ACTION_FLAG_DISPLAY[action_flag], object_repr, message)
+    # Check that user referred to by the entry actually exists
     if not User.objects.filter(pk=user_id).exists():
         logger.error(
             'Cannot record change "%s" to %s: user pk=%s does not exist',

common/management/commands/generate_fake_data.py

@@ -1,3 +1,4 @@
+import logging
 import random
 
 from django.core.management.base import BaseCommand
@@ -7,7 +8,7 @@ from common.tests.factories.files import FileFactory
 from common.tests.factories.teams import TeamFactory
 from files.models import File
 from constants.version_permissions import VERSION_PERMISSION_FILE, VERSION_PERMISSION_NETWORK
-from constants.licenses import LICENSE_GPL2
+from constants.licenses import LICENSE_GPL2, LICENSE_GPL3
 from extensions.models import Extension, Tag
 
 FILE_SOURCES = {
@@ -54,12 +55,22 @@ to setup the `addon preferences`.
 
 ...
 '''
+LICENSES = (LICENSE_GPL2.id, LICENSE_GPL3.id)
 
 
 class Command(BaseCommand):
     help = 'Generate fake data with extensions, users and versions using test factories.'
 
     def handle(self, *args, **options):
+        verbosity = int(options['verbosity'])
+        root_logger = logging.getLogger('root')
+        if verbosity > 2:
+            root_logger.setLevel(logging.DEBUG)
+        elif verbosity > 1:
+            root_logger.setLevel(logging.INFO)
+        else:
+            root_logger.setLevel(logging.WARNING)
+
         tags = {
             type_id: list(Tag.objects.filter(type=type_id).values_list('name', flat=True))
             for type_id, _ in Extension.TYPES
@@ -100,7 +111,7 @@ class Command(BaseCommand):
         # Create a few publicly listed extensions
         for i in range(10):
             extension__type = random.choice(Extension.TYPES)[0]
-            create_approved_version(
+            version = create_approved_version(
                 file__status=File.STATUSES.APPROVED,
                 # extension__status=Extension.STATUSES.APPROVED,
                 extension__type=extension__type,
@@ -116,16 +127,20 @@ class Command(BaseCommand):
                     )
                 ],
             )
+            for i in range(random.randint(1, len(LICENSES))):
+                version.licenses.add(LICENSES[i])
 
         # Create a few unlisted extension versions
         for i in range(5):
             extension__type = random.choice(Extension.TYPES)[0]
-            create_version(
+            version = create_version(
                 file__status=random.choice(
                     (File.STATUSES.DISABLED, File.STATUSES.DISABLED_BY_AUTHOR)
                 ),
                 tags=random.sample(tags[extension__type], k=1),
             )
+            for i in range(random.randint(1, len(LICENSES))):
+                version.licenses.add(LICENSES[i])
 
         example_version.extension.average_score = 5.0
         example_version.extension.save(update_fields={'average_score'})

common/model_mixins.py

@@ -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
 
@@ -59,19 +59,23 @@ class TrackChangesMixin(models.Model):
 
     track_changes_to_fields: Set[str]
 
-    def _compare(self, old_instance: object) -> bool:
+    def _was_modified(self, old_instance: object, update_fields=None) -> bool:
-        """Returns True if model fields have changed.
+        """Returns True if the record is modified.
 
         Only checks fields listed in self.track_changes_to_fields.
         """
         for field in self.track_changes_to_fields:
+            # If update_fields was given and this field was NOT in it,
+            # its value definitely won't be changed:
+            if update_fields is not None and field not in update_fields:
+                continue
             old_val = getattr(old_instance, field, ...)
             new_val = getattr(self, field, ...)
             if old_val != new_val:
                 return True
         return False
 
-    def pre_save_record(self) -> Tuple[bool, OldStateType]:
+    def pre_save_record(self, *args, **kwargs) -> Tuple[bool, OldStateType]:
         """Tracks the previous state of this object.
 
         Only records fields listed in self.track_changes_to_fields.
@@ -86,7 +90,8 @@ class TrackChangesMixin(models.Model):
         except type(self).DoesNotExist:
             return True, {}
 
-        was_modified = self._compare(db_instance)
+        update_fields = kwargs.get('update_fields')
+        was_modified = self._was_modified(db_instance, update_fields=update_fields)
         old_instance_data = {
             attr: copy.deepcopy(getattr(db_instance, attr)) for attr in self.track_changes_to_fields
         }
@@ -147,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:
-            self.date_deleted = timezone.now()
-            self.save()
-            if hasattr(self, 'file'):
-                # .file should always exist but we don't want to break delete regardless
-                self.file.delete()
-
-        logger.warning('%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.warning('%r pk=%r deleted', self.__class__, self.pk)

common/static/common/styles/main.sass

@@ -40,6 +40,12 @@ $container-width: map-get($container-max-widths, 'xl')
   +media-xs
     width: 60px
 
+/* Temporarily here until it can be moved to web-assets v2. */
+.nav-global-links-right
+  gap: 0 var(--spacer-2)
+  .navbar-search
+    margin: 0
+
 .navbar-search
   width: 160px
 

common/templates/common/base.html

@@ -135,7 +135,7 @@
             {% if user.is_authenticated %}
               <li class="nav-item dropdown">
                 <button id="navbarDropdown" aria-expanded="false" aria-haspopup="true" data-toggle-menu-id="nav-account-dropdown" role="button" class="nav-link dropdown-toggle js-dropdown-toggle">
-                  <span>{% include "users/components/profile_display.html" with user=user %}</span>
+                  <i class="i-user"></i>
                   <i class="i-chevron-down"></i>
                 </button>
                 <ul id="nav-account-dropdown" aria-labelledby="navbarDropdown" class="dropdown-menu dropdown-menu-right js-dropdown-menu">

common/templatetags/common.py

@@ -1,10 +1,7 @@
-from urllib.parse import urljoin, urlparse
+from urllib.parse import urlparse
 import json
 import logging
-import os
 
-from django.conf import settings
-from django.contrib.sites.shortcuts import get_current_site
 from django.template import Library, loader
 from django.template.defaultfilters import stringfilter
 from django.utils.safestring import mark_safe
@@ -23,47 +20,11 @@ register = Library()
 logger = logging.getLogger(__name__)
 
 
-@register.filter
-def absolutify(url: str, request=None) -> str:
-    """Return an absolute URL."""
-    if url and url.startswith(('http://', 'https://')):
-        return url
-
-    proto = 'http' if settings.DEBUG else 'https'
-
-    return urljoin(f'{proto}://', get_current_site(request).domain, url)
-
-
 @register.simple_tag(takes_context=True)
 def absolute_url(context, path: str) -> str:
     """Return an absolute URL of a given path."""
     request = context.get('request')
-    return absolutify(path, request=request)
+    return utils.absolutify(path, request=request)
-
-
-# A (temporary?) copy of this is in services/utils.py. See bug 1055654.
-def user_media_path(what):
-    """Make it possible to override storage paths in settings.
-
-    By default, all storage paths are in the MEDIA_ROOT.
-
-    This is backwards compatible.
-
-    """
-    default = os.path.join(settings.MEDIA_ROOT, what)
-    key = f'{what.upper()}_PATH'
-    return getattr(settings, key, default)
-
-
-# A (temporary?) copy of this is in services/utils.py. See bug 1055654.
-def user_media_url(what):
-    """
-    Generate default media url, and make possible to override it from
-    settings.
-    """
-    default = f'{settings.MEDIA_URL}{what}/'
-    key = '{}_URL'.format(what.upper().replace('-', '_'))
-    return getattr(settings, key, default)
 
 
 class PaginationRenderer:

common/tests/factories/extensions.py

@@ -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

common/tests/factories/users.py

@@ -1,6 +1,7 @@
 import random
 
 from django.contrib.auth import get_user_model
+from django.contrib.auth.models import Group
 from factory.django import DjangoModelFactory
 import factory
 
@@ -22,6 +23,8 @@ class OAuthUserTokenFactory(DjangoModelFactory):
     class Meta:
         model = OAuthToken
 
+    oauth_user_id = factory.Sequence(lambda n: n + 899999)
+
     user = factory.SubFactory('common.tests.factories.users.UserFactory')
 
 
@@ -40,3 +43,10 @@ class UserFactory(DjangoModelFactory):
 
     oauth_tokens = factory.RelatedFactoryList(OAuthUserTokenFactory, factory_related_name='user')
     oauth_info = factory.RelatedFactory(OAuthUserInfoFactory, factory_related_name='user')
+
+
+def create_moderator():
+    user = UserFactory()
+    moderators = Group.objects.get(name='moderators')
+    user.groups.add(moderators)
+    return user

common/views/errors.py

@@ -22,11 +22,3 @@ class ErrorView(TemplateView):
         response = self.render_to_response(context)
         response.status_code = self.status
         return response
-
-
-def csrf_failure(request, reason=''):
-    import django.views.csrf
-
-    return django.views.csrf.csrf_failure(
-        request, reason=reason, template_name='errors/403_csrf.html'
-    )

constants/activity.py

@@ -0,0 +1,18 @@
+class Verb:
+    """These constants are used to dispatch Action records,
+    changing the values will result in a mismatch with historical values stored in db.
+    """
+
+    APPROVED = 'approved'
+    COMMENTED = 'commented'
+    RATED_EXTENSION = 'rated extension'
+    REPORTED_EXTENSION = 'reported extension'
+    REPORTED_RATING = 'reported rating'
+    REQUESTED_CHANGES = 'requested changes'
+    REQUESTED_REVIEW = 'requested review'
+
+
+class Flag:
+    AUTHOR = 'author'
+    MODERATOR = 'moderator'
+    REVIEWER = 'reviewer'

constants/base.py

@@ -58,8 +58,13 @@ EXTENSION_TYPE_PLURAL = {
     EXTENSION_TYPE_CHOICES.THEME: _('Themes'),
 }
 EXTENSION_SLUGS_PATH = '|'.join(EXTENSION_TYPE_SLUGS.values())
+EXTENSION_SLUG_TYPES = {v: k for k, v in EXTENSION_TYPE_SLUGS_SINGULAR.items()}
 
-VALID_SOURCE_EXTENSIONS = ('zip',)
+ALLOWED_EXTENSION_MIMETYPES = ('application/zip', )
+# FIXME: this controls the initial widget rendered server-side, and server-side validation
+# but not the additional JS-appended preview file inputs.
+# If this list changes, the "accept" attribute also has to be updated in appendImageUploadForm.
+ALLOWED_PREVIEW_MIMETYPES = ('image/jpg', 'image/jpeg', 'image/png', 'image/webp', 'video/mp4')
 
 # Rating scores
 RATING_SCORE_CHOICES = Choices(
@@ -88,10 +93,10 @@ TEAM_ROLE_CHOICES = (
 # Abuse
 ABUSE_TYPE_EXTENSION = 1
 ABUSE_TYPE_USER = 2
-ABUSE_TYPE_REVIEW = 3
+ABUSE_TYPE_RATING = 3
 
 ABUSE_TYPE = Choices(
     ('ABUSE_EXTENSION', ABUSE_TYPE_EXTENSION, "Extension"),
     ('ABUSE_USER', ABUSE_TYPE_USER, "User"),
-    ('ABUSE_REVIEW', ABUSE_TYPE_REVIEW, "Review"),
+    ('ABUSE_RATING', ABUSE_TYPE_RATING, "Rating"),
 )

extensions/admin.py

@@ -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',
@@ -184,8 +182,8 @@ class VersionAdmin(admin.ModelAdmin):
 
 class MaintainerAdmin(admin.ModelAdmin):
     model = Maintainer
-    list_display = ('extension', 'user', 'date_deleted')
+    list_display = ('extension', 'user')
-    readonly_fields = ('extension', 'user', 'date_deleted')
+    readonly_fields = ('extension', 'user')
 
 
 class LicenseAdmin(admin.ModelAdmin):

extensions/apps.py

@@ -6,4 +6,7 @@ class ExtensionsConfig(AppConfig):
     name = 'extensions'
 
     def ready(self):
+        from actstream import registry
         import extensions.signals  # noqa: F401
+
+        registry.register(self.get_model('Extension'))

extensions/forms.py

@@ -1,6 +1,10 @@
 import logging
 
 from django import forms
+from django.utils.translation import gettext_lazy as _
+
+from files.validators import FileMIMETypeValidator
+from constants.base import ALLOWED_PREVIEW_MIMETYPES
 
 import extensions.models
 import files.models
@@ -33,13 +37,25 @@ EditPreviewFormSet = forms.inlineformset_factory(
 
 
 class AddPreviewFileForm(forms.ModelForm):
+    msg_unexpected_file_type = _('Choose a JPEG, PNG or WebP image, or an MP4 video')
+
     class Meta:
         model = files.models.File
         fields = ('caption', 'source')
-        widgets = {
-            'source': forms.ClearableFileInput(attrs={'accept': 'image/*,video/*'}),
-        }
 
+    source = forms.FileField(
+        allow_empty_file=False,
+        required=True,
+        validators=[
+            FileMIMETypeValidator(
+                allowed_mimetypes=ALLOWED_PREVIEW_MIMETYPES,
+                message=msg_unexpected_file_type,
+            ),
+        ],
+        widget=forms.ClearableFileInput(
+            attrs={'accept': ','.join(ALLOWED_PREVIEW_MIMETYPES)},
+        ),
+    )
     caption = forms.CharField(max_length=255, required=False)
 
     def __init__(self, *args, **kwargs):
@@ -60,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
@@ -109,6 +122,12 @@ class ExtensionUpdateForm(forms.ModelForm):
         )
 
 
+class ExtensionDeleteForm(forms.ModelForm):
+    class Meta:
+        model = extensions.models.Extension
+        fields = []
+
+
 class VersionForm(forms.ModelForm):
     class Meta:
         model = extensions.models.Version
@@ -129,7 +148,7 @@ class VersionForm(forms.ModelForm):
         return self.initial['file']
 
 
-class DeleteViewForm(forms.ModelForm):
+class VersionDeleteForm(forms.ModelForm):
     class Meta:
         model = extensions.models.Version
         fields = []

extensions/migrations/0026_remove_extension_date_deleted_and_more.py

@@ -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',
+        ),
+    ]

extensions/models.py

@@ -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,36 +106,27 @@ 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(
+        return self.filter(maintainer__user_id=user_id)
-            maintainer__user_id=user_id, maintainer__date_deleted__isnull=True
-        )
 
     def listed_or_authored_by(self, user_id: int):
-        return self.exclude_deleted.filter(
+        return self.filter(
-            Q(status=self.model.STATUSES.APPROVED)
+            Q(status=self.model.STATUSES.APPROVED) | Q(maintainer__user_id=user_id)
-            | Q(maintainer__user_id=user_id, maintainer__date_deleted__isnull=True)
         ).distinct()
 
 
-class Extension(
+class Extension(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Model):
-    CreatedModifiedMixin, RatingMixin, TrackChangesMixin, SoftDeleteMixin, models.Model
-):
     track_changes_to_fields = {
         'status',
         'name',
@@ -175,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)
 
@@ -195,6 +185,10 @@ class Extension(
     def type_slug(self) -> str:
         return EXTENSION_TYPE_SLUGS[self.type]
 
+    @property
+    def status_slug(self) -> str:
+        return utils.slugify(EXTENSION_STATUS_CHOICES[self.status - 1][1])
+
     def clean(self) -> None:
         if not self.slug:
             self.slug = utils.slugify(self.name)
@@ -230,15 +224,35 @@ class Extension(
         self.status = self.STATUSES.APPROVED
         self.save()
 
+    @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])
 
+    def get_draft_url(self):
+        return reverse('extensions:draft', args=[self.type_slug, self.slug])
+
     def get_manage_url(self):
         return reverse('extensions:manage', args=[self.type_slug, self.slug])
 
     def get_manage_versions_url(self):
         return reverse('extensions:manage-versions', args=[self.type_slug, self.slug])
 
+    def get_delete_url(self):
+        return reverse('extensions:delete', args=[self.type_slug, self.slug])
+
     def get_new_version_url(self):
         return reverse('extensions:new-version', args=[self.type_slug, self.slug])
 
@@ -276,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,
             )
@@ -293,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
@@ -303,14 +316,9 @@ class Extension(
 
     def can_request_review(self):
         """Return whether an add-on can request a review or not."""
-        if (
+        if self.is_disabled or self.status in (
-            self.is_deleted
-            or self.is_disabled
-            or self.status
-            in (
             self.STATUSES.APPROVED,
             self.STATUSES.AWAITING_REVIEW,
-            )
         ):
             return False
 
@@ -319,7 +327,7 @@ class Extension(
         return latest_version is not None and not latest_version.file.reviewed
 
     @property
-    def is_approved(self) -> str:
+    def is_approved(self) -> bool:
         return self.status == self.STATUSES.APPROVED
 
     @property
@@ -341,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(
+        return self.authors.filter(maintainer__user_id=user.pk).exists()
-            maintainer__user_id=user.pk, maintainer__date_deleted__isnull=True
-        ).exists()
 
     def can_rate(self, user) -> bool:
         """Return True if given user can rate this extension.
@@ -411,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
@@ -438,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',
@@ -565,18 +566,20 @@ class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, SoftDeleteMi
     def __str__(self) -> str:
         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
+        # To be public, version file must have a public status.
-        # extension, and its attached file must have a public status.
+        return self.file is not None and self.file.status == self.file.STATUSES.APPROVED
-        try:
+
-            return (
+    @property
-                not self.is_deleted
+    def cannot_be_deleted_reasons(self) -> List[str]:
-                and self.extension.is_listed
+        """Return a list of reasons why this version cannot be deleted."""
-                and self.file is not None
+        reasons = []
-                and self.file.status == self.file.STATUSES.APPROVED
+        if self.is_listed:
-            )
+            reasons.append('version_is_listed')
-        except models.ObjectDoesNotExist:
+        if self.ratings.count() > 0:
-            return False
+            reasons.append('version_has_ratings')
+        return reasons
 
     @property
     def pending_rejection(self):
@@ -636,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)
@@ -660,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(

extensions/signals.py

@@ -1,20 +1,42 @@
 from typing import Union
+import logging
 
-from django.db.models.signals import pre_save, post_save, post_delete
+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, post_delete
 from django.dispatch import receiver
-import django.dispatch
 
+from constants.activity import Flag
 import extensions.models
-import extensions.tasks
 import files.models
 
-version_changed = django.dispatch.Signal()
+logger = logging.getLogger(__name__)
-version_uploaded = django.dispatch.Signal()
+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:
+@receiver(post_delete, sender=extensions.models.Version)
-    instance.file.delete()
+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)
@@ -22,9 +44,10 @@ def _delete_file(sender: object, instance: extensions.models.Preview, **kwargs:
 def _record_changes(
     sender: object,
     instance: Union[extensions.models.Extension, extensions.models.Version],
+    update_fields: object,
     **kwargs: object,
 ) -> None:
-    was_changed, old_state = instance.pre_save_record()
+    was_changed, old_state = instance.pre_save_record(update_fields=update_fields)
 
     if hasattr(instance, 'name'):
         instance.sanitize('name', was_changed, old_state, **kwargs)
@@ -39,15 +62,10 @@ def _update_search_index(sender, instance, **kw):
     pass  # TODO: update search index
 
 
-def send_notifications(sender=None, instance=None, signal=None, **kw):
-    pass  # TODO: send email notification about new version upload
-
-
 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
     )
 
@@ -82,8 +100,69 @@ 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()
 
 
-version_uploaded.connect(send_notifications, dispatch_uid='send_notifications')
+@receiver(post_save, sender=extensions.models.Extension)
+def _setup_followers(
+    sender: object,
+    instance: extensions.models.Extension,
+    created: bool,
+    **kwargs: object,
+) -> None:
+    if not created:
+        return
+
+    for user in instance.authors.all():
+        follow(user, instance, send_action=False, flag=Flag.AUTHOR)
+    for user in Group.objects.get(name='moderators').user_set.all():
+        follow(user, instance, send_action=False, flag=Flag.MODERATOR)
+
+
+@receiver(m2m_changed, sender=extensions.models.Extension.authors.through)
+def _update_authors_follow(instance, action, model, reverse, pk_set, **kwargs):
+    if action not in ['post_add', 'post_remove']:
+        return
+
+    if model == extensions.models.Extension and not reverse:
+        targets = extensions.models.Extension.objects.filter(pk__in=pk_set)
+        users = [instance]
+    else:
+        targets = [instance]
+        users = User.objects.filter(pk__in=pk_set)
+
+    for user in users:
+        for extension in targets:
+            if action == 'post_remove':
+                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'})

extensions/static/extensions/scripts/manage.js

@@ -41,7 +41,7 @@ function appendImageUploadForm() {
         <input class="js-input-img-caption form-control" id="${formsetPrefix}-${i}-caption" type="text" maxlength="255" name="${formsetPrefix}-${i}-caption" placeholder="Describe the preview">
       </div>
       <div class="align-items-center d-flex justify-content-between">
-        <input accept="image/*" class="form-control js-input-img" id="id_${formsetPrefix}-${i}-source" type="file" name="${formsetPrefix}-${i}-source">
+        <input accept="image/jpg,image/jpeg,image/png,image/webp,video/mp4" class="form-control js-input-img" id="id_${formsetPrefix}-${i}-source" type="file" name="${formsetPrefix}-${i}-source">
         <ul class="pt-0">
           <li>
             <button class="btn btn-link btn-sm js-btn-reset-img-upload-form ps-2 pe-0"><i class="i-refresh"></i> Reset</button>

extensions/templates/extensions/confirm_delete.html

@@ -0,0 +1,31 @@
+{% extends "common/base.html" %}
+{% load i18n %}
+{% block content %}
+  <div class="row">
+    <div class="col col-md-8 mx-auto my-4">
+      <div class="box">
+        <h2>
+          {% blocktranslate with extension_name=extension_name %} Delete {{ extension_name }}?{% endblocktranslate %}
+        </h2>
+        <p>
+          {% blocktranslate with extension_name=extension_name %}
+            All extension previews and versions files will be deleted.
+          {% endblocktranslate %}
+        </p>
+        <div class="btn-row-fluid">
+          <a href="#" class="btn js-btn-back">
+            <i class="i-cancel"></i>
+            <span>{% trans 'Cancel' %}</span>
+          </a>
+          <form method="post">
+          {% csrf_token %}
+            <button type="submit" class="btn btn-block btn-danger">
+              <i class="i-trash"></i>
+              <span>{% trans 'Confirm Deletion' %}</span>
+            </button>
+          </form>
+        </div>
+      </div>
+    </div>
+  </div>
+{% endblock content %}

extensions/templates/extensions/detail.html

@@ -5,6 +5,12 @@
 {% block page_title %}{{ extension.name }}{% endblock page_title %}
 
 {% block content %}
+  {% if extension.latest_version %}
+    {% with latest=extension.latest_version %}
+      {% include "files/components/scan_details.html" with file=latest.file %}
+    {% endwith %}
+  {% endif %}
+
   {% has_maintainer extension as is_maintainer %}
   {% with latest=extension.latest_version %}
 
@@ -58,7 +64,7 @@
 
       {# Permissions #}
       {% block extension_permissions %}
-      {% if extension.get_type_display|lower == 'add-on' %}
+      {% if extension.type_slug == 'add-on' %}
       <hr class="my-4">
       <section id="permissions" class="ext-detail-permissions">
         <h2 class="mb-3">{% trans "Permissions" %}</h2>

extensions/templates/extensions/draft_finalise.html

@@ -82,10 +82,23 @@
 
                   <section class="card p-3 mt-3">
                     <div class="btn-col">
-                      <button type="submit" class="btn btn-primary">
+                      <button type="submit" name="save_draft" class="btn btn-primary btn-warning">
                         <i class="i-check"></i>
                         <span>{% trans 'Save Draft' %}</span>
                       </button>
+                      <button type="submit" name="submit_draft" class="btn btn-primary">
+                        <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>

extensions/templates/extensions/manage/update.html

@@ -100,21 +100,12 @@
                 <button id="btn-save" type="submit" class="btn btn-primary">
                   <i class="i-check"></i>
                   <span>
-                  {% if extenson.is_approved %}
-                  {% trans 'Save Draft' %}
-                  {% else %}
                   {% trans 'Save Changes' %}
-                  {% endif %}
                   </span>
                 </button>
 
                 <hr>
 
-                {% if extension.status == extension. %}
-                  <button type="submit" class="btn btn-block disabled" disabled>{% trans 'Mark ready for review' %}</button>
-                {% elif 'Incomplete' != extension.get_status_display %}
-                {% endif %}
-
                 <a href="{{ extension.get_new_version_url }}" class="btn">
                   <i class="i-upload"></i>
                   <span>{% trans 'Upload New Version' %}</span>
@@ -124,6 +115,11 @@
                   <span>{% trans 'Version History' %}</span>
                 </a>
 
+                <a href="{{ extension.get_delete_url }}" class="btn btn-danger">
+                  <i class="i-trash"></i>
+                  <span>{% trans 'Delete Extension' %}</span>
+                </a>
+
                 {% if request.user.is_staff %}
                   <a href="{% url 'admin:extensions_extension_change' extension.pk %}" class="btn btn-admin">
                     <span>Admin</span>

extensions/templates/extensions/version_confirm_delete.html

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

extensions/templates/extensions/version_list.html

@@ -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>

extensions/tests/files/invalid-archive.zip

@@ -0,0 +1 @@
+wat

extensions/tests/test_delete.py

@@ -0,0 +1,114 @@
+from django.test import TestCase
+
+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_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()
+        self.client.force_login(user)
+        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()
+        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
+
+        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()

extensions/tests/test_manifest.py

@@ -20,10 +20,10 @@ import toml
 
 META_DATA = {
     "id": "an_id",
-    "name": "Theme",
+    "name": "A random add-on",
-    "tagline": "A theme",
+    "tagline": "An add-on",
     "version": "0.1.0",
-    "type": "theme",
+    "type": "add-on",
     "license": [LICENSE_GPL3.slug],
     "blender_version_min": "4.2.0",
     "blender_version_max": "4.2.0",
@@ -61,7 +61,7 @@ class CreateFileTest(TestCase):
             extension__extension_id=extension_id,
             version='0.1.5-alpha+f52258de',
             file=FileFactory(
-                type=File.TYPES.THEME,
+                type=File.TYPES.BPY,
                 status=File.STATUSES.APPROVED,
             ),
         )
@@ -74,6 +74,14 @@ class CreateFileTest(TestCase):
 
         version = combined_meta_data.get("version", "0.1.0")
         extension_id = combined_meta_data.get("id", "foobar").strip()
+        type_slug = combined_meta_data['type']
+        init_path = None
+
+        if type_slug == 'add-on':
+            # Add the required __init__.py file
+            init_path = os.path.join(self.temp_directory, '__init__.py')
+            with open(init_path, 'w') as init_file:
+                init_file.write('')
 
         with open(manifest_path, "w") as manifest_file:
             toml.dump(combined_meta_data, manifest_file)
@@ -81,6 +89,10 @@ class CreateFileTest(TestCase):
         with zipfile.ZipFile(output_path, "w") as my_zip:
             arcname = f"{extension_id}-{version}/{os.path.basename(manifest_path)}"
             my_zip.write(manifest_path, arcname=arcname)
+            if init_path:
+                # Write the __init__.py file too
+                arcname = f"{extension_id}-{version}/{os.path.basename(init_path)}"
+                my_zip.write(init_path, arcname=arcname)
 
         os.remove(manifest_path)
         return output_path
@@ -105,14 +117,9 @@ class ValidateManifestTest(CreateFileTest):
             )
 
         self.assertEqual(response.status_code, 200)
-        self.assertDictEqual(
+        error = response.context['form'].errors.get('source')[0]
-            response.context['form'].errors,
+        self.assertIn('id-with-hyphens', error)
-            {
+        self.assertIn('No hyphens', error)
-                'source': [
-                    'Invalid id from extension manifest: "id-with-hyphens". No hyphens are allowed.'
-                ]
-            },
-        )
 
     def test_validation_manifest_extension_id_spaces(self):
         self.assertEqual(Extension.objects.count(), 0)
@@ -130,14 +137,8 @@ class ValidateManifestTest(CreateFileTest):
             )
 
         self.assertEqual(response.status_code, 200)
-        self.assertDictEqual(
+        error = response.context['form'].errors.get('source')[0]
-            response.context['form'].errors,
+        self.assertIn('"id with spaces"', error)
-            {
-                'source': [
-                    'Invalid id from extension manifest: "id with spaces". Use a valid id consisting of Unicode letters, numbers or underscores.'
-                ]
-            },
-        )
 
     def test_validation_manifest_extension_id_clash(self):
         """Test if we add two extensions with the same extension_id"""
@@ -161,14 +162,9 @@ class ValidateManifestTest(CreateFileTest):
             )
 
         self.assertEqual(response.status_code, 200)
-        self.assertDictEqual(
+        error = response.context['form'].errors.get('source')[0]
-            response.context['form'].errors,
+        self.assertIn('blender_kitsu', error)
-            {
+        self.assertIn('already being used', error)
-                'source': [
-                    'The extension id in the manifest ("blender_kitsu") is already being used by another extension.'
-                ]
-            },
-        )
 
     def test_validation_manifest_extension_id_mismatch(self):
         """Test if we try to add a new version to an extension with a mismatched extension_id"""
@@ -192,14 +188,10 @@ class ValidateManifestTest(CreateFileTest):
             )
 
         self.assertEqual(response.status_code, 200)
-        self.assertDictEqual(
+        error = response.context['form'].errors.get('source')[0]
-            response.context['form'].errors,
+        self.assertIn('non_kitsu', error)
-            {
+        self.assertIn('blender_kitsu', error)
-                'source': [
+        self.assertIn('doesn\'t match the expected one', error)
-                    'The extension id in the manifest ("non_kitsu") doesn\'t match the expected one for this extension ("blender_kitsu").'
-                ]
-            },
-        )
 
     def test_validation_manifest_extension_id_repeated_version(self):
         """Test if we try to add a version to an extension without changing the version number"""
@@ -268,6 +260,53 @@ class ValidateManifestTest(CreateFileTest):
 
         self.assertEqual(response.status_code, 302)
 
+    def test_validation_manifest_unsafe_id_hyphen(self):
+        """Make sure the error message for id_hyphen is sane
+
+        This is not a particular important check, but it uses the same logic of other parts of the code.
+        So I'm using this test as a way to make sure all this logic is sound
+        """
+        self.assertEqual(Extension.objects.count(), 0)
+        user = UserFactory()
+        self.client.force_login(user)
+
+        file_data = {
+            "id": "id-with-hyphens",
+        }
+
+        bad_file = self._create_file_from_data("theme.zip", file_data, self.user)
+        with open(bad_file, 'rb') as fp:
+            response = self.client.post(
+                self._get_submit_url(), {'source': fp, 'agreed_with_terms': True}
+            )
+
+        self.assertEqual(response.status_code, 200)
+        error = response.context['form'].errors.get('source')
+        self.assertEqual(len(error), 1)
+        self.assertIn('"id-with-hyphens"', error[0])
+
+    def test_name_left_as_is(self):
+        user = UserFactory()
+        self.client.force_login(user)
+
+        file_data = {
+            # If we ever need to restrict content of Extension's name,
+            # it should be done at the manifest validation step.
+            "name": "Name. - With  Extra spaces and other characters Ж",
+        }
+
+        extension_file = self._create_file_from_data("theme.zip", file_data, self.user)
+        with open(extension_file, 'rb') as fp:
+            response = self.client.post(
+                self._get_submit_url(), {'source': fp, 'agreed_with_terms': True}
+            )
+
+        self.assertEqual(response.status_code, 302)
+        file = File.objects.first()
+        extension = file.extension
+        self.assertEqual(extension.slug, 'an-id')
+        self.assertEqual(extension.name, 'Name. - With  Extra spaces and other characters Ж')
+
 
 class ValidateManifestFields(TestCase):
     fixtures = ['licenses', 'version_permissions']
@@ -368,7 +407,7 @@ class ValidateManifestFields(TestCase):
         self.assertEqual(
             e.exception.messages,
             [
-                'Manifest value error: blender_version_min should be at least "4.2.0"',
+                'Manifest value error: <code>blender_version_min</code> should be at least "4.2.0"',
             ],
         )
 
@@ -378,7 +417,7 @@ class ValidateManifestFields(TestCase):
         self.assertEqual(
             e.exception.messages,
             [
-                'Manifest value error: blender_version_min should be at least "4.2.0"',
+                'Manifest value error: <code>blender_version_min</code> should be at least "4.2.0"',
             ],
         )
 
@@ -395,8 +434,9 @@ class ValidateManifestFields(TestCase):
         with self.assertRaises(ValidationError) as e:
             ManifestValidator(data)
 
-        message_begin = "Manifest value error: license expects a list of supported licenses, e.g.: ['SPDX:GPL-2.0-or-later']. Visit"
+        message_begin = "Manifest value error: <code>license</code> expects a list of"
         self.assertIn(message_begin, e.exception.messages[0])
+        self.assertIn('[\'SPDX:GPL-2.0-or-later\']', e.exception.messages[0])
 
         data['license'] = ['SPDX:GPL-2.0-only']
         with self.assertRaises(ValidationError) as e:
@@ -417,8 +457,9 @@ class ValidateManifestFields(TestCase):
         with self.assertRaises(ValidationError) as e:
             ManifestValidator(data)
 
-        message_begin = "Manifest value error: tags expects a list of supported add-on tags, e.g.: ['Animation', 'Sequencer']. Visit"
+        message_begin = "Manifest value error: <code>tags</code> expects a list of"
         self.assertIn(message_begin, e.exception.messages[0])
+        self.assertIn('[\'Animation\', \'Sequencer\']', e.exception.messages[0])
 
         data['tags'] = ['Dark']
         with self.assertRaises(ValidationError) as e:
@@ -430,6 +471,7 @@ class ValidateManifestFields(TestCase):
             **self.mandatory_fields,
             **self.optional_fields,
         }
+        data.pop('permissions')
         data['type'] = 'theme'
 
         data['tags'] = ['Light']
@@ -439,8 +481,9 @@ class ValidateManifestFields(TestCase):
         with self.assertRaises(ValidationError) as e:
             ManifestValidator(data)
 
-        message_begin = "Manifest value error: tags expects a list of supported theme tags, e.g.: ['Dark', 'Accessibility']. Visit"
+        message_begin = "Manifest value error: <code>tags</code> expects a list of"
         self.assertIn(message_begin, e.exception.messages[0])
+        self.assertIn('[\'Dark\', \'Accessibility\']', e.exception.messages[0])
 
         data['tags'] = ['Render']
         with self.assertRaises(ValidationError) as e:
@@ -463,22 +506,30 @@ class ValidateManifestFields(TestCase):
         with self.assertRaises(ValidationError) as e:
             ManifestValidator(data)
 
-        message_begin = "Manifest value error: permissions expects a list of supported permissions, e.g.: ['files', 'network']. The supported permissions are: "
+        message_begin = "Manifest value error: <code>permissions</code> expects a list of"
         self.assertIn(message_begin, e.exception.messages[0])
+        self.assertIn('[\'files\', \'network\']', e.exception.messages[0])
 
-        # Check the licenses that will always be around.
+        # Check the permissions that will always be around.
         message_end = e.exception.messages[0][len(message_begin) :]
         self.assertIn('files', message_end)
         self.assertIn('network', message_end)
 
-        # Check for any other license that we may add down the line.
+        # Check for any other permission that we may add down the line.
         for permission in VersionPermission.objects.all():
             self.assertIn(permission.slug, message_end)
 
         data['permissions'] = []
         with self.assertRaises(ValidationError) as e:
             ManifestValidator(data)
+        self.assertEqual(1, len(e.exception.messages))
 
+        # Make sure permissions are only defined for add-ons, but fail for themes.
+        data['permissions'] = ['files']
+        data['type'] = EXTENSION_TYPE_SLUGS_SINGULAR[EXTENSION_TYPE_CHOICES.THEME]
+        data['tags'] = []
+        with self.assertRaises(ValidationError) as e:
+            ManifestValidator(data)
         self.assertEqual(1, len(e.exception.messages))
 
     def test_tagline(self):
@@ -516,6 +567,7 @@ class ValidateManifestFields(TestCase):
             **self.mandatory_fields,
             **self.optional_fields,
         }
+        data.pop('permissions')
 
         # Good cops.
         data['type'] = EXTENSION_TYPE_SLUGS_SINGULAR[EXTENSION_TYPE_CHOICES.BPY]
@@ -539,6 +591,22 @@ class ValidateManifestFields(TestCase):
             ManifestValidator(data)
             self.assertEqual(1, len(e.exception.messages))
 
+    def test_schema_version(self):
+        data = {
+            **self.mandatory_fields,
+            **self.optional_fields,
+        }
+
+        data['schema_version'] = "1.0.0"
+        ManifestValidator(data)
+
+        data['schema_version'] = "0.0.9"
+        with self.assertRaises(ValidationError) as e:
+            ManifestValidator(data)
+        self.assertEqual(1, len(e.exception.messages))
+        self.assertIn('0.0.9', e.exception.messages[0])
+        self.assertIn('not supported', e.exception.messages[0])
+
 
 class VersionPermissionsTest(CreateFileTest):
     fixtures = ['licenses', 'version_permissions']

extensions/tests/test_submit.py

@@ -29,6 +29,7 @@ EXPECTED_EXTENSION_DATA = {
         'size_bytes': 53959,
         'tags': ['Sequencer'],
         'version_str': '0.1.0',
+        'slug': 'edit-breakdown',
     },
     'blender_gis-2.2.8.zip': {
         'metadata': {
@@ -43,6 +44,7 @@ EXPECTED_EXTENSION_DATA = {
         'size_bytes': 434471,
         'tags': ['3D View'],
         'version_str': '2.2.8',
+        'slug': 'blender-gis',
     },
     'amaranth-1.0.8.zip': {
         'metadata': {
@@ -57,8 +59,30 @@ EXPECTED_EXTENSION_DATA = {
         'size_bytes': 72865,
         'tags': [],
         'version_str': '1.0.8',
+        'slug': 'amaranth',
     },
 }
+EXPECTED_VALIDATION_ERRORS = {
+    'empty.txt': {'source': ['Only .zip files are accepted.']},
+    'empty.zip': {'source': ['The submitted file is empty.']},
+    'invalid-archive.zip': {'source': ['Only .zip files are accepted.']},
+    'invalid-manifest-path.zip': {
+        'source': [
+            'The manifest file should be at the top level of the archive, or one level deep.',
+        ],
+    },
+    'invalid-addon-no-init.zip': {
+        'source': ['An add-on should have an __init__.py file.'],
+    },
+    'invalid-addon-dir-no-init.zip': {
+        'source': ['An add-on should have an __init__.py file.'],
+    },
+    'invalid-no-manifest.zip': {
+        'source': ['The manifest file is missing.'],
+    },
+    'invalid-manifest-toml.zip': {'source': ['Could not parse the manifest file.']},
+    'invalid-theme-multiple-xmls.zip': {'source': ['A theme should have exactly one XML file.']},
+}
 
 
 class SubmitFileTest(TestCase):
@@ -75,6 +99,7 @@ class SubmitFileTest(TestCase):
         blender_version_min: str,
         size_bytes: int,
         file_hash: str,
+        slug: str,
         **other_metadata,
     ):
         self.assertEqual(File.objects.count(), 0)
@@ -88,6 +113,9 @@ class SubmitFileTest(TestCase):
         self.assertEqual(File.objects.count(), 1)
         file = File.objects.first()
         self.assertEqual(response['Location'], file.get_submit_url())
+        extension = file.extension
+        self.assertEqual(extension.slug, slug)
+        self.assertEqual(extension.name, name)
         self.assertEqual(file.original_name, file_name)
         self.assertEqual(file.size_bytes, size_bytes)
         self.assertEqual(file.original_hash, file_hash)
@@ -116,33 +144,28 @@ class SubmitFileTest(TestCase):
             {'agreed_with_terms': ['This field is required.']},
         )
 
-    def test_validation_errors_invalid_extension(self):
+    def test_validation_errors(self):
         self.assertEqual(Extension.objects.count(), 0)
         user = UserFactory()
         self.client.force_login(user)
 
-        with open(TEST_FILES_DIR / 'empty.txt', 'rb') as fp:
+        for test_archive, extected_errors in EXPECTED_VALIDATION_ERRORS.items():
+            with self.subTest(test_archive=test_archive):
+                with open(TEST_FILES_DIR / test_archive, 'rb') as fp:
                     response = self.client.post(self.url, {'source': fp, 'agreed_with_terms': True})
 
                 self.assertEqual(response.status_code, 200)
-        self.assertDictEqual(
+                self.assertDictEqual(response.context['form'].errors, extected_errors)
-            response.context['form'].errors,
-            {'source': ['File extension “txt” is not allowed. Allowed extensions are: zip.']},
-        )
 
-    def test_validation_errors_empty_file(self):
+    def test_addon_without_top_level_directory(self):
         self.assertEqual(Extension.objects.count(), 0)
         user = UserFactory()
         self.client.force_login(user)
 
-        with open(TEST_FILES_DIR / 'empty.zip', 'rb') as fp:
+        with open(TEST_FILES_DIR / 'addon-without-dir.zip', 'rb') as fp:
             response = self.client.post(self.url, {'source': fp, 'agreed_with_terms': True})
 
-        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.status_code, 302)
-        self.assertDictEqual(
-            response.context['form'].errors,
-            {'source': ['The submitted file is empty.']},
-        )
 
     def test_theme_file(self):
         self.assertEqual(File.objects.count(), 0)
@@ -224,12 +247,24 @@ class SubmitFinaliseTest(TestCase):
             hash=file_data['file_hash'],
             metadata=file_data['metadata'],
         )
+        self.version = create_version(
+            file=self.file,
+            extension__name=file_data['metadata']['name'],
+            extension__slug=file_data['metadata']['id'].replace("_", "-"),
+            extension__website=None,
+            tagline=file_data['metadata']['tagline'],
+            version=file_data['metadata']['version'],
+            blender_version_min=file_data['metadata']['blender_version_min'],
+            schema_version=file_data['metadata']['schema_version'],
+        )
 
     def test_get_finalise_addon_redirects_if_anonymous(self):
         response = self.client.post(self.file.get_submit_url(), {})
 
         self.assertEqual(response.status_code, 302)
-        self.assertEqual(response['Location'], f'/oauth/login?next=/submit/{self.file.pk}/')
+        self.assertEqual(
+            response['Location'], f'/oauth/login?next=/add-ons/{self.file.extension.slug}/draft/'
+        )
 
     def test_get_finalise_addon_not_allowed_if_different_user(self):
         user = UserFactory()
@@ -237,7 +272,9 @@ class SubmitFinaliseTest(TestCase):
 
         response = self.client.post(self.file.get_submit_url(), {})
 
-        self.assertEqual(response.status_code, 403)
+        # Technically this could (should) be a 403, but changing this means changing
+        # the MaintainedExtensionMixin which is used in multiple places.
+        self.assertEqual(response.status_code, 404)
 
     def test_post_finalise_addon_validation_errors(self):
         self.client.force_login(self.file.user)
@@ -258,8 +295,8 @@ class SubmitFinaliseTest(TestCase):
 
     def test_post_finalise_addon_creates_addon_with_version_awaiting_review(self):
         self.assertEqual(File.objects.count(), 1)
-        self.assertEqual(Extension.objects.count(), 0)
+        self.assertEqual(Extension.objects.count(), 1)
-        self.assertEqual(Version.objects.count(), 0)
+        self.assertEqual(Version.objects.count(), 1)
         self.assertEqual(File.objects.filter(type=File.TYPES.IMAGE).count(), 0)
 
         self.client.force_login(self.file.user)
@@ -279,6 +316,8 @@ class SubmitFinaliseTest(TestCase):
             'form-0-caption': ['First Preview Caption Text'],
             'form-1-id': '',
             'form-1-caption': ['Second Preview Caption Text'],
+            # Submit for Approval.
+            'submit_draft': '',
         }
         file_name1 = 'test_preview_image_0001.png'
         file_name2 = 'test_preview_image_0002.png'
@@ -313,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
@@ -357,7 +396,7 @@ class NewVersionTest(TestCase):
         self.assertEqual(response.status_code, 200)
         self.assertDictEqual(
             response.context['form'].errors,
-            {'source': ['File extension “txt” is not allowed. Allowed extensions are: zip.']},
+            {'source': ['Only .zip files are accepted.']},
         )
 
     def test_validation_errors_empty_file(self):

extensions/tests/test_update.py

@@ -100,7 +100,7 @@ class UpdateTest(TestCase):
             'form-TOTAL_FORMS': ['2'],
         }
         file_name1 = 'test_preview_image_0001.png'
-        file_name2 = 'test_preview_image_0002.png'
+        file_name2 = 'test_preview_image_0001.webp'
         url = extension.get_manage_url()
         user = extension.authors.first()
         self.client.force_login(user)
@@ -134,15 +134,15 @@ class UpdateTest(TestCase):
         )
         self.assertEqual(
             file2.original_hash,
-            'sha256:f8ef448d66e2506055e2586d4cb20dc8758b19cd6e8b052231fcbcad2e2be4b3',
+            'sha256:213648f19f0cc7ef8e266e87a0a7a66f0079eb80de50d539895466e645137616',
         )
         self.assertEqual(
-            file2.hash, 'sha256:f8ef448d66e2506055e2586d4cb20dc8758b19cd6e8b052231fcbcad2e2be4b3'
+            file2.hash, 'sha256:213648f19f0cc7ef8e266e87a0a7a66f0079eb80de50d539895466e645137616'
         )
         self.assertEqual(file1.original_name, file_name1)
         self.assertEqual(file2.original_name, file_name2)
         self.assertEqual(file1.size_bytes, 1163)
-        self.assertEqual(file2.size_bytes, 1693)
+        self.assertEqual(file2.size_bytes, 154)
         self.assertTrue(
             file1.source.url.startswith(
                 '/media/images/64/643e15eb6c4831173bbcf71b8c85efc70cf3437321bf2559b39aa5e9acfd5340',
@@ -151,10 +151,10 @@ class UpdateTest(TestCase):
         self.assertTrue(file1.source.url.endswith('.png'))
         self.assertTrue(
             file2.source.url.startswith(
-                '/media/images/f8/f8ef448d66e2506055e2586d4cb20dc8758b19cd6e8b052231fcbcad2e2be4b3',
+                '/media/images/21/213648f19f0cc7ef8e266e87a0a7a66f0079eb80de50d539895466e645137616',
             )
         )
-        self.assertTrue(file2.source.url.endswith('.png'))
+        self.assertTrue(file2.source.url.endswith('.webp'))
         for f in (file1, file2):
             self.assertEqual(f.user_id, user.pk)
 
@@ -203,3 +203,80 @@ class UpdateTest(TestCase):
             response.context['add_preview_formset'].forms[0].errors,
             {'source': ['File with this Hash already exists.']},
         )
+
+    def test_post_upload_validation_error_unexpected_preview_format_gif(self):
+        extension = create_approved_version().extension
+
+        data = {
+            **POST_DATA,
+            'form-TOTAL_FORMS': ['2'],
+        }
+        file_name1 = 'test_preview_image_0001.gif'
+        file_name2 = 'test_preview_image_0001.png'
+        url = extension.get_manage_url()
+        user = extension.authors.first()
+        self.client.force_login(user)
+        with open(TEST_FILES_DIR / file_name1, 'rb') as fp1, open(
+            TEST_FILES_DIR / file_name2, 'rb'
+        ) as fp2:
+            files = {
+                'form-0-source': fp1,
+                'form-1-source': fp2,
+            }
+            response = self.client.post(url, {**data, **files})
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            response.context['add_preview_formset'].forms[0].errors,
+            {'source': ['Choose a JPEG, PNG or WebP image, or an MP4 video']},
+        )
+
+    def test_post_upload_validation_error_unexpected_preview_format_tif(self):
+        extension = create_approved_version().extension
+
+        data = {
+            **POST_DATA,
+            'form-TOTAL_FORMS': ['2'],
+        }
+        file_name1 = 'test_preview_image_0001.png'
+        file_name2 = 'test_preview_image_0001.tif'
+        url = extension.get_manage_url()
+        user = extension.authors.first()
+        self.client.force_login(user)
+        with open(TEST_FILES_DIR / file_name1, 'rb') as fp1, open(
+            TEST_FILES_DIR / file_name2, 'rb'
+        ) as fp2:
+            files = {
+                'form-0-source': fp1,
+                'form-1-source': fp2,
+            }
+            response = self.client.post(url, {**data, **files})
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            response.context['add_preview_formset'].forms[1].errors,
+            {'source': ['Choose a JPEG, PNG or WebP image, or an MP4 video']},
+        )
+
+    def test_post_upload_validation_error_unexpected_preview_format_renamed_gif(self):
+        extension = create_approved_version().extension
+
+        data = {
+            **POST_DATA,
+            'form-TOTAL_FORMS': ['1'],
+        }
+        file_name1 = 'test_preview_image_renamed_gif.png'
+        url = extension.get_manage_url()
+        user = extension.authors.first()
+        self.client.force_login(user)
+        with open(TEST_FILES_DIR / file_name1, 'rb') as fp1:
+            files = {
+                'form-0-source': fp1,
+            }
+            response = self.client.post(url, {**data, **files})
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            response.context['add_preview_formset'].forms[0].errors,
+            {'source': ['Choose a JPEG, PNG or WebP image, or an MP4 video']},
+        )

extensions/tests/test_views.py

@@ -52,8 +52,10 @@ class PublicViewsTest(_BaseTestCase):
         response = self.client.get(url, HTTP_ACCEPT=HTTP_ACCEPT)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response['Content-Type'], 'application/json')
-        self.assertEqual(len(response.json()), 3)
+        json = response.json()
-        for _, v in response.json().items():
+        self.assertEqual(len(json['data']), 3)
+        for v in json['data']:
+            self.assertIn('id', v)
             self.assertIn('name', v)
             self.assertIn('tagline', v)
             self.assertIn('version', v)
@@ -96,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()
 
@@ -133,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()
@@ -157,8 +146,16 @@ class ExtensionManageViewTest(_BaseTestCase):
 
         self.assertEqual(response.status_code, 302)
 
+    def test_cannot_view_manage_extension_page_for_drafts(self):
+        extension = _create_extension()
+
+        response = self.client.get(extension.get_manage_url())
+
+        self.assertEqual(response.status_code, 302)
+
     def test_can_view_manage_extension_page_if_maintaner(self):
         extension = _create_extension()
+        extension.approve()
 
         self.client.force_login(extension.authors.first())
         response = self.client.get(extension.get_manage_url())
@@ -183,7 +180,7 @@ class ListedExtensionsTest(_BaseTestCase):
         self.assertEqual(response['Content-Type'], 'application/json')
 
         # Basic sanity check to make sure we are getting the result of listed
-        listed_count = len(response.json())
+        listed_count = len(response.json()['data'])
         self.assertEqual(Extension.objects.listed.count(), listed_count)
         return listed_count
 
@@ -191,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()

extensions/urls.py

@@ -7,7 +7,6 @@ from extensions.views import api, public, submit, manage
 app_name = 'extensions'
 urlpatterns = [
     path('submit/', submit.UploadFileView.as_view(), name='submit'),
-    path('submit/<int:pk>/', submit.SubmitFileView.as_view(), name='submit-finalise'),
     # TODO: move /accounts pages under its own app when User model is replaced
     path('accounts/extensions/', manage.ManageListView.as_view(), name='manage-list'),
     # FIXME: while there's no profile page, redirect to My Extensions instead
@@ -19,7 +18,6 @@ urlpatterns = [
     path('api/v1/extensions/', api.ExtensionsAPIView.as_view(), name='api'),
     # Public pages
     path('', public.HomeView.as_view(), name='home'),
-    path('', api.ExtensionsAPIView.as_view(), name='home-api'),
     path('search/', public.SearchView.as_view(), name='search'),
     path('author/<int:user_id>/', public.SearchView.as_view(), name='by-author'),
     path('search/', public.SearchView.as_view(), name='search'),
@@ -35,11 +33,21 @@ urlpatterns = [
         rf'^(?P<type_slug>{EXTENSION_SLUGS_PATH})/',
         include(
             [
+                path(
+                    '<slug:slug>/draft/',
+                    manage.DraftExtensionView.as_view(),
+                    name='draft',
+                ),
                 path(
                     '<slug:slug>/manage/',
                     manage.UpdateExtensionView.as_view(),
                     name='manage',
                 ),
+                path(
+                    '<slug:slug>/delete/',
+                    manage.DeleteExtensionView.as_view(),
+                    name='delete',
+                ),
                 path(
                     '<slug:slug>/manage/versions/',
                     manage.ManageVersionsView.as_view(),

extensions/views/api.py

@@ -53,6 +53,7 @@ class ListedExtensionsSerializer(serializers.ModelSerializer):
             return {}
 
         data = {
+            'id': instance.extension_id,
             'schema_version': instance.latest_version.schema_version,
             'name': instance.name,
             'version': instance.latest_version.version,
@@ -75,16 +76,12 @@ class ListedExtensionsSerializer(serializers.ModelSerializer):
             'tags': [str(tag) for tag in instance.latest_version.tags.all()],
         }
 
-        return {instance.extension_id: clean_json_dictionary_from_optional_fields(data)}
+        return clean_json_dictionary_from_optional_fields(data)
 
 
 class ExtensionsAPIView(APIView):
     serializer_class = ListedExtensionsSerializer
 
-    @staticmethod
-    def _convert_list_to_dict(data):
-        return {k: v for d in data for k, v in d.items()}
-
     @extend_schema(
         parameters=[
             OpenApiParameter(
@@ -99,5 +96,12 @@ class ExtensionsAPIView(APIView):
         serializer = self.serializer_class(
             Extension.objects.listed, blender_version=blender_version, request=request, many=True
         )
-        data_as_dict = self._convert_list_to_dict(serializer.data)
+        data = serializer.data
-        return Response(data_as_dict)
+        return Response(
+            {
+                # TODO implement extension blocking by moderators
+                'blocklist': [],
+                'data': data,
+                'version': 'v1',
+            }
+        )

extensions/views/manage.py

@@ -5,20 +5,28 @@ from django.contrib.messages.views import SuccessMessageMixin
 from django.db import transaction
 from django.shortcuts import get_object_or_404, reverse
 from django.views.generic import DetailView, ListView
-from django.views.generic.edit import CreateView, UpdateView, DeleteView
+from django.views.generic.edit import CreateView, UpdateView, DeleteView, FormView
 
 
-from .mixins import ExtensionQuerysetMixin, OwnsFileMixin, MaintainedExtensionMixin
+from .mixins import (
+    ExtensionQuerysetMixin,
+    OwnsFileMixin,
+    MaintainedExtensionMixin,
+    DraftVersionMixin,
+    DraftMixin,
+)
 from extensions.forms import (
     EditPreviewFormSet,
     AddPreviewFormSet,
+    ExtensionDeleteForm,
     ExtensionUpdateForm,
     VersionForm,
-    DeleteViewForm,
+    VersionDeleteForm,
 )
 from extensions.models import Extension, Version
 from files.forms import FileForm
 from files.models import File
+from reviewers.models import ApprovalActivity
 from stats.models import ExtensionView
 import ratings.models
 
@@ -90,6 +98,7 @@ class UpdateExtensionView(
     LoginRequiredMixin,
     MaintainedExtensionMixin,
     SuccessMessageMixin,
+    DraftMixin,
     UpdateView,
 ):
     model = Extension
@@ -152,14 +161,45 @@ class UpdateExtensionView(
         return self.form_invalid(form, edit_preview_formset, add_preview_formset)
 
 
+class DeleteExtensionView(
+    LoginRequiredMixin,
+    UserPassesTestMixin,
+    DeleteView,
+):
+    model = Extension
+    template_name = 'extensions/confirm_delete.html'
+    form_class = ExtensionDeleteForm
+
+    def get_success_url(self):
+        return reverse('extensions:manage-list')
+
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data(**kwargs)
+        context['extension_name'] = self.object.name
+        context['confirm_url'] = self.object.get_delete_url()
+        return context
+
+    def test_func(self) -> bool:
+        obj = self.get_object()
+        # Only maintainers allowed
+        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
     template_name = 'extensions/version_confirm_delete.html'
-    form_class = DeleteViewForm
+    form_class = VersionDeleteForm
 
     def get_success_url(self):
         return reverse(
@@ -194,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,
@@ -293,3 +341,95 @@ class UpdateVersionView(LoginRequiredMixin, UserPassesTestMixin, UpdateView):
     def test_func(self) -> bool:
         # Only maintainers are allowed to perform this
         return self.get_object().extension.has_maintainer(self.request.user)
+
+
+class DraftExtensionView(
+    LoginRequiredMixin,
+    MaintainedExtensionMixin,
+    DraftVersionMixin,
+    UserPassesTestMixin,
+    SuccessMessageMixin,
+    FormView,
+):
+    template_name = 'extensions/draft_finalise.html'
+    form_class = VersionForm
+
+    @property
+    def success_message(self) -> str:
+        if self.extension.status == Extension.STATUSES.INCOMPLETE:
+            return "Updated successfully"
+        return "Submitted to the Approval Queue"
+
+    def test_func(self) -> bool:
+        return self.extension.status == Extension.STATUSES.INCOMPLETE
+
+    def get_form_kwargs(self):
+        form_kwargs = super().get_form_kwargs()
+        form_kwargs['instance'] = self.extension.versions.first()
+        return form_kwargs
+
+    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
+
+    def get_context_data(self, form=None, extension_form=None, add_preview_formset=None, **kwargs):
+        """Add all the additional forms to the context."""
+        context = super().get_context_data(**kwargs)
+        if not (add_preview_formset and extension_form):
+            extension_form = ExtensionUpdateForm(instance=self.extension)
+            add_preview_formset = AddPreviewFormSet(extension=self.extension, request=self.request)
+        context['extension_form'] = extension_form
+        context['add_preview_formset'] = add_preview_formset
+        return context
+
+    def post(self, request, *args, **kwargs):
+        """Handle bound forms and valid/invalid logic with the extra forms."""
+        form = self.get_form()
+        extension_form = ExtensionUpdateForm(
+            self.request.POST, self.request.FILES, instance=self.extension
+        )
+        add_preview_formset = AddPreviewFormSet(
+            self.request.POST, self.request.FILES, extension=self.extension, request=self.request
+        )
+        if form.is_valid() and extension_form.is_valid() and add_preview_formset.is_valid():
+            return self.form_valid(form, extension_form, add_preview_formset)
+        return self.form_invalid(form, extension_form, add_preview_formset)
+
+    @transaction.atomic
+    def form_valid(self, form, extension_form, add_preview_formset):
+        """Save all the forms in correct order.
+
+        Extension must be saved first.
+        """
+        try:
+            # Send the extension and version to the review
+            if 'submit_draft' in self.request.POST:
+                extension_form.instance.status = extension_form.instance.STATUSES.AWAITING_REVIEW
+            extension_form.save()
+            add_preview_formset.save()
+            form.save()
+            if 'submit_draft' in self.request.POST:
+                # TODO allow to submit a custom message via the form
+                ApprovalActivity(
+                    user=self.request.user,
+                    extension=extension_form.instance,
+                    type=ApprovalActivity.ActivityType.AWAITING_REVIEW,
+                    message="initial submission",
+                ).save()
+            return super().form_valid(form)
+        except forms.ValidationError as e:
+            if 'hash' in e.error_dict:
+                add_preview_formset.forms[0].add_error('source', e.error_dict['hash'])
+        return self.form_invalid(form, extension_form, add_preview_formset)
+
+    def form_invalid(self, form, extension_form, add_preview_formset):
+        return self.render_to_response(
+            self.get_context_data(form, extension_form, add_preview_formset)
+        )
+
+    def get_success_url(self):
+        return self.extension.get_manage_url()

extensions/views/mixins.py

@@ -1,5 +1,5 @@
 from django.contrib.auth.mixins import UserPassesTestMixin
-from django.shortcuts import get_object_or_404
+from django.shortcuts import get_object_or_404, redirect
 
 from extensions.models import Extension
 from files.models import File
@@ -50,7 +50,29 @@ 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(
+        self.extension = get_object_or_404(Extension, slug=self.kwargs['slug'])
-            Extension.objects.exclude_deleted, slug=self.kwargs['slug']
-        )
         return super().dispatch(*args, **kwargs)
+
+
+class DraftVersionMixin:
+    """Fetch the version object which is being edited as a draft."""
+
+    def dispatch(self, *args, **kwargs):
+        self.version = self.extension.versions.first()
+        return super().dispatch(*args, **kwargs)
+
+
+class DraftMixin:
+    """If the extension is incomplete, returns the FinalizeDraftView"""
+
+    def dispatch(self, request, *args, **kwargs):
+        extension = (
+            Extension.objects.listed_or_authored_by(user_id=self.request.user.pk)
+            .filter(status=Extension.STATUSES.INCOMPLETE)
+            .first()
+        )
+
+        if not extension:
+            return super().dispatch(request, *args, **kwargs)
+
+        return redirect(extension.get_draft_url())

extensions/views/submit.py

@@ -1,25 +1,18 @@
 import logging
 
-from django import forms
 from django.contrib.auth.mixins import LoginRequiredMixin
 from django.db import transaction
-from django.shortcuts import reverse
+from django.views.generic.edit import CreateView
-from django.views.generic.edit import CreateView, FormView
 
-from .mixins import OwnsFileMixin
+from .mixins import DraftMixin
 from extensions.models import Version, Extension
-from extensions.forms import (
-    ExtensionUpdateForm,
-    VersionForm,
-    AddPreviewFormSet,
-)
 from files.forms import FileForm
 from files.models import File
 
 logger = logging.getLogger(__name__)
 
 
-class UploadFileView(LoginRequiredMixin, CreateView):
+class UploadFileView(LoginRequiredMixin, DraftMixin, CreateView):
     model = File
     template_name = 'extensions/submit.html'
     form_class = FileForm
@@ -30,30 +23,12 @@ class UploadFileView(LoginRequiredMixin, CreateView):
         return kwargs
 
     def get_success_url(self):
-        return reverse('extensions:submit-finalise', kwargs={'pk': self.object.pk})
+        return self.extension.get_draft_url()
 
-
+    @transaction.atomic
-class SubmitFileView(LoginRequiredMixin, OwnsFileMixin, FormView):
+    def form_valid(self, form):
-    template_name = 'extensions/submit_finalise.html'
+        """Create an extension and a version already, associated with the user."""
-    form_class = VersionForm
+        self.file = form.instance
-
-    def _get_extension(self) -> 'Extension':
-        # Get the existing Extension and Version, if they were partially saved already
-        existing_version = getattr(self.file, 'version', None)
-        if existing_version:
-            extension = existing_version.extension
-            assert (
-                extension.can_request_review
-            ), 'TODO: cannot request review for extension: {extension.get_status_display}'
-            assert not extension.authors.count() or extension.authors.filter(
-                user__id=self.request.user.pk
-            ), 'TODO: cannot request review for extension: maintained by someone else'
-            logger.warning(
-                'Found existing version pk=%s for file pk=%s',
-                existing_version.pk,
-                self.file.pk,
-            )
-            return extension
 
         parsed_extension_fields = self.file.parsed_extension_fields
         if parsed_extension_fields:
@@ -69,72 +44,21 @@ class SubmitFileView(LoginRequiredMixin, OwnsFileMixin, FormView):
                     extension.pk,
                     self.file.pk,
                 )
-                return extension
+                return False
-        return Extension.objects.update_or_create(type=self.file.type, **parsed_extension_fields)[0]
 
-    def _get_version(self, extension) -> 'Version':
+        # Make sure an extension has a user associated to it from the beginning, otherwise
-        return Version.objects.update_or_create(
+        # it will prevent it from being re-uploaded and yet not show on My Extensions.
-            extension=extension, file=self.file, **self.file.parsed_version_fields
+        self.extension = Extension.objects.update_or_create(
+            type=self.file.type, **parsed_extension_fields
+        )[0]
+        self.extension.authors.add(self.request.user)
+        self.extension.save()
+
+        # Need to save the form to be able to use the file to create the version.
+        self.object = self.file = form.save()
+
+        Version.objects.update_or_create(
+            extension=self.extension, file=self.file, **self.file.parsed_version_fields
         )[0]
 
-    def get_form_kwargs(self):
-        form_kwargs = super().get_form_kwargs()
-        form_kwargs['instance'] = self._get_version(self.extension)
-        return form_kwargs
-
-    def get_initial(self):
-        """Return initial values for the version, based on the file."""
-        initial = super().get_initial()
-        initial['file'] = self.file
-        initial.update(**self.file.parsed_version_fields)
-        return initial
-
-    def get_context_data(self, form=None, extension_form=None, add_preview_formset=None, **kwargs):
-        """Add all the additional forms to the context."""
-        context = super().get_context_data(**kwargs)
-        if not (add_preview_formset and extension_form):
-            extension_form = ExtensionUpdateForm(instance=self.extension)
-            add_preview_formset = AddPreviewFormSet(extension=self.extension, request=self.request)
-        context['extension_form'] = extension_form
-        context['add_preview_formset'] = add_preview_formset
-        return context
-
-    def post(self, request, *args, **kwargs):
-        """Handle bound forms and valid/invalid logic with the extra forms."""
-        form = self.get_form()
-        extension_form = ExtensionUpdateForm(
-            self.request.POST, self.request.FILES, instance=self.extension
-        )
-        add_preview_formset = AddPreviewFormSet(
-            self.request.POST, self.request.FILES, extension=self.extension, request=self.request
-        )
-        if form.is_valid() and extension_form.is_valid() and add_preview_formset.is_valid():
-            return self.form_valid(form, extension_form, add_preview_formset)
-        return self.form_invalid(form, extension_form, add_preview_formset)
-
-    @transaction.atomic
-    def form_valid(self, form, extension_form, add_preview_formset):
-        """Save all the forms in correct order.
-
-        Extension must be saved first.
-        """
-        try:
-            # Send the extension and version to the review
-            extension_form.instance.status = extension_form.instance.STATUSES.AWAITING_REVIEW
-            extension_form.save()
-            self.extension.authors.add(self.request.user)
-            add_preview_formset.save()
-            form.save()
         return super().form_valid(form)
-        except forms.ValidationError as e:
-            if 'hash' in e.error_dict:
-                add_preview_formset.forms[0].add_error('source', e.error_dict['hash'])
-        return self.form_invalid(form, extension_form, add_preview_formset)
-
-    def form_invalid(self, form, extension_form, add_preview_formset):
-        return self.render_to_response(
-            self.get_context_data(form, extension_form, add_preview_formset)
-        )
-
-    def get_success_url(self):
-        return self.file.extension.get_manage_url()

files/admin.py

@@ -1,6 +1,28 @@
 from django.contrib import admin
+import background_task.admin
+import background_task.models
 
-from .models import File
+from .models import File, FileValidation
+import files.signals
+
+
+def scan_selected_files(self, request, queryset):
+    """Scan selected files."""
+    for instance in queryset:
+        files.signals.schedule_scan(instance)
+
+
+class FileValidationInlineAdmin(admin.StackedInline):
+    model = FileValidation
+    readonly_fields = ('date_created', 'date_modified', 'is_ok', 'results')
+    extra = 0
+
+    def _nope(self, request, obj):
+        return False
+
+    has_add_permission = _nope
+    has_change_permission = _nope
+    has_delete_permission = _nope
 
 
 @admin.register(File)
@@ -9,20 +31,19 @@ class FileAdmin(admin.ModelAdmin):
     save_on_top = True
 
     list_filter = (
+        'validation__is_ok',
         'type',
         'status',
         'date_status_changed',
         'date_approved',
-        'date_deleted',
     )
-    list_display = ('original_name', 'extension', 'user', 'date_created', 'type', 'status')
+    list_display = ('original_name', 'extension', 'user', 'date_created', 'type', 'status', 'is_ok')
 
     list_select_related = ('version__extension', 'user')
 
     readonly_fields = (
         'id',
         'date_created',
-        'date_deleted',
         'date_modified',
         'date_approved',
         'date_status_changed',
@@ -61,7 +82,6 @@ class FileAdmin(admin.ModelAdmin):
                     'date_modified',
                     'date_status_changed',
                     'date_approved',
-                    'date_deleted',
                 )
             },
         ),
@@ -77,3 +97,56 @@ class FileAdmin(admin.ModelAdmin):
             },
         ),
     )
+
+    inlines = [FileValidationInlineAdmin]
+    actions = [scan_selected_files]
+
+    def is_ok(self, obj):
+        return obj.validation.is_ok if hasattr(obj, 'validation') else None
+
+    is_ok.boolean = True
+
+
+try:
+    admin.site.unregister(background_task.models.Task)
+    admin.site.unregister(background_task.models.CompletedTask)
+except admin.site.NotRegistered:
+    pass
+
+
+class TaskMixin:
+    """Modify a few properties of background tasks displayed in admin."""
+
+    def no_errors(self, obj):
+        """Replace background_task's "has_error".
+
+        Make Django's red/green boolean icons less confusing
+        in the context of "there's an error during task run".
+        """
+        return not bool(obj.last_error)
+
+    no_errors.boolean = True
+
+
+@admin.register(background_task.models.Task)
+@admin.register(background_task.models.CompletedTask)
+class TaskAdmin(background_task.admin.TaskAdmin, TaskMixin):
+    date_hierarchy = 'run_at'
+    list_display = [
+        'run_at',
+        'task_name',
+        'task_params',
+        'attempts',
+        'no_errors',
+        'locked_by',
+        'locked_by_pid_running',
+    ]
+    list_filter = (
+        'task_name',
+        'run_at',
+        'failed_at',
+        'locked_at',
+        'attempts',
+        'creator_content_type',
+    )
+    search_fields = ['task_name', 'task_params', 'last_error', 'verbose_name']

files/forms.py

@@ -5,16 +5,14 @@ import tempfile
 
 from django import forms
 from django.utils.safestring import mark_safe
+from django.utils.translation import gettext_lazy as _
 
 from .validators import (
-    CustomFileExtensionValidator,
     ExtensionIDManifestValidator,
+    FileMIMETypeValidator,
     ManifestValidator,
 )
-from constants.base import (
+from constants.base import EXTENSION_SLUG_TYPES, ALLOWED_EXTENSION_MIMETYPES
-    EXTENSION_TYPE_SLUGS_SINGULAR,
-    VALID_SOURCE_EXTENSIONS,
-)
 import files.models
 import files.utils as utils
 
@@ -25,6 +23,22 @@ logger = logging.getLogger(__name__)
 
 
 class FileForm(forms.ModelForm):
+    msg_only_zip_files = _('Only .zip files are accepted.')
+
+    # Mimicking how django.forms.fields.Field handles validation error messages.
+    # TODO: maybe this should be a custom SourceFileField with all these validators and messages
+    error_messages = {
+        'invalid_manifest_path': _(
+            'The manifest file should be at the top level of the archive, or one level deep.'
+        ),
+        # TODO: surface TOML parsing errors?
+        'invalid_manifest_toml': _('Could not parse the manifest file.'),
+        'invalid_missing_init': _('An add-on should have an __init__.py file.'),
+        'missing_or_multiple_theme_xml': _('A theme should have exactly one XML file.'),
+        'invalid_zip_archive': msg_only_zip_files,
+        'missing_manifest_toml': _('The manifest file is missing.'),
+    }
+
     class Meta:
         model = files.models.File
         fields = ('source', 'type', 'metadata', 'agreed_with_terms', 'user')
@@ -32,8 +46,16 @@ class FileForm(forms.ModelForm):
     source = forms.FileField(
         allow_empty_file=False,
         required=True,
-        validators=[CustomFileExtensionValidator(allowed_extensions=VALID_SOURCE_EXTENSIONS)],
+        validators=[
-        help_text=('Only .zip file are accepted.'),
+            FileMIMETypeValidator(
+                allowed_mimetypes=ALLOWED_EXTENSION_MIMETYPES,
+                message=error_messages['invalid_zip_archive'],
+            ),
+        ],
+        widget=forms.ClearableFileInput(
+            attrs={'accept': ','.join(ALLOWED_EXTENSION_MIMETYPES)}
+        ),
+        help_text=msg_only_zip_files,
     )
     agreed_with_terms = forms.BooleanField(
         initial=False,
@@ -117,22 +139,19 @@ class FileForm(forms.ModelForm):
 
             errors = []
             if not zipfile.is_zipfile(file_path):
-                errors.append('File is not .zip')
+                raise forms.ValidationError(self.error_messages['invalid_zip_archive'])
 
-            manifest = utils.read_manifest_from_zip(file_path)
+            manifest, error_codes = utils.read_manifest_from_zip(file_path)
+            for code in error_codes:
+                errors.append(forms.ValidationError(self.error_messages[code]))
+            if errors:
+                self.add_error('source', errors)
 
-            if manifest is None:
+            if manifest:
-                errors.append('A valid manifest file could not be found')
-            else:
                 ManifestValidator(manifest)
                 ExtensionIDManifestValidator(manifest, self.extension)
 
-            extension_types = {v: k for k, v in EXTENSION_TYPE_SLUGS_SINGULAR.items()}
-            if errors:
-                raise forms.ValidationError({'source': errors}, code='invalid')
-
                 self.cleaned_data['metadata'] = manifest
-            # TODO: Error handling
+                self.cleaned_data['type'] = EXTENSION_SLUG_TYPES[manifest['type']]
-            self.cleaned_data['type'] = extension_types[manifest['type']]
 
             return self.cleaned_data

files/migrations/0005_rename_validation_filevalidation_results_and_more.py

@@ -0,0 +1,40 @@
+# Generated by Django 4.2.11 on 2024-04-12 09:05
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('files', '0004_alter_file_status'),
+    ]
+
+    operations = [
+        migrations.RenameField(
+            model_name='filevalidation',
+            old_name='validation',
+            new_name='results',
+        ),
+        migrations.AlterField(
+            model_name='filevalidation',
+            name='results',
+            field=models.JSONField(),
+        ),
+        migrations.RemoveField(
+            model_name='filevalidation',
+            name='errors',
+        ),
+        migrations.RemoveField(
+            model_name='filevalidation',
+            name='notices',
+        ),
+        migrations.RemoveField(
+            model_name='filevalidation',
+            name='warnings',
+        ),
+        migrations.RenameField(
+            model_name='filevalidation',
+            old_name='is_valid',
+            new_name='is_ok',
+        ),
+    ]

files/migrations/0006_remove_file_date_deleted.py

@@ -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',
+        ),
+    ]

files/migrations/0007_alter_file_status.py

@@ -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 not extension or 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),
+    ]

files/models.py

@@ -1,15 +1,12 @@
 from pathlib import Path
 from typing import Dict, Any
 import logging
-import mimetypes
-import re
 
 from django.contrib.auth import get_user_model
 from django.db import models
-from django.urls import reverse
 
-from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin
+from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin
-from files.utils import get_sha256
+from files.utils import get_sha256, guess_mimetype_from_ext
 from constants.base import (
     FILE_STATUS_CHOICES,
     FILE_TYPE_CHOICES,
@@ -51,8 +48,8 @@ def thumbnail_upload_to(instance, filename):
     return path
 
 
-class File(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Model):
+class File(CreatedModifiedMixin, TrackChangesMixin, models.Model):
-    track_changes_to_fields = {'status', 'size_bytes', 'hash', 'date_deleted'}
+    track_changes_to_fields = {'status', 'size_bytes', 'hash'}
 
     TYPES = FILE_TYPE_CHOICES
     STATUSES = FILE_STATUS_CHOICES
@@ -79,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
@@ -138,8 +135,7 @@ class File(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Mode
             self.original_name = self.source.name
 
         if not self.content_type:
-            content_type, _ = mimetypes.guess_type(self.original_name)
+            self.content_type = guess_mimetype_from_ext(self.original_name)
-            self.content_type = content_type
         if self.content_type:
             if 'image' in self.content_type:
                 self.type = self.TYPES.IMAGE
@@ -180,14 +176,10 @@ class File(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Mode
         data = self.metadata
 
         extension_id = data.get('id')
-        original_name = data.get('name', self.original_name)
+        name = data.get('name', self.original_name)
-        name_as_path = Path(original_name)
-        for suffix in name_as_path.suffixes:
-            original_name = original_name.replace(suffix, '')
-        name = re.sub(r'[-_ ]+', ' ', original_name)
         return {
             'name': name,
-            'slug': utils.slugify(name),
+            'slug': utils.slugify(extension_id),
             'extension_id': extension_id,
             'website': data.get('website'),
         }
@@ -209,15 +201,12 @@ class File(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Mode
         }
 
     def get_submit_url(self) -> str:
-        return reverse('extensions:submit-finalise', kwargs={'pk': self.pk})
+        return self.extension.get_draft_url()
 
 
 class FileValidation(CreatedModifiedMixin, TrackChangesMixin, models.Model):
-    track_changes_to_fields = {'is_valid', 'errors', 'warnings', 'notices', 'validation'}
+    track_changes_to_fields = {'is_ok', 'results'}
 
     file = models.OneToOneField(File, related_name='validation', on_delete=models.CASCADE)
-    is_valid = models.BooleanField(default=False)
+    is_ok = models.BooleanField(default=False)
-    errors = models.IntegerField(default=0)
+    results = models.JSONField()
-    warnings = models.IntegerField(default=0)
-    notices = models.IntegerField(default=0)
-    validation = models.TextField()

files/signals.py

@@ -1,11 +1,40 @@
-from django.db.models.signals import pre_save
+import logging
+
+from django.db.models.signals import pre_save, post_save, pre_delete
 from django.dispatch import receiver
 
 import files.models
+import files.tasks
+
+logger = logging.getLogger(__name__)
 
 
 @receiver(pre_save, sender=files.models.File)
-def _record_changes(sender: object, instance: files.models.File, **kwargs: object) -> None:
+def _record_changes(
-    was_changed, old_state = instance.pre_save_record()
+    sender: object, instance: files.models.File, update_fields: object, **kwargs: object
+) -> None:
+    was_changed, old_state = instance.pre_save_record(update_fields=update_fields)
 
     instance.record_status_change(was_changed, old_state, **kwargs)
+
+
+def schedule_scan(file: files.models.File) -> None:
+    """Schedule a scan of a given file."""
+    logger.info('Scheduling a scan for file pk=%s', file.pk)
+    verbose_name = f'clamdscan of "{file.source.name}"'
+    files.tasks.clamdscan(file_id=file.pk, creator=file, verbose_name=verbose_name)
+
+
+@receiver(post_save, sender=files.models.File)
+def _scan_new_file(
+    sender: object, instance: files.models.File, created: bool, **kwargs: object
+) -> None:
+    if not created:
+        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)

files/tasks.py

@@ -0,0 +1,29 @@
+import logging
+import os.path
+
+from background_task import background
+from background_task.tasks import TaskSchedule
+from django.conf import settings
+
+import files.models
+import files.utils
+
+logger = logging.getLogger(__name__)
+
+
+@background(schedule={'action': TaskSchedule.RESCHEDULE_EXISTING})
+def clamdscan(file_id: int):
+    """Run a scan of a given file and save its output as a FileValidation record."""
+    file = files.models.File.objects.get(pk=file_id)
+    abs_path = os.path.join(settings.MEDIA_ROOT, file.source.path)
+    scan_status, scan_found = files.utils.run_clamdscan(abs_path)
+    logger.info('File pk=%s scanned: %s', file.pk, (scan_status, scan_found))
+    scan_result = {'clamdscan': [scan_status, scan_found]}
+    is_ok = scan_status == 'OK'
+    file_validation, is_new = files.models.FileValidation.objects.get_or_create(
+        file=file, defaults={'results': scan_result, 'is_ok': is_ok}
+    )
+    if not is_new:
+        file_validation.results = scan_result
+        file_validation.is_ok = is_ok
+        file_validation.save(update_fields={'results', 'is_ok', 'date_modified'})

files/templates/files/components/scan_details.html

@@ -0,0 +1,21 @@
+{% load common i18n %}
+{# FIXME: we might want to rephrase is_moderator in terms of Django's (group) permissions #}
+{% if perms.files.view_file or request.user.is_moderator %}
+  {% with file_validation=file.validation %}
+    {% if file_validation and not file_validation.is_ok %}
+      <section>
+        <div class="card pb-3 pt-4 px-4 mb-3 ext-detail-download-danger">
+          <h3>⚠&nbsp;{% trans "Suspicious upload" %}</h3>
+          {% blocktrans asvar alert_text %}Scan of the {{ file }} indicates malicious content.{% endblocktrans %}
+          <h4>
+            {{ alert_text }}
+            {% if perms.files.view_file %}{# Moderators don't necessarily have access to the admin #}
+              {% url 'admin:files_file_change' file.pk as admin_file_url %}
+              <a href="{{ admin_file_url }}" target="_blank">{% trans "See details" %}</a>
+            {% endif %}
+          </h4>
+        </div>
+      </section>
+    {% endif %}
+  {% endwith %}
+{% endif %}

files/templates/files/components/scan_details_flag.html

@@ -0,0 +1,10 @@
+{% load common i18n %}
+{# FIXME: we might want to rephrase is_moderator in terms of Django's (group) permissions #}
+{% if perms.files.view_file or request.user.is_moderator %}
+  {% with file_validation=file.validation %}
+    {% if file_validation and not file_validation.is_ok %}
+      {% blocktrans asvar alert_text %}Scan of the {{ file }} indicates malicious content.{% endblocktrans %}
+      <b class="text-danger pt-2" title="{{ alert_text }}">⚠</b>
+    {% endif %}
+  {% endwith %}
+{% endif %}

files/tests/test_models.py

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

files/tests/test_signals.py

@@ -0,0 +1,107 @@
+import os
+import shutil
+import tempfile
+import unittest
+
+from background_task.models import Task
+from django.conf import settings
+from django.test import TestCase, override_settings
+
+from common.tests.factories.files import FileFactory
+import files.models
+import files.tasks
+
+
+@unittest.skipUnless(shutil.which('clamd'), 'requires clamd')
+@override_settings(MEDIA_ROOT='/tmp/')
+class FileScanTest(TestCase):
+    def setUp(self):
+        super().setUp()
+        self.temp_directory = tempfile.mkdtemp(prefix=settings.MEDIA_ROOT)
+
+    def tearDown(self):
+        super().tearDown()
+        shutil.rmtree(self.temp_directory)
+
+    def test_scan_flags_found_invalid(self):
+        test_file_path = os.path.join(self.temp_directory, 'test_file.zip')
+        test_content = (
+            b'X5O!P%@AP[4\PZX54(P^)7CC)7}$EICAR-STANDARD-ANTIVIRUS-TEST-FILE!$H+H*'  # noqa: W605
+        )
+        with open(test_file_path, 'wb+') as test_file:
+            test_file.write(test_content)
+
+        file = FileFactory(source=test_file_path)
+        self.assertFalse(hasattr(file, 'validation'))
+
+        # A background task should have been created
+        task = Task.objects.created_by(creator=file).first()
+        self.assertIsNotNone(task)
+        self.assertEqual(task.task_name, 'files.tasks.clamdscan')
+        self.assertEqual(task.task_params, f'[[], {{"file_id": {file.pk}}}]')
+
+        # Actually run the task as if by background runner
+        task_args, task_kwargs = task.params()
+        files.tasks.clamdscan.task_function(*task_args, **task_kwargs)
+
+        file.refresh_from_db()
+        self.assertFalse(file.validation.is_ok)
+        result = file.validation.results['clamdscan']
+        self.assertEqual(result, ['FOUND', 'Win.Test.EICAR_HDB-1'])
+
+    def test_scan_flags_found_invalid_updates_existing_validation(self):
+        test_file_path = os.path.join(self.temp_directory, 'test_file.zip')
+        test_content = (
+            b'X5O!P%@AP[4\PZX54(P^)7CC)7}$EICAR-STANDARD-ANTIVIRUS-TEST-FILE!$H+H*'  # noqa: W605
+        )
+        with open(test_file_path, 'wb+') as test_file:
+            test_file.write(test_content)
+
+        file = FileFactory(source=test_file_path)
+        # Make sure validation record exists before scanner runs
+        existing_validation = files.models.FileValidation(file=file, results={})
+        existing_validation.save()
+        self.assertTrue(hasattr(file, 'validation'))
+        old_date_modified = existing_validation.date_modified
+
+        # A background task should have been created
+        task = Task.objects.created_by(creator=file).first()
+        self.assertIsNotNone(task)
+        self.assertEqual(task.task_name, 'files.tasks.clamdscan')
+        self.assertEqual(task.task_params, f'[[], {{"file_id": {file.pk}}}]')
+
+        # Actually run the task as if by background runner
+        task_args, task_kwargs = task.params()
+        files.tasks.clamdscan.task_function(*task_args, **task_kwargs)
+
+        self.assertFalse(file.validation.is_ok)
+        file.validation.refresh_from_db()
+        result = file.validation.results['clamdscan']
+        self.assertEqual(result, ['FOUND', 'Win.Test.EICAR_HDB-1'])
+        self.assertEqual(existing_validation.pk, file.validation.pk)
+
+        existing_validation.refresh_from_db()
+        self.assertGreater(existing_validation.date_modified, old_date_modified)
+
+    def test_scan_flags_nothing_found_valid(self):
+        test_file_path = os.path.join(self.temp_directory, 'test_file.zip')
+        with open(test_file_path, 'wb+') as test_file:
+            test_file.write(b'some file')
+
+        file = FileFactory(source=test_file_path)
+        self.assertFalse(hasattr(file, 'validation'))
+
+        # A background task should have been created
+        task = Task.objects.created_by(creator=file).first()
+        self.assertIsNotNone(task)
+        self.assertEqual(task.task_name, 'files.tasks.clamdscan')
+        self.assertEqual(task.task_params, f'[[], {{"file_id": {file.pk}}}]')
+
+        # Actually run the task as if by background runner
+        task_args, task_kwargs = task.params()
+        files.tasks.clamdscan.task_function(*task_args, **task_kwargs)
+
+        file.refresh_from_db()
+        self.assertTrue(file.validation.is_ok)
+        result = file.validation.results['clamdscan']
+        self.assertEqual(result, ['OK', None])

files/tests/test_utils.py

@@ -1,6 +1,6 @@
 from django.test import TestCase
 
-from files.utils import find_file_inside_zip_list
+from files.utils import find_path_by_name, find_exact_path, filter_paths_by_ext
 
 
 class UtilsTest(TestCase):
@@ -10,7 +10,7 @@ class UtilsTest(TestCase):
         name_list = [
             "blender_manifest.toml",
         ]
-        manifest_file = find_file_inside_zip_list(self.manifest, name_list)
+        manifest_file = find_path_by_name(name_list, self.manifest)
         self.assertEqual(manifest_file, "blender_manifest.toml")
 
     def test_find_manifest_nested(self):
@@ -23,21 +23,21 @@ class UtilsTest(TestCase):
             "foobar-1.0.3/manifest.toml",
             "foobar-1.0.3/manifest.json",
         ]
-        manifest_file = find_file_inside_zip_list(self.manifest, name_list)
+        manifest_file = find_path_by_name(name_list, self.manifest)
         self.assertEqual(manifest_file, "foobar-1.0.3/blender_manifest.toml")
 
     def test_find_manifest_no_zipped_folder(self):
         name_list = [
             "foobar-1.0.3/blender_manifest.toml",
         ]
-        manifest_file = find_file_inside_zip_list(self.manifest, name_list)
+        manifest_file = find_path_by_name(name_list, self.manifest)
         self.assertEqual(manifest_file, "foobar-1.0.3/blender_manifest.toml")
 
     def test_find_manifest_no_manifest(self):
         name_list = [
             "foobar-1.0.3/",
         ]
-        manifest_file = find_file_inside_zip_list(self.manifest, name_list)
+        manifest_file = find_path_by_name(name_list, self.manifest)
         self.assertEqual(manifest_file, None)
 
     def test_find_manifest_with_space(self):
@@ -47,5 +47,54 @@ class UtilsTest(TestCase):
             "foobar-1.0.3/blender_manifest.toml.txt",
             "blender_manifest.toml/my_files.py",
         ]
-        manifest_file = find_file_inside_zip_list(self.manifest, name_list)
+        manifest_file = find_path_by_name(name_list, self.manifest)
         self.assertEqual(manifest_file, None)
+
+    def test_find_exact_path_found(self):
+        name_list = [
+            'foobar-1.0.3/theme.xml',
+            'foobar-1.0.3/theme1.xml',
+            'foobar-1.0.3/theme2.txt',
+            'foobar-1.0.3/__init__.py',
+            'foobar-1.0.3/foobar/__init__.py',
+            'foobar-1.0.3/foobar-1.0.3/__init__.py',
+            'blender_manifest.toml',
+        ]
+        path = find_exact_path(name_list, 'foobar-1.0.3/__init__.py')
+        self.assertEqual(path, 'foobar-1.0.3/__init__.py')
+
+    def test_find_exact_path_nothing_found(self):
+        name_list = [
+            'foobar-1.0.3/theme.xml',
+            'foobar-1.0.3/theme1.xml',
+            'foobar-1.0.3/theme2.txt',
+            'foobar-1.0.3/foobar/__init__.py',
+            'foobar-1.0.3/foobar-1.0.3/__init__.py',
+            'blender_manifest.toml',
+        ]
+        path = find_exact_path(name_list, 'foobar-1.0.3/__init__.py')
+        self.assertIsNone(path)
+
+    def test_filter_paths_by_ext_found(self):
+        name_list = [
+            'foobar-1.0.3/theme.xml',
+            'foobar-1.0.3/theme1.xml',
+            'foobar-1.0.3/theme2.txt',
+            'foobar-1.0.3/__init__.py',
+            'foobar-1.0.3/foobar-1.0.3/__init__.py',
+            'blender_manifest.toml',
+        ]
+        paths = filter_paths_by_ext(name_list, '.xml')
+        self.assertEqual(list(paths), ['foobar-1.0.3/theme.xml', 'foobar-1.0.3/theme1.xml'])
+
+    def test_filter_paths_by_ext_nothing_found(self):
+        name_list = [
+            'foobar-1.0.3/theme.xml',
+            'foobar-1.0.3/theme1.md.xml',
+            'foobar-1.0.3/theme2.txt',
+            'foobar-1.0.3/__init__.py',
+            'foobar-1.0.3/foobar-1.0.3/__init__.py',
+            'blender_manifest.toml',
+        ]
+        paths = filter_paths_by_ext(name_list, '.md')
+        self.assertEqual(list(paths), [])

files/utils.py

@@ -1,12 +1,17 @@
 from pathlib import Path
 import hashlib
 import io
-import os
 import logging
-import zipfile
+import mimetypes
+import os
+import os.path
 import toml
+import typing
+import zipfile
 
 from lxml import etree
+import clamd
+import magic
 
 logger = logging.getLogger(__name__)
 MODULE_DIR = Path(__file__).resolve().parent
@@ -46,33 +51,124 @@ def get_sha256_from_value(value: str):
     return hash_.hexdigest()
 
 
-def find_file_inside_zip_list(file_to_read: str, name_list: list) -> str:
+def find_path_by_name(paths: typing.List[str], name: str) -> typing.Optional[str]:
-    """Return the first occurance of file_to_read insize a zip name_list"""
+    """Return the first occurrence of file name in a given list of paths."""
-    for file_path in name_list:
+    for file_path in paths:
         # Remove leading/trailing whitespace from file path
         file_path_stripped = file_path.strip()
         # Check if the basename of the stripped path is equal to the target file name
-        if os.path.basename(file_path_stripped) == file_to_read:
+        if os.path.basename(file_path_stripped) == name:
             return file_path_stripped
     return None
 
 
+def find_exact_path(paths: typing.List[str], exact_path: str) -> typing.Optional[str]:
+    """Return a first path equal to a given one if it exists in a given list of paths."""
+    matching_paths = (path for path in paths if path == exact_path)
+    return next(matching_paths, None)
+
+
+def filter_paths_by_ext(paths: typing.List[str], ext: str) -> typing.Iterable[str]:
+    """Generate a list of paths having a given extension from a given list of paths."""
+    for file_path in paths:
+        # Get file path's extension
+        _, file_path_ext = os.path.splitext(file_path)
+        # Check if this file's extension matches the extension we are looking for
+        if file_path_ext.lower() == ext.lower():
+            yield file_path
+
+
 def read_manifest_from_zip(archive_path):
-    file_to_read = 'blender_manifest.toml'
+    """Read and validate extension's manifest file and contents of the archive.
+
+    In any extension archive, a valid `blender_manifest.toml` file is expected
+    to be found at the top level of the archive, or inside a single nested directory.
+    Additionally, depending on the extension type defined in the manifest,
+    the archive is expected to have a particular file structure:
+
+        * for themes, a single XML file is expected next to the manifest;
+
+        * for add-ons, the following structure is expected:
+
+            ```
+            some-addon.zip
+            └─ an-optional-dir
+                ├─ blender_manifest.toml
+                ├─ __init__.py
+                └─ (...)
+            ```
+    """
+    manifest_name = 'blender_manifest.toml'
+    error_codes = []
     try:
         with zipfile.ZipFile(archive_path) as myzip:
-            manifest_filepath = find_file_inside_zip_list(file_to_read, myzip.namelist())
+            bad_file = myzip.testzip()
+            if bad_file is not None:
+                logger.error('Bad file in ZIP')
+                error_codes.append('invalid_zip_archive')
+                return None, error_codes
+
+            file_list = myzip.namelist()
+            manifest_filepath = find_path_by_name(file_list, manifest_name)
 
             if manifest_filepath is None:
-                logger.info(f"File '{file_to_read}' not found in the archive.")
+                logger.info(f"File '{manifest_name}' not found in the archive.")
-                return None
+                error_codes.append('missing_manifest_toml')
+                return None, error_codes
+
+            # Manifest file is expected to be no deeper than one directory down
+            if os.path.dirname(os.path.dirname(manifest_filepath)) != '':
+                error_codes.append('invalid_manifest_path')
+                return None, error_codes
 
             # Extract the file content
             with myzip.open(manifest_filepath) as file_content:
-                # TODO: handle TOML loading error
                 toml_content = toml.loads(file_content.read().decode())
-                return toml_content
+
+            # If manifest was parsed successfully, do additional type-specific validation
+            type_slug = toml_content['type']
+            if type_slug == 'theme':
+                theme_xmls = filter_paths_by_ext(file_list, '.xml')
+                if len(list(theme_xmls)) != 1:
+                    error_codes.append('missing_or_multiple_theme_xml')
+            elif type_slug == 'add-on':
+                # __init__.py is expected to be next to the manifest
+                expected_init_path = os.path.join(os.path.dirname(manifest_filepath), '__init__.py')
+                init_filepath = find_exact_path(file_list, expected_init_path)
+                if not init_filepath:
+                    error_codes.append('invalid_missing_init')
+
+            return toml_content, error_codes
+
+    except toml.decoder.TomlDecodeError as e:
+        logger.error(f"Manifest Error: {e.msg}")
+        error_codes.append('invalid_manifest_toml')
 
     except Exception as e:
         logger.error(f"Error extracting from archive: {e}")
-        return None
+        error_codes.append('invalid_zip_archive')
+
+    return None, error_codes
+
+
+def guess_mimetype_from_ext(file_name: str) -> str:
+    """Guess MIME-type from the extension of the given file name."""
+    mimetype_from_ext, _ = mimetypes.guess_type(file_name)
+    return mimetype_from_ext
+
+
+def guess_mimetype_from_content(file_obj) -> str:
+    """Guess MIME-type based on a portion of the given file's bytes."""
+    mimetype_from_bytes = magic.from_buffer(file_obj.read(2048), mime=True)
+    # This file might be read again by validation or other utilities
+    file_obj.seek(0)
+    return mimetype_from_bytes
+
+
+def run_clamdscan(abs_path: str) -> tuple:
+    logger.info('Scanning file at path=%s', abs_path)
+    clamd_socket = clamd.ClamdUnixSocket()
+    with open(abs_path, 'rb') as f:
+        result = clamd_socket.instream(f)['stream']
+    logger.info('File at path=%s scanned: %s', abs_path, result)
+    return result

files/validators.py

@@ -1,40 +1,55 @@
-from pathlib import Path
 from semantic_version import Version
 import logging
 
-from django.utils.translation import gettext_lazy as _
 from django.core.exceptions import ValidationError
-from django.core.validators import FileExtensionValidator, validate_unicode_slug
+from django.core.validators import validate_unicode_slug
+from django.utils.deconstruct import deconstructible
+from django.utils.html import escape
+from django.utils.safestring import mark_safe
 
 from extensions.models import Extension, License, VersionPermission, Tag
 from constants.base import EXTENSION_TYPE_SLUGS_SINGULAR, EXTENSION_TYPE_CHOICES
+from files.utils import guess_mimetype_from_ext, guess_mimetype_from_content
 
 logger = logging.getLogger(__name__)
 
 
-class CustomFileExtensionValidator(FileExtensionValidator):
+@deconstructible
-    """Allows extensions such as tar.gz."""
+class FileMIMETypeValidator:
+    code = 'invalid_mimetype'
+
+    def __init__(self, allowed_mimetypes, message, code=None):
+        self.allowed_mimetypes = {_type.lower() for _type in allowed_mimetypes}
+        if message is not None:
+            self.message = message
+        if code is not None:
+            self.code = code
 
     def __call__(self, value):
-        suffixes = Path(value.name).suffixes
+        # Guess MIME-type based on extension first
-        # Possible extensions without a leading dot
+        mimetype_from_ext = guess_mimetype_from_ext(value.name)
-        extensions = {
+        if mimetype_from_ext not in self.allowed_mimetypes:
-            ''.join(suffixes[-1:])[1:].lower(),
+            raise ValidationError(self.message, code=self.code)
-            ''.join(suffixes)[1:].lower(),
+        # Guess MIME-type based on file's content
-            ''.join(suffixes[1:])[1:].lower(),
+        mimetype_from_bytes = guess_mimetype_from_content(value)
-        }
+        if mimetype_from_bytes not in self.allowed_mimetypes:
-        if self.allowed_extensions is not None and all(
+            raise ValidationError(self.message, code=self.code)
-            extension not in self.allowed_extensions for extension in extensions
+        if mimetype_from_ext != mimetype_from_bytes:
-        ):
+            # This shouldn't happen, but libmagic's and mimetypes' mappings
-            full_extension = ''.join(suffixes).lower()[1:]
+            # might differ from distro to distro.
-            raise ValidationError(
+            logger.exception(
-                self.message,
+                "MIME-type from extension (%s) doesn't match content (%s)",
-                code=self.code,
+                mimetype_from_ext,
-                params={
+                mimetype_from_bytes,
-                    "extension": full_extension,
+            )
-                    "allowed_extensions": ", ".join(self.allowed_extensions),
+            raise ValidationError(self.message, code=self.code)
-                    "value": value,
+
-                },
+    def __eq__(self, other):
+        return (
+            isinstance(other, self.__class__)
+            and self.allowed_mimetypes == other.allowed_mimetypes
+            and self.message == other.message
+            and self.code == other.code
         )
 
 
@@ -54,7 +69,7 @@ class ExtensionIDManifestValidator:
             raise ValidationError(
                 {
                     'source': [
-                        _('Missing field in blender_manifest.toml: "id"'),
+                        mark_safe('Missing field in blender_manifest.toml: <code>id</code>'),
                     ],
                 },
                 code='invalid',
@@ -64,9 +79,9 @@ class ExtensionIDManifestValidator:
             raise ValidationError(
                 {
                     'source': [
-                        _(
+                        mark_safe(
-                            f'Invalid id from extension manifest: "{extension_id}". '
+                            "Invalid <code>id</code> from extension manifest: "
-                            'No hyphens are allowed.'
+                            f'"{escape(extension_id)}". No hyphens are allowed.'
                         ),
                     ],
                 },
@@ -79,9 +94,10 @@ class ExtensionIDManifestValidator:
             raise ValidationError(
                 {
                     'source': [
-                        _(
+                        mark_safe(
-                            f'Invalid id from extension manifest: "{extension_id}". '
+                            f'Invalid <code>id</code> from extension manifest: '
-                            'Use a valid id consisting of Unicode letters, numbers or underscores.'
+                            f'"{escape(extension_id)}". '
+                            f'Use a valid id consisting of Unicode letters, numbers or underscores.'
                         ),
                     ],
                 },
@@ -93,9 +109,10 @@ class ExtensionIDManifestValidator:
                 raise ValidationError(
                     {
                         'source': [
-                            _(
+                            mark_safe(
-                                f'The extension id in the manifest ("{extension_id}") '
+                                f'The extension <code>id</code> in the manifest '
-                                'is already being used by another extension.'
+                                f'("{escape(extension_id)}") '
+                                f'is already being used by another extension.'
                             ),
                         ],
                     },
@@ -105,10 +122,10 @@ class ExtensionIDManifestValidator:
             raise ValidationError(
                 {
                     'source': [
-                        _(
+                        mark_safe(
-                            f'The extension id in the manifest ("{extension_id}") '
+                            f'The extension <code>id</code> in the manifest '
-                            'doesn\'t match the expected one for this extension '
+                            f'("{escape(extension_id)}") doesn\'t match the expected one for'
-                            f'("{extension_to_be_updated.extension_id}").'
+                            f'this extension ("{escape(extension_to_be_updated.extension_id)}").'
                         ),
                     ],
                 },
@@ -130,7 +147,9 @@ class SimpleValidator(ManifestFieldValidator):
         if not hasattr(cls, '_type') or not hasattr(cls, '_type_name'):
             assert not "SimpleValidator must be inherited not be used directly."
         if type(value) != cls._type:
-            return f'Manifest value error: {name} should be of type: {cls._type_name}'
+            return mark_safe(
+                f'Manifest value error: <code>{name}</code> should be of type: {cls._type_name}'
+            )
 
 
 class StringValidator(SimpleValidator):
@@ -166,10 +185,10 @@ class LicenseValidator(ListValidator):
         if not is_error:
             return
 
-        error_message = (
+        error_message = mark_safe(
-            f'Manifest value error: {name} expects a list of supported licenses, '
+            f'Manifest value error: <code>license</code> expects a list of '
-            f'e.g.: {cls.example}. Visit '
+            f'<a href="https://docs.blender.org/manual/en/dev/extensions/licenses.html">'
-            'https://docs.blender.org/manual/en/dev/extensions/licenses.html to learn more.'
+            f'supported licenses</a>. e.g., {cls.example}.'
         )
 
         return error_message
@@ -196,12 +215,11 @@ class TagsValidatorBase:
             return
 
         error_message = (
-            f'Manifest value error: {name} expects a list of supported {type_name} '
+            f'Manifest value error: <code>tags</code> expects a list of '
-            f'tags, e.g.: {cls.example}. '
+            f'<a href="https://docs.blender.org/manual/en/dev/extensions/tags.html" '
-            'Visit https://docs.blender.org/manual/en/dev/extensions/tags.html to learn more.'
+            f'target="_blank"> supported {type_name} tags</a>. e.g., {cls.example}. '
         )
-
+        return mark_safe(error_message)
-        return error_message
 
 
 class TagsAddonsValidator(TagsValidatorBase):
@@ -252,7 +270,7 @@ class TypeValidator:
             return
 
         error_message = (
-            f'Manifest value error: {name} expects one of the supported types. '
+            f'Manifest value error: <code>{name}</code> expects one of the supported types. '
             f'The supported types are: '
         )
 
@@ -260,7 +278,7 @@ class TypeValidator:
             error_message += f'"{_type}", '
         error_message = error_message[:-2] + '.'
 
-        return error_message
+        return mark_safe(error_message)
 
 
 class PermissionsValidator:
@@ -271,6 +289,20 @@ class PermissionsValidator:
         """Return error message if doesn´t contain a valid list of permissions."""
         is_error = False
 
+        extension_type = manifest.get('type')
+        is_theme = extension_type == EXTENSION_TYPE_SLUGS_SINGULAR[EXTENSION_TYPE_CHOICES.THEME]
+        is_bpy = extension_type == EXTENSION_TYPE_SLUGS_SINGULAR[EXTENSION_TYPE_CHOICES.BPY]
+
+        if is_theme:
+            error_message = mark_safe(
+                'Manifest value error: <code>permissions</code> not required for themes.'
+            )
+            return error_message
+
+        # Let the wrong type error be handled elsewhere.
+        if not is_bpy:
+            return
+
         if (type(value) != list) or (not value):
             is_error = True
         else:
@@ -284,15 +316,15 @@ class PermissionsValidator:
             return
 
         error_message = (
-            f'Manifest value error: {name} expects a list of supported permissions, '
+            f'Manifest value error: <code>permissions</code> expects a list of '
-            f'e.g.: {cls.example}. The supported permissions are: '
+            f'supported permissions. e.g.: {cls.example}. The supported permissions are: '
         )
 
         for permission_ob in VersionPermission.objects.all():
             error_message += f'{permission_ob.slug}, '
         error_message = error_message[:-2] + '.'
 
-        return error_message
+        return mark_safe(error_message)
 
 
 class VersionValidator:
@@ -306,9 +338,32 @@ class VersionValidator:
         except (ValueError, TypeError):
             # ValueError happens when passing an invalid version, like "2.9"
             # TypeError happens when e.g., passing an integer
-            return (
+            return mark_safe(
-                f'Manifest value error: {name} should follow a semantic version, '
+                f'Manifest value error: <code>{name}</code> should follow a '
-                f'e.g., "{cls.example}". Visit https://semver.org/ to learn more.'
+                f'<a href="https://semver.org/" target="_blank">semantic version</a>. '
+                f'e.g., "{cls.example}"'
+            )
+
+
+class SchemaVersionValidator(VersionValidator):
+    example = '1.0.0'
+
+    @classmethod
+    def validate(cls, *, name: str, value: str, manifest: dict) -> str:
+        """Return error message if cannot validate, otherwise returns nothing"""
+        if err_message := super().validate(name=name, value=value, manifest=manifest):
+            return err_message
+
+        valid_schemas = {
+            "1.0.0",
+        }
+
+        if value not in valid_schemas:
+            # TODO make a user manual page with the list of the different schemas.
+            return mark_safe(
+                f'Manifest value error: <code>schema</code> version ({escape(value)}) '
+                f'<a href="https://docs.blender.org/manual/en/dev/extensions/'
+                f'getting_started.html#manifest" target="_blank">not supported</a>.'
             )
 
 
@@ -330,7 +385,7 @@ class VersionVersionValidator(VersionValidator):
         version = extension.versions.filter(version=value).first()
 
         if version:
-            return (
+            return mark_safe(
                 f'The version {value} was already uploaded for this extension '
                 f'({extension.name})'
             )
@@ -347,7 +402,9 @@ class VersionMinValidator(VersionValidator):
 
         # Extensions were created in 4.2.0
         if Version(value) < Version('4.2.0'):
-            return 'Manifest value error: blender_version_min should be at least "4.2.0"'
+            return mark_safe(
+                'Manifest value error: <code>blender_version_min</code> should be at least "4.2.0"'
+            )
 
 
 class VersionMaxValidator(VersionValidator):
@@ -364,18 +421,20 @@ class TaglineValidator(StringValidator):
             return err_message
 
         if not value:
-            return 'Manifest value error: tagline cannot be empty.'
+            return mark_safe('Manifest value error: <code>tagline</code> cannot be empty.')
 
         if value[-1] in {'.', '!', '?'}:
-            return "Manifest value error: tagline cannot end with any punctuation (.!?)."
+            return mark_safe(
+                "Manifest value error: <code>tagline</code> cannot end with any punctuation (.!?)."
+            )
 
         # TODO: get this from the model, the following code was supposed to work, however
         # it does not (it did for Extensions).
         # Version._meta.get_field('tagline').max_length
         max_length = 64
         if len(value) > max_length:
-            return (
+            return mark_safe(
-                f'Manifest value error: tagline is too long ({len(value)}), '
+                f'Manifest value error: <code>tagline</code> is too long ({len(value)}), '
                 f'max-length: {max_length} characters'
             )
 
@@ -389,7 +448,7 @@ class ManifestValidator:
         'license': LicenseValidator,
         'maintainer': StringValidator,
         'name': StringValidator,
-        'schema_version': VersionValidator,
+        'schema_version': SchemaVersionValidator,
         'tagline': TaglineValidator,
         'type': TypeValidator,
         'version': VersionVersionValidator,
@@ -432,11 +491,9 @@ class ManifestValidator:
 
         if missing_fields:
             errors.append(
-                _(
                 'The following values are missing from the manifest file: '
                 f'{", ".join(missing_fields)}'
             )
-            )
 
         # Add the wrong field error messages to the general errors.
         errors += wrong_fields

notifications/admin.py

@@ -0,0 +1,20 @@
+from django.contrib import admin
+
+from notifications.models import Notification
+
+
+class NotificationAdmin(admin.ModelAdmin):
+    readonly_fields = (
+        'recipient',
+        'action',
+        'email_sent',
+        'processed_by_mailer_at',
+        'read_at',
+    )
+    fields = readonly_fields
+
+    def get_queryset(self, request):
+        return Notification.objects.all()
+
+
+admin.site.register(Notification, NotificationAdmin)

notifications/apps.py

@@ -0,0 +1,9 @@
+from django.apps import AppConfig
+
+
+class NotificationsConfig(AppConfig):
+    default_auto_field = 'django.db.models.BigAutoField'
+    name = 'notifications'
+
+    def ready(self):
+        import notifications.signals  # noqa: F401

notifications/management/commands/ensure_followers.py

@@ -0,0 +1,35 @@
+"""Create all necessary follow records."""
+import logging
+
+from actstream.actions import follow
+from django.contrib.auth.models import Group
+from django.core.management.base import BaseCommand
+
+from constants.activity import Flag
+from extensions.models import Extension
+from reviewers.models import ApprovalActivity
+
+logger = logging.getLogger(__name__)
+logger.setLevel(logging.INFO)
+
+
+class Command(BaseCommand):
+    def handle(self, *args, **options):  # noqa: D102
+        # TODO? keep a record of explicit unfollow requests to avoid re-following
+        extensions = Extension.objects.all()
+        moderators = Group.objects.get(name='moderators').user_set.all()
+        for extension in extensions:
+            authors = extension.authors.all()
+            for recipient in authors:
+                _follow_with_log(recipient, extension, Flag.AUTHOR)
+            for recipient in moderators:
+                _follow_with_log(recipient, extension, Flag.MODERATOR)
+
+        approval_activity_items = ApprovalActivity.objects.all().select_related('extension', 'user')
+        for item in approval_activity_items:
+            _follow_with_log(item.user, item.extension, Flag.REVIEWER)
+
+
+def _follow_with_log(user, target, flag):
+    follow(user, target, send_action=False, flag=flag)
+    logger.info(f'{user} follows {target} with flag={flag}')

notifications/management/commands/send_notification_emails.py

@@ -0,0 +1,52 @@
+"""Send user notifications as emails, at most once delivery."""
+import logging
+
+from django.conf import settings
+from django.core.mail import send_mail
+from django.core.management.base import BaseCommand
+from django.utils import timezone
+
+from notifications.models import Notification
+
+logger = logging.getLogger(__name__)
+logger.setLevel(logging.INFO)
+
+
+class Command(BaseCommand):
+    def handle(self, *args, **options):  # noqa: D102
+        unprocessed_notifications = Notification.objects.filter(processed_by_mailer_at=None)
+        for n in unprocessed_notifications:
+            logger.info(f'processing Notification pk={n.pk}')
+            n.processed_by_mailer_at = timezone.now()
+            recipient = n.recipient
+            if not recipient.is_subscribed_to_notification_emails:
+                logger.info(f'{recipient} is not subscribed, skipping')
+                n.save()
+                continue
+            # check that email is confirmed to avoid spamming unsuspecting email owners
+            if recipient.confirmed_email_at is None:
+                logger.info(f'{recipient} has unconfirmed email, skipping')
+                n.save()
+                continue
+            # FIXME test with only internal emails first
+            if not recipient.email.endswith('@blender.org'):
+                logger.info('skipping: not an internal email')
+                n.save()
+                continue
+            n.email_sent = True
+            # first mark as processed, then send: avoid spamming in case of a crash-loop
+            n.save()
+            logger.info(f'sending an email to {recipient}: {n.action}')
+            send_notification_email(n)
+
+
+def send_notification_email(notification):
+    # TODO construct a proper phrase, depending on the verb,
+    # possibly share a template with NotificationsView
+    subject, message = notification.format_email()
+    send_mail(
+        subject,
+        message,
+        settings.DEFAULT_FROM_EMAIL,
+        [notification.recipient.email],
+    )

notifications/migrations/0001_initial.py

@@ -0,0 +1,33 @@
+# Generated by Django 4.2.11 on 2024-04-16 15:56
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    initial = True
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('actstream', '0003_add_follow_flag'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='Notification',
+            fields=[
+                ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('recipient', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)),
+                ('action', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='actstream.action')),
+                ('email_sent', models.BooleanField(default=False)),
+                ('processed_by_mailer_at', models.DateTimeField(default=None, null=True)),
+                ('read_at', models.DateTimeField(default=None, null=True)),
+            ],
+            options={
+                'indexes': [models.Index(fields=['processed_by_mailer_at'], name='notificatio_process_fc95bc_idx'), models.Index(fields=['recipient', 'read_at'], name='notificatio_recipie_564b1f_idx')],
+                'unique_together': {('recipient', 'action')},
+            },
+        ),
+    ]

notifications/models.py

@@ -0,0 +1,56 @@
+from actstream.models import Action
+from django.contrib.auth import get_user_model
+from django.db import models
+
+from constants.activity import Verb
+from utils import absolutify
+
+User = get_user_model()
+
+
+class Notification(models.Model):
+    """Notification records are created in Action's post_save signal.
+    When a user marks a notification as read, read_at is set.
+    send_notification_emails management command runs periodically in background and sends all
+    notifications that haven't been processed yet, read_at is not checked when sending emails.
+    email_sent flag is used only to record the fact that we attempted to send an email.
+    A user can unsubscribe from notification emails in their profile settings.
+    """
+
+    recipient = models.ForeignKey(User, null=False, on_delete=models.CASCADE)
+    action = models.ForeignKey(Action, null=False, on_delete=models.CASCADE)
+    email_sent = models.BooleanField(default=False, null=False)
+    processed_by_mailer_at = models.DateTimeField(default=None, null=True)
+    read_at = models.DateTimeField(default=None, null=True)
+
+    class Meta:
+        indexes = [
+            models.Index(fields=['processed_by_mailer_at']),
+            models.Index(fields=['recipient', 'read_at']),
+        ]
+        unique_together = ['recipient', 'action']
+
+    def format_email(self):
+        action = self.action
+        subject = f'New Activity: {action.actor.full_name} {action.verb} {action.target}'
+        url = self.get_absolute_url()
+        mesage = f'{action.actor.full_name} {action.verb} {action.target}: {url}'
+        return (subject, mesage)
+
+    def get_absolute_url(self):
+        if self.action.verb == Verb.RATED_EXTENSION:
+            url = self.action.target.get_ratings_url()
+        elif self.action.verb in [
+            Verb.APPROVED,
+            Verb.COMMENTED,
+            Verb.REQUESTED_CHANGES,
+            Verb.REQUESTED_REVIEW,
+        ]:
+            url = self.action.target.get_review_url()
+        elif self.action.action_object is not None:
+            url = self.action.action_object.get_absolute_url()
+        else:
+            url = self.action.target.get_absolute_url()
+
+        # TODO? url cloacking to mark visited notifications as read automatically
+        return absolutify(url)

notifications/signals.py

@@ -0,0 +1,57 @@
+import logging
+
+from actstream.models import Action, Follow
+from django.contrib.auth import get_user_model
+from django.db.models.signals import post_save
+from django.dispatch import receiver
+
+from constants.activity import Flag, Verb
+from notifications.models import Notification
+
+logger = logging.getLogger(__name__)
+
+VERB2FLAGS = {
+    Verb.APPROVED: [Flag.AUTHOR, Flag.REVIEWER],
+    Verb.COMMENTED: [Flag.AUTHOR, Flag.REVIEWER],
+    Verb.RATED_EXTENSION: [Flag.AUTHOR],
+    Verb.REPORTED_EXTENSION: [Flag.MODERATOR],
+    Verb.REPORTED_RATING: [Flag.MODERATOR],
+    Verb.REQUESTED_CHANGES: [Flag.AUTHOR, Flag.REVIEWER],
+    Verb.REQUESTED_REVIEW: [Flag.MODERATOR, Flag.REVIEWER],
+}
+
+
+@receiver(post_save, sender=Action)
+def _create_notifications(
+    sender: object,
+    instance: Action,
+    created: bool,
+    raw: bool,
+    **kwargs: object,
+) -> None:
+    if raw:
+        return
+    if not created:
+        return
+
+    if not instance.target:
+        logger.warning(f'ignoring an unexpected Action without a target, verb={instance.verb}')
+        return
+
+    notifications = []
+
+    flags = VERB2FLAGS.get(instance.verb, None)
+    if not flags:
+        logger.warning(f'no follower flags for verb={instance.verb}, nobody will be notified')
+        return
+
+    followers = Follow.objects.for_object(instance.target).filter(flag__in=flags)
+    user_ids = followers.values_list('user', flat=True)
+    followers = get_user_model().objects.filter(id__in=user_ids)
+
+    for recipient in followers:
+        if recipient == instance.actor:
+            continue
+        notifications.append(Notification(recipient=recipient, action=instance))
+    if len(notifications) > 0:
+        Notification.objects.bulk_create(notifications)

notifications/templates/notifications/notification_list.html

@@ -0,0 +1,22 @@
+{% extends "common/base.html" %}
+{% load i18n %}
+
+{% block page_title %}{% blocktranslate %}Notifications{% endblocktranslate %}{% endblock page_title %}
+
+{% block content %}
+{% if notification_list %}
+  {% for notification in notification_list %}
+    <div class="row">
+      {{ notification.action }}
+      {% if notification.read_at %}
+      {% else %}
+      {% blocktranslate %}Mark as read{% endblocktranslate %}
+      {% endif %}
+    </div>
+  {% endfor %}
+{% else %}
+<p>
+  {% blocktranslate %}You have no notifications{% endblocktranslate %}
+</p>
+{% endif %}
+{% endblock content %}

notifications/tests/test_follow_logic.py

@@ -0,0 +1,142 @@
+from pathlib import Path
+
+from django.test import TestCase
+from django.urls import reverse
+
+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, create_moderator
+from files.models import File
+from notifications.models import Notification
+from reviewers.models import ApprovalActivity
+
+TEST_FILES_DIR = Path(__file__).resolve().parent / '../../extensions/tests/files'
+
+
+class TestTasks(TestCase):
+    fixtures = ['dev', 'licenses']
+
+    def test_ratings(self):
+        extension = create_approved_version(ratings=[]).extension
+        author = extension.authors.first()
+        notification_nr = Notification.objects.filter(recipient=author).count()
+        some_user = UserFactory()
+        self.client.force_login(some_user)
+        url = extension.get_rate_url()
+        response = self.client.post(url, {'score': 3, 'text': 'rating text'})
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(extension.ratings.count(), 1)
+        new_notification_nr = Notification.objects.filter(recipient=author).count()
+        self.assertEqual(new_notification_nr, notification_nr + 1)
+
+    def test_abuse(self):
+        extension = create_approved_version(ratings=[]).extension
+        moderator = create_moderator()
+        notification_nr = Notification.objects.filter(recipient=moderator).count()
+        some_user = UserFactory()
+        self.client.force_login(some_user)
+        url = extension.get_report_url()
+        self.client.post(
+            url,
+            {
+                'message': 'test message',
+                'reason': '127',
+                'version': '',
+            },
+        )
+        new_notification_nr = Notification.objects.filter(recipient=moderator).count()
+        self.assertEqual(new_notification_nr, notification_nr + 1)
+
+    def test_new_extension_submitted(self):
+        moderator = create_moderator()
+        notification_nr = Notification.objects.filter(recipient=moderator).count()
+        some_user = UserFactory()
+        file_data = {
+            'metadata': {
+                'tagline': 'Get insight on the complexity of an edit',
+                'id': 'edit_breakdown',
+                'name': 'Edit Breakdown',
+                'version': '0.1.0',
+                'blender_version_min': '4.2.0',
+                'type': 'add-on',
+                'schema_version': "1.0.0",
+            },
+            'file_hash': 'sha256:4f3664940fc41641c7136a909270a024bbcfb2f8523a06a0d22f85c459b0b1ae',
+            'size_bytes': 53959,
+            'tags': ['Sequencer'],
+            'version_str': '0.1.0',
+            'slug': 'edit-breakdown',
+        }
+        file = FileFactory(
+            type=File.TYPES.BPY,
+            user=some_user,
+            original_hash=file_data['file_hash'],
+            hash=file_data['file_hash'],
+            metadata=file_data['metadata'],
+        )
+        create_version(
+            file=file,
+            extension__name=file_data['metadata']['name'],
+            extension__slug=file_data['metadata']['id'].replace("_", "-"),
+            extension__website=None,
+            tagline=file_data['metadata']['tagline'],
+            version=file_data['metadata']['version'],
+            blender_version_min=file_data['metadata']['blender_version_min'],
+            schema_version=file_data['metadata']['schema_version'],
+        )
+        self.client.force_login(some_user)
+        data = {
+            # Most of these values should come from the form's initial values, set in the template
+            # Version fields
+            'release_notes': 'initial release',
+            # Extension fields
+            'description': 'Rather long and verbose description',
+            'support': 'https://example.com/issues',
+            # Previews
+            'form-TOTAL_FORMS': ['2'],
+            'form-INITIAL_FORMS': ['0'],
+            'form-MIN_NUM_FORMS': ['0'],
+            'form-MAX_NUM_FORMS': ['1000'],
+            'form-0-id': '',
+            'form-0-caption': ['First Preview Caption Text'],
+            'form-1-id': '',
+            'form-1-caption': ['Second Preview Caption Text'],
+            # Submit for Approval.
+            'submit_draft': '',
+        }
+        file_name1 = 'test_preview_image_0001.png'
+        file_name2 = 'test_preview_image_0002.png'
+        with open(TEST_FILES_DIR / file_name1, 'rb') as fp1, open(
+            TEST_FILES_DIR / file_name2, 'rb'
+        ) as fp2:
+            files = {
+                'form-0-source': fp1,
+                'form-1-source': fp2,
+            }
+            self.client.post(file.get_submit_url(), {**data, **files})
+        new_notification_nr = Notification.objects.filter(recipient=moderator).count()
+        self.assertEqual(new_notification_nr, notification_nr + 1)
+
+    def test_approval_queue_activity(self):
+        extension = create_approved_version(ratings=[]).extension
+        author = extension.authors.first()
+        moderator = create_moderator()
+        some_user = UserFactory()
+        notification_nrs = {}
+        for user in [author, moderator, some_user]:
+            notification_nrs[user.pk] = Notification.objects.filter(recipient=user).count()
+        # both moderator and some_user start following only after their first comment
+        self._leave_a_comment(moderator, extension, 'need to check this')
+        self._leave_a_comment(some_user, extension, 'this is bad')
+        self._leave_a_comment(moderator, extension, 'thanks for the heads up')
+        new_notification_nrs = {}
+        for user in [author, moderator, some_user]:
+            new_notification_nrs[user.pk] = Notification.objects.filter(recipient=user).count()
+        self.assertEqual(new_notification_nrs[author.pk], notification_nrs[author.pk] + 3)
+        self.assertEqual(new_notification_nrs[moderator.pk], notification_nrs[moderator.pk] + 1)
+        self.assertEqual(new_notification_nrs[some_user.pk], notification_nrs[some_user.pk] + 1)
+
+    def _leave_a_comment(self, user, extension, text):
+        self.client.force_login(user)
+        url = reverse('reviewers:approval-comment', args=[extension.slug])
+        self.client.post(url, {'type': ApprovalActivity.ActivityType.COMMENT, 'message': text})

notifications/urls.py

@@ -0,0 +1,25 @@
+from django.urls import path, include
+
+import notifications.views as views
+
+app_name = 'notifications'
+urlpatterns = [
+    path(
+        'notifications/',
+        include(
+            [
+                path('', views.NotificationsView.as_view(), name='notifications'),
+                path(
+                    'mark-read-all/',
+                    views.MarkReadAllView.as_view(),
+                    name='notifications-mark-read-all',
+                ),
+                path(
+                    '<int:pk>/mark-read/',
+                    views.MarkReadView.as_view(),
+                    name='notifications-mark-read',
+                ),
+            ],
+        ),
+    ),
+]

notifications/views.py

@@ -0,0 +1,49 @@
+"""Notifications pages."""
+from django.contrib.auth.mixins import LoginRequiredMixin
+from django.http import HttpResponseForbidden
+from django.http.response import JsonResponse
+from django.utils import timezone
+from django.views.generic import ListView
+from django.views.generic.detail import SingleObjectMixin
+from django.views.generic.edit import FormView
+from django.views import View
+
+from notifications.models import Notification
+
+
+class NotificationsView(LoginRequiredMixin, ListView):
+    model = Notification
+    ordering = None  # FIXME
+    paginate_by = 10
+
+    def get_queryset(self):
+        return Notification.objects.filter(recipient=self.request.user)
+
+
+class MarkReadAllView(LoginRequiredMixin, FormView):
+    model = Notification
+    raise_exception = True
+
+    def post(self, request, *args, **kwargs):
+        """Mark all previously unread notifications as read."""
+        unread = self.model.objects.filter(recipient=request.user, read_at__isnull=True)
+        now = timezone.now()
+        for notification in unread:
+            notification.read_at = now
+
+        Notification.objects.bulk_update(unread, ['read_at'])
+
+        return JsonResponse({})
+
+
+class MarkReadView(LoginRequiredMixin, SingleObjectMixin, View):
+    model = Notification
+    raise_exception = True
+
+    def post(self, request, *args, **kwargs):
+        notification = self.get_object()
+        if notification.recipient != request.user:
+            return HttpResponseForbidden()
+        notification.read_at = timezone.now()
+        notification.save(update_fields=['read_at'])
+        return JsonResponse({})

playbooks/common/handlers/main.yaml

@@ -5,6 +5,7 @@
   ansible.builtin.systemd: name={{ item }} daemon_reload=yes state=restarted enabled=yes
   with_items:
     - "{{ service_name }}"
+    - "{{ service_name }}-background"
   tags:
     - always
 

playbooks/install.yaml

@@ -9,6 +9,8 @@
     - name: Installing required packages
       ansible.builtin.apt: name={{ item }} state=present
       with_items:
+        - clamav-daemon
+        - clamav-unofficial-sigs
         - git
         - libpq-dev
         - nginx-full

playbooks/tasks/setup_other_services.yaml

@@ -7,6 +7,12 @@
   with_fileglob:
     - ../templates/other-services/*.service
 
+- name: Enabling clamav-daemon
+  ansible.builtin.systemd:
+    name: clamav-daemon
+    state: started
+    enabled: true
+
 - name: Enabling systemd services
   ansible.builtin.systemd:
     name: "{{ service_name }}-{{ item }}"

playbooks/templates/asgi/asgi.service

@@ -11,8 +11,6 @@ ExecReload=/bin/kill -s HUP $MAINPID
 Restart=always
 KillMode=mixed
 Type=notify
-StandardError=syslog
-StandardOutput=syslog
 SyslogIdentifier={{ service_name }}
 NotifyAccess=all
 WorkingDirectory={{ dir.source }}

playbooks/templates/other-services/background-restart.service

@@ -4,6 +4,4 @@ Description=restart {{ background_service_name }} task handler
 [Service]
 Type=oneshot
 ExecStart=/bin/systemctl restart {{ background_service_name }}
-StandardError=syslog
-StandardOutput=syslog
 SyslogIdentifier={{ service_name }}

playbooks/templates/other-services/background.service

@@ -7,11 +7,9 @@ User={{ user }}
 Group={{ group }}
 EnvironmentFile={{ env_file }}
 ExecStart={{ dir.source }}/.venv/bin/python {{ dir.source }}/manage.py process_tasks
+ExecStop=kill -s SIGTSTP $MAINPID
 Restart=always
-KillSignal=SIGQUIT
 Type=idle
-StandardError=syslog
-StandardOutput=syslog
 SyslogIdentifier={{ service_name }}
 NotifyAccess=all
 WorkingDirectory={{ dir.source }}

playbooks/templates/other-services/clearsessions.service

@@ -7,8 +7,6 @@ User={{ user }}
 Group={{ group }}
 EnvironmentFile={{ env_file }}
 ExecStart={{ dir.source }}/.venv/bin/python {{ dir.source }}/manage.py clearsessions
-StandardError=syslog
-StandardOutput=syslog
 SyslogIdentifier={{ service_name }}
 NotifyAccess=all
 WorkingDirectory={{ dir.source }}

playbooks/templates/other-services/queue-deletion-requests.service

@@ -7,8 +7,6 @@ User={{ user }}
 Group={{ group }}
 EnvironmentFile={{ env_file }}
 ExecStart={{ dir.source }}/.venv/bin/python {{ dir.source }}/manage.py queue_deletion_request
-StandardError=syslog
-StandardOutput=syslog
 SyslogIdentifier={{ service_name }}
 NotifyAccess=all
 WorkingDirectory={{ dir.source }}

playbooks/templates/other-services/send-notification-emails.service

@@ -0,0 +1,20 @@
+[Unit]
+Description=Send notification emails for {{ project_name }} {{ env|capitalize }}
+
+[Service]
+Type=oneshot
+User={{ user }}
+Group={{ group }}
+EnvironmentFile={{ env_file }}
+ExecStart={{ dir.source }}/.venv/bin/python {{ dir.source }}/manage.py send_notification_emails
+SyslogIdentifier={{ service_name }}
+NotifyAccess=all
+WorkingDirectory={{ dir.source }}
+
+PrivateTmp=true
+ProtectHome=true
+ProtectSystem=full
+CapabilityBoundingSet=~CAP_SYS_ADMIN
+
+[Install]
+WantedBy=multi-user.target

playbooks/templates/other-services/send-notification-emails.timer

@@ -0,0 +1,8 @@
+[Timer]
+OnCalendar=*-*-* *:*:00
+RandomizedDelaySec=3
+AccuracySec=1us
+Persistent=true
+
+[Install]
+WantedBy=timer.target

playbooks/templates/other-services/write-stats.service

@@ -7,8 +7,6 @@ User={{ user }}
 Group={{ group }}
 EnvironmentFile={{ env_file }}
 ExecStart={{ dir.source }}/.venv/bin/python {{ dir.source }}/manage.py write_stats
-StandardError=syslog
-StandardOutput=syslog
 SyslogIdentifier={{ service_name }}
 NotifyAccess=all
 WorkingDirectory={{ dir.source }}

ratings/admin.py

@@ -16,12 +16,12 @@ class RatingTypeFilter(admin.SimpleListFilter):
     parameter_name = 'type'
 
     def lookups(self, request, model_admin):
-        """
+        """Return a list of lookup option tuples.
-        Returns a list of tuples. The first element in each
+
-        tuple is the coded value for the option that will
+        The first element in each tuple is the coded value
-        appear in the URL query. The second element is the
+        for the option that will appear in the URL query.
-        human-readable name for the option that will appear
+        The second element is the human-readable name for
-        in the right sidebar.
+        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):
-        """
+        """Return the filtered queryset.
-        Returns the filtered queryset based on the value
+
-        provided in the query string and retrievable via
+        Filter based on the value provided in the query string
-        `self.value()`.
+        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,7 @@ class RatingAdmin(admin.ModelAdmin):
     fields = ('status',) + readonly_fields
     list_display = (
         'date_created',
-        'is_deleted',
+        'extension',
         'user',
         'ip_address',
         'score',
@@ -71,7 +70,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 +93,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)

ratings/apps.py

@@ -6,4 +6,7 @@ class RatingsConfig(AppConfig):
     name = 'ratings'
 
     def ready(self):
+        from actstream import registry
         import ratings.signals  # noqa: F401
+
+        registry.register(self.get_model('Rating'))

ratings/migrations/0005_remove_rating_date_deleted.py

@@ -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',
+        ),
+    ]