FIX: form template upload type validation (#24148)

When submitting files through the form template upload field, we were having an issue where, although a validation error message was being presented to the user, the upload was still coming through, because `PickFilesButton`'s validation happens **after** the Uppy mixin finished the upload and hit `uploadDone`.

This PR adds a new overridable method to the Uppy mixin and overrides it with the custom validation, which now happens before the file is sent.

Additionally, we're now also using `uploadingOrProcessing` as the source of truth to show the upload/uploading label, which seems more reliable.
This commit is contained in:
Renato Atilio 2023-11-03 10:11:25 -03:00 committed by GitHub
parent 2b2f6b1c25
commit 2201f8f7dc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 57 additions and 14 deletions

View File

@ -12,38 +12,51 @@ export default class FormTemplateFieldUpload extends Component.extend(
UppyUploadMixin
) {
@tracked uploadValue;
@tracked uploadComplete = false;
@tracked uploadedFiles = [];
@tracked disabled = this.uploading;
@tracked disabled = this.uploadingOrProcessing;
@tracked fileUploadElementId = `${dasherize(this.id)}-uploader`;
@tracked fileInputSelector = `#${this.fileUploadElementId}`;
type = "composer";
@computed("uploading", "uploadValue")
@computed("uploadingOrProcessing")
get uploadStatusLabel() {
if (!this.uploading && !this.uploadValue) {
return "form_templates.upload_field.upload";
}
return this.uploadingOrProcessing
? "form_templates.upload_field.uploading"
: "form_templates.upload_field.upload";
}
if (!this.uploading && this.uploadValue) {
this.uploadComplete = true;
return "form_templates.upload_field.upload";
}
/**
* The validation from PickFilesButton._filesPicked, where acceptedFormatsOverride
* is validated and displays a message, happens after the upload is complete.
*
* Overriding this method allows us to validate the file before the upload
*
* @param file
* @returns {boolean}
*/
isUploadedFileAllowed(file) {
// same logic from PickFilesButton._hasAcceptedExtensionOrType
const fileTypes = this.attributes.file_types;
const extension = file.name.split(".").pop();
return "form_templates.upload_field.uploading";
return (
!fileTypes ||
fileTypes.includes(`.${extension}`) ||
fileTypes.includes(file.type)
);
}
uploadDone(upload) {
// If re-uploading, clear the existing file if multiple aren't allowed
if (!this.attributes.allow_multiple && this.uploadComplete) {
if (!this.attributes.allow_multiple && this.uploadValue) {
this.uploadedFiles = [];
this.uploadValue = "";
}
const uploadMarkdown = this.buildMarkdown(upload);
this.uploadedFiles.pushObject(upload);
const uploadMarkdown = this.buildMarkdown(upload);
if (this.uploadValue && this.allowMultipleFiles) {
// multiple file upload
this.uploadValue = `${this.uploadValue}\n${uploadMarkdown}`;

View File

@ -47,6 +47,17 @@ export default Mixin.create(UppyS3Multipart, ExtendableUploader, {
return {};
},
/**
* Overridable for custom file validations, executed before uploading.
*
* @param {object} file
*
* @returns {boolean}
*/
isUploadedFileAllowed() {
return true;
},
uploadingOrProcessing: or("uploading", "processing"),
@on("willDestroyElement")
@ -112,7 +123,9 @@ export default Mixin.create(UppyS3Multipart, ExtendableUploader, {
},
this.validateUploadedFilesOptions()
);
const isValid = validateUploadedFile(currentFile, validationOpts);
const isValid =
validateUploadedFile(currentFile, validationOpts) &&
this.isUploadedFileAllowed(currentFile);
this.setProperties({
uploadProgress: 0,
uploading: isValid && this.autoStartUploads,

View File

@ -370,6 +370,7 @@ describe "Composer Form Templates", type: :system do
expect(find("#dialog-holder .dialog-body p", visible: :all)).to have_content(
I18n.t("js.pick_files_button.unsupported_file_picked", { types: ".jpg, .png" }),
)
expect(page).to have_no_css(".form-template-field__uploaded-files")
end
it "creates a post with multiple uploads" do
@ -401,6 +402,22 @@ describe "Composer Form Templates", type: :system do
)
end
it "overrides uploaded file if allow_multiple false" do
topic_title = "Peter Parker's Medication"
category_page.visit(category_with_upload_template)
category_page.new_topic_button.click
attach_file "prescription-uploader",
"#{Rails.root}/spec/fixtures/images/logo.png",
make_visible: true
composer.fill_title(topic_title)
attach_file "prescription-uploader",
"#{Rails.root}/spec/fixtures/images/fake.jpg",
make_visible: true
expect(find(".form-template-field__uploaded-files")).to have_css("li", count: 1)
end
it "shows labels and descriptions when a form template is assigned to the category" do
category_page.visit(category_with_template_6)
category_page.new_topic_button.click