Use FormData for docs/images add-multiple's edit handling

[th3hamm0r]

The ajax-request now uses multipart/form-data, similar to image-chooser-modal.js/document-chooser-modal.js, which allows to add and use FileFields in the bulk-upload-views.

This PR is a bit related to #8314, because we needed an additional FileField for our images, where the user can attach a license proof in some cases (depends on other custom fields and validation).
This works in the image chooser, but did not work in the bulk-upload-tool (add-multiple.js), because those file-inputs have not been transferred to the backend with jquery's .serialize().
So as a quickfix, we had to customize/patch get_image_multi_form(), to exclude the license proof for now, but to allow customizations in a clean way, I've created #8314.

Please check the following:

• Do the tests still pass?¹
• Does the code comply with the style guide?
  • Run make lint from the Wagtail root.
• For front-end changes: Did you test on all of Wagtail’s supported environments?²
  • Please list the exact browser and operating system versions you tested:
    Google Chrome 100.0.4896.75, Firefox 99.0 (both on Linux Mint/Ubuntu)
  • Please list which assistive technologies ³ you tested:
• For new features: Has the documentation been updated accordingly?

Please describe additional details for testing this change.

Footnotes

1. Development Testing ↩

2. Browser and device support ↩

3. Accessibility Target ↩

[squash-labs]

Manage this branch in Squash

Test this branch here: https://th3hamm0rfeatureuse-multipart-sub0n.squash.io

[th3hamm0r]

In the hope for a faster review, maybe this helps:
My above changes basically replaced jquery's form.serialize() with the vanilla FormData class, very similar to the image-chooser-modal.js and the document-chooser-modal.js:

wagtail/client/src/entrypoints/images/image-chooser-modal.js

Lines 6 to 45 in e4d563e

	var formdata = new FormData(this);
	if (!$('#id_image-chooser-upload-title', modal.body).val()) {
	var li = $('#id_image-chooser-upload-title', modal.body).closest('li');
	if (!li.hasClass('error')) {
	li.addClass('error');
	$('#id_image-chooser-upload-title', modal.body)
	.closest('.field-content')
	.append(
	'<p class="error-message"><span>This field is required.</span></p>',
	);
	}
	setTimeout(cancelSpinner, 500);
	} else {
	$.ajax({
	url: this.action,
	data: formdata,
	processData: false,
	contentType: false,
	type: 'POST',
	dataType: 'text',
	success: modal.loadResponseText,
	error: function (response, textStatus, errorThrown) {
	var message =
	jsonData.error_message +
	'<br />' +
	errorThrown +
	' - ' +
	response.status;
	$('#tab-upload').append(
	'<div class="help-block help-critical">' +
	'<strong>' +
	jsonData.error_label +
	': </strong>' +
	message +
	'</div>',
	);
	},
	});
	}

wagtail/client/src/entrypoints/documents/document-chooser-modal.js

Lines 6 to 32 in e4d563e

	var formdata = new FormData(this);
	$.ajax({
	url: this.action,
	data: formdata,
	processData: false,
	contentType: false,
	type: 'POST',
	dataType: 'text',
	success: modal.loadResponseText,
	error: function (response, textStatus, errorThrown) {
	var message =
	jsonData.error_message +
	'<br />' +
	errorThrown +
	' - ' +
	response.status;
	$('#tab-upload', modal.body).append(
	'<div class="help-block help-critical">' +
	'<strong>' +
	jsonData.error_label +
	': </strong>' +
	message +
	'</div>',
	);
	},
	});

[lb-]

In principle this look good, why the change to $.ajax instead of just keeping $.post (sorry my jquery is a bit rusty - it may be obvious).

I will try to do some testing locally and get this approved.

Thanks for the explanation.

[th3hamm0r]

In principle this look good, why the change to $.ajax instead of just keeping $.post (sorry my jquery is a bit rusty - it may be obvious).

No, there is no obvious reason. First I wanted to keep it in sync with the very similar code for the modals, and second, from my experience with jquery and the task to reduce its usage where possible, only using ajax() and none of the helpers like post, getJson,... makes it a lot easier to find and adapt (e.g. adding new options).

I will try to do some testing locally and get this approved.

Thanks for the explanation.

Nice, thanks!

[gasman]

As I understand it, $.ajax is needed in order to pass the processData: false option - without that, jquery will try to interpret the FormData object as a key/value dict instead of using the multipart serialisation we want.

(Haven't tested to confirm that - I just happened to be refactoring the chooser modal JS yesterday and was wondering why it used the long-winded $.ajax route instead of modal-workflow.js's stock ajaxifyForm, and that's the conclusion I came to :-) )

[lb-]

Thanks, that makes sense.

[lb-]

This looks great, put it through a bunch of scenarios and cannot see any issues.
Thanks for the explanation @th3hamm0r

Tested

• after upload of images/docs - trigger a server side error
• after upload of images/docs - delete the item & ensure it is not saved
• after upload of images/docs - trigger a success save (should hide and changes be made)
• test bad upload on drop - not part of this change but ensure it does not try to grab content that is not there (no form)

[lb-]

pushed up a rebase, with prettier formatting (added after this PR was created) + changelog.
Will check a few tests pass and merge.