extensions/models.py

@@ -677,10 +677,6 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model):
     def single_file(self):
         return len(self.files.all()) == 1
 
-    @property
-    def can_upload_more_files(self):
-        return len(self.get_remaining_platforms()) > 0
-
     def add_file(self, file: File):
         current_platforms = set([p.slug for p in self.platforms.all()])
         if not current_platforms:
@@ -880,16 +876,6 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model):
             },
         )
 
-    def get_version_upload_url(self) -> str:
-        return reverse(
-            'extensions:version-upload',
-            kwargs={
-                'type_slug': self.extension.type_slug,
-                'slug': self.extension.slug,
-                'pk': self.pk,
-            },
-        )
-
     @property
     def update_url(self) -> str:
         return reverse(

extensions/templates/extensions/new_version_finalise.html

@@ -48,12 +48,6 @@
                 </a>
               {% endfor %}
               {% endwith %}
-              {% if form.instance.can_upload_more_files %}
-                <a href="{{ form.instance.get_version_upload_url }}" class="btn btn-primary">
-                  <i class="i-upload"></i>
-                  {% trans 'Upload files for other platforms' %}
-                </a>
-              {% endif %}
               </div>
             </section>
           </div>

extensions/templates/extensions/submit.html

@@ -17,9 +17,7 @@
     <div class="col-md-10 mx-auto">
       <div class="card p-5">
         <h1>
-          {% if version %}
-          {% trans 'Upload New File for' %} {{ version }}
-          {% elif extension %}
+          {% if extension %}
           {% trans 'Upload New' %} {{ extension.get_type_display }} {% trans 'Version' %}
           {% else %}
           {% trans 'Upload Extension' %}

extensions/tests/test_models.py

@@ -90,25 +90,33 @@ class VersionTest(TestCase):
 
         self.assertEqual(response.status_code, 200, path)
 
-    def test_can_upload_more_files(self):
+    def test_get_remaining_platforms(self):
         version_platforms_none = create_version(metadata__platforms=None)
-        self.assertFalse(version_platforms_none.can_upload_more_files)
+        self.assertEqual(version_platforms_none.get_remaining_platforms(), set())
 
         version_platforms_empty = create_version(metadata__platforms=[])
-        self.assertFalse(version_platforms_empty.can_upload_more_files)
+        self.assertEqual(version_platforms_empty.get_remaining_platforms(), set())
 
         version_platforms_some = create_version(metadata__platforms=['linux-x64', 'windows-x64'])
-        self.assertTrue(version_platforms_some.can_upload_more_files)
+        self.assertTrue(version_platforms_some.get_remaining_platforms())
 
         version_two_files = create_version(metadata__platforms=['linux-x64'])
         file = FileFactory(metadata__platforms=['windows-x64'])
         version_two_files.files.add(file)
-        self.assertTrue(version_two_files.can_upload_more_files)
+        self.assertTrue(version_two_files.get_remaining_platforms())
 
         version_platforms_all = create_version(
             metadata__platforms=[p.slug for p in Platform.objects.all()]
         )
-        self.assertFalse(version_platforms_all.can_upload_more_files)
+        self.assertEqual(version_platforms_all.get_remaining_platforms(), set())
+
+        skip_platforms = set(['linux-x64', 'windows-x64'])
+        version_platforms_without_some = create_version(
+            metadata__platforms=[
+                p.slug for p in Platform.objects.all() if p.slug not in skip_platforms
+            ]
+        )
+        self.assertEqual(version_platforms_without_some.get_remaining_platforms(), skip_platforms)
 
     def test_collect_platforms_across_files(self):
         version = create_version(metadata__platforms=['linux-x64'])

extensions/tests/test_submit.py

@@ -619,7 +619,7 @@ class MultiPlatformUploadTest(TestCase):
 
         with open(file_windows, 'rb') as fp:
             response = self.client.post(
-                extension.latest_version.get_version_upload_url(),
+                extension.get_new_version_url(),
                 {'source': fp, 'agreed_with_terms': True},
             )
         self.assertEqual(response.status_code, 302)
@@ -627,6 +627,17 @@ class MultiPlatformUploadTest(TestCase):
         self.assertEqual(extension.latest_version.files.count(), 2)
         self.assertEqual(extension.latest_version.platforms.count(), 2)
 
+        # can't upload the same file a second time
+        with open(file_windows, 'rb') as fp:
+            response = self.client.post(
+                extension.get_new_version_url(),
+                {'source': fp, 'agreed_with_terms': True},
+            )
+        self.assertEqual(response.status_code, 200)
+        extension.refresh_from_db()
+        self.assertEqual(extension.latest_version.files.count(), 2)
+        self.assertEqual(extension.latest_version.platforms.count(), 2)
+
 
 class DraftsWarningTest(TestCase):
     fixtures = ['licenses']

extensions/urls.py

@@ -96,11 +96,6 @@ urlpatterns = [
                     public.extension_version_platform_download,
                     name='version-platform-download',
                 ),
-                path(
-                    '<slug:slug>/manage/versions/<int:pk>/upload/',
-                    manage.UploadVersionFileView.as_view(),
-                    name='version-upload',
-                ),
                 path('<slug:slug>/versions/', manage.VersionsView.as_view(), name='versions'),
             ],
         ),

extensions/views/api.py

@@ -202,7 +202,6 @@ class UploadExtensionVersionView(APIView):
             )
 
         form = FileFormSkipAgreed(
-            allow_existing_version=True,
             data={},
             extension=extension,
             request=request,

extensions/views/manage.py

@@ -214,7 +214,11 @@ class NewVersionView(
     @transaction.atomic
     def form_valid(self, form):
         response = super().form_valid(form)
-        self.extension.create_version_from_file(self.object)
+        manifest_version = self.object.metadata['version']
+        if version := self.extension.versions.filter(version=manifest_version).first():
+            version.add_file(self.object)
+        else:
+            version = self.extension.create_version_from_file(self.object)
         return response
 
     def get_success_url(self):
@@ -273,46 +277,6 @@ class UpdateVersionView(LoginRequiredMixin, UserPassesTestMixin, UpdateView):
         return self.get_object().extension.has_maintainer(self.request.user)
 
 
-class UploadVersionFileView(
-    LoginRequiredMixin,
-    MaintainedExtensionMixin,
-    CreateView,
-):
-    """Upload a new file for an existing version. Multi-OS support."""
-
-    form_class = FileForm
-    template_name = 'extensions/submit.html'
-
-    def get_context_data(self, **kwargs):
-        ctx = super().get_context_data(**kwargs)
-        ctx['extension'] = self.extension
-        ctx['version'] = self.version
-        return ctx
-
-    def get_form_kwargs(self):
-        kwargs = super().get_form_kwargs()
-        kwargs['request'] = self.request
-        kwargs['extension'] = self.extension
-        kwargs['allow_existing_version'] = True
-        return kwargs
-
-    @transaction.atomic
-    def form_valid(self, form):
-        response = super().form_valid(form)
-        self.version.add_file(self.object)
-        return response
-
-    def get_success_url(self):
-        return reverse(
-            'extensions:new-version-finalise',
-            kwargs={
-                'type_slug': self.extension.type_slug,
-                'slug': self.extension.slug,
-                'pk': self.object.pk,
-            },
-        )
-
-
 class DraftExtensionView(
     LoginRequiredMixin,
     MaintainedExtensionMixin,

files/forms.py

@@ -74,7 +74,6 @@ class FileForm(forms.ModelForm):
     )
 
     def __init__(self, *args, **kwargs):
-        self.allow_existing_version = kwargs.pop('allow_existing_version', None)
         self.extension = kwargs.pop('extension', None)
         self.request = kwargs.pop('request')
         for field in self.base_fields:
@@ -169,7 +168,6 @@ class FileForm(forms.ModelForm):
                 ExtensionVersionManifestValidator(
                     manifest,
                     self.extension,
-                    self.allow_existing_version,
                 )
 
                 self.cleaned_data['metadata'] = manifest

files/validators.py

@@ -166,14 +166,9 @@ class ExtensionNameManifestValidator:
 
 
 class ExtensionVersionManifestValidator:
-    """Validates version existence in db and available platforms.
+    """Validates version existence in db and available platforms."""
 
-    allow_existing_version is meant to be used from API and UploadVersionFileNew
-    when a version either may exist or definitely exists.
-    TODO? create separate flags for these scenarios?
-    """
-
-    def __init__(self, manifest, extension_to_be_updated, allow_existing_version=False):
+    def __init__(self, manifest, extension_to_be_updated):
 
         # If the extension wasn't created yet, any version is valid
         if not extension_to_be_updated:
@@ -184,17 +179,6 @@ class ExtensionVersionManifestValidator:
         version = extension_to_be_updated.versions.filter(version=manifest_version).first()
         if not version:
             return
-        if not allow_existing_version:
-            raise ValidationError(
-                {
-                    'source': [
-                        f'The version {escape(manifest_version)} was already uploaded for this '
-                        f'extension ({extension_to_be_updated.name})'
-                    ],
-                },
-                code='invalid',
-            )
-
         # check for platforms
         # TODO test coverage
         remaining_platforms = version.get_remaining_platforms()