From f82e6fa48bad7214b53f424bbf8b313152ae5ee9 Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Wed, 2 Sep 2020 19:35:35 +0200 Subject: [PATCH] 72403: Process output logs only retrieved at button press + tests --- .../detail/process-detail.component.html | 22 ++++--- .../detail/process-detail.component.spec.ts | 66 +++++++++++++------ .../detail/process-detail.component.ts | 51 ++++++++++---- src/app/process-page/process-page.resolver.ts | 2 +- src/assets/i18n/en.json5 | 4 ++ 5 files changed, 103 insertions(+), 42 deletions(-) diff --git a/src/app/process-page/detail/process-detail.component.html b/src/app/process-page/detail/process-detail.component.html index e76d24d17a..e13770a0a3 100644 --- a/src/app/process-page/detail/process-detail.component.html +++ b/src/app/process-page/detail/process-detail.component.html @@ -34,13 +34,19 @@
{{ process.processStatus }}
- - 
{{ (outputLogs$ | async)?.join('\n') }}
-

- {{ 'process.detail.logs.none' | translate }} -

- + + + + 
{{ (outputLogs$ | async)?.join('\n') }}
+

+ {{ 'process.detail.logs.none' | translate }} +

+ - {{'process.detail.back' | translate}} +
+ {{'process.detail.back' | translate}} +
diff --git a/src/app/process-page/detail/process-detail.component.spec.ts b/src/app/process-page/detail/process-detail.component.spec.ts index c1efd233e8..8d4a3f0e62 100644 --- a/src/app/process-page/detail/process-detail.component.spec.ts +++ b/src/app/process-page/detail/process-detail.component.spec.ts @@ -1,7 +1,7 @@ import { ProcessOutputDataService } from '../../core/data/process-output-data.service'; import { ProcessOutput } from '../processes/process-output.model'; import { ProcessDetailComponent } from './process-detail.component'; -import { async, ComponentFixture, fakeAsync, TestBed } from '@angular/core/testing'; +import { async, ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing'; import { VarDirective } from '../../shared/utils/var.directive'; import { TranslateModule } from '@ngx-translate/core'; import { RouterTestingModule } from '@angular/router/testing'; @@ -93,7 +93,10 @@ describe('ProcessDetailComponent', () => { declarations: [ProcessDetailComponent, ProcessDetailFieldComponent, VarDirective, FileSizePipe], imports: [TranslateModule.forRoot(), RouterTestingModule.withRoutes([])], providers: [ - { provide: ActivatedRoute, useValue: { data: observableOf({ process: createSuccessfulRemoteDataObject(process) }) } }, + { + provide: ActivatedRoute, + useValue: { data: observableOf({ process: createSuccessfulRemoteDataObject(process) }) } + }, { provide: ProcessDataService, useValue: processService }, { provide: ProcessOutputDataService, useValue: processOutputService }, { provide: DSONameService, useValue: nameService } @@ -105,15 +108,16 @@ describe('ProcessDetailComponent', () => { beforeEach(() => { fixture = TestBed.createComponent(ProcessDetailComponent); component = fixture.componentInstance; - fixture.detectChanges(); }); it('should display the script\'s name', () => { + fixture.detectChanges(); const name = fixture.debugElement.query(By.css('#process-name')).nativeElement; expect(name.textContent).toContain(process.scriptName); }); it('should display the process\'s parameters', () => { + fixture.detectChanges(); const args = fixture.debugElement.query(By.css('#process-arguments')).nativeElement; process.parameters.forEach((param) => { expect(args.textContent).toContain(`${param.name} ${param.value}`) @@ -121,27 +125,52 @@ describe('ProcessDetailComponent', () => { }); it('should display the process\'s output files', () => { + fixture.detectChanges(); const processFiles = fixture.debugElement.query(By.css('#process-files')).nativeElement; expect(processFiles.textContent).toContain(fileName); }); - it('should display the process\'s output logs', () => { - const outputProcess = fixture.debugElement.query(By.css('#process-output pre')).nativeElement; - expect(outputProcess.textContent).toContain('Process started'); + describe('if press show output logs', () => { + beforeEach(fakeAsync(() => { + spyOn(component, 'showProcessOutputLogs').and.callThrough(); + fixture.detectChanges(); + const showOutputButton = fixture.debugElement.query(By.css('#showOutputButton')); + showOutputButton.triggerEventHandler('click', { + preventDefault: () => {/**/ + } + }); + tick(); + })); + it('should trigger showProcessOutputLogs', () => { + expect(component.showProcessOutputLogs).toHaveBeenCalled(); + }); + it('should display the process\'s output logs', () => { + fixture.detectChanges(); + const outputProcess = fixture.debugElement.query(By.css('#process-output pre')); + expect(outputProcess.nativeElement.textContent).toContain('Process started'); + }); }); - describe('if process has no output logs (yet)', () => { + describe('if press show output logs and process has no output logs (yet)', () => { beforeEach(fakeAsync(() => { - jasmine.getEnv().allowRespy(true); - const emptyProcessOutput = Object.assign(new ProcessOutput(), { - logs: [] - }); - spyOn(processOutputService, 'findByHref').and.returnValue(createSuccessfulRemoteDataObject$(emptyProcessOutput)); - fixture = TestBed.createComponent(ProcessDetailComponent); - component = fixture.componentInstance; - fixture.detectChanges(); - }) - ); + jasmine.getEnv().allowRespy(true); + const emptyProcessOutput = Object.assign(new ProcessOutput(), { + logs: [] + }); + spyOn(processOutputService, 'findByHref').and.returnValue(createSuccessfulRemoteDataObject$(emptyProcessOutput)); + fixture = TestBed.createComponent(ProcessDetailComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + spyOn(component, 'showProcessOutputLogs').and.callThrough(); + fixture.detectChanges(); + const showOutputButton = fixture.debugElement.query(By.css('#showOutputButton')); + showOutputButton.triggerEventHandler('click', { + preventDefault: () => {/**/ + } + }); + tick(); + fixture.detectChanges(); + })); it('should not display the process\'s output logs', () => { const outputProcess = fixture.debugElement.query(By.css('#process-output pre')); expect(outputProcess).toBeNull(); @@ -150,7 +179,6 @@ describe('ProcessDetailComponent', () => { const noOutputProcess = fixture.debugElement.query(By.css('#no-output-logs-message')).nativeElement; expect(noOutputProcess).toBeDefined(); }); - } - ) + }); }); diff --git a/src/app/process-page/detail/process-detail.component.ts b/src/app/process-page/detail/process-detail.component.ts index af6e3238a6..f6b628f0f8 100644 --- a/src/app/process-page/detail/process-detail.component.ts +++ b/src/app/process-page/detail/process-detail.component.ts @@ -1,12 +1,13 @@ -import { Component, OnInit } from '@angular/core'; +import { Component, NgZone, OnInit } from '@angular/core'; import { ActivatedRoute, Router } from '@angular/router'; +import { BehaviorSubject } from 'rxjs/internal/BehaviorSubject'; import { Observable } from 'rxjs/internal/Observable'; import { ProcessOutputDataService } from '../../core/data/process-output-data.service'; import { RemoteData } from '../../core/data/remote-data'; import { DSpaceObject } from '../../core/shared/dspace-object.model'; import { ProcessOutput } from '../processes/process-output.model'; import { Process } from '../processes/process.model'; -import { map, switchMap } from 'rxjs/operators'; +import { finalize, map, switchMap, take } from 'rxjs/operators'; import { getFirstSucceededRemoteDataPayload, redirectToPageNotFoundOn404 } from '../../core/shared/operators'; import { AlertType } from '../../shared/alert/aletr-type'; import { ProcessDataService } from '../../core/data/processes/process-data.service'; @@ -44,11 +45,21 @@ export class ProcessDetailComponent implements OnInit { */ outputLogs$: Observable; + /** + * Boolean on whether or not to show the output logs + */ + showOutputLogs = false; + /** + * When it's retrieving the output logs from backend, to show loading component + */ + retrievingOutputLogs$ = new BehaviorSubject(false); + constructor(protected route: ActivatedRoute, protected router: Router, protected processService: ProcessDataService, protected processOutputService: ProcessOutputDataService, - protected nameService: DSONameService) { + protected nameService: DSONameService, + private zone: NgZone) { } /** @@ -65,17 +76,6 @@ export class ProcessDetailComponent implements OnInit { getFirstSucceededRemoteDataPayload(), switchMap((process: Process) => this.processService.getFiles(process.processId)) ); - - const processOutputRD$: Observable> = this.processRD$.pipe( - getFirstSucceededRemoteDataPayload(), - switchMap((process: Process) => this.processOutputService.findByHref(process._links.output.href)) - ); - this.outputLogs$ = processOutputRD$.pipe( - getFirstSucceededRemoteDataPayload(), - map((processOutput: ProcessOutput) => { - return processOutput.logs; - }) - ) } /** @@ -86,4 +86,27 @@ export class ProcessDetailComponent implements OnInit { return bitstream instanceof DSpaceObject ? this.nameService.getName(bitstream) : 'unknown'; } + /** + * Retrieves the process logs, while setting the loading subject to true. + * Sets the outputLogs when retrieved and sets the showOutputLogs boolean to show them and hide the button. + */ + showProcessOutputLogs() { + this.retrievingOutputLogs$.next(true); + this.zone.runOutsideAngular(() => { + const processOutputRD$: Observable> = this.processRD$.pipe( + getFirstSucceededRemoteDataPayload(), + switchMap((process: Process) => this.processOutputService.findByHref(process._links.output.href)) + ); + this.outputLogs$ = processOutputRD$.pipe( + getFirstSucceededRemoteDataPayload(), + map((processOutput: ProcessOutput) => { + this.showOutputLogs = true; + return processOutput.logs; + }), + finalize(() => this.zone.run(() => this.retrievingOutputLogs$.next(false))), + ) + }); + this.outputLogs$.pipe(take(1)).subscribe(); + } + } diff --git a/src/app/process-page/process-page.resolver.ts b/src/app/process-page/process-page.resolver.ts index 57c749e1cb..84821a2574 100644 --- a/src/app/process-page/process-page.resolver.ts +++ b/src/app/process-page/process-page.resolver.ts @@ -24,7 +24,7 @@ export class ProcessPageResolver implements Resolve> { * or an error if something went wrong */ resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable> { - return this.processService.findById(route.params.id, followLink('script'), followLink('output') ).pipe( + return this.processService.findById(route.params.id, followLink('script')).pipe( find((RD) => hasValue(RD.error) || RD.hasSucceeded), ); } diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 75db3eceb1..036fd87ec8 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -2085,6 +2085,10 @@ "process.detail.output" : "Process Output", + "process.detail.logs.button": "Retrieve process output", + + "process.detail.logs.loading": "Retrieving", + "process.detail.logs.none": "This process has no output yet", "process.detail.output-files" : "Output Files",