From 40d13ce6628e4de1c92f96a5bfd10bcbcd06b1d1 Mon Sep 17 00:00:00 2001
From: Martin Brennan <mjrbrennan@gmail.com>
Date: Mon, 13 Dec 2021 15:24:00 +1000
Subject: [PATCH] DEV: Only support multipart for backup S3 uploads with Uppy
 (#15270)

In the composer, we already only allow for S3 multipart uploads
if enable_direct_s3_uploads is true, so in the backups uploader
that is based on Uppy we want to do the same thing. In future
if self-hosters need some way to not use S3 multipart in these
scenarios for whatever reason we can revisit this then (which
should be as simple as adding a enable_multipart_s3_uploads site
setting).
---
 .../admin/addon/templates/backups-index.hbs        |  2 +-
 .../app/components/uppy-backup-uploader.js         | 14 +++++---------
 .../discourse/app/mixins/composer-upload-uppy.js   |  2 --
 3 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/app/assets/javascripts/admin/addon/templates/backups-index.hbs b/app/assets/javascripts/admin/addon/templates/backups-index.hbs
index da2e488425b..7c30f00ef09 100644
--- a/app/assets/javascripts/admin/addon/templates/backups-index.hbs
+++ b/app/assets/javascripts/admin/addon/templates/backups-index.hbs
@@ -12,7 +12,7 @@
         class="btn-default"}}
     {{/if}}
   {{else}}
-    {{#if (and siteSettings.enable_direct_s3_uploads siteSettings.enable_experimental_backup_uploader)}}
+    {{#if siteSettings.enable_experimental_backup_uploader}}
       {{uppy-backup-uploader done=(route-action "remoteUploadSuccess")}}
     {{else}}
       {{backup-uploader done=(route-action "remoteUploadSuccess")}}
diff --git a/app/assets/javascripts/discourse/app/components/uppy-backup-uploader.js b/app/assets/javascripts/discourse/app/components/uppy-backup-uploader.js
index ac07d28a400..9311b35b751 100644
--- a/app/assets/javascripts/discourse/app/components/uppy-backup-uploader.js
+++ b/app/assets/javascripts/discourse/app/components/uppy-backup-uploader.js
@@ -1,5 +1,5 @@
 import Component from "@ember/component";
-import { alias, not } from "@ember/object/computed";
+import { alias } from "@ember/object/computed";
 import I18n from "I18n";
 import UppyUploadMixin from "discourse/mixins/uppy-upload";
 import discourseComputed from "discourse-common/utils/decorators";
@@ -12,15 +12,11 @@ export default Component.extend(UppyUploadMixin, {
   uploadRootPath: "/admin/backups",
   uploadUrl: "/admin/backups/upload",
 
-  // TODO (martin) Add functionality to make this usable _without_ multipart
-  // uploads, direct to S3, which needs to call get-presigned-put on the
-  // BackupsController (which extends ExternalUploadHelpers) rather than
-  // the old create_upload_url route. The two are functionally equivalent;
-  // they both generate a presigned PUT url for the upload to S3, and do
-  // the whole thing in one request rather than multipart.
-
   // direct s3 backups
-  useMultipartUploadsIfAvailable: not("localBackupStorage"),
+  @discourseComputed("localBackupStorage")
+  useMultipartUploadsIfAvailable(localBackupStorage) {
+    return !localBackupStorage && this.siteSettings.enable_direct_s3_uploads;
+  },
 
   // local backups
   useChunkedUploads: alias("localBackupStorage"),
diff --git a/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js b/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js
index a39c77c2cb3..badd1590abe 100644
--- a/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js
+++ b/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js
@@ -205,8 +205,6 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
       this._useXHRUploads();
     }
 
-    // TODO (martin) develop upload handler guidance and an API to use; will
-    // likely be using uppy plugins for this
     this._uppyInstance.on("file-added", (file) => {
       if (isPrivateMessage) {
         file.meta.for_private_message = true;