Discource-C/discourse
1
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-02-22 12:17:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

FIX: Quirks around starting new uploads when one was in progress (#18393)

Browse Source 
This commit addresses issues around starting new uploads in a composer etc.
when one or more uploads are already processing or uploading.
There were a couple of issues:

1. When all preprocessors were complete, we were not resetting
    `completeProcessing` to 0, which meant that `needProcessing`
    would never match `completeProcessing` if a new upload was
    started.
2. We were relying on the uppy "complete" event which is supposed
    to fire when all uploads are complete, but this doesn't seem to take
    into account new uploads that are added. Instead now we can rely on
    our own `inProgressUploads` tracker, and consider all uploads complete
    when there are no `inProgressUploads` in flight
This commit is contained in:
Martin Brennan 2022-09-30 11:01:40 +10:00 committed by GitHub
parent f60e6837c6
commit 9daa6328b5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 115 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js
View File

@ -339,20 +339,18 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
file.name,
upload
);
if (this.inProgressUploads.length === 0) {
this.appEvents.trigger(
`${this.composerEventPrefix}:all-uploads-complete`
);
this._reset();
}
});
});
this._uppyInstance.on("upload-error", this._handleUploadError);
this._uppyInstance.on("complete", () => {
run(() => {
this.appEvents.trigger(
`${this.composerEventPrefix}:all-uploads-complete`
);
this._reset();
});
});
this._uppyInstance.on("cancel-all", () => {
// uppyInstance.reset() also fires cancel-all, so we want to
// only do the manual cancelling work if the user clicked cancel
@ -553,6 +551,7 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
isUploading: false,
isProcessingUpload: false,
isCancellable: false,
inProgressUploads: [],
});
this._resetPreProcessors();
this.fileInputEl.value = "";

2
app/assets/javascripts/discourse/app/mixins/extendable-uploader.js
View File

@ -139,6 +139,7 @@ export default Mixin.create(UploadDebugging, {
_addNeedProcessing(fileCount) {
this._eachPreProcessor((pluginName, status) => {
status.needProcessing += fileCount;
status.allComplete = false;
});
},
@ -167,6 +168,7 @@ export default Mixin.create(UploadDebugging, {
) {
preProcessorStatus.allComplete = true;
preProcessorStatus.needProcessing = 0;
preProcessorStatus.completeProcessing = 0;
if (this._allPreprocessorsComplete()) {
callback(true);

27
app/assets/javascripts/discourse/app/mixins/uppy-upload.js
View File

@ -221,6 +221,9 @@ export default Mixin.create(UppyS3Multipart, ExtendableUploader, {
);
this._triggerInProgressUploadsEvent();
if (this.inProgressUploads.length === 0) {
this._allUploadsComplete();
}
})
.catch((errResponse) => {
displayErrorForUpload(errResponse, this.siteSettings, file.name);
@ -235,7 +238,11 @@ export default Mixin.create(UppyS3Multipart, ExtendableUploader, {
upload
);
this.uploadDone(deepMerge(upload, { file_name: file.name }));
this._triggerInProgressUploadsEvent();
if (this.inProgressUploads.length === 0) {
this._allUploadsComplete();
}
}
});
@ -260,17 +267,6 @@ export default Mixin.create(UppyS3Multipart, ExtendableUploader, {
});
});
this._uppyInstance.on("complete", () => {
run(() => {
if (this.isDestroying || this.isDestroyed) {
return;
}
this.appEvents.trigger(`upload-mixin:${this.id}:all-uploads-complete`);
this._reset();
});
});
// TODO (martin) preventDirectS3Uploads is necessary because some of
// the current upload mixin components, for example the emoji uploader,
// send the upload to custom endpoints that do fancy things in the rails
@ -485,4 +481,13 @@ export default Mixin.create(UppyS3Multipart, ExtendableUploader, {
_uploadDropTargetOptions() {
return { target: this.element };
},
_allUploadsComplete() {
if (this.isDestroying || this.isDestroyed) {
return;
}
this.appEvents.trigger(`upload-mixin:${this.id}:all-uploads-complete`);
this._reset();
},
});

111
app/assets/javascripts/discourse/tests/acceptance/composer-uploads-uppy-test.js
View File

@ -13,6 +13,8 @@ import { skip, test } from "qunit";
import { Promise } from "rsvp";
import sinon from "sinon";
let uploadNumber = 1;
function pretender(server, helper) {
server.post("/uploads/lookup-urls", () => {
return helper.response([
@ -21,27 +23,53 @@ function pretender(server, helper) {
short_path: "/uploads/short-url/yoj8pf9DdIeHRRULyw7i57GAYdz.jpeg",
short_url: "upload://yoj8pf9DdIeHRRULyw7i57GAYdz.jpeg",
},
{
url: "/images/discourse-logo-sketch-small.png",
short_path: "/uploads/short-url/sdfljsdfgjlkwg4328.jpeg",
short_url: "upload://sdfljsdfgjlkwg4328.jpeg",
},
]);
});
server.post(
"/uploads.json",
() => {
return helper.response({
extension: "jpeg",
filesize: 126177,
height: 800,
human_filesize: "123 KB",
id: 202,
original_filename: "avatar.PNG.jpg",
retain_hours: null,
short_path: "/uploads/short-url/yoj8pf9DdIeHRRULyw7i57GAYdz.jpeg",
short_url: "upload://yoj8pf9DdIeHRRULyw7i57GAYdz.jpeg",
thumbnail_height: 320,
thumbnail_width: 690,
url: "/images/discourse-logo-sketch-small.png",
width: 1920,
});
let response = null;
if (uploadNumber === 1) {
response = {
extension: "jpeg",
filesize: 126177,
height: 800,
human_filesize: "123 KB",
id: 202,
original_filename: "avatar.PNG.jpg",
retain_hours: null,
short_path: "/uploads/short-url/yoj8pf9DdIeHRRULyw7i57GAYdz.jpeg",
short_url: "upload://yoj8pf9DdIeHRRULyw7i57GAYdz.jpeg",
thumbnail_height: 320,
thumbnail_width: 690,
url: "/images/discourse-logo-sketch-small.png",
width: 1920,
};
uploadNumber += 1;
} else {
response = {
extension: "jpeg",
filesize: 4322,
height: 800,
human_filesize: "566 KB",
id: 202,
original_filename: "avatar2.PNG.jpg",
retain_hours: null,
short_path: "/uploads/short-url/sdfljsdfgjlkwg4328.jpeg",
short_url: "upload://sdfljsdfgjlkwg4328.jpeg",
thumbnail_height: 320,
thumbnail_width: 690,
url: "/images/discourse-logo-sketch-small.png",
width: 1920,
};
}
return helper.response(response);
},
500 // this delay is important to slow down the uploads a bit so we can click elements in the UI like the cancel button
);
@ -54,6 +82,9 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
simultaneous_uploads: 2,
enable_rich_text_paste: true,
});
needs.hooks.afterEach(() => {
uploadNumber = 1;
});
test("should insert the Uploading placeholder then the complete image placeholder", async function (assert) {
await visit("/");
@ -62,7 +93,8 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
const appEvents = loggedInUser().appEvents;
const done = assert.async();
appEvents.on("composer:all-uploads-complete", () => {
appEvents.on("composer:all-uploads-complete", async () => {
await settled();
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n![avatar.PNG|690x320](upload://yoj8pf9DdIeHRRULyw7i57GAYdz.jpeg)\n"
@ -81,6 +113,35 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
appEvents.trigger("composer:add-files", image);
});
test("should handle adding one file for upload then adding another when the first is still in progress", async function (assert) {
await visit("/");
await click("#create-topic");
await fillIn(".d-editor-input", "The image:\n");
const appEvents = loggedInUser().appEvents;
const done = assert.async();
appEvents.on("composer:all-uploads-complete", async () => {
await settled();
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n![avatar.PNG|690x320](upload://yoj8pf9DdIeHRRULyw7i57GAYdz.jpeg)\n![avatar2.PNG|690x320](upload://sdfljsdfgjlkwg4328.jpeg)\n"
);
done();
});
let image2Added = false;
appEvents.on("composer:upload-started", () => {
if (!image2Added) {
appEvents.trigger("composer:add-files", image2);
image2Added = true;
}
});
const image1 = createFile("avatar.png");
const image2 = createFile("avatar2.png");
appEvents.trigger("composer:add-files", image1);
});
test("should handle placeholders correctly even if the OS rewrites ellipses", async function (assert) {
const execCommand = document.execCommand;
sinon.stub(document, "execCommand").callsFake(function (...args) {
@ -96,7 +157,8 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
const appEvents = loggedInUser().appEvents;
const done = assert.async();
appEvents.on("composer:all-uploads-complete", () => {
appEvents.on("composer:all-uploads-complete", async () => {
await settled();
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n![avatar.PNG|690x320](upload://yoj8pf9DdIeHRRULyw7i57GAYdz.jpeg)\n"
@ -221,7 +283,8 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
);
});
appEvents.on("composer:all-uploads-complete", () => {
appEvents.on("composer:all-uploads-complete", async () => {
await settled();
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n![avatar.PNG|690x320](upload://yoj8pf9DdIeHRRULyw7i57GAYdz.jpeg)\n"
@ -251,7 +314,8 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
);
});
appEvents.on("composer:all-uploads-complete", () => {
appEvents.on("composer:all-uploads-complete", async () => {
await settled();
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n![avatar.PNG|690x320](upload://yoj8pf9DdIeHRRULyw7i57GAYdz.jpeg)\n Text after the image."
@ -284,7 +348,8 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
);
});
appEvents.on("composer:all-uploads-complete", () => {
appEvents.on("composer:all-uploads-complete", async () => {
await settled();
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n![avatar.PNG|690x320](upload://yoj8pf9DdIeHRRULyw7i57GAYdz.jpeg)\n Text after the image."
@ -309,7 +374,8 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
);
});
appEvents.on("composer:all-uploads-complete", () => {
appEvents.on("composer:all-uploads-complete", async () => {
await settled();
assert.strictEqual(
query(".d-editor-input").value,
"![avatar.PNG|690x320](upload://yoj8pf9DdIeHRRULyw7i57GAYdz.jpeg)\n"
@ -335,7 +401,8 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
);
});
appEvents.on("composer:all-uploads-complete", () => {
appEvents.on("composer:all-uploads-complete", async () => {
await settled();
assert.strictEqual(
query(".d-editor-input").value,
"The image:\n![avatar.PNG|690x320](upload://yoj8pf9DdIeHRRULyw7i57GAYdz.jpeg)\n"