Browse Source

Fixed #34488 -- Made ClearableFileInput preserve "Clear" checked attribute when form is invalid.

Marcelo Galigniana 1 year ago
parent
commit
8a6c0203c4

+ 1 - 1
django/contrib/admin/templates/admin/widgets/clearable_file_input.html

@@ -1,6 +1,6 @@
 {% if widget.is_initial %}<p class="file-upload">{{ widget.initial_text }}: <a href="{{ widget.value.url }}">{{ widget.value }}</a>{% if not widget.required %}
 <span class="clearable-file-input">
-<input type="checkbox" name="{{ widget.checkbox_name }}" id="{{ widget.checkbox_id }}"{% if widget.attrs.disabled %} disabled{% endif %}>
+<input type="checkbox" name="{{ widget.checkbox_name }}" id="{{ widget.checkbox_id }}"{% include "django/forms/widgets/attrs.html" %}>
 <label for="{{ widget.checkbox_id }}">{{ widget.clear_checkbox_label }}</label></span>{% endif %}<br>
 {{ widget.input_text }}:{% endif %}
 <input type="{{ widget.type }}" name="{{ widget.name }}"{% include "django/forms/widgets/attrs.html" %}>{% if widget.is_initial %}</p>{% endif %}

+ 3 - 0
django/forms/widgets.py

@@ -433,6 +433,7 @@ class ClearableFileInput(FileInput):
     initial_text = _("Currently")
     input_text = _("Change")
     template_name = "django/forms/widgets/clearable_file_input.html"
+    checked = False
 
     def clear_checkbox_name(self, name):
         """
@@ -475,10 +476,12 @@ class ClearableFileInput(FileInput):
             }
         )
         context["widget"]["attrs"].setdefault("disabled", False)
+        context["widget"]["attrs"]["checked"] = self.checked
         return context
 
     def value_from_datadict(self, data, files, name):
         upload = super().value_from_datadict(data, files, name)
+        self.checked = self.clear_checkbox_name(name) in data
         if not self.is_required and CheckboxInput().value_from_datadict(
             data, files, self.clear_checkbox_name(name)
         ):

+ 68 - 30
tests/admin_widgets/tests.py

@@ -1772,39 +1772,59 @@ class RelatedFieldWidgetSeleniumTests(AdminWidgetSeleniumTestCase):
 
 @skipUnless(Image, "Pillow not installed")
 class ImageFieldWidgetsSeleniumTests(AdminWidgetSeleniumTestCase):
-    def test_clearablefileinput_widget(self):
+    name_input_id = "id_name"
+    photo_input_id = "id_photo"
+    tests_files_folder = "%s/files" % os.getcwd()
+    clear_checkbox_id = "photo-clear_id"
+
+    def _submit_and_wait(self):
+        from selenium.webdriver.common.by import By
+
+        with self.wait_page_loaded():
+            self.selenium.find_element(
+                By.CSS_SELECTOR, "input[value='Save and continue editing']"
+            ).click()
+
+    def _run_image_upload_path(self):
         from selenium.webdriver.common.by import By
 
         self.admin_login(username="super", password="secret", login_url="/")
         self.selenium.get(
             self.live_server_url + reverse("admin:admin_widgets_student_add"),
         )
+        # Add a student.
+        name_input = self.selenium.find_element(By.ID, self.name_input_id)
+        name_input.send_keys("Joe Doe")
+        photo_input = self.selenium.find_element(By.ID, self.photo_input_id)
+        photo_input.send_keys(f"{self.tests_files_folder}/test.png")
+        self._submit_and_wait()
+        student = Student.objects.last()
+        self.assertEqual(student.name, "Joe Doe")
+        self.assertRegex(student.photo.name, r"^photos\/(test|test_.+).png")
 
-        photo_input_id = "id_photo"
-        save_and_edit_button_css_selector = "input[value='Save and continue editing']"
-        tests_files_folder = "%s/files" % os.getcwd()
-        clear_checkbox_id = "photo-clear_id"
-
-        def _submit_and_wait():
-            with self.wait_page_loaded():
-                self.selenium.find_element(
-                    By.CSS_SELECTOR, save_and_edit_button_css_selector
-                ).click()
+    def test_clearablefileinput_widget(self):
+        from selenium.webdriver.common.by import By
 
-        # Add a student.
-        title_input = self.selenium.find_element(By.ID, "id_name")
-        title_input.send_keys("Joe Doe")
-        photo_input = self.selenium.find_element(By.ID, photo_input_id)
-        photo_input.send_keys(f"{tests_files_folder}/test.png")
-        _submit_and_wait()
+        self._run_image_upload_path()
+        self.selenium.find_element(By.ID, self.clear_checkbox_id).click()
+        self._submit_and_wait()
         student = Student.objects.last()
         self.assertEqual(student.name, "Joe Doe")
-        self.assertEqual(student.photo.name, "photos/test.png")
+        self.assertEqual(student.photo.name, "")
+        # "Currently" with "Clear" checkbox and "Change" are not shown.
+        photo_field_row = self.selenium.find_element(By.CSS_SELECTOR, ".field-photo")
+        self.assertNotIn("Currently", photo_field_row.text)
+        self.assertNotIn("Change", photo_field_row.text)
+
+    def test_clearablefileinput_widget_invalid_file(self):
+        from selenium.webdriver.common.by import By
+
+        self._run_image_upload_path()
         # Uploading non-image files is not supported by Safari with Selenium,
         # so upload a broken one instead.
-        photo_input = self.selenium.find_element(By.ID, photo_input_id)
-        photo_input.send_keys(f"{tests_files_folder}/brokenimg.png")
-        _submit_and_wait()
+        photo_input = self.selenium.find_element(By.ID, self.photo_input_id)
+        photo_input.send_keys(f"{self.tests_files_folder}/brokenimg.png")
+        self._submit_and_wait()
         self.assertEqual(
             self.selenium.find_element(By.CSS_SELECTOR, ".errorlist li").text,
             (
@@ -1813,12 +1833,30 @@ class ImageFieldWidgetsSeleniumTests(AdminWidgetSeleniumTestCase):
             ),
         )
         # "Currently" with "Clear" checkbox and "Change" still shown.
-        cover_field_row = self.selenium.find_element(By.CSS_SELECTOR, ".field-photo")
-        self.assertIn("Currently", cover_field_row.text)
-        self.assertIn("Change", cover_field_row.text)
-        # "Clear" box works.
-        self.selenium.find_element(By.ID, clear_checkbox_id).click()
-        _submit_and_wait()
-        student.refresh_from_db()
-        self.assertEqual(student.name, "Joe Doe")
-        self.assertEqual(student.photo.name, "")
+        photo_field_row = self.selenium.find_element(By.CSS_SELECTOR, ".field-photo")
+        self.assertIn("Currently", photo_field_row.text)
+        self.assertIn("Change", photo_field_row.text)
+
+    def test_clearablefileinput_widget_preserve_clear_checkbox(self):
+        from selenium.webdriver.common.by import By
+
+        self._run_image_upload_path()
+        # "Clear" is not checked by default.
+        self.assertIs(
+            self.selenium.find_element(By.ID, self.clear_checkbox_id).is_selected(),
+            False,
+        )
+        # "Clear" was checked, but a validation error is raised.
+        name_input = self.selenium.find_element(By.ID, self.name_input_id)
+        name_input.clear()
+        self.selenium.find_element(By.ID, self.clear_checkbox_id).click()
+        self._submit_and_wait()
+        self.assertEqual(
+            self.selenium.find_element(By.CSS_SELECTOR, ".errorlist li").text,
+            "This field is required.",
+        )
+        # "Clear" persists checked.
+        self.assertIs(
+            self.selenium.find_element(By.ID, self.clear_checkbox_id).is_selected(),
+            True,
+        )

+ 4 - 1
tests/forms_tests/widget_tests/test_clearablefileinput.py

@@ -17,7 +17,8 @@ class FakeFieldFile:
 
 
 class ClearableFileInputTest(WidgetTest):
-    widget = ClearableFileInput()
+    def setUp(self):
+        self.widget = ClearableFileInput()
 
     def test_clear_input_renders(self):
         """
@@ -148,6 +149,7 @@ class ClearableFileInputTest(WidgetTest):
             name="myfile",
         )
         self.assertIs(value, False)
+        self.assertIs(self.widget.checked, True)
 
     def test_clear_input_checked_returns_false_only_if_not_required(self):
         """
@@ -164,6 +166,7 @@ class ClearableFileInputTest(WidgetTest):
             name="myfile",
         )
         self.assertEqual(value, field)
+        self.assertIs(widget.checked, True)
 
     def test_html_does_not_mask_exceptions(self):
         """