[#45772] finalizing direct upload code increment

- added error toast when dropping many files
- added unit tests for new query

frontend/src/app/shared/components/storages/openproject-storages.module.ts

@@ -54,7 +54,6 @@ import {
 import {
   LoadingFileListComponent,
 } from 'core-app/shared/components/storages/loading-file-list/loading-file-list.component';
-import { UploadStorageFilesService } from 'core-app/shared/components/storages/services/upload-storage-files.service';
 
 @NgModule({
   imports: [
@@ -79,7 +78,6 @@ import { UploadStorageFilesService } from 'core-app/shared/components/storages/s
   providers: [
     SortFilesPipe,
     CookieService,
-    UploadStorageFilesService,
   ],
 })
 export class OpenprojectStoragesModule {}

frontend/src/app/shared/components/storages/storage/storage.component.ts

@@ -136,6 +136,8 @@ export class StorageComponent extends UntilDestroyedMixin implements OnInit, OnD
       uploadFailed: (fileName:string):string => this.i18n.t('js.storages.file_links.upload_error', { fileName }),
       linkingAfterUploadFailed: (fileName:string, workPackageId:string):string =>
         this.i18n.t('js.storages.file_links.link_uploaded_file_error', { fileName, workPackageId }),
+      draggingManyFiles: (storageType:string):string => this.i18n.t('js.storages.file.dragging_many_files', { storageType }),
+      uploadingLabel: this.i18n.t('js.label_upload_notification'),
     },
     dropBox: {
       uploadLabel: this.i18n.t('js.storages.upload_files'),
@@ -144,6 +146,7 @@ export class StorageComponent extends UntilDestroyedMixin implements OnInit, OnD
     },
     emptyList: ():string => this.i18n.t('js.storages.file_links.empty', { storageType: this.storageType }),
     openStorage: ():string => this.i18n.t('js.storages.open_storage', { storageType: this.storageType }),
+    nextcloud: this.i18n.t('js.storages.types.nextcloud'),
   };
 
   public get storageFilesLocation():string {
@@ -281,7 +284,6 @@ export class StorageComponent extends UntilDestroyedMixin implements OnInit, OnD
     this.storageFilesResourceService
       .uploadLink(this.uploadResourceLink(file.name, locationId))
       .pipe(
-        // switchMap((link) => this.uploadStorageFilesService.uploadFile(link, file)),
         switchMap((link) => this.uploadAndNotify(link, file)),
         catchError((error) => {
           isUploadError = true;
@@ -331,8 +333,11 @@ export class StorageComponent extends UntilDestroyedMixin implements OnInit, OnD
         responseType: 'json',
       },
     ).pipe(share());
-    const message = this.i18n.t('js.label_upload_notification');
-    const notification = this.toastService.add({ data: [[file, observable]], type: 'upload', message });
+    const notification = this.toastService.add({
+      data: [[file, observable]],
+      type: 'upload',
+      message: this.text.toast.uploadingLabel,
+    });
 
     return observable
       .pipe(
@@ -460,7 +465,7 @@ export class StorageComponent extends UntilDestroyedMixin implements OnInit, OnD
   }
 
   private initializeStorageTypes() {
-    this.storageTypeMap[nextcloud] = this.i18n.t('js.storages.types.nextcloud');
+    this.storageTypeMap[nextcloud] = this.text.nextcloud;
   }
 
   public onDropFiles(event:DragEvent):void {
@@ -469,7 +474,13 @@ export class StorageComponent extends UntilDestroyedMixin implements OnInit, OnD
     this.draggingOverDropZone = false;
     this.dragging = 0;
 
-    this.openSelectLocationDialog(event.dataTransfer.files);
+    const files = event.dataTransfer.files;
+    if (files.length !== 1) {
+      this.toastService.addError(this.text.toast.draggingManyFiles(this.storageType));
+      return;
+    }
+
+    this.openSelectLocationDialog(files);
   }
 
   public onDragOver(event:DragEvent):void {

modules/storages/config/locales/js-en.yml

@@ -30,6 +30,7 @@ en:
 
       files:
         directory_not_writeable: "You do not have to permission to add files to this folder."
+        dragging_many_files: "The upload to %{storageType} supports only one file at once."
 
       file_links:
         empty: >

modules/storages/spec/common/peripherals/storage_requests_spec.rb

@@ -356,6 +356,99 @@ describe Storages::Peripherals::StorageRequests, webmock: true do
   end
 
   describe '#upload_link_query' do
+    let(:query_payload) { Struct.new(:parent).new(42) }
+    let(:upload_token) { 'valid-token' }
+
+    before do
+      allow(OAuthClients::ConnectionManager).to receive(:new).and_return(connection_manager)
+      stub_request(:post, "#{url}/apps/integration_openproject/direct-upload-token")
+        .with(body: { folder_id: query_payload.parent })
+        .to_return(
+          status: 200,
+          body: {
+            token: upload_token,
+            expires_on: 1673883865
+          }.to_json
+        )
+    end
+
+    describe 'with Nextcloud storage type selected' do
+      it 'must return an upload link URL' do
+        subject
+          .upload_link_query(user:)
+          .match(
+            on_success: ->(query) do
+              query.call(query_payload).match(
+                on_success: ->(link) {
+                  expect(link.destination.path).to be_eql("/apps/integration_openproject/direct-upload/#{upload_token}")
+                  expect(link.destination.host).to be_eql(URI(url).host)
+                  expect(link.destination.scheme).to be_eql(URI(url).scheme)
+                  expect(link.destination.user).to be_nil
+                  expect(link.destination.password).to be_nil
+                  expect(link.method).to eq(:post)
+                },
+                on_failure: ->(error) {
+                  raise "Files query could not be executed: #{error}"
+                }
+              )
+            end,
+            on_failure: ->(error) do
+              raise "Files query could not be created: #{error}"
+            end
+          )
+      end
+    end
+
+    describe 'with not supported storage type selected' do
+      before do
+        allow(storage).to receive(:provider_type).and_return('not_supported_storage_type'.freeze)
+      end
+
+      it 'must raise ArgumentError' do
+        expect { subject.upload_link_query(user:) }.to raise_error(ArgumentError)
+      end
+    end
+
+    describe 'with missing OAuth token' do
+      before do
+        allow(connection_manager).to receive(:get_access_token).and_return(ServiceResult.failure)
+      end
+
+      it 'must return ":not_authorized" ServiceResult' do
+        expect(subject.upload_link_query(user:)).to be_failure
+      end
+    end
+
+    shared_examples_for 'outbound is failing' do |code, symbol|
+      describe "with outbound request returning #{code}" do
+        before do
+          stub_request(:post, "#{url}/apps/integration_openproject/direct-upload-token").to_return(status: code)
+        end
+
+        it "must return :#{symbol} ServiceResult" do
+          subject
+            .upload_link_query(user:)
+            .match(
+              on_success: ->(query) do
+                result = query.call(query_payload)
+                expect(result).to be_failure
+                expect(result.errors.code).to be(symbol)
+              end,
+              on_failure: ->(error) do
+                raise "Files query could not be created: #{error}"
+              end
+            )
+        end
+      end
+    end
+
+    include_examples 'outbound is failing', 400, :error
+    include_examples 'outbound is failing', 401, :not_authorized
+    include_examples 'outbound is failing', 404, :not_found
+    include_examples 'outbound is failing', 500, :error
+  end
+
+  describe '#legacy_upload_link_query', with_flag: { legacy_upload_preparation: true } do
     let(:query_payload) do
       Struct.new(:fileName, :parent).new("ape.png", "/Pictures")
     end
@@ -407,6 +500,7 @@ describe Storages::Peripherals::StorageRequests, webmock: true do
                   expect(link.destination.scheme).to be_eql(URI(url).scheme)
                   expect(link.destination.user).not_to be_nil
                   expect(link.destination.password).not_to be_nil
+                  expect(link.method).to eq(:put)
                 },
                 on_failure: ->(error) {
                   raise "Files query could not be executed: #{error}"
@@ -426,7 +520,7 @@ describe Storages::Peripherals::StorageRequests, webmock: true do
       end
 
       it 'must raise ArgumentError' do
-        expect { subject.download_link_query(user:) }.to raise_error(ArgumentError)
+        expect { subject.upload_link_query(user:) }.to raise_error(ArgumentError)
       end
     end
 
@@ -436,7 +530,7 @@ describe Storages::Peripherals::StorageRequests, webmock: true do
       end
 
       it 'must return ":not_authorized" ServiceResult' do
-        expect(subject.download_link_query(user:)).to be_failure
+        expect(subject.upload_link_query(user:)).to be_failure
       end
     end