extensions/models.py

@@ -237,6 +237,9 @@ class Extension(
     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])
 

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 extension.is_approved %}
                   {% trans 'Save Changes' %}
-                  {% else %}
-                  {% trans 'Save Draft' %}
-                  {% 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>

extensions/tests/test_submit.py

@@ -224,12 +224,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 +249,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 +272,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(Version.objects.count(), 0)
+        self.assertEqual(Extension.objects.count(), 1)
+        self.assertEqual(Version.objects.count(), 1)
         self.assertEqual(File.objects.filter(type=File.TYPES.IMAGE).count(), 0)
 
         self.client.force_login(self.file.user)

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
@@ -35,6 +34,11 @@ 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(),

extensions/views/manage.py

@@ -5,10 +5,16 @@ 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,
@@ -91,6 +97,7 @@ class UpdateExtensionView(
     MaintainedExtensionMixin,
     SuccessMessageMixin,
     UpdateView,
+    DraftMixin,
 ):
     model = Extension
     template_name = 'extensions/manage/update.html'
@@ -293,3 +300,78 @@ 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,
+    FormView,
+):
+    template_name = 'extensions/draft_finalise.html'
+    form_class = VersionForm
+
+    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()
+        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
+            extension_form.instance.status = extension_form.instance.STATUSES.AWAITING_REVIEW
+            extension_form.save()
+            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.extension.get_manage_url()

extensions/views/mixins.py

@@ -1,5 +1,6 @@
 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 django.core.exceptions import BadRequest
 
 from extensions.models import Extension
 from files.models import File
@@ -54,3 +55,30 @@ class ExtensionMixin:
             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()
+        if self.version is None:
+            error_message = f'Extension "{self.extension.name}" has no versions.'
+            raise BadRequest(error_message)
+        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, FormView
+from django.views.generic.edit import CreateView
 
-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()
 
-
-class SubmitFileView(LoginRequiredMixin, OwnsFileMixin, FormView):
-    template_name = 'extensions/submit_finalise.html'
-    form_class = VersionForm
-
-    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
+    @transaction.atomic
+    def form_valid(self, form):
+        """Create an extension and a version already, associated with the user."""
+        self.file = form.instance
 
         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 Extension.objects.update_or_create(type=self.file.type, **parsed_extension_fields)[0]
+                return False
 
-    def _get_version(self, extension) -> 'Version':
-        return Version.objects.update_or_create(
-            extension=extension, file=self.file, **self.file.parsed_version_fields
+        # Make sure an extension has a user associated to it from the beginning, otherwise
+        # it will prevent it from being re-uploaded and yet not show on My Extensions.
+        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()
+        return super().form_valid(form)

files/models.py

@@ -6,7 +6,6 @@ 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 files.utils import get_sha256
@@ -209,7 +208,7 @@ 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):