fix(thumbnail): fix & optimize thumbnail request handling to prevent …

…excessive api calls on page size change (#1633) * refactor: remove unused and outdated files * fix failing e2e test * fix failing e2e test * fix for cypress config type error & keep only one cypress config file for e2e test local/github * fix jesmine asserstion fake error * keep only one cypress.config.ts * fix failing e2e * revert scicat-sdk-ts import * Refactor dataset table component and dashboard component - Add pageChange event to dataset table component - Dispatch changePageAction in dashboard component - Remove unused imports and methods in dashboard component - Remove ngIf condition in dataset table component - Emit pageChange event in dataset table component - Update thumbnail pipe to use ThumbnailService - Create ThumbnailService to fetch thumbnails - Cache thumbnails in ThumbnailService * added thumbnailFetchLimitPerPage config variable and included cache expiration logic * fixed issue with missing columns in dataset table upon logout * fix failing test and added test for thumbnail.service
SciCatProject · Nov 1, 2024 · 2ae03e8 · 2ae03e8
1 parent c417e49
commit 2ae03e8
Show file tree

Hide file tree

Showing 14 changed files with 334 additions and 145 deletions.
diff --git a/CI/e2e/frontend.config.e2e.json b/CI/e2e/frontend.config.e2e.json
@@ -30,6 +30,7 @@
   "logbookEnabled": true,
   "loginFormEnabled": true,
   "maxDirectDownloadSize": 1047521824,
+  "thumbnailFetchLimitPerPage": 100,
   "metadataPreviewEnabled": true,
   "metadataStructure": "",
   "multipleDownloadAction": "http:/localhost/zip",

diff --git a/src/app/app-config.service.spec.ts b/src/app/app-config.service.spec.ts
@@ -37,6 +37,7 @@ const appConfig: AppConfig = {
   lbBaseURL: "http://127.0.0.1:3000",
   logbookEnabled: true,
   loginFormEnabled: true,
+  thumbnailFetchLimitPerPage: 500,
   maxDirectDownloadSize: 5000000000,
   metadataPreviewEnabled: true,
   metadataStructure: "",

diff --git a/src/app/app-config.service.ts b/src/app/app-config.service.ts
@@ -97,6 +97,7 @@ export interface AppConfig {
   metadataEditingUnitListDisabled?: boolean;
   defaultDatasetsListSettings: DatasetsListSettings;
   labelMaps: LabelMaps;
+  thumbnailFetchLimitPerPage: number;
 }
 
 @Injectable()

diff --git a/src/app/datasets/dashboard/dashboard.component.html b/src/app/datasets/dashboard/dashboard.component.html
@@ -40,6 +40,7 @@
           [selectedSets]="selectedSets$ | async"
           (settingsClick)="onSettingsClick()"
           (rowClick)="onRowClick($event)"
+          (pageChange)="onPageChange($event)"
         ></dataset-table>
       </div>
     </div>

diff --git a/src/app/datasets/dashboard/dashboard.component.spec.ts b/src/app/datasets/dashboard/dashboard.component.spec.ts
@@ -11,7 +11,10 @@ import { Store, StoreModule } from "@ngrx/store";
 import { MockActivatedRoute, MockStore } from "shared/MockStubs";
 import { DashboardComponent } from "./dashboard.component";
 import { of } from "rxjs";
-import { addDatasetAction } from "state-management/actions/datasets.actions";
+import {
+  addDatasetAction,
+  changePageAction,
+} from "state-management/actions/datasets.actions";
 import { User, Dataset, DerivedDataset } from "shared/sdk";
 import {
   selectColumnAction,
@@ -32,6 +35,7 @@ import { MatCheckboxChange } from "@angular/material/checkbox";
 import { MatCardModule } from "@angular/material/card";
 import { MatIconModule } from "@angular/material/icon";
 import { AppConfigService } from "app-config.service";
+import { PageChangeEvent } from "shared/modules/table/table.component";
 
 class MockMatDialog {
   open() {
@@ -264,8 +268,26 @@ describe("DashboardComponent", () => {
     });
   });
 
-  describe("#updateColumnSubscription()", () => {
-    it("should navigate to a dataset", () => {
+  describe("#onPageChange()", () => {
+    it("should dispatch a changePangeAction", () => {
+      dispatchSpy = spyOn(store, "dispatch");
+
+      const event: PageChangeEvent = {
+        pageIndex: 0,
+        pageSize: 25,
+        length: 25,
+      };
+      component.onPageChange(event);
+
+      expect(dispatchSpy).toHaveBeenCalledTimes(1);
+      expect(dispatchSpy).toHaveBeenCalledWith(
+        changePageAction({ page: event.pageIndex, limit: event.pageSize }),
+      );
+    });
+  });
+
+  describe("#tableColumn$ observable", () => {
+    it("should show 'select' column when user is logged in", () => {
       const testColumn: TableColumn = {
         name: "test",
         order: 0,
@@ -276,18 +298,16 @@ describe("DashboardComponent", () => {
         name: "select",
         order: 1,
         type: "standard",
-        enabled: false,
+        enabled: true,
       };
       selectColumns.setResult([testColumn, selectColumn]);
       selectIsLoggedIn.setResult(true);
-      component.updateColumnSubscription();
 
-      component.tableColumns$.subscribe((result) =>
-        expect(result.length).toEqual(2),
-      );
+      component.tableColumns$.subscribe((result) => {
+        expect(result.length).toEqual(2);
+      });
 
       selectIsLoggedIn.setResult(false);
-      component.updateColumnSubscription();
       component.tableColumns$.subscribe((result) =>
         expect(result).toEqual([testColumn]),
       );

diff --git a/src/app/datasets/dashboard/dashboard.component.ts b/src/app/datasets/dashboard/dashboard.component.ts
@@ -14,6 +14,7 @@ import {
   addDatasetAction,
   fetchDatasetCompleteAction,
   fetchMetadataKeysAction,
+  changePageAction,
 } from "state-management/actions/datasets.actions";
 
 import {
@@ -38,7 +39,7 @@ import { Dataset, DerivedDataset } from "shared/sdk";
 import {
   selectColumnAction,
   deselectColumnAction,
-  setDatasetTableColumnsAction,
+  loadDefaultSettings,
 } from "state-management/actions/user.actions";
 import { SelectColumnEvent } from "datasets/dataset-table-settings/dataset-table-settings.component";
 import { AppConfigService } from "app-config.service";
@@ -55,16 +56,16 @@ export class DashboardComponent implements OnInit, OnDestroy {
     .pipe(filter((has) => has));
   loggedIn$ = this.store.select(selectIsLoggedIn);
   selectedSets$ = this.store.select(selectSelectedDatasets);
-  tableColumns$ = this.store
-    .select(selectColumns)
-    .pipe(
-      map((columns) => columns.filter((column) => column.name !== "select")),
-    );
-  selectableColumns$ = this.store
-    .select(selectColumns)
-    .pipe(
-      map((columns) => columns.filter((column) => column.name !== "select")),
-    );
+  selectColumns$ = this.store.select(selectColumns);
+
+  tableColumns$ = combineLatest([this.selectColumns$, this.loggedIn$]).pipe(
+    map(([columns, loggedIn]) =>
+      columns.filter((column) => loggedIn || column.name !== "select"),
+    ),
+  );
+  selectableColumns$ = this.selectColumns$.pipe(
+    map((columns) => columns.filter((column) => column.name !== "select")),
+  );
   public nonEmpty$ = this.store
     .select(selectDatasetsInBatch)
     .pipe(map((batch) => batch.length > 0));
@@ -88,6 +89,12 @@ export class DashboardComponent implements OnInit, OnDestroy {
     private route: ActivatedRoute,
   ) {}
 
+  onPageChange(event: { pageIndex: number; pageSize: number }) {
+    this.store.dispatch(
+      changePageAction({ page: event.pageIndex, limit: event.pageSize }),
+    );
+  }
+
   onSettingsClick(): void {
     this.sideNav.toggle();
     if (this.sideNav.opened) {
@@ -157,49 +164,27 @@ export class DashboardComponent implements OnInit, OnDestroy {
     });
   }
 
-  updateColumnSubscription(): void {
-    this.subscriptions.push(
-      this.loggedIn$.subscribe((status) => {
-        // NOTE: this.appConfig.localColumns is for backward compatibility.
-        //       it should be removed once localColumns is removed from the appConfig
-        const columns =
-          this.appConfig.defaultDatasetsListSettings?.columns ||
-          this.appConfig.localColumns;
-        this.store.dispatch(setDatasetTableColumnsAction({ columns }));
-        if (!status) {
-          this.tableColumns$ = this.store
-            .select(selectColumns)
-            .pipe(
-              map((columns) =>
-                columns.filter((column) => column.name !== "select"),
-              ),
-            );
-        } else {
-          this.tableColumns$ = this.store.select(selectColumns);
-        }
-      }),
-    );
-  }
-
   ngOnInit() {
     this.store.dispatch(prefillBatchAction());
     this.store.dispatch(fetchMetadataKeysAction());
-    this.store.dispatch(fetchDatasetsAction());
-
-    this.updateColumnSubscription();
 
     this.subscriptions.push(
       combineLatest([this.pagination$, this.readyToFetch$, this.loggedIn$])
         .pipe(
-          map(([pagination, _, loggedIn]) => [pagination, loggedIn]),
+          map(([pagination, , loggedIn]) => [pagination, loggedIn]),
           distinctUntilChanged(deepEqual),
         )
-        .subscribe((obj) => {
+        .subscribe(([pagination, loggedIn]) => {
           this.store.dispatch(fetchDatasetsAction());
           this.store.dispatch(fetchFacetCountsAction());
           this.router.navigate(["/datasets"], {
-            queryParams: { args: JSON.stringify(obj[0]) },
+            queryParams: { args: JSON.stringify(pagination) },
           });
+          if (!loggedIn) {
+            this.store.dispatch(
+              loadDefaultSettings({ config: this.appConfig }),
+            );
+          }
         }),
     );
 

diff --git a/src/app/datasets/dataset-table/dataset-table.component.html b/src/app/datasets/dataset-table/dataset-table.component.html
@@ -206,13 +206,11 @@
         </div>
       </mat-header-cell>
       <mat-cell *matCellDef="let dataset">
-        <ng-container *ngIf="datasetsPerPage$ | async as datasetsPerPage">
-          <img
-            *ngIf="datasetsPerPage < 50"
-            src="{{ dataset.pid | thumbnail | async }}"
-            height="60px"
-          />
-        </ng-container>
+        <img
+          src="{{ dataset.pid | thumbnail | async }}"
+          height="60px"
+          loading="lazy"
+        />
       </mat-cell>
     </ng-container>
 

diff --git a/src/app/datasets/dataset-table/dataset-table.component.spec.ts b/src/app/datasets/dataset-table/dataset-table.component.spec.ts
@@ -403,49 +403,6 @@ describe("DatasetTableComponent", () => {
     });
   });
 
-  describe("#onPageChange()", () => {
-    const name = "image";
-    const columnType = "standard";
-
-    it("should dispatch a changePangeAction and a selectColumnAction if pageSize is less than 50", () => {
-      dispatchSpy = spyOn(store, "dispatch");
-
-      const event: PageChangeEvent = {
-        pageIndex: 0,
-        pageSize: 25,
-        length: 25,
-      };
-      component.onPageChange(event);
-
-      expect(dispatchSpy).toHaveBeenCalledTimes(2);
-      expect(dispatchSpy).toHaveBeenCalledWith(
-        changePageAction({ page: event.pageIndex, limit: event.pageSize }),
-      );
-      expect(dispatchSpy).toHaveBeenCalledWith(
-        selectColumnAction({ name, columnType }),
-      );
-    });
-
-    it("should dispatch a changePangeAction and a deselectColumnAction if pageSize is larger than or equal to 50", () => {
-      dispatchSpy = spyOn(store, "dispatch");
-
-      const event: PageChangeEvent = {
-        pageIndex: 0,
-        pageSize: 50,
-        length: 25,
-      };
-      component.onPageChange(event);
-
-      expect(dispatchSpy).toHaveBeenCalledTimes(2);
-      expect(dispatchSpy).toHaveBeenCalledWith(
-        changePageAction({ page: event.pageIndex, limit: event.pageSize }),
-      );
-      expect(dispatchSpy).toHaveBeenCalledWith(
-        deselectColumnAction({ name, columnType }),
-      );
-    });
-  });
-
   describe("#onSortChange()", () => {
     it("should dispatch a sortByColumnAction", () => {
       dispatchSpy = spyOn(store, "dispatch");

diff --git a/src/app/datasets/dataset-table/dataset-table.component.ts b/src/app/datasets/dataset-table/dataset-table.component.ts
@@ -18,7 +18,6 @@ import {
   selectDatasetAction,
   deselectDatasetAction,
   selectAllDatasetsAction,
-  changePageAction,
   sortByColumnAction,
 } from "state-management/actions/datasets.actions";
 
@@ -29,14 +28,10 @@ import {
   selectTotalSets,
   selectDatasetsInBatch,
 } from "state-management/selectors/datasets.selectors";
-import { PageChangeEvent } from "shared/modules/table/table.component";
-import {
-  selectColumnAction,
-  deselectColumnAction,
-} from "state-management/actions/user.actions";
 import { get } from "lodash";
 import { AppConfigService } from "app-config.service";
 import { selectCurrentUser } from "state-management/selectors/user.selectors";
+import { PageEvent } from "@angular/material/paginator";
 export interface SortChangeEvent {
   active: string;
   direction: "asc" | "desc" | "";
@@ -67,6 +62,10 @@ export class DatasetTableComponent implements OnInit, OnDestroy, OnChanges {
   @Input() tableColumns: TableColumn[] | null = null;
   displayedColumns: string[] = [];
   @Input() selectedSets: Dataset[] | null = null;
+  @Output() pageChange = new EventEmitter<{
+    pageIndex: number;
+    pageSize: number;
+  }>();
 
   datasets: Dataset[] = [];
   // datasetDerivationsMaps: DatasetDerivationsMap[] = [];
@@ -79,6 +78,13 @@ export class DatasetTableComponent implements OnInit, OnDestroy, OnChanges {
     public appConfigService: AppConfigService,
     private store: Store,
   ) {}
+
+  onPageChange(event: PageEvent) {
+    this.pageChange.emit({
+      pageIndex: event.pageIndex,
+      pageSize: event.pageSize,
+    });
+  }
   doSettingsClick(event: MouseEvent) {
     this.settingsClick.emit(event);
   }
@@ -178,21 +184,6 @@ export class DatasetTableComponent implements OnInit, OnDestroy, OnChanges {
     }
   }
 
-  onPageChange(event: PageChangeEvent): void {
-    this.store.dispatch(
-      changePageAction({ page: event.pageIndex, limit: event.pageSize }),
-    );
-    if (event.pageSize < 50) {
-      this.store.dispatch(
-        selectColumnAction({ name: "image", columnType: "standard" }),
-      );
-    } else {
-      this.store.dispatch(
-        deselectColumnAction({ name: "image", columnType: "standard" }),
-      );
-    }
-  }
-
   onSortChange(event: SortChangeEvent): void {
     const { active, direction } = event;
     let column = active.split("_")[1];

diff --git a/src/app/shared/pipes/json-head.pipe.ts b/src/app/shared/pipes/json-head.pipe.ts
@@ -36,7 +36,7 @@ export class JsonHeadPipe implements PipeTransform {
       val2 = " ";
     }
     outputString =
-      key1 + ":" + (val1?.value.toString() || val1) + "\n" + key2 + ":" + val2;
+      key1 + ":" + (val1?.value?.toString() || val1) + "\n" + key2 + ":" + val2;
     if (key1 === undefined) {
       outputString = "No metadata found";
     }