Skip to content

Commit 17a9fc9

Browse files
committed
MAGETWO-87641: Validation message displays many times after upload incorrect file extension
- Aggregate error messages and display error modal when upload chain has completed - Add this functionality to media-uploader.js widget (used in catalog/product)
1 parent 6a69901 commit 17a9fc9

File tree

5 files changed

+349
-24
lines changed

5 files changed

+349
-24
lines changed

app/code/Magento/Backend/view/adminhtml/web/js/media-uploader.js

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,19 @@ define([
1313
'jquery',
1414
'mage/template',
1515
'Magento_Ui/js/modal/alert',
16+
'Magento_Ui/js/form/element/file-uploader',
1617
'mage/translate',
1718
'jquery/file-uploader'
18-
], function ($, mageTemplate, alert) {
19+
], function ($, mageTemplate, alert, FileUploader) {
1920
'use strict';
2021

22+
var fileUploader = new FileUploader({
23+
dataScope: '',
24+
isMultipleFiles: true
25+
});
26+
27+
fileUploader.initUploader();
28+
2129
$.widget('mage.mediaUploader', {
2230

2331
/**
@@ -79,10 +87,9 @@ define([
7987
if (data.result && !data.result.error) {
8088
self.element.trigger('addItem', data.result);
8189
} else {
82-
alert({
83-
content: $.mage.__('We don\'t recognize or support this file extension type.')
84-
});
90+
fileUploader.aggregateError(data.files[0].name, data.result.error);
8591
}
92+
8693
self.element.find('#' + data.fileId).remove();
8794
},
8895

@@ -108,7 +115,9 @@ define([
108115
.delay(2000)
109116
.hide('highlight')
110117
.remove();
111-
}
118+
},
119+
120+
stop: fileUploader.uploaderConfig.stop
112121
});
113122

114123
this.element.find('input[type=file]').fileupload('option', {

app/code/Magento/Ui/i18n/en_US.csv

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,3 +201,4 @@ CSV,CSV
201201
"Please enter at least {0} characters.","Please enter at least {0} characters."
202202
"Please enter a value between {0} and {1} characters long.","Please enter a value between {0} and {1} characters long."
203203
"Please enter a value between {0} and {1}.","Please enter a value between {0} and {1}."
204+
"was not uploaded","was not uploaded"

app/code/Magento/Ui/view/base/web/js/form/element/file-uploader.js

Lines changed: 82 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,16 @@ define([
1313
'Magento_Ui/js/modal/alert',
1414
'Magento_Ui/js/lib/validation/validator',
1515
'Magento_Ui/js/form/element/abstract',
16+
'mage/backend/notification',
17+
'mage/translate',
1618
'jquery/file-uploader'
17-
], function ($, _, utils, uiAlert, validator, Element) {
19+
], function ($, _, utils, uiAlert, validator, Element, notification, $t) {
1820
'use strict';
1921

2022
return Element.extend({
2123
defaults: {
2224
value: [],
25+
aggregatedErrors: [],
2326
maxFileSize: false,
2427
isMultipleFiles: false,
2528
placeholderType: 'document', // 'image', 'video'
@@ -278,9 +281,15 @@ define([
278281
* @returns {FileUploader} Chainable.
279282
*/
280283
notifyError: function (msg) {
281-
uiAlert({
284+
var data = {
282285
content: msg
283-
});
286+
};
287+
288+
if (this.isMultipleFiles) {
289+
data.modalClass = '_image-box';
290+
}
291+
292+
uiAlert(data);
284293

285294
return this;
286295
},
@@ -309,13 +318,20 @@ define([
309318
},
310319

311320
/**
312-
* Abstract handler which is invoked when files are choosed for upload.
321+
* Handler which is invoked when files are choosed for upload.
313322
* May be used for implementation of aditional validation rules,
314323
* e.g. total files and a total size rules.
315324
*
316-
* @abstract
325+
* @param {Event} e - Event object.
326+
* @param {Object} data - File data that will be uploaded.
327+
* @returns {Boolean|void} - false if further processing should halt or undefined if processing should continue
317328
*/
318-
onFilesChoosed: function () {},
329+
onFilesChoosed: function (e, data) {
330+
// no option exists in fileuploader for restricting upload chains to single files; this enforces that policy
331+
if (!this.isMultipleFiles) {
332+
data.files.splice(1);
333+
}
334+
},
319335

320336
/**
321337
* Handler which is invoked prior to the start of a file upload.
@@ -329,30 +345,45 @@ define([
329345
target = $(e.target);
330346

331347
if (allowed.passed) {
332-
target.on('fileuploadsend', function (event, postData) {
333-
postData.data.append('param_name', this.paramName);
334-
}.bind(data));
335-
336348
target.fileupload('process', data).done(function () {
337349
data.submit();
338350
});
339351
} else {
340-
this.notifyError(allowed.message);
352+
this.aggregateError(file.name, allowed.message);
353+
354+
// if all files in upload chain are invalid, stop callback is never called; this resolves promise
355+
if (this.aggregatedErrors.length === data.originalFiles.length) {
356+
this.uploaderConfig.stop();
357+
}
341358
}
342359
},
343360

361+
/**
362+
* Add error message associated with filename for display when upload chain is complete
363+
*
364+
* @param {String} filename
365+
* @param {String} message
366+
*/
367+
aggregateError: function (filename, message) {
368+
this.aggregatedErrors.push({
369+
filename: filename,
370+
message: message
371+
});
372+
},
373+
344374
/**
345375
* Handler of the file upload complete event.
346376
*
347377
* @param {Event} e
348378
* @param {Object} data
349379
*/
350380
onFileUploaded: function (e, data) {
351-
var file = data.result,
381+
var uploadedFilename = data.files[0].name,
382+
file = data.result,
352383
error = file.error;
353384

354385
error ?
355-
this.notifyError(error) :
386+
this.aggregateError(uploadedFilename, error) :
356387
this.addFile(file);
357388
},
358389

@@ -367,7 +398,45 @@ define([
367398
* Load stop event handler.
368399
*/
369400
onLoadingStop: function () {
401+
var aggregatedErrorMessages = [];
402+
370403
this.isLoading = false;
404+
405+
if (!this.aggregatedErrors.length) {
406+
return;
407+
}
408+
409+
if (!this.isMultipleFiles) { // only single file upload occurred; use first file's error message
410+
aggregatedErrorMessages.push(this.aggregatedErrors[0].message);
411+
} else { // construct message from all aggregatedErrors
412+
_.each(this.aggregatedErrors, function (error) {
413+
notification().add({
414+
error: true,
415+
message: '%s' + error.message, // %s to be used as placeholder for html injection
416+
417+
/**
418+
* Adds constructed error notification to aggregatedErrorMessages
419+
*
420+
* @param {String} constructedMessage
421+
*/
422+
insertMethod: function (constructedMessage) {
423+
var errorMsgBodyHtml = '<strong>%s</strong> %s.<br>'
424+
.replace('%s', error.filename)
425+
.replace('%s', $t('was not uploaded'));
426+
427+
// html is escaped in message body for notification widget; prepend unescaped html here
428+
constructedMessage = constructedMessage.replace('%s', errorMsgBodyHtml);
429+
430+
aggregatedErrorMessages.push(constructedMessage);
431+
}
432+
});
433+
});
434+
}
435+
436+
this.notifyError(aggregatedErrorMessages.join(''));
437+
438+
// clear out aggregatedErrors array for this completed upload chain
439+
this.aggregatedErrors = [];
371440
},
372441

373442
/**

app/design/adminhtml/Magento/backend/web/css/source/components/_modals_extend.less

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@
8080
&.confirm,
8181
&.prompt {
8282
.modal-inner-wrap {
83-
.message {
83+
.message:not(.message-error) {
8484
background: @modal-popup-colorless__background;
8585
}
8686
}

0 commit comments

Comments
 (0)