feat(files): updated download logic

Author: nsemets

src/app/features/files/components/files-tree-explorer/files-tree-explorer.component.spec.ts

@@ -13,15 +13,13 @@ import { FileMenuType } from '@osf/shared/enums/file-menu-type.enum';
 import { CurrentResourceType } from '@osf/shared/enums/resource-type.enum';
 import { FileFolderModel } from '@osf/shared/models/files/file-folder.model';
 import { FileLabelModel } from '@osf/shared/models/files/file-label.model';
-import { DataciteService } from '@osf/shared/services/datacite/datacite.service';
-import { FilesService } from '@osf/shared/services/files.service';
+import { FileDownloadService } from '@osf/shared/services/file-download.service';
 import { FilesShareEmbedService } from '@osf/shared/services/files-share-embed.service';
 import { ViewOnlyLinkHelperService } from '@osf/shared/services/view-only-link-helper.service';
 
 import { FileModelMock } from '@testing/mocks/file.model.mock';
 import { provideOSFCore } from '@testing/osf.testing.provider';
-import { DataciteServiceMock, DataciteServiceMockType } from '@testing/providers/datacite.service.mock';
-import { FilesServiceMock, FilesServiceMockType } from '@testing/providers/files-service.mock';
+import { FileDownloadServiceMock, FileDownloadServiceMockType } from '@testing/providers/file-download-service.mock';
 import {
   FilesShareEmbedServiceMock,
   FilesShareEmbedServiceMockType,
@@ -37,8 +35,7 @@ describe('FilesTreeExplorerComponent', () => {
   let component: FilesTreeExplorerComponent;
   let fixture: ComponentFixture<FilesTreeExplorerComponent>;
   let routerMock: RouterMockType;
-  let filesService: FilesServiceMockType;
-  let dataciteService: DataciteServiceMockType;
+  let fileDownloadService: FileDownloadServiceMockType;
   let filesShareEmbedService: FilesShareEmbedServiceMockType;
   let viewOnlyHelper: ViewOnlyLinkHelperMockType;
 
@@ -62,8 +59,7 @@ describe('FilesTreeExplorerComponent', () => {
 
   function setup() {
     routerMock = RouterMockBuilder.create().withUrl('/node-1/files').build();
-    filesService = FilesServiceMock.simple();
-    dataciteService = DataciteServiceMock.simple();
+    fileDownloadService = FileDownloadServiceMock.simple();
     viewOnlyHelper = ViewOnlyLinkHelperMock.simple(false);
     filesShareEmbedService = FilesShareEmbedServiceMock.simple();
 
@@ -75,8 +71,7 @@ describe('FilesTreeExplorerComponent', () => {
       providers: [
         provideOSFCore(),
         MockProvider(Router, routerMock),
-        MockProvider(FilesService, filesService),
-        MockProvider(DataciteService, dataciteService),
+        MockProvider(FileDownloadService, fileDownloadService),
         MockProvider(FilesShareEmbedService, filesShareEmbedService),
         MockProvider(ViewOnlyLinkHelperService, viewOnlyHelper),
       ],
@@ -244,14 +239,20 @@ describe('FilesTreeExplorerComponent', () => {
     expect(emitSpy).toHaveBeenNthCalledWith(2, { file, action: MoveCopyAction.Copy });
   });
 
-  it('should download folder with resolved zip link', () => {
+  it('should download folder from file menu action', () => {
     setup();
-    const openSpy = vi.spyOn(window, 'open').mockReturnValue({ focus: vi.fn() } as unknown as Window);
+    const file = FileModelMock.simple({
+      kind: FileKind.Folder,
+      links: { ...FileModelMock.simple().links, upload: '/upload-folder' },
+    });
 
-    component.downloadFolder('/upload-folder');
+    component.onFileMenuAction({ value: FileMenuType.Download }, file);
 
-    expect(filesService.getFolderDownloadLink).toHaveBeenCalledWith('/upload-folder');
-    expect(openSpy).toHaveBeenCalledWith('/upload-folder?zip=', '_blank');
+    expect(fileDownloadService.downloadFileOrFolder).toHaveBeenCalledWith({
+      resourceId: 'node-1',
+      resourceType: CurrentResourceType.Projects,
+      file,
+    });
   });
 
   it('should open share link in new tab for non self target', () => {

src/app/features/files/components/files-tree-explorer/files-tree-explorer.component.ts

@@ -8,7 +8,6 @@ import {
   ChangeDetectionStrategy,
   Component,
   computed,
-  DestroyRef,
   effect,
   inject,
   input,
@@ -17,7 +16,6 @@ import {
   PLATFORM_ID,
   signal,
 } from '@angular/core';
-import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
 import { Router } from '@angular/router';
 
 import { FileMenuComponent } from '@osf/shared/components/file-menu/file-menu.component';
@@ -34,8 +32,7 @@ import { FileFolderModel } from '@osf/shared/models/files/file-folder.model';
 import { FileLabelModel } from '@osf/shared/models/files/file-label.model';
 import { FileMenuAction, FileMenuFlags } from '@osf/shared/models/files/file-menu-action.model';
 import { FilePageLinkModel } from '@osf/shared/models/files/file-page-link.model';
-import { DataciteService } from '@osf/shared/services/datacite/datacite.service';
-import { FilesService } from '@osf/shared/services/files.service';
+import { FileDownloadService } from '@osf/shared/services/file-download.service';
 import { FilesShareEmbedService } from '@osf/shared/services/files-share-embed.service';
 import { ViewOnlyLinkHelperService } from '@osf/shared/services/view-only-link-helper.service';
 
@@ -60,10 +57,8 @@ import { MenuMoveCopyPayload } from '../../models/menu-move-copy.model';
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
 export class FilesTreeExplorerComponent {
-  private readonly destroyRef = inject(DestroyRef);
   private readonly router = inject(Router);
-  private readonly filesService = inject(FilesService);
-  private readonly dataciteService = inject(DataciteService);
+  private readonly fileDownloadService = inject(FileDownloadService);
   private readonly filesShareEmbedService = inject(FilesShareEmbedService);
   private readonly viewOnlyService = inject(ViewOnlyLinkHelperService);
   private readonly isBrowser = isPlatformBrowser(inject(PLATFORM_ID));
@@ -189,29 +184,11 @@ export class FilesTreeExplorerComponent {
   }
 
   downloadFileOrFolder(file: FileModel) {
-    this.dataciteService
-      .logFileDownload(this.resourceId(), this.resourceType())
-      .pipe(takeUntilDestroyed(this.destroyRef))
-      .subscribe();
-
-    if (file.kind === FileKind.File) {
-      this.downloadFile(file.links.download);
-    } else {
-      this.downloadFolder(file.links.upload);
-    }
-  }
-
-  downloadFile(link: string): void {
-    if (this.isBrowser) {
-      window.open(link)?.focus();
-    }
-  }
-
-  downloadFolder(downloadLink: string): void {
-    if (downloadLink) {
-      const link = this.filesService.getFolderDownloadLink(downloadLink);
-      window.open(link, '_blank')?.focus();
-    }
+    this.fileDownloadService.downloadFileOrFolder({
+      resourceId: this.resourceId(),
+      resourceType: this.resourceType(),
+      file,
+    });
   }
 
   onLazyLoad(event: TreeLazyLoadEvent) {

src/app/features/files/pages/files/files.component.spec.ts

@@ -23,19 +23,19 @@ import { CurrentResource } from '@osf/shared/models/current-resource.model';
 import { FileFolderModel } from '@osf/shared/models/files/file-folder.model';
 import { FileLabelModel } from '@osf/shared/models/files/file-label.model';
 import { CustomDialogService } from '@osf/shared/services/custom-dialog.service';
+import { FileDownloadService } from '@osf/shared/services/file-download.service';
 import { FilesService } from '@osf/shared/services/files.service';
 import { FilesTreeActionsService } from '@osf/shared/services/files-tree-actions.service';
 import { ToastService } from '@osf/shared/services/toast.service';
 import { ViewOnlyLinkHelperService } from '@osf/shared/services/view-only-link-helper.service';
 import { CurrentResourceSelectors, GetResourceDetails } from '@osf/shared/stores/current-resource';
-import { DataciteService } from '@shared/services/datacite/datacite.service';
 
 import { MOCK_CONFIGURED_ADDON } from '@testing/mocks/configured-addon.mock';
 import { FileModelMock } from '@testing/mocks/file.model.mock';
 import { OSF_FILE_MOCK } from '@testing/mocks/osf-file.mock';
 import { provideOSFCore } from '@testing/osf.testing.provider';
 import { CustomDialogServiceMock, CustomDialogServiceMockType } from '@testing/providers/custom-dialog-provider.mock';
-import { DataciteServiceMock, DataciteServiceMockType } from '@testing/providers/datacite.service.mock';
+import { FileDownloadServiceMock, FileDownloadServiceMockType } from '@testing/providers/file-download-service.mock';
 import { FilesServiceMock, FilesServiceMockType } from '@testing/providers/files-service.mock';
 import { ActivatedRouteMockBuilder } from '@testing/providers/route-provider.mock';
 import { RouterMockBuilder, RouterMockType } from '@testing/providers/router-provider.mock';
@@ -97,7 +97,7 @@ describe('FilesComponent', () => {
     uploadFiles: Mock;
   };
   let customDialogService: CustomDialogServiceMockType;
-  let dataciteService: DataciteServiceMockType;
+  let fileDownloadService: FileDownloadServiceMockType;
 
   const currentFolder: FileFolderModel = {
     ...OSF_FILE_MOCK,
@@ -121,7 +121,7 @@ describe('FilesComponent', () => {
     toastService = ToastServiceMock.simple();
     viewOnlyHelper = ViewOnlyLinkHelperMock.simple(false);
     customDialogService = CustomDialogServiceMock.simple();
-    dataciteService = DataciteServiceMock.simple();
+    fileDownloadService = FileDownloadServiceMock.simple();
 
     filesActionsService = {
       deleteSelected: vi.fn(),
@@ -195,7 +195,7 @@ describe('FilesComponent', () => {
         MockProvider(ToastService, toastService),
         MockProvider(ViewOnlyLinkHelperService, viewOnlyHelper),
         MockProvider(CustomDialogService, customDialogService),
-        MockProvider(DataciteService, dataciteService),
+        MockProvider(FileDownloadService, fileDownloadService),
         MockProvider(FilesActionsService, filesActionsService),
         MockProvider(FilesTreeActionsService, filesTreeActionsService),
         MockProvider(FilesUploadService, filesUploadService),
@@ -468,28 +468,28 @@ describe('FilesComponent', () => {
     expect(store.dispatch).toHaveBeenCalledWith(new GetFiles('/files-link', 1));
   });
 
-  it('should log download and open folder zip link', () => {
+  it('should download current folder as zip', () => {
     setup();
-    (store.dispatch as Mock).mockClear();
-    const openSpy = vi.spyOn(window, 'open').mockReturnValue({ focus: vi.fn() } as unknown as Window);
 
     component.downloadFolder();
 
-    expect(dataciteService.logFileDownload).toHaveBeenCalledWith('node-1', 'nodes');
-    expect(filesService.getFolderDownloadLink).toHaveBeenCalledWith('/v2/files/file-123/download/');
-    expect(openSpy).toHaveBeenCalledWith('/v2/files/file-123/download/?zip=', '_blank');
-    openSpy.mockRestore();
+    expect(fileDownloadService.downloadFolderAsZip).toHaveBeenCalledWith({
+      resourceId: 'node-1',
+      resourceType: 'nodes',
+      downloadLink: '/v2/files/file-123/download/',
+    });
   });
 
   it('should skip download when resource id is missing', () => {
     setup({ resourceId: '' });
-    const openSpy = vi.spyOn(window, 'open');
 
     component.downloadFolder();
 
-    expect(dataciteService.logFileDownload).not.toHaveBeenCalled();
-    expect(openSpy).not.toHaveBeenCalled();
-    openSpy.mockRestore();
+    expect(fileDownloadService.downloadFolderAsZip).toHaveBeenCalledWith({
+      resourceId: '',
+      resourceType: 'nodes',
+      downloadLink: '/v2/files/file-123/download/',
+    });
   });
 
   it('should open files browser info dialog', () => {

src/app/features/files/pages/files/files.component.ts

@@ -39,6 +39,7 @@ import { CurrentResourceType, ResourceType } from '@osf/shared/enums/resource-ty
 import { mapRootFoldersToStorageLabels } from '@osf/shared/helpers/storage-addon-options.helper';
 import { FilePageLinkModel } from '@osf/shared/models/files/file-page-link.model';
 import { CustomDialogService } from '@osf/shared/services/custom-dialog.service';
+import { FileDownloadService } from '@osf/shared/services/file-download.service';
 import { FilesService } from '@osf/shared/services/files.service';
 import { FilesTreeActionsService } from '@osf/shared/services/files-tree-actions.service';
 import { ToastService } from '@osf/shared/services/toast.service';
@@ -48,7 +49,6 @@ import { StorageItem } from '@shared/models/addons/storage-item.model';
 import { FileModel } from '@shared/models/files/file.model';
 import { FileFolderModel } from '@shared/models/files/file-folder.model';
 import { FileLabelModel } from '@shared/models/files/file-label.model';
-import { DataciteService } from '@shared/services/datacite/datacite.service';
 
 import { FileBrowserInfoComponent } from '../../components/file-browser-info/file-browser-info.component';
 import { FilesSelectionActionsComponent } from '../../components/files-selection-actions/files-selection-actions.component';
@@ -107,7 +107,7 @@ export class FilesComponent {
   private readonly customDialogService = inject(CustomDialogService);
   private readonly translateService = inject(TranslateService);
   private readonly router = inject(Router);
-  private readonly dataciteService = inject(DataciteService);
+  private readonly fileDownloadService = inject(FileDownloadService);
   private readonly filesActionsService = inject(FilesActionsService);
   private readonly filesTreeActionsService = inject(FilesTreeActionsService);
   private readonly filesUploadService = inject(FilesUploadService);
@@ -506,17 +506,11 @@ export class FilesComponent {
   }
 
   downloadFolder(): void {
-    const resourceId = this.resourceId();
-    const resourcePath = this.resourceMetadata()?.type ?? 'nodes';
-    const downloadLink = this.currentFolder()?.links.download ?? '';
-    if (resourceId && downloadLink) {
-      this.dataciteService
-        .logFileDownload(resourceId, resourcePath)
-        .pipe(takeUntilDestroyed(this.destroyRef))
-        .subscribe();
-      const link = this.filesService.getFolderDownloadLink(downloadLink);
-      window.open(link, '_blank')?.focus();
-    }
+    this.fileDownloadService.downloadFolderAsZip({
+      resourceId: this.resourceId() ?? '',
+      resourceType: this.resourceMetadata()?.type ?? 'nodes',
+      downloadLink: this.currentFolder()?.links.download ?? '',
+    });
   }
 
   showInfoDialog() {

src/app/features/project/overview/components/files-widget/files-widget.component.html

@@ -15,7 +15,7 @@ <h2>{{ 'project.overview.files.filesPreview' | translate }}</h2>
       <p-skeleton width="6rem" height="2.5rem" />
     </div>
   } @else {
-    <p-tabs [value]="currentRootFolder()?.folder?.id || ''" (valueChange)="onStorageChange($event)" scrollable>
+    <p-tabs [value]="currentRootFolder()?.folder?.id" (valueChange)="onStorageChange($event)" scrollable>
       <p-tablist>
         @for (option of storageAddons(); track option.folder.id) {
           <p-tab class="addon-tab p-2" [value]="option.folder.id" [disabled]="isFilesLoading()">
@@ -30,6 +30,17 @@ <h2>{{ 'project.overview.files.filesPreview' | translate }}</h2>
         }
       </p-tablist>
     </p-tabs>
+
+    <div class="flex">
+      <p-button
+        raised
+        outlined
+        [icon]="'fas fa-download'"
+        [label]="'files.actions.downloadAsZip' | translate"
+        [disabled]="isButtonDisabled()"
+        (onClick)="downloadFolder()"
+      />
+    </div>
   }
 
   <osf-files-tree

src/app/features/project/overview/components/files-widget/files-widget.component.spec.ts

@@ -18,18 +18,20 @@ import {
   SetFilesCurrentFolder,
 } from '@osf/features/files/store';
 import { SupportedFeature } from '@osf/shared/enums/addon-supported-features.enum';
-import { ResourceType } from '@osf/shared/enums/resource-type.enum';
+import { CurrentResourceType, ResourceType } from '@osf/shared/enums/resource-type.enum';
 import { FileFolderModel } from '@osf/shared/models/files/file-folder.model';
 import { FilePageLinkModel } from '@osf/shared/models/files/file-page-link.model';
 import { NodeShortInfoModel } from '@osf/shared/models/nodes/node-with-children.model';
 import { SelectOption } from '@osf/shared/models/select-option.model';
+import { FileDownloadService } from '@osf/shared/services/file-download.service';
 import { FilesService } from '@osf/shared/services/files.service';
 import { ViewOnlyLinkHelperService } from '@osf/shared/services/view-only-link-helper.service';
 
 import { MOCK_CONFIGURED_ADDON } from '@testing/mocks/configured-addon.mock';
 import { FileModelMock } from '@testing/mocks/file.model.mock';
 import { OSF_FILE_MOCK } from '@testing/mocks/osf-file.mock';
 import { provideOSFCore } from '@testing/osf.testing.provider';
+import { FileDownloadServiceMock, FileDownloadServiceMockType } from '@testing/providers/file-download-service.mock';
 import { FilesServiceMock, FilesServiceMockType } from '@testing/providers/files-service.mock';
 import { RouterMockBuilder, RouterMockType } from '@testing/providers/router-provider.mock';
 import {
@@ -55,6 +57,7 @@ describe('FilesWidgetComponent', () => {
   let store: Store;
   let routerMock: RouterMockType;
   let filesService: FilesServiceMockType;
+  let fileDownloadService: FileDownloadServiceMockType;
   let viewOnlyHelper: ViewOnlyLinkHelperMockType;
 
   const rootFolder: FileFolderModel = {
@@ -78,6 +81,7 @@ describe('FilesWidgetComponent', () => {
       .withSerializeUrl(vi.fn().mockReturnValue('/serialized'))
       .build();
     filesService = FilesServiceMock.simple();
+    fileDownloadService = FileDownloadServiceMock.simple();
     viewOnlyHelper = ViewOnlyLinkHelperMock.simple(overrides.hasViewOnly ?? false);
     viewOnlyHelper.getViewOnlyParamFromUrl.mockReturnValue('token');
 
@@ -105,6 +109,7 @@ describe('FilesWidgetComponent', () => {
         provideOSFCore(),
         MockProvider(Router, routerMock),
         MockProvider(FilesService, filesService),
+        MockProvider(FileDownloadService, fileDownloadService),
         MockProvider(ViewOnlyLinkHelperService, viewOnlyHelper),
         provideMockStore({ signals: mergeSignalOverrides(defaultSignals, overrides.selectorOverrides) }),
       ],
@@ -179,6 +184,34 @@ describe('FilesWidgetComponent', () => {
     expect(store.dispatch).toHaveBeenCalledWith(new SetFilesCurrentFolder(rootFolder));
   });
 
+  it('should disable download button when folder has no files', () => {
+    setup();
+
+    expect(component.isButtonDisabled()).toBe(true);
+  });
+
+  it('should enable download button when folder has files', () => {
+    setup({
+      selectorOverrides: [{ selector: FilesSelectors.getFilesTotalCount, value: 2 }],
+    });
+
+    expect(component.isButtonDisabled()).toBe(false);
+  });
+
+  it('should download current folder as zip', () => {
+    setup({
+      selectorOverrides: [{ selector: FilesSelectors.getFilesTotalCount, value: 1 }],
+    });
+
+    component.downloadFolder();
+
+    expect(fileDownloadService.downloadFolderAsZip).toHaveBeenCalledWith({
+      resourceId: 'project-1',
+      resourceType: CurrentResourceType.Projects,
+      downloadLink: '/v2/files/file-123/download/',
+    });
+  });
+
   it('should open file directly when guid exists', () => {
     setup();
     const openSpy = vi.spyOn(window, 'open').mockImplementation(() => null);