[CSTPER-138] Fixed issue where results are the same when pagination changed while retrieving external source providers

2025-10-10 11:33:04 +00:00 · 2020-10-21 18:01:29 +02:00
parent 988747e392
commit 826957a66d
4 changed files with 129 additions and 58 deletions
--- a/src/app/submission/import-external/import-external-searchbar/submission-import-external-searchbar.component.ts
+++ b/src/app/submission/import-external/import-external-searchbar/submission-import-external-searchbar.component.ts
@@ -1,6 +1,6 @@
-import { ChangeDetectorRef, Component, EventEmitter, Input, OnInit, Output } from '@angular/core';
+import { ChangeDetectorRef, Component, EventEmitter, Input, OnDestroy, OnInit, Output } from '@angular/core';

-import { of as observableOf, Observable } from 'rxjs';
+import { Observable, of as observableOf, Subscription } from 'rxjs';
 import { catchError, tap } from 'rxjs/operators';

 import { ExternalSourceService } from '../../../core/data/external-source.service';
@@ -12,6 +12,7 @@ import { createSuccessfulRemoteDataObject } from '../../../shared/remote-data.ut
 import { FindListOptions } from '../../../core/data/request.models';
 import { getFirstSucceededRemoteDataPayload } from '../../../core/shared/operators';
 import { HostWindowService } from '../../../shared/host-window.service';
+import { hasValue } from '../../../shared/empty.util';

 /**
 * Interface for the selected external source element.
@@ -37,7 +38,7 @@ export interface ExternalSourceData {
  styleUrls: ['./submission-import-external-searchbar.component.scss'],
  templateUrl: './submission-import-external-searchbar.component.html'
 })
-export class SubmissionImportExternalSearchbarComponent implements OnInit {
+export class SubmissionImportExternalSearchbarComponent implements OnInit, OnDestroy {
  /**
   * The init external source value.
   */
@@ -76,6 +77,11 @@ export class SubmissionImportExternalSearchbarComponent implements OnInit {
   */
  protected findListOptions: FindListOptions;

+  /**
+   * The subscription to unsubscribe
+   */
+  protected sub: Subscription;
+
  /**
   * Initialize the component variables.
   * @param {ExternalSourceService} externalService
@@ -145,7 +151,7 @@ export class SubmissionImportExternalSearchbarComponent implements OnInit {
        elementsPerPage: 5,
        currentPage: this.findListOptions.currentPage + 1,
      });
-      this.externalService.findAll(this.findListOptions).pipe(
+      this.sub = this.externalService.findAll(this.findListOptions).pipe(
        catchError(() => {
          const pageInfo = new PageInfo();
          const paginatedList = new PaginatedList(pageInfo, []);
@@ -159,7 +165,7 @@ export class SubmissionImportExternalSearchbarComponent implements OnInit {
        })
        this.pageInfo = externalSource.payload.pageInfo;
        this.cdr.detectChanges();
-      })
+      });
    }
  }

@@ -169,4 +175,13 @@ export class SubmissionImportExternalSearchbarComponent implements OnInit {
  public search(): void {
    this.externalSourceData.emit({ sourceId: this.selectedElement.id, query: this.searchString });
  }
+
+  /**
+   * Unsubscribe from all subscriptions
+   */
+  ngOnDestroy(): void {
+    if (hasValue(this.sub)) {
+      this.sub.unsubscribe();
+    }
+  }
 }
--- a/src/app/submission/import-external/submission-import-external.component.html
+++ b/src/app/submission/import-external/submission-import-external.component.html
@@ -4,7 +4,7 @@
      <h2 id="header" class="pb-2">{{'submission.import-external.title' | translate}}</h2>
      <ds-submission-import-external-searchbar
        [initExternalSourceData]="routeData"
-        (externalSourceData) = "getExternalsourceData($event)">
+        (externalSourceData) = "getExternalSourceData($event)">
      </ds-submission-import-external-searchbar>
    </div>
  </div>
@@ -20,7 +20,8 @@
                                [context]="context"
                                [importable]="true"
                                [importConfig]="importConfig"
-                                (importObject)="import($event)">
+                                (importObject)="import($event)"
+                                (pageChange)="paginationChange();">
        </ds-viewable-collection>
        <ds-loading *ngIf="(isLoading$ | async)"
                    message="{{'loading.search-results' | translate}}"></ds-loading>
--- a/src/app/submission/import-external/submission-import-external.component.spec.ts
+++ b/src/app/submission/import-external/submission-import-external.component.spec.ts
@@ -1,15 +1,19 @@
 import { Component, NO_ERRORS_SCHEMA } from '@angular/core';
-import { async, TestBed, ComponentFixture, inject } from '@angular/core/testing';
+import { async, ComponentFixture, inject, TestBed } from '@angular/core/testing';
+
+import { getTestScheduler } from 'jasmine-marbles';
 import { TranslateModule } from '@ngx-translate/core';
 import { Router } from '@angular/router';
 import { NgbModal } from '@ng-bootstrap/ng-bootstrap';
-import { of as observableOf, of } from 'rxjs/internal/observable/of';
+import { of as observableOf } from 'rxjs';
+import { TestScheduler } from 'rxjs/testing';
+
 import { SubmissionImportExternalComponent } from './submission-import-external.component';
 import { ExternalSourceService } from '../../core/data/external-source.service';
 import { getMockExternalSourceService } from '../../shared/mocks/external-source.service.mock';
 import { SearchConfigurationService } from '../../core/shared/search/search-configuration.service';
 import { RouteService } from '../../core/services/route.service';
-import { createTestComponent, createPaginatedList } from '../../shared/testing/utils.test';
+import { createPaginatedList, createTestComponent } from '../../shared/testing/utils.test';
 import { RouterStub } from '../../shared/testing/router.stub';
 import { VarDirective } from '../../shared/utils/var.directive';
 import { routeServiceStub } from '../../shared/testing/route-service.stub';
@@ -18,17 +22,20 @@ import { PaginationComponentOptions } from '../../shared/pagination/pagination-c
 import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils';
 import { ExternalSourceEntry } from '../../core/shared/external-source-entry.model';
 import { SubmissionImportExternalPreviewComponent } from './import-external-preview/submission-import-external-preview.component';
+import { BehaviorSubject } from 'rxjs/internal/BehaviorSubject';

 describe('SubmissionImportExternalComponent test suite', () => {
  let comp: SubmissionImportExternalComponent;
  let compAsAny: any;
  let fixture: ComponentFixture<SubmissionImportExternalComponent>;
+  let scheduler: TestScheduler;
  const ngbModal = jasmine.createSpyObj('modal', ['open']);
-  const mockSearchOptions = of(new PaginatedSearchOptions({
+  const mockSearchOptions = observableOf(new PaginatedSearchOptions({
    pagination: Object.assign(new PaginationComponentOptions(), {
      pageSize: 10,
      currentPage: 0
-    })
+    }),
+    query: 'test'
  }));
  const searchConfigServiceStub = {
    paginatedSearchOptions: mockSearchOptions
@@ -83,6 +90,7 @@ describe('SubmissionImportExternalComponent test suite', () => {
      fixture = TestBed.createComponent(SubmissionImportExternalComponent);
      comp = fixture.componentInstance;
      compAsAny = comp;
+      scheduler = getTestScheduler();
    });

    afterEach(() => {
@@ -102,25 +110,31 @@ describe('SubmissionImportExternalComponent test suite', () => {
    });

    it('Should init component properly (with route data)', () => {
-      const expectedEntries = createSuccessfulRemoteDataObject(createPaginatedList([]));
-      const searchOptions = new PaginatedSearchOptions({
-        pagination: Object.assign(new PaginationComponentOptions(), {
-          pageSize: 10,
-          currentPage: 0
-        })
-      });
-      spyOn(compAsAny.routeService, 'getQueryParameterValue').and.returnValue(observableOf('dummy'));
+      spyOn(compAsAny, 'retrieveExternalSources');
+      spyOn(compAsAny.routeService, 'getQueryParameterValue').and.returnValues(observableOf('source'), observableOf('dummy'));
      fixture.detectChanges();

-      expect(comp.routeData).toEqual({ sourceId: 'dummy', query: 'dummy' });
+      expect(compAsAny.retrieveExternalSources).toHaveBeenCalledWith('source', 'dummy');
+    });
+
+    it('Should call \'getExternalSourceEntries\' properly', () => {
+      comp.routeData = { sourceId: '', query: '' };
+      comp.isLoading$ = new BehaviorSubject(false);
+      scheduler.schedule(() => compAsAny.retrieveExternalSources('orcidV2', 'test'));
+      scheduler.flush();
+
+      expect(comp.routeData).toEqual({ sourceId: 'orcidV2', query: 'test' });
      expect(comp.isLoading$.value).toBe(true);
-      expect(comp.entriesRD$.value).toEqual(expectedEntries);
-      expect(compAsAny.externalService.getExternalSourceEntries).toHaveBeenCalledWith('dummy', searchOptions);
+      expect(compAsAny.externalService.getExternalSourceEntries).toHaveBeenCalled();
    });

    it('Should call \'router.navigate\'', () => {
+      spyOn(compAsAny, 'retrieveExternalSources');
+      compAsAny.router.navigate.and.returnValue( new Promise(() => {return;}))
      const event = { sourceId: 'orcidV2', query: 'dummy' };
-      comp.getExternalsourceData(event);
+
+      scheduler.schedule(() => comp.getExternalSourceData(event));
+      scheduler.flush();

      expect(compAsAny.router.navigate).toHaveBeenCalledWith([], { queryParams: { source: event.sourceId, query: event.query }, replaceUrl: true });
    });
--- a/src/app/submission/import-external/submission-import-external.component.ts
+++ b/src/app/submission/import-external/submission-import-external.component.ts
@@ -1,22 +1,25 @@
-import { Component, OnInit } from '@angular/core';
+import { Component, OnDestroy, OnInit } from '@angular/core';
 import { Router } from '@angular/router';
-import { combineLatest, BehaviorSubject } from 'rxjs';
+
+import { BehaviorSubject, combineLatest, Subscription } from 'rxjs';
+import { filter, flatMap, take } from 'rxjs/operators';
+import { NgbModal, NgbModalRef } from '@ng-bootstrap/ng-bootstrap';
+
 import { ExternalSourceService } from '../../core/data/external-source.service';
 import { ExternalSourceData } from './import-external-searchbar/submission-import-external-searchbar.component';
 import { RemoteData } from '../../core/data/remote-data';
 import { PaginatedList } from '../../core/data/paginated-list';
 import { ExternalSourceEntry } from '../../core/shared/external-source-entry.model';
 import { SearchConfigurationService } from '../../core/shared/search/search-configuration.service';
-import { switchMap, filter, take } from 'rxjs/operators';
-import { PaginatedSearchOptions } from '../../shared/search/paginated-search-options.model';
 import { Context } from '../../core/shared/context.model';
 import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model';
 import { RouteService } from '../../core/services/route.service';
 import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils';
-import { NgbModal, NgbModalRef } from '@ng-bootstrap/ng-bootstrap';
 import { SubmissionImportExternalPreviewComponent } from './import-external-preview/submission-import-external-preview.component';
 import { fadeIn } from '../../shared/animations/fade';
 import { PageInfo } from '../../core/shared/page-info.model';
+import { hasValue, isNotEmpty } from '../../shared/empty.util';
+import { getFinishedRemoteData } from '../../core/shared/operators';

 /**
 * This component allows to submit a new workspaceitem importing the data from an external source.
@@ -25,9 +28,10 @@ import { PageInfo } from '../../core/shared/page-info.model';
  selector: 'ds-submission-import-external',
  styleUrls: ['./submission-import-external.component.scss'],
  templateUrl: './submission-import-external.component.html',
-  animations: [ fadeIn ]
+  animations: [fadeIn]
 })
-export class SubmissionImportExternalComponent implements OnInit {
+export class SubmissionImportExternalComponent implements OnInit, OnDestroy {
+
  /**
   * The external source search data from the routing service.
   */
@@ -61,7 +65,7 @@ export class SubmissionImportExternalComponent implements OnInit {
   */
  public initialPagination = Object.assign(new PaginationComponentOptions(), {
    id: 'submission-external-source-relation-list',
-    pageSize: 5
+    pageSize: 10
  });
  /**
   * The context to displaying lists for
@@ -72,6 +76,11 @@ export class SubmissionImportExternalComponent implements OnInit {
   */
  public modalRef: NgbModalRef;

+  /**
+   * The subscription to unsubscribe
+   */
+  protected subs: Subscription[] = [];
+
  /**
   * Initialize the component variables.
   * @param {SearchConfigurationService} searchConfigService
@@ -86,58 +95,45 @@ export class SubmissionImportExternalComponent implements OnInit {
    private routeService: RouteService,
    private router: Router,
    private modalService: NgbModal,
-  ) { }
+  ) {
+  }

  /**
   * Get the entries for the selected external source and set initial configuration.
   */
  ngOnInit(): void {
-    this.label      = 'Journal';
-    this.listId     = 'list-submission-external-sources';
-    this.context    = Context.EntitySearchModalWithNameVariants;
+    this.label = 'Journal';
+    this.listId = 'list-submission-external-sources';
+    this.context = Context.EntitySearchModalWithNameVariants;
    this.repeatable = false;
-    this.routeData  = { sourceId: '', query: '' };
+    this.routeData = { sourceId: '', query: '' };
    this.importConfig = {
      buttonLabel: 'submission.sections.describe.relationship-lookup.external-source.import-button-title.' + this.label
    };
    this.entriesRD$ = new BehaviorSubject(createSuccessfulRemoteDataObject(new PaginatedList(new PageInfo(), [])));
    this.isLoading$ = new BehaviorSubject(false);
-    combineLatest(
+    this.subs.push(combineLatest(
      [
        this.routeService.getQueryParameterValue('source'),
        this.routeService.getQueryParameterValue('query')
      ]).pipe(
-        take(1),
-        filter(([source, query]) => source && query && source !== '' && query !== ''),
-        filter(([source, query]) => source !== this.routeData.sourceId || query !== this.routeData.query),
-        switchMap(([source, query]) => {
-          this.routeData.sourceId = source;
-          this.routeData.query = query;
-          this.isLoading$.next(true);
-          return this.searchConfigService.paginatedSearchOptions.pipe(
-            switchMap((searchOptions: PaginatedSearchOptions) => {
-              return this.externalService.getExternalSourceEntries(this.routeData.sourceId, searchOptions);
-            }),
-            take(1)
-          )
-        }),
-    ).subscribe((rdData) => {
-      this.entriesRD$.next(rdData);
-      this.isLoading$.next(false);
-    });
+      take(1)
+    ).subscribe(([source, query]: [string, string]) => {
+      this.retrieveExternalSources(source, query);
+    }));
  }

  /**
   * Get the data from the searchbar and changes the router data.
   */
-  public getExternalsourceData(event: ExternalSourceData): void {
+  public getExternalSourceData(event: ExternalSourceData): void {
    this.router.navigate(
      [],
      {
        queryParams: { source: event.sourceId, query: event.query },
        replaceUrl: true
      }
-    );
+    ).then(() => this.retrieveExternalSources(event.sourceId, event.query));
  }

  /**
@@ -151,4 +147,49 @@ export class SubmissionImportExternalComponent implements OnInit {
    const modalComp = this.modalRef.componentInstance;
    modalComp.externalSourceEntry = entry;
  }
+
+  /**
+   * Retrieve external sources on pagination change
+   */
+  paginationChange() {
+    this.retrieveExternalSources(this.routeData.sourceId, this.routeData.query);
+  }
+
+  /**
+   * Unsubscribe from all subscriptions
+   */
+  ngOnDestroy(): void {
+    this.subs
+      .filter((sub) => hasValue(sub))
+      .forEach((sub) => sub.unsubscribe());
+  }
+
+  /**
+   * Retrieve external source entries
+   *
+   * @param source The source tupe
+   * @param query The query string to search
+   */
+  private retrieveExternalSources(source: string, query: string): void {
+    if (isNotEmpty(source) && isNotEmpty(query)) {
+      this.routeData.sourceId = source;
+      this.routeData.query = query;
+      this.isLoading$.next(true);
+      this.subs.push(
+        this.searchConfigService.paginatedSearchOptions.pipe(
+          filter((searchOptions) => searchOptions.query === query),
+          take(1),
+          flatMap((searchOptions) => this.externalService.getExternalSourceEntries(this.routeData.sourceId, searchOptions).pipe(
+            getFinishedRemoteData(),
+            take(1)
+          )),
+          take(1)
+        ).subscribe((rdData) => {
+          this.entriesRD$.next(rdData);
+          this.isLoading$.next(false);
+        })
+      );
+    }
+  }
+
 }