From 596b0dba6959fd94c5c44e876d208d5decf2375e Mon Sep 17 00:00:00 2001 From: Kuno Vercammen Date: Wed, 3 Apr 2024 16:01:34 +0200 Subject: [PATCH] 113904: Returning unkown when showing process of deleted user --- .../form/process-form.component.ts | 43 ++++++++++++-- .../process-overview-table.component.spec.ts | 57 +++++++++++++++++++ .../table/process-overview-table.component.ts | 36 +++++++++--- src/assets/i18n/en.json5 | 2 + 4 files changed, 125 insertions(+), 13 deletions(-) diff --git a/src/app/process-page/form/process-form.component.ts b/src/app/process-page/form/process-form.component.ts index d361926bc7..fef725a8ef 100644 --- a/src/app/process-page/form/process-form.component.ts +++ b/src/app/process-page/form/process-form.component.ts @@ -100,11 +100,11 @@ export class ProcessFormComponent implements OnInit { } const stringParameters: ProcessParameter[] = this.parameters.map((parameter: ProcessParameter) => { - return { - name: parameter.name, - value: this.checkValue(parameter), - }; - }, + return { + name: parameter.name, + value: this.checkValue(parameter), + }; + }, ); this.scriptService.invoke(this.selectedScript.id, stringParameters, this.files) .pipe(getFirstCompletedRemoteData()) @@ -177,5 +177,36 @@ export class ProcessFormComponent implements OnInit { }; void this.router.navigate([getProcessListRoute()], extras); } -} + updateScript($event: Script) { + this.selectedScript = $event; + this.parameters = undefined; + this.updateName(); + } + + updateName(): void { + if (isEmpty(this.customName)) { + this.processName = this.generatedProcessName; + } else { + this.processName = this.customName; + } + } + + get generatedProcessName() { + const paramsString = this.parameters?.map((p: ProcessParameter) => { + const value = this.parseValue(p.value); + return isEmpty(value) ? p.name : `${p.name} ${value}`; + }).join(' ') || ''; + return isEmpty(paramsString) ? this.selectedScript.name : `${this.selectedScript.name} ${paramsString}`; + } + + private parseValue(value: any) { + if (typeof value === 'boolean') { + return undefined; + } + if (value instanceof File) { + return value.name; + } + return value?.toString(); + } +} diff --git a/src/app/process-page/overview/table/process-overview-table.component.spec.ts b/src/app/process-page/overview/table/process-overview-table.component.spec.ts index 9684e22ce2..2a3f237525 100644 --- a/src/app/process-page/overview/table/process-overview-table.component.spec.ts +++ b/src/app/process-page/overview/table/process-overview-table.component.spec.ts @@ -18,6 +18,16 @@ import { AuthService } from '../../../core/auth/auth.service'; import { ProcessDataService } from '../../../core/data/processes/process-data.service'; import { EPersonDataService } from '../../../core/eperson/eperson-data.service'; import { EPerson } from '../../../core/eperson/models/eperson.model'; +import { ProcessBulkDeleteService } from '../process-bulk-delete.service'; +import { ProcessStatus } from '../../processes/process-status.model'; +import { createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; +import { createPaginatedList } from '../../../shared/testing/utils.test'; +import { PaginationServiceStub } from '../../../shared/testing/pagination-service.stub'; +import { BehaviorSubject } from 'rxjs'; +import { NgbModal, NgbCollapse } from '@ng-bootstrap/ng-bootstrap'; +import { VarDirective } from '../../../shared/utils/var.directive'; +import { TranslateModule, TranslateService } from '@ngx-translate/core'; +import { RouterTestingModule } from '@angular/router/testing'; import { PaginationService } from '../../../core/pagination/pagination.service'; import { RouteService } from '../../../core/services/route.service'; import { ThemedLoadingComponent } from '../../../shared/loading/themed-loading.component'; @@ -46,6 +56,7 @@ describe('ProcessOverviewTableComponent', () => { let modalService: NgbModal; let authService; // : AuthService; Not typed as the mock does not fully implement AuthService let routeService: RouteService; + let translateService: TranslateService; let processes: Process[]; let ePerson: EPerson; @@ -217,4 +228,50 @@ describe('ProcessOverviewTableComponent', () => { }); }); +/* + describe('getEPersonName', () => { + beforeEach(() => { + init(); + translateService = getMockTranslateService(); + }); + + it('should return the name when the ID is valid', () => { + const id = 'valid_id'; + const expectedName = 'John Doe'; + + spyOn(dsoNameService, 'getName').and.returnValue(expectedName); + + component.getEPersonName(id).subscribe(name => { + expect(name).toEqual(expectedName); + }); + + expect(ePersonService.findById).toHaveBeenCalledWith(id); + }); + + fit('should return "Unknown" when the ID is invalid', () => { + const id = 'invalid_id'; + const translationKey = 'unknown_user'; + const expectedMessage = 'Unknown'; + + spyOn(translateService, 'get').and.returnValue(of(expectedMessage)); + + component.getEPersonName(id).subscribe(name => { + expect(name).toEqual(expectedMessage); + }); + + expect(ePersonService.findById).toHaveBeenCalledWith(id); + expect(translateService.get).toHaveBeenCalledWith(translationKey); + }); + + it('should return an empty observable when the ID is null', () => { + const id = null; + + component.getEPersonName(id).subscribe(name => { + expect(name).toBeUndefined(); + }); + + expect(ePersonService.findById).not.toHaveBeenCalled(); + }); + }); +*/ }); diff --git a/src/app/process-page/overview/table/process-overview-table.component.ts b/src/app/process-page/overview/table/process-overview-table.component.ts index 4df95e8780..5c5eabd3fd 100644 --- a/src/app/process-page/overview/table/process-overview-table.component.ts +++ b/src/app/process-page/overview/table/process-overview-table.component.ts @@ -47,6 +47,7 @@ import { redirectOn4xx } from '../../../core/shared/authorized.operators'; import { getAllCompletedRemoteData, getFirstSucceededRemoteDataPayload, + getAllCompletedRemoteData, getFirstCompletedRemoteData } from '../../../core/shared/operators'; import { hasValue } from '../../../shared/empty.util'; import { ThemedLoadingComponent } from '../../../shared/loading/themed-loading.component'; @@ -60,6 +61,17 @@ import { ProcessOverviewService, ProcessSortField, } from '../process-overview.service'; +import { map, switchMap, toArray, take, filter } from 'rxjs/operators'; +import { EPerson } from '../../../core/eperson/models/eperson.model'; +import { PaginationService } from 'src/app/core/pagination/pagination.service'; +import { FindListOptions } from '../../../core/data/find-list-options.model'; +import { redirectOn4xx } from '../../../core/shared/authorized.operators'; +import { Router } from '@angular/router'; +import { AuthService } from '../../../core/auth/auth.service'; +import { isPlatformBrowser } from '@angular/common'; +import { RouteService } from '../../../core/services/route.service'; +import { hasValue, isNotEmpty } from '../../../shared/empty.util'; +import { TranslateService } from '@ngx-translate/core'; const NEW_PROCESS_PARAM = 'new_process_id'; @@ -159,6 +171,7 @@ export class ProcessOverviewTableComponent implements OnInit, OnDestroy { protected routeService: RouteService, protected router: Router, protected auth: AuthService, + private translateService: TranslateService, @Inject(PLATFORM_ID) protected platformId: object, ) { } @@ -176,7 +189,7 @@ export class ProcessOverviewTableComponent implements OnInit, OnDestroy { // Creates an ID from the first 2 characters of the process status. // Should two process status values ever start with the same substring, // increase the number of characters until the ids are distinct. - this.paginationId = this.processStatus.toLowerCase().substring(0,2); + this.paginationId = this.processStatus.toLowerCase().substring(0, 2); const defaultPaginationOptions = Object.assign(new PaginationComponentOptions(), { id: this.paginationId, @@ -218,7 +231,7 @@ export class ProcessOverviewTableComponent implements OnInit, OnDestroy { // Map RemoteData> to RemoteData> switchMap((processesRD: RemoteData>) => { // Create observable emitting all processes one by one - return observableFrom(processesRD.payload.page).pipe( + return observableFrom(processesRD.payload.page).pipe( // Map every Process to ProcessOverviewTableEntry mergeMap((process: Process) => { return this.getEPersonName(process.userId).pipe( @@ -243,7 +256,6 @@ export class ProcessOverviewTableComponent implements OnInit, OnDestroy { }), ); }), - ).subscribe((next: RemoteData>) => { this.processesRD$.next(next); })); @@ -267,10 +279,20 @@ export class ProcessOverviewTableComponent implements OnInit, OnDestroy { * @param id ID of the EPerson */ getEPersonName(id: string): Observable { - return this.ePersonDataService.findById(id).pipe( - getFirstSucceededRemoteDataPayload(), - map((eperson: EPerson) => this.dsoNameService.getName(eperson)), - ); + if (isNotEmpty(id)) { + return this.ePersonDataService.findById(id).pipe( + getFirstCompletedRemoteData(), + switchMap((rd: RemoteData) => { + if (rd.hasSucceeded) { + return observableFrom([this.dsoNameService.getName(rd.payload)]); + } else { + return this.translateService.get('process.overview.unknown.user'); + } + }) + ); + } else { + return this.translateService.get('process.overview.unknown.user'); + } } /** diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 6072874140..8043c25974 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3846,6 +3846,8 @@ "process.overview.delete.header": "Delete processes", + "process.overview.unknown.user": "Unknown", + "process.bulk.delete.error.head": "Error on deleteing process", "process.bulk.delete.error.body": "The process with ID {{processId}} could not be deleted. The remaining processes will continue being deleted. ",