From 826957a66d59f4cf2d8dde81a1dab75f82cc0030 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Wed, 21 Oct 2020 18:01:29 +0200 Subject: [PATCH] [CSTPER-138] Fixed issue where results are the same when pagination changed while retrieving external source providers --- ...ion-import-external-searchbar.component.ts | 25 +++- .../submission-import-external.component.html | 5 +- ...bmission-import-external.component.spec.ts | 48 +++++--- .../submission-import-external.component.ts | 109 ++++++++++++------ 4 files changed, 129 insertions(+), 58 deletions(-) diff --git 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 index 743f3ea73a..58fadacdcc 100644 --- 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(); + } + } } diff --git a/src/app/submission/import-external/submission-import-external.component.html b/src/app/submission/import-external/submission-import-external.component.html index c4de97b934..919eec3f4a 100644 --- 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 @@ + (externalSourceData) = "getExternalSourceData($event)"> @@ -20,7 +20,8 @@ [context]="context" [importable]="true" [importConfig]="importConfig" - (importObject)="import($event)"> + (importObject)="import($event)" + (pageChange)="paginationChange();"> diff --git a/src/app/submission/import-external/submission-import-external.component.spec.ts b/src/app/submission/import-external/submission-import-external.component.spec.ts index 4cbe204a91..fdfa972cc0 100644 --- 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; + 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 }); }); diff --git a/src/app/submission/import-external/submission-import-external.component.ts b/src/app/submission/import-external/submission-import-external.component.ts index a230d12caf..110a399287 100644 --- 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); + }) + ); + } + } + }