uploads: Improve abstraction of upload.js by removing object exports.

The `upload_objects_by_message_edit_row` map object was being exported
to handle the uploads during the editing of a message. To improve the
abstraction, we move the logic being used to access
`upload_objects_by_message_edit_row` and itself into `upload.js`.

Similarly, the `compose_upload_object` constant which was being exported
to handle the cancelling of compose uploads. This commit removes this
export and instead defines a new method `compose_upload_cancel` to
handle the same.

web/src/compose_setup.js

@@ -34,7 +34,7 @@ import * as user_topics from "./user_topics";
 
 export function abort_xhr() {
     $("#compose-send-button").prop("disabled", false);
-    upload.compose_upload_object.cancelAll();
+    upload.compose_upload_cancel();
 }
 
 function setup_compose_actions_hooks() {

web/src/message_edit.js

@@ -577,11 +577,10 @@ function start_edit_with_content($row, content, edit_box_open_callback) {
         edit_box_open_callback();
     }
     const row_id = rows.id($row);
-    const upload_object = upload.setup_upload({
+    upload.setup_upload({
         mode: "edit",
         row: row_id,
     });
-    upload.upload_objects_by_message_edit_row.set(row_id, upload_object);
 }
 
 export function start($row, edit_box_open_callback) {
@@ -791,25 +790,11 @@ export function end_inline_topic_edit($row) {
     message_lists.current.hide_edit_topic_on_recipient_row($row);
 }
 
-function remove_uploads_from_row(row_id) {
-    const uploads_for_row = upload.upload_objects_by_message_edit_row.get(row_id);
-    if (!uploads_for_row) {
-        return;
-    }
-    // We need to cancel all uploads, reset their progress,
-    // and clear the files upon ending the edit.
-    upload.deactivate_upload(uploads_for_row, {
-        mode: "edit",
-        row: row_id,
-    });
-    // Since we removed all the uploads from the row, we should
-    // now remove the corresponding upload object from the store.
-    upload.upload_objects_by_message_edit_row.delete(row_id);
-}
-
 export function end_message_row_edit($row) {
     const row_id = rows.id($row);
-    remove_uploads_from_row(row_id);
+
+    // Clean up the upload handler
+    upload.deactivate_upload({mode: "edit", row: row_id});
 
     const message = message_lists.current.get(row_id);
     if (message !== undefined && currently_editing_messages.has(message.id)) {

web/src/upload.js

@@ -4,6 +4,7 @@ import $ from "jquery";
 
 import render_upload_banner from "../templates/compose_banner/upload_banner.hbs";
 
+import * as blueslip from "./blueslip";
 import * as compose_actions from "./compose_actions";
 import * as compose_banner from "./compose_banner";
 import * as compose_reply from "./compose_reply";
@@ -17,8 +18,12 @@ import * as rows from "./rows";
 import {realm} from "./state_data";
 
 let drag_drop_img = null;
-export let compose_upload_object;
+let compose_upload_object;
-export const upload_objects_by_message_edit_row = new Map();
+const upload_objects_by_message_edit_row = new Map();
+
+export function compose_upload_cancel() {
+    compose_upload_object.cancelAll();
+}
 
 // Show the upload button only if the browser supports it.
 export function feature_check($upload_button) {
@@ -292,6 +297,10 @@ export function setup_upload(config) {
         },
     });
 
+    if (config.mode === "edit") {
+        upload_objects_by_message_edit_row.set(config.row, uppy);
+    }
+
     uppy.on("upload-progress", (file, progress) => {
         const percent_complete = (100 * progress.bytesUploaded) / progress.bytesTotal;
         $(`${get_item("upload_banner_identifier", config, file.id)} .moving_bar`).css({
@@ -437,16 +446,38 @@ export function setup_upload(config) {
     return uppy;
 }
 
-export function deactivate_upload(uppy, config) {
+export function deactivate_upload(config) {
     // Remove event listeners added for handling uploads.
     $(get_item("file_input_identifier", config)).off("change");
     get_item("banner_container", config).off("click");
     get_item("drag_drop_container", config).off("dragover dragenter drop paste");
 
-    // Uninstall all plugins and close down the Uppy instance.
+    let uppy;
-    // Also runs uppy.cancelAll() before uninstalling - which
+
-    // cancels all uploads, resets progress and removes all files.
+    if (config.mode === "edit") {
-    uppy.close();
+        uppy = upload_objects_by_message_edit_row.get(config.row);
+    } else if (config.mode === "compose") {
+        uppy = compose_upload_object;
+    }
+
+    if (!uppy) {
+        return;
+    }
+
+    try {
+        // Uninstall all plugins and close down the Uppy instance.
+        // Also runs uppy.cancelAll() before uninstalling - which
+        // cancels all uploads, resets progress and removes all files.
+        uppy.close();
+    } catch (error) {
+        blueslip.error("Failed to close upload object.", {config}, error);
+    }
+
+    if (config.mode === "edit") {
+        // Since we removed all the uploads from the row, we should
+        // now remove the corresponding upload object from the store.
+        upload_objects_by_message_edit_row.delete(config.row);
+    }
 }
 
 export function initialize() {

web/tests/compose.test.js

@@ -508,10 +508,8 @@ test_ui("initialize", ({override}) => {
     realm.max_file_upload_size_mib = 512;
 
     let uppy_cancel_all_called = false;
-    override(upload, "compose_upload_object", {
+    override(upload, "compose_upload_cancel", () => {
-        cancelAll() {
+        uppy_cancel_all_called = true;
-            uppy_cancel_all_called = true;
-        },
     });
     override(upload, "feature_check", noop);