fix: fixed and improved some user interface issues. (#1666)

* disabled sample edit * enhanced large file download instructions and update config * commented out sample edit test * fix the issue of the red cross icon appearing for PDF images in the dataset table * fix for thumbnail image being cut off * fixes based on AI PR review * added unit test for the change * rename for large file message function * fix based on comments * fix failing test --------- Co-authored-by: Martin <[email protected]> Co-authored-by: Max Novelli <[email protected]>
SciCatProject · Nov 28, 2024 · 0d37dfb · 0d37dfb
1 parent 5f490c0
commit 0d37dfb
Show file tree

Hide file tree

Showing 16 changed files with 95 additions and 91 deletions.
diff --git a/src/app/app-config.service.spec.ts b/src/app/app-config.service.spec.ts
@@ -54,6 +54,8 @@ const appConfig: AppConfig = {
   searchPublicDataEnabled: true,
   searchSamples: true,
   sftpHost: "login.esss.dk",
+  sourceFolder: "default",
+  maxFileSizeWarning: "Some files are above the max size",
   shareEnabled: true,
   shoppingCartEnabled: true,
   shoppingCartOnHeader: true,

diff --git a/src/app/app-config.service.ts b/src/app/app-config.service.ts
@@ -82,6 +82,8 @@ export interface AppConfig {
   searchPublicDataEnabled: boolean;
   searchSamples: boolean;
   sftpHost: string | null;
+  sourceFolder?: string;
+  maxFileSizeWarning?: string;
   shareEnabled: boolean;
   shoppingCartEnabled: boolean;
   shoppingCartOnHeader: boolean;

diff --git a/src/app/datasets/dashboard/dashboard.component.ts b/src/app/datasets/dashboard/dashboard.component.ts
@@ -73,7 +73,6 @@ export class DashboardComponent implements OnInit, OnDestroy {
   subscriptions: Subscription[] = [];
 
   appConfig = this.appConfigService.getConfig();
-
   currentUser: User = new User();
   userGroups: string[] = [];
   clearColumnSearch = false;

diff --git a/src/app/datasets/datafiles/datafiles.component.html b/src/app/datasets/datafiles/datafiles.component.html
@@ -2,16 +2,7 @@
   <ng-template [ngIf]="maxFileSize && tooLargeFile">
     <span>
       <mat-icon class="warning-icon"> warning </mat-icon>
-      One or more files are too large to be downloaded directly.
-      {{
-        sftpHost
-          ? "These files are available via sftp host " +
-            sftpHost +
-            " in directory " +
-            sourcefolder +
-            "."
-          : ""
-      }}
+      <span [innerHTML]="hasFileAboveMaxSizeWarning()"></span>
     </span>
   </ng-template>
 
@@ -54,7 +45,7 @@ <h3>No files associated to this dataset</h3>
       <input type="hidden" name="auth_token" [value]="auth_token" />
       <input type="hidden" name="jwt" [value]="jwt?.jwt" />
       <input type="hidden" name="dataset" [value]="datasetPid" />
-      <input type="hidden" name="directory" [value]="sourcefolder" />
+      <input type="hidden" name="directory" [value]="sourceFolder" />
       <input
         *ngFor="let file of getAllFiles(); index as i"
         type="hidden"
@@ -86,7 +77,7 @@ <h3>No files associated to this dataset</h3>
       <input type="hidden" name="jwt" [value]="jwt?.jwt" />
       <input type="hidden" name="auth_token" [value]="auth_token" />
       <input type="hidden" name="dataset" [value]="datasetPid" />
-      <input type="hidden" name="directory" [value]="sourcefolder" />
+      <input type="hidden" name="directory" [value]="sourceFolder" />
       <input
         *ngFor="let file of getSelectedFiles(); index as i"
         type="hidden"

diff --git a/src/app/datasets/datafiles/datafiles.component.spec.ts b/src/app/datasets/datafiles/datafiles.component.spec.ts
@@ -19,6 +19,7 @@ import { MatButtonModule } from "@angular/material/button";
 import { AppConfigService } from "app-config.service";
 import { MatDialogModule, MatDialogRef } from "@angular/material/dialog";
 import { DatafilesActionsComponent } from "datasets/datafiles-actions/datafiles-actions.component";
+import { FileSizePipe } from "shared/pipes/filesize.pipe";
 
 describe("DatafilesComponent", () => {
   let component: DatafilesComponent;
@@ -100,6 +101,7 @@ describe("DatafilesComponent", () => {
             provide: DatafilesActionsComponent,
             useClass: MockDatafilesActionsComponent,
           },
+          { provide: FileSizePipe },
         ],
       },
     });
@@ -140,7 +142,7 @@ describe("DatafilesComponent", () => {
       },
     ];
     component.tableData = component.files;
-    component.sourcefolder = "/test/";
+    component.sourceFolder = "/test/";
     fixture.detectChanges();
   });
 

diff --git a/src/app/datasets/datafiles/datafiles.component.ts b/src/app/datasets/datafiles/datafiles.component.ts
@@ -62,7 +62,6 @@ export class DatafilesComponent
   subscriptions: Subscription[] = [];
 
   files: Array<any> = [];
-  sourcefolder = "";
   datasetPid = "";
   actionDataset: ActionDataset;
 
@@ -76,7 +75,12 @@ export class DatafilesComponent
     this.appConfig.fileserverButtonLabel || "Download";
   multipleDownloadAction: string | null = this.appConfig.multipleDownloadAction;
   maxFileSize: number | null = this.appConfig.maxDirectDownloadSize;
-  sftpHost: string | null = this.appConfig.sftpHost;
+  sourceFolder: string =
+    this.appConfig.sourceFolder || "No source folder provided";
+  sftpHost: string = this.appConfig.sftpHost || "No sftp host provided";
+  maxFileSizeWarning: string | null =
+    this.appConfig.maxFileSizeWarning ||
+    `Some files are above the max size ${this.fileSizePipe.transform(this.maxFileSize)}`;
   jwt: any;
   auth_token: any;
 
@@ -110,6 +114,7 @@ export class DatafilesComponent
     private store: Store,
     private cdRef: ChangeDetectorRef,
     private dialog: MatDialog,
+    private fileSizePipe: FileSizePipe,
     private userApi: UserApi,
   ) {}
 
@@ -204,11 +209,40 @@ export class DatafilesComponent
     }
   }
 
+  hasFileAboveMaxSizeWarning() {
+    /**
+     * Template for a file size warning message.
+     * Placeholders:
+     * - <maxDirectDownloadSize>: Maximum file size allowed (e.g., "10 MB").
+     * - <sftpHost>: SFTP host for downloading large files.
+     * - <sourceFolder>: Directory path on the SFTP host.
+     *
+     * Example usage:
+     * Some files are above <maxDirectDownloadSize>. These file can be accessed via sftp host: <sftpHost> in directory: <sourceFolder>
+     */
+
+    const valueMapping = {
+      sftpHost: this.sftpHost,
+      sourceFolder: this.sourceFolder,
+      maxDirectDownloadSize: this.fileSizePipe.transform(this.maxFileSize),
+    };
+
+    let warning = this.maxFileSizeWarning;
+
+    Object.keys(valueMapping).forEach((key) => {
+      warning = warning.replace(
+        "<" + key + ">",
+        `<strong>${valueMapping[key]}</strong>`,
+      );
+    });
+
+    return warning;
+  }
   ngAfterViewInit() {
     this.subscriptions.push(
       this.dataset$.subscribe((dataset) => {
         if (dataset) {
-          this.sourcefolder = dataset.sourceFolder;
+          this.sourceFolder = dataset.sourceFolder;
           this.datasetPid = dataset.pid;
           this.actionDataset = <ActionDataset>dataset;
         }
@@ -287,7 +321,7 @@ export class DatafilesComponent
     return (
       this.fileserverBaseURL +
       "&origin_path=" +
-      encodeURIComponent(this.sourcefolder)
+      encodeURIComponent(this.sourceFolder)
     );
   }
 }
diff --git a/src/app/datasets/dataset-detail/dataset-detail.component.html b/src/app/datasets/dataset-detail/dataset-detail.component.html
@@ -268,13 +268,6 @@
                 <a (click)="onClickSample(sample.sampleId)">
                   <span>{{ sample.description }}</span>
                 </a>
-                <span
-                  class="sample-edit"
-                  (click)="openSampleEditDialog()"
-                  *ngIf="appConfig.editDatasetSampleEnabled && editingAllowed"
-                >
-                  <mat-icon>edit</mat-icon>
-                </span>
               </td>
             </tr>
             <tr *ngIf="dataset['instrumentId'] && instrument">

diff --git a/src/app/datasets/dataset-detail/dataset-detail.component.spec.ts b/src/app/datasets/dataset-detail/dataset-detail.component.spec.ts
@@ -341,34 +341,6 @@ describe("DatasetDetailComponent", () => {
     });
   });
 
-  describe("#openSampleEditDialog()", () => {
-    it("should open the sample edit dialog and dispatch updatePropertyAction", () => {
-      const dispatchSpy = spyOn(store, "dispatch");
-      component.dataset = new Dataset();
-      component.dataset.ownerGroup = "test";
-      component.sample = new Sample();
-      const sampleId = "testId";
-      component.sample.sampleId = sampleId;
-      const pid = "testPid";
-      component.dataset.pid = pid;
-      const property = { sampleId };
-      const dialogOpenSpy = spyOn(component.dialog, "open").and.returnValue({
-        afterClosed: () => of({ sample: { sampleId: "testId" } }),
-      } as MatDialogRef<SampleEditComponent>);
-      component.openSampleEditDialog();
-
-      expect(dialogOpenSpy).toHaveBeenCalledTimes(1);
-      expect(dialogOpenSpy).toHaveBeenCalledWith(SampleEditComponent, {
-        width: "1000px",
-        data: { ownerGroup: "test", sampleId: "testId" },
-      });
-      expect(dispatchSpy).toHaveBeenCalledTimes(1);
-      expect(dispatchSpy).toHaveBeenCalledWith(
-        updatePropertyAction({ pid, property }),
-      );
-    });
-  });
-
   describe("#onSaveMetadata()", () => {
     it("should dispatch an updatePropertyAction", () => {
       const dispatchSpy = spyOn(store, "dispatch");

diff --git a/src/app/datasets/dataset-detail/dataset-detail.component.ts b/src/app/datasets/dataset-detail/dataset-detail.component.ts
@@ -4,7 +4,7 @@ import { ENTER, COMMA, SPACE } from "@angular/cdk/keycodes";
 import { MatChipInputEvent } from "@angular/material/chips";
 
 import { MatDialog } from "@angular/material/dialog";
-import { SampleEditComponent } from "datasets/sample-edit/sample-edit.component";
+// import { SampleEditComponent } from "datasets/sample-edit/sample-edit.component";
 import { DialogComponent } from "shared/modules/dialog/dialog.component";
 import { combineLatest, fromEvent, Observable, Subscription } from "rxjs";
 import { Store } from "@ngrx/store";
@@ -277,29 +277,6 @@ export class DatasetDetailComponent
     this.router.navigateByUrl("/samples/" + id);
   }
 
-  openSampleEditDialog() {
-    if (this.dataset) {
-      this.dialog
-        .open(SampleEditComponent, {
-          width: "1000px",
-          data: {
-            ownerGroup: this.dataset.ownerGroup,
-            sampleId: this.sample?.sampleId,
-          },
-        })
-        .afterClosed()
-        .subscribe((res) => {
-          if (res && this.dataset) {
-            const { sample } = res;
-            this.sample = sample;
-            const pid = this.dataset.pid;
-            const property = { sampleId: sample.sampleId };
-            this.store.dispatch(updatePropertyAction({ pid, property }));
-          }
-        });
-    }
-  }
-
   onSlidePublic(event: MatSlideToggleChange) {
     if (this.dataset) {
       const pid = this.dataset.pid;
@@ -351,11 +328,7 @@ export class DatasetDetailComponent
   }
 
   getImageUrl(encoded: string) {
-    const mimeType = this.base64MimeType(encoded);
-    if (mimeType === "application/pdf") {
-      return "assets/images/pdf-icon.svg";
-    }
-    return encoded;
+    return this.attachmentService.getImageUrl(encoded);
   }
 
   openAttachment(encoded: string) {

diff --git a/src/app/datasets/dataset-table/dataset-table.component.html b/src/app/datasets/dataset-table/dataset-table.component.html
@@ -208,7 +208,7 @@
       <mat-cell *matCellDef="let dataset">
         <img
           src="{{ dataset.pid | thumbnail | async }}"
-          height="60px"
+          height="42px"
           loading="lazy"
         />
       </mat-cell>

diff --git a/src/app/datasets/datasets.module.ts b/src/app/datasets/datasets.module.ts
@@ -89,7 +89,6 @@ import { DatasetsFilterSettingsComponent } from "./datasets-filter/settings/data
 import { CdkDrag, CdkDragHandle, CdkDropList } from "@angular/cdk/drag-drop";
 import { FiltersModule } from "shared/modules/filters/filters.module";
 import { userReducer } from "state-management/reducers/user.reducer";
-import { AttachmentService } from "shared/services/attachment.service";
 
 @NgModule({
   imports: [

diff --git a/src/app/shared/modules/file-uploader/file-uploader.component.ts b/src/app/shared/modules/file-uploader/file-uploader.component.ts
@@ -109,11 +109,7 @@ export class FileUploaderComponent {
   }
 
   getImageUrl(encoded: string) {
-    const mimeType = this.base64MimeType(encoded);
-    if (mimeType === "application/pdf") {
-      return "assets/images/pdf-icon.svg";
-    }
-    return encoded;
+    return this.attachmentService.getImageUrl(encoded);
   }
 
   openAttachment(encoded: string) {

diff --git a/src/app/shared/services/attachment.service.spec.ts b/src/app/shared/services/attachment.service.spec.ts
@@ -29,11 +29,25 @@ describe("AttachmentService", () => {
     });
 
     it("should return null for non-string input", () => {
-      const mimeType = service.base64MimeType(null as any);
+      const mimeType = service.base64MimeType(null);
       expect(mimeType).toBeNull();
     });
   });
 
+  describe("getImageUrl", () => {
+    it("should return the pdf icon if the file is pdf", () => {
+      const encoded = "data:application/pdf;base64,SGVsbG8sIFdvcmxkIQ==";
+      const imageUrl = service.getImageUrl(encoded);
+      expect(imageUrl).toBe("assets/images/pdf-icon.svg");
+    });
+
+    it("should return the encoded string if the file is not pdf", () => {
+      const encoded = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUA";
+      const imageUrl = service.getImageUrl(encoded);
+      expect(imageUrl).toBe(encoded);
+    });
+  });
+
   describe("openAttachment", () => {
     it("should open a new window with the correct object URL", () => {
       const encoded = "data:text/plain;base64,SGVsbG8sIFdvcmxkIQ==";

diff --git a/src/app/shared/services/attachment.service.ts b/src/app/shared/services/attachment.service.ts
@@ -3,6 +3,10 @@ import { Injectable } from "@angular/core";
 @Injectable()
 export class AttachmentService {
   base64MimeType(encoded: string): string {
+    if (!encoded) {
+      return null;
+    }
+
     let result = null;
 
     if (typeof encoded !== "string") {
@@ -17,7 +21,24 @@ export class AttachmentService {
 
     return result;
   }
+
+  getImageUrl(encoded: string) {
+    if (!encoded) {
+      return null;
+    }
+
+    const mimeType = this.base64MimeType(encoded);
+    if (mimeType === "application/pdf") {
+      return "assets/images/pdf-icon.svg";
+    }
+    return encoded;
+  }
+
   openAttachment(encoded: string) {
+    if (!encoded) {
+      return null;
+    }
+
     const mimeType = this.base64MimeType(encoded);
     const strippedData = encoded.replace(
       new RegExp(`^data:${mimeType};base64,`),

diff --git a/src/app/shared/services/thumbnail.service.spec.ts b/src/app/shared/services/thumbnail.service.spec.ts
@@ -5,6 +5,7 @@ import { ThumbnailService } from "./thumbnail.service";
 import { selectDatasetsPerPage } from "state-management/selectors/datasets.selectors";
 import { MockStore, provideMockStore } from "@ngrx/store/testing";
 import { of, throwError } from "rxjs";
+import { AttachmentService } from "./attachment.service";
 
 describe("ThumbnailService", () => {
   let service: ThumbnailService;
@@ -27,6 +28,9 @@ describe("ThumbnailService", () => {
           provide: AppConfigService,
           useValue: { getConfig },
         },
+        {
+          provide: AttachmentService,
+        },
         provideMockStore({
           selectors: [
             {