diff --git a/.vscode/settings.json b/.vscode/settings.json index bcfb564f8..09091c2e9 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -2,6 +2,9 @@ "typescript.tsdk": "node_modules\\typescript\\lib", "cSpell.words": [ "Datatable", + "datatable", + "luxon", + "toastr", "typeahead", "Ecoacoustics" ] diff --git a/src/app/components/harvest/pages/list/list.component.spec.ts b/src/app/components/harvest/pages/list/list.component.spec.ts index 46c040b95..e9236d442 100644 --- a/src/app/components/harvest/pages/list/list.component.spec.ts +++ b/src/app/components/harvest/pages/list/list.component.spec.ts @@ -1,5 +1,10 @@ import { Injector } from "@angular/core"; -import { discardPeriodicTasks, fakeAsync, flush, tick } from "@angular/core/testing"; +import { + discardPeriodicTasks, + fakeAsync, + flush, + tick, +} from "@angular/core/testing"; import { MockBawApiModule } from "@baw-api/baw-apiMock.module"; import { HARVEST } from "@baw-api/ServiceTokens"; import { ConfirmationComponent } from "@components/harvest/components/modal/confirmation.component"; @@ -34,10 +39,7 @@ describe("ListComponent", () => { mocks: [ToastrService], }); - function setup( - project: Project, - mockHarvest: Harvest - ) { + function setup(project: Project, mockHarvest: Harvest) { spec = createComponent({ detectChanges: false, data: { @@ -101,9 +103,7 @@ describe("ListComponent", () => { return spec.query("baw-user-link"); } - function getElementByInnerText( - text: string - ): T { + function getElementByInnerText(text: string): T { return spec.debugElement.query( (element) => element.nativeElement.innerText === text )?.nativeElement as T; @@ -133,13 +133,12 @@ describe("ListComponent", () => { it("should not show abort button when harvest cannot be aborted", () => { // ensure harvest status is not an abortable state - const unAbortableHarvest = new Harvest( generateHarvest({ status: "scanning" }) ); - - setup( - defaultProject, - unAbortableHarvest + const unAbortableHarvest = new Harvest( + generateHarvest({ status: "scanning" }) ); + setup(defaultProject, unAbortableHarvest); + // assert abort button is not rendered expect(spec.query("button[name='list-abort-button']")).toBeFalsy(); }); @@ -176,13 +175,17 @@ describe("ListComponent", () => { const mockUserTimeZone = "Australia/Perth"; // +08:00 UTC const harvestUtcCreatedAt = DateTime.fromISO("2020-01-01T00:00:00.000Z"); const expectedLocalCreatedAt = "2020-01-01 08:00:00"; - defaultHarvest = new Harvest(generateHarvest({ createdAt: harvestUtcCreatedAt })); + defaultHarvest = new Harvest( + generateHarvest({ createdAt: harvestUtcCreatedAt }) + ); // To simplify tests, set the Luxon.Settings.defaultZone to a mock timezone (+08:00 UTC) Settings.defaultZone = mockUserTimeZone; setup(defaultProject, defaultHarvest); - const createdAtLabel = getElementByInnerText(expectedLocalCreatedAt); + const createdAtLabel = getElementByInnerText( + expectedLocalCreatedAt + ); expect(createdAtLabel).toExist(); }); @@ -208,4 +211,32 @@ describe("ListComponent", () => { expect(creatorColumn.innerText).toEqual(expectedUserName); }); + + it("should use projection in the api requests to load a subset of harvest properties", () => { + const harvestApi = setup(defaultProject, defaultHarvest); + + expect(harvestApi.filter).toHaveBeenCalledWith( + jasmine.objectContaining({ + projection: { + include: [ + "id", + "projectId", + "name", + "createdAt", + "creatorId", + "streaming", + "status", + ], + }, + }), + defaultProject + ); + }); + + // if this test if failing, it is either because the harvest api is not called, or called twice (or more) + // either in the harvest list page is faulty or our datatable pagination directive is faulty + it("should call the harvest api once on load", () => { + const harvestApi = setup(defaultProject, defaultHarvest); + expect(harvestApi.filter).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/app/components/harvest/pages/list/list.component.ts b/src/app/components/harvest/pages/list/list.component.ts index 54db9d87c..d5d9fd2b9 100644 --- a/src/app/components/harvest/pages/list/list.component.ts +++ b/src/app/components/harvest/pages/list/list.component.ts @@ -48,12 +48,25 @@ class ListComponent extends PageComponent implements OnInit { public ngOnInit(): void { this.project = this.route.snapshot.data[projectKey].model; this.canCreateHarvestCapability = this.project.can("createHarvest").can; - // A BehaviorSubject is need on fitlers$ to update the ngx-datatable harvest list & models + // A BehaviorSubject is need on filters$ to update the ngx-datatable harvest list & models // The this.filters$ is triggered in abortUpload() this.filters$ = new BehaviorSubject({ sorting: { direction: "desc", orderBy: "createdAt", + }, + // projection allows us only to emit the fields that we want + // this improves performance and reduces the amount of data sent + projection: { + include: [ + "id", + "projectId", + "name", + "createdAt", + "creatorId", + "streaming", + "status", + ] } }); } diff --git a/src/app/directives/datatable/pagination.directive.spec.ts b/src/app/directives/datatable/pagination.directive.spec.ts index 0ba982b24..8b5a91873 100644 --- a/src/app/directives/datatable/pagination.directive.spec.ts +++ b/src/app/directives/datatable/pagination.directive.spec.ts @@ -202,6 +202,17 @@ describe("DatatablePaginationDirective", () => { flushGetModels(); assertLoading(false); })); + + it("should only emit one api request when loaded", fakeAsync(() => { + const mockGetModels = jasmine.createSpy("getModels").and.callFake(() => of(defaultModels)); + + generateModels(); + setup({ filters: {}, getModels: mockGetModels }); + + flushGetModels(); + + expect(mockGetModels).toHaveBeenCalledTimes(1); + })); }); describe("total", () => { diff --git a/src/app/directives/datatable/pagination.directive.ts b/src/app/directives/datatable/pagination.directive.ts index 6980c9073..6eef20f46 100644 --- a/src/app/directives/datatable/pagination.directive.ts +++ b/src/app/directives/datatable/pagination.directive.ts @@ -123,10 +123,6 @@ export class DatatablePaginationDirective * retrieved from the api request */ private rows$: Observable; - /** - * Observable tracking the total number of models for the current base filter - */ - private total$: Observable; /** Observable tracking when the table is loading new data */ private loading$ = new BehaviorSubject(false); /** @@ -182,12 +178,6 @@ export class DatatablePaginationDirective tap((): void => this.loading$.next(false)) ); - this.total$ = this.rows$.pipe( - // Get the total number of models for the current filter from the - // responses metadata - map((models): number => models[0]?.getMetadata().paging.total ?? 0) - ); - this.subscribeToTableOutputs(); this.setTableInputs(); } @@ -234,9 +224,10 @@ export class DatatablePaginationDirective /** Sets table inputs with latest values from observables */ private setTableInputs(): void { - // Set table rows on change + // Set table rows and the total number of rows on change this.rows$.pipe(takeUntil(this.unsubscribe)).subscribe((rows): void => { this.datatable.rows = rows; + this.datatable.count = rows[0]?.getMetadata().paging.total ?? 0; }); // Set loading state on change @@ -246,11 +237,6 @@ export class DatatablePaginationDirective this.datatable.loadingIndicator = isLoading; }); - // Set count on change - this.total$.pipe(takeUntil(this.unsubscribe)).subscribe((total): void => { - this.datatable.count = total; - }); - // Set page number on change this.pageAndSort$ .pipe(takeUntil(this.unsubscribe))