Pass all necessary template context to chooser view after upload validation error

Fixes #4548

CHANGELOG.txt

@@ -25,6 +25,7 @@ Changelog
  * Fix: Site.get_site_root_paths() preferring other sites over the default when some sites share the same root_page (Andy Babic)
  * Fix: Pages with missing model definitions no longer crash the API (Abdulmalik Abdulwahab)
  * Fix: Strip null characters from paths when checking for redirects (Andrew Crewdson)
+ * Fix: Rich text image chooser no longer skips format selection after a validation error (Matt Westcott)
 
 
 2.1.1 (xx.xx.xxxx) - IN DEVELOPMENT

docs/releases/2.2.rst

@@ -38,6 +38,7 @@ Bug fixes
  * Site.get_site_root_paths() preferring other sites over the default when some sites share the same root_page (Andy Babic)
  * Pages with missing model definitions no longer crash the API (Abdulmalik Abdulwahab)
  * Strip null characters from paths when checking for redirects (Andrew Crewdson)
+ * Rich text image chooser no longer skips format selection after a validation error (Matt Westcott)
 
 Upgrade considerations
 ======================

wagtail/images/tests/test_admin_views.py

@@ -746,6 +746,22 @@ class TestImageChooserUploadView(TestCase, WagtailTestUtils):
         self.assertContains(response, "Page 1 of ")
         self.assertEqual(12, len(response.context['images']))
 
+    def test_select_format_flag_after_upload_form_error(self):
+        submit_url = reverse('wagtailimages:chooser_upload') + '?select_format=true'
+        response = self.client.post(submit_url, {
+            'title': "Test image",
+            'file': SimpleUploadedFile('not_an_image.txt', b'this is not an image'),
+        })
+
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailimages/chooser/chooser.html')
+        self.assertFormError(response, 'uploadform', 'file', "Not a supported image format. Supported formats: GIF, JPEG, PNG.")
+
+        # the action URL of the re-rendered form should include the select_format=true parameter
+        # (NB the HTML in the response is embedded in a JS string, so need to escape accordingly)
+        expected_action_attr = 'action=\\"%s\\"' % submit_url
+        self.assertContains(response, expected_action_attr)
+
     @override_settings(DEFAULT_FILE_STORAGE='wagtail.tests.dummy_external_storage.DummyExternalStorage')
     def test_upload_with_external_storage(self):
         response = self.client.post(reverse('wagtailimages:chooser_upload'), {

wagtail/images/views/chooser.py

@@ -17,7 +17,7 @@ from wagtail.utils.pagination import paginate
 permission_checker = PermissionPolicyChecker(permission_policy)
 
 
-def get_chooser_context():
+def get_chooser_js_data():
     """construct context variables needed by the chooser JS"""
     return {
         'step': 'chooser',
@@ -46,6 +46,23 @@ def get_image_result_data(image):
     }
 
 
+def get_chooser_context(request):
+    """Helper function to return common template context variables for the main chooser view"""
+
+    collections = Collection.objects.all()
+    if len(collections) < 2:
+        collections = None
+
+    return {
+        'searchform': SearchForm(),
+        'is_searching': False,
+        'query_string': None,
+        'will_select_format': request.GET.get('select_format'),
+        'popular_tags': popular_tags_for_model(get_image_model()),
+        'collections': collections,
+    }
+
+
 def chooser(request):
     Image = get_image_model()
 
@@ -61,7 +78,6 @@ def chooser(request):
     for hook in hooks.get_hooks('construct_image_chooser_queryset'):
         images = hook(images, request)
 
-    q = None
     if (
         'q' in request.GET or 'p' in request.GET or 'tag' in request.GET or
         'collection_id' in request.GET
@@ -80,6 +96,7 @@ def chooser(request):
             is_searching = True
         else:
             is_searching = False
+            q = None
 
             tag_name = request.GET.get('tag')
             if tag_name:
@@ -95,24 +112,17 @@ def chooser(request):
             'will_select_format': request.GET.get('select_format')
         })
     else:
-        searchform = SearchForm()
-
-        collections = Collection.objects.all()
-        if len(collections) < 2:
-            collections = None
-
         paginator, images = paginate(request, images, per_page=12)
 
-        return render_modal_workflow(request, 'wagtailimages/chooser/chooser.html', None, {
+        context = get_chooser_context(request)
+        context.update({
             'images': images,
             'uploadform': uploadform,
-            'searchform': searchform,
-            'is_searching': False,
-            'query_string': q,
-            'will_select_format': request.GET.get('select_format'),
-            'popular_tags': popular_tags_for_model(Image),
-            'collections': collections,
-        }, json_data=get_chooser_context())
+        })
+        return render_modal_workflow(
+            request, 'wagtailimages/chooser/chooser.html', None, context,
+            json_data=get_chooser_js_data()
+        )
 
 
 def image_chosen(request, image_id):
@@ -129,8 +139,6 @@ def chooser_upload(request):
     Image = get_image_model()
     ImageForm = get_image_form(Image)
 
-    searchform = SearchForm()
-
     if request.method == 'POST':
         image = Image(uploaded_by_user=request.user)
         form = ImageForm(request.POST, request.FILES, instance=image, user=request.user)
@@ -164,12 +172,21 @@ def chooser_upload(request):
         form = ImageForm(user=request.user)
 
     images = Image.objects.order_by('-created_at')
+
+    # allow hooks to modify the queryset
+    for hook in hooks.get_hooks('construct_image_chooser_queryset'):
+        images = hook(images, request)
+
     paginator, images = paginate(request, images, per_page=12)
 
+    context = get_chooser_context(request)
+    context.update({
+        'images': images,
+        'uploadform': form,
+    })
     return render_modal_workflow(
-        request, 'wagtailimages/chooser/chooser.html', None,
-        {'images': images, 'uploadform': form, 'searchform': searchform},
-        json_data=get_chooser_context()
+        request, 'wagtailimages/chooser/chooser.html', None, context,
+        json_data=get_chooser_js_data()
     )