fix(crashmanager): prevent duplicate testcase extensions

server/crashmanager/forms.py

@@ -58,7 +58,7 @@ class Meta:
             "description": "Bug description",
             "security": "This is a security bug",
             "security_group": "Security group",
-            "testcase_filename": "Filename that will be used for the testcase",
+            "testcase_filename": "Testcase basename (without extension)",
             "blocks": "Blocks",
             "dependson": "Depends On",
         }
@@ -89,7 +89,7 @@ class Meta:
         labels = {
             "name": "Template name",
             "comment": "Comment",
-            "testcase_filename": "Filename that will be used for the testcase",
+            "testcase_filename": "Testcase basename (without extension)",
         }
         widgets = {
             "name": TextInput(),

server/crashmanager/migrations/0021_bugzilla_template_basename.py

@@ -0,0 +1,20 @@
+from django.db import migrations
+
+
+def strip_template_extensions(apps, schema_editor):
+    Template = apps.get_model("crashmanager", "BugzillaTemplate")
+    templates = Template.objects.using(schema_editor.connection.alias)
+    for template in templates.exclude(testcase_filename="").iterator():
+        filename = template.testcase_filename
+        if "." in filename:
+            templates.filter(pk=template.pk).update(
+                testcase_filename=filename.rsplit(".", 1)[0]
+            )
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("crashmanager", "0020_alter_bucket_optimizedsignature_and_more"),
+    ]
+
+    operations = [migrations.RunPython(strip_template_extensions)]

server/crashmanager/models.py

@@ -932,6 +932,7 @@ class BugzillaTemplate(models.Model):
     security = models.BooleanField(blank=False, default=False)
     security_group = models.TextField(blank=True)
     comment = models.TextField(blank=True)
+    # Stores only the basename; attachments use the original testcase extension.
     testcase_filename = models.TextField(blank=True)
     blocks = models.TextField(blank=True)
     dependson = models.TextField(blank=True)

server/crashmanager/tests/test_bugs.py

@@ -10,9 +10,13 @@
 """
 
 import logging
+from importlib import import_module
+from types import SimpleNamespace
 
 import pytest
 import requests
+from django.apps import apps
+from django.db import connection
 from django.urls import reverse
 
 from crashmanager.models import BugzillaTemplate
@@ -287,3 +291,23 @@ def test_create_external_bug_comment_simple_get(client, cm):
     )
     LOG.debug(response)
     assert response.status_code == requests.codes["ok"]
+
+
+@pytest.mark.parametrize(
+    ("filename", "basename"),
+    [
+        ("testcase.zip", "testcase"),
+        ("testcase.html", "testcase"),
+        ("testcase.min.js", "testcase.min"),
+        ("testcase", "testcase"),
+        ("", ""),
+    ],
+)
+def test_template_basename_migration(cm, filename, basename):
+    template = cm.create_template()
+    template.testcase_filename = filename
+    template.save()
+    migration = import_module("crashmanager.migrations.0021_bugzilla_template_basename")
+    migration.strip_template_extensions(apps, SimpleNamespace(connection=connection))
+    template.refresh_from_db()
+    assert template.testcase_filename == basename

server/frontend/src/components/Bugs/Comments/PublicationForm.vue

@@ -235,7 +235,7 @@ import { defineComponent } from "vue";
 import * as api from "../../../api";
 import * as bugzillaApi from "../../../bugzilla_api";
 import * as HandlebarsHelpers from "../../../handlebars_helpers";
-import { errorParser } from "../../../helpers";
+import { errorParser, parseFilename, buildFilename } from "../../../helpers";
 import CrashDataSection from "../CrashDataSection.vue";
 import HelpPopover from "../HelpPopover.vue";
 import TestCaseSection from "../TestCaseSection.vue";
@@ -395,7 +395,7 @@ export default defineComponent({
       return attachmentFilenameExtension;
     },
     filenameWithExtension() {
-      return this.fileName + "." + this.fileExtension;
+      return buildFilename(this.fileName, this.fileExtension);
     },
     fileMimetype() {
       const mimeType = mime.getType(this.filenameWithExtension);
@@ -417,6 +417,7 @@ export default defineComponent({
       this.provider = this.providers.find((p) => p.id === newVal);
     },
     selectedTemplate(newVal) {
+      this.newFileName = null;
       this.template = this.templates.find((t) => t.id === newVal);
     },
   },
@@ -573,19 +574,13 @@ export default defineComponent({
     getFileDetails() {
       let attachmentFilename = "";
       let attachmentFilenameExtension = "";
-      if (this.entry) {
-        // extract file name
-        const splittedAttachmentFilename = this.template?.testcase_filename
-          ? this.template.testcase_filename
-          : this.entry.testcase.split("/");
-
-        const attachmentFilenameAndExtension =
-          splittedAttachmentFilename[
-            splittedAttachmentFilename?.length - 1
-          ].split(".");
-
-        attachmentFilename = attachmentFilenameAndExtension[0];
-        attachmentFilenameExtension = attachmentFilenameAndExtension[1];
+      if (this.entry?.testcase) {
+        const { basename, extension } = parseFilename(
+          this.entry.testcase,
+          this.template?.testcase_filename,
+        );
+        attachmentFilename = basename;
+        attachmentFilenameExtension = extension;
       }
 
       return { attachmentFilename, attachmentFilenameExtension };

server/frontend/src/components/Bugs/PublicationForm.vue

@@ -525,16 +525,6 @@
               class="form-control"
             />
           </div>
-          <div class="col-md-2">
-            <label for="file_extension">File extension:</label>
-            <input
-              id="file_extension"
-              type="text"
-              class="form-control"
-              disabled
-              :value="fileExtension"
-            />
-          </div>
         </div>
 
         <hr />
@@ -823,7 +813,9 @@ export default defineComponent({
     });
 
     watch([entry, template], () => {
-      if (entry.value) {
+      if (props.isBugTemplateCreation) {
+        fileName.value = template.value?.testcase_filename ?? "";
+      } else if (entry.value?.testcase) {
         const { basename, extension } = parseFilename(
           entry.value.testcase,
           template.value?.testcase_filename,
@@ -1123,7 +1115,7 @@ export default defineComponent({
 
       const payload = {
         ...template.value,
-        testcase_filename: filenameWithExtension.value,
+        testcase_filename: fileName.value,
         product: product.value,
         component: component.value,
         op_sys: opSys.value,

server/frontend/src/components/Bugs/TestCaseSection.vue

@@ -58,7 +58,7 @@
 </template>
 
 <script>
-import { defineComponent, onMounted, ref, watch } from "vue";
+import { computed, defineComponent, onMounted, ref, watch } from "vue";
 import * as api from "../../api";
 
 export default defineComponent({
@@ -85,29 +85,25 @@ export default defineComponent({
     },
     fileExtension: {
       type: String,
-      required: true,
+      default: null,
     },
     fileName: {
       type: String,
       required: true,
     },
   },
 
-  emits: [
-    "update-not-attach-test",
-    "update-filename",
-    "update-content",
-    "update-attachment-extension",
-  ],
+  emits: ["update-not-attach-test", "update-filename", "update-content"],
   setup(props, { emit }) {
     const notAttachTest = ref(false);
-    const filename = ref("");
-    const filenameExtension = ref("");
+    const filename = computed({
+      get: () => props.fileName,
+      set: (value) => emit("update-filename", value),
+    });
     const content = ref("Content loading...");
 
     onMounted(async () => {
       notAttachTest.value = props.initialNotAttachTest;
-      filename.value = props.fileName;
 
       if (!props.entry.testcase_isbinary) {
         content.value = await api.retrieveCrashTestCase(props.entry.id);
@@ -119,15 +115,6 @@ export default defineComponent({
       emit("update-not-attach-test", newValue);
     });
 
-    watch(filename, (newValue) => {
-      if (newValue) {
-        emit("update-filename", newValue);
-      } else {
-        // Prevent removing the whole section when the filename input is empty
-        emit("update-filename", " ");
-      }
-    });
-
     watch(content, (newValue) => {
       emit("update-content", newValue);
     });
@@ -136,7 +123,6 @@ export default defineComponent({
       notAttachTest,
       filename,
       content,
-      filenameExtension,
     };
   },
 });

server/frontend/src/helpers.js

@@ -224,7 +224,7 @@ export function formatMonthly(d) {
 /**
  * Parse a testcase file path into basename and extension.
  * @param {string} testcasePath - Testcase file path.
- * @param {string|null} templateBasename - Optional template basename or full filename.
+ * @param {string|null} templateBasename - Optional template basename (without an extension).
  * @returns {{basename: string, extension: string|null}} Object with basename and extension.
  */
 export function parseFilename(testcasePath, templateBasename = null) {
@@ -237,19 +237,9 @@ export function parseFilename(testcasePath, templateBasename = null) {
     ? originalParts[originalParts.length - 1]
     : null;
 
-  let basename;
-  if (templateBasename) {
-    // Templates store the complete filename, while the form edits the basename.
-    const suffix = extension ? `.${extension}` : null;
-    basename =
-      suffix && templateBasename.endsWith(suffix)
-        ? templateBasename.slice(0, -suffix.length)
-        : templateBasename;
-  } else {
-    basename = hasExtension
-      ? originalParts.slice(0, -1).join(".")
-      : originalParts[0];
-  }
+  const basename =
+    templateBasename ||
+    (hasExtension ? originalParts.slice(0, -1).join(".") : originalParts[0]);
 
   return { basename, extension };
 }