Draft: Make sure extensions always have a user #54
@ -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])
|
||||
|
||||
|
@ -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>
|
||||
|
@ -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)
|
||||
|
@ -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(),
|
||||
|
@ -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()
|
||||
|
@ -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())
|
||||
|
@ -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()
|
||||
|
@ -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):
|
||||
|
Loading…
Reference in New Issue
Block a user