Skip to content

Commit b406e71

Browse files
authored
Merge pull request DSpace#3014 from atmire/w2p-113904_AddSupportForNullUsers
Handle Null Users Gracefully in Process Overview Page
2 parents 2aa0f95 + 2d5d4a0 commit b406e71

4 files changed

Lines changed: 105 additions & 14 deletions

File tree

src/app/process-page/form/process-form.component.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,5 +177,27 @@ export class ProcessFormComponent implements OnInit {
177177
};
178178
void this.router.navigate([getProcessListRoute()], extras);
179179
}
180-
}
181180

181+
updateScript($event: Script) {
182+
this.selectedScript = $event;
183+
this.parameters = undefined;
184+
}
185+
186+
get generatedProcessName() {
187+
const paramsString = this.parameters?.map((p: ProcessParameter) => {
188+
const value = this.parseValue(p.value);
189+
return isEmpty(value) ? p.name : `${p.name} ${value}`;
190+
}).join(' ') || '';
191+
return isEmpty(paramsString) ? this.selectedScript.name : `${this.selectedScript.name} ${paramsString}`;
192+
}
193+
194+
private parseValue(value: any) {
195+
if (typeof value === 'boolean') {
196+
return undefined;
197+
}
198+
if (value instanceof File) {
199+
return value.name;
200+
}
201+
return value?.toString();
202+
}
203+
}

src/app/process-page/overview/table/process-overview-table.component.spec.ts

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ import {
1010
NgbCollapse,
1111
NgbModal,
1212
} from '@ng-bootstrap/ng-bootstrap';
13-
import { TranslateModule } from '@ngx-translate/core';
13+
import {
14+
TranslateModule,
15+
TranslateService,
16+
} from '@ngx-translate/core';
1417
import { BehaviorSubject } from 'rxjs';
1518
import { take } from 'rxjs/operators';
1619

@@ -50,6 +53,8 @@ describe('ProcessOverviewTableComponent', () => {
5053
let processes: Process[];
5154
let ePerson: EPerson;
5255

56+
let translateServiceSpy: jasmine.SpyObj<TranslateService>;
57+
5358
function init() {
5459
processes = [
5560
Object.assign(new Process(), {
@@ -58,23 +63,28 @@ describe('ProcessOverviewTableComponent', () => {
5863
startTime: '2020-03-19 00:30:00',
5964
endTime: '2020-03-19 23:30:00',
6065
processStatus: ProcessStatus.COMPLETED,
66+
userId: 'testid',
6167
}),
6268
Object.assign(new Process(), {
6369
processId: 2,
6470
scriptName: 'script-b',
6571
startTime: '2020-03-20 00:30:00',
6672
endTime: '2020-03-20 23:30:00',
6773
processStatus: ProcessStatus.FAILED,
74+
userId: 'testid',
6875
}),
6976
Object.assign(new Process(), {
7077
processId: 3,
7178
scriptName: 'script-c',
7279
startTime: '2020-03-21 00:30:00',
7380
endTime: '2020-03-21 23:30:00',
7481
processStatus: ProcessStatus.RUNNING,
82+
userId: 'testid',
7583
}),
7684
];
7785
ePerson = Object.assign(new EPerson(), {
86+
id: 'testid',
87+
uuid: 'testid',
7888
metadata: {
7989
'eperson.firstname': [
8090
{
@@ -133,6 +143,8 @@ describe('ProcessOverviewTableComponent', () => {
133143
beforeEach(waitForAsync(() => {
134144
init();
135145

146+
translateServiceSpy = jasmine.createSpyObj('TranslateService', ['get']);
147+
136148
void TestBed.configureTestingModule({
137149
declarations: [NgbCollapse],
138150
imports: [TranslateModule.forRoot(), RouterTestingModule.withRoutes([]), VarDirective, ProcessOverviewTableComponent],
@@ -217,4 +229,43 @@ describe('ProcessOverviewTableComponent', () => {
217229
});
218230

219231
});
232+
233+
describe('getEPersonName function', () => {
234+
it('should return unknown user when id is null', (done: DoneFn) => {
235+
const id = null;
236+
const expectedTranslation = 'process.overview.unknown.user';
237+
238+
translateServiceSpy.get(expectedTranslation);
239+
240+
component.getEPersonName(id).subscribe((result: string) => {
241+
expect(result).toBe(expectedTranslation);
242+
done();
243+
});
244+
expect(translateServiceSpy.get).toHaveBeenCalledWith('process.overview.unknown.user');
245+
});
246+
247+
it('should return unknown user when id is invalid', (done: DoneFn) => {
248+
const id = '';
249+
const expectedTranslation = 'process.overview.unknown.user';
250+
251+
translateServiceSpy.get(expectedTranslation);
252+
253+
component.getEPersonName(id).subscribe((result: string) => {
254+
expect(result).toBe(expectedTranslation);
255+
done();
256+
});
257+
expect(translateServiceSpy.get).toHaveBeenCalledWith('process.overview.unknown.user');
258+
});
259+
260+
it('should return EPerson name when id is correct', (done: DoneFn) => {
261+
const id = 'testid';
262+
const expectedName = 'John Doe';
263+
264+
component.getEPersonName(id).subscribe((result: string) => {
265+
expect(result).toEqual(expectedName);
266+
done();
267+
});
268+
expect(translateServiceSpy.get).not.toHaveBeenCalled();
269+
});
270+
});
220271
});

src/app/process-page/overview/table/process-overview-table.component.ts

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ import {
1818
RouterLink,
1919
} from '@angular/router';
2020
import { NgbCollapseModule } from '@ng-bootstrap/ng-bootstrap';
21-
import { TranslateModule } from '@ngx-translate/core';
21+
import {
22+
TranslateModule,
23+
TranslateService,
24+
} from '@ngx-translate/core';
2225
import {
2326
BehaviorSubject,
2427
from as observableFrom,
@@ -46,9 +49,12 @@ import { RouteService } from '../../../core/services/route.service';
4649
import { redirectOn4xx } from '../../../core/shared/authorized.operators';
4750
import {
4851
getAllCompletedRemoteData,
49-
getFirstSucceededRemoteDataPayload,
52+
getFirstCompletedRemoteData,
5053
} from '../../../core/shared/operators';
51-
import { hasValue } from '../../../shared/empty.util';
54+
import {
55+
hasValue,
56+
isNotEmpty,
57+
} from '../../../shared/empty.util';
5258
import { ThemedLoadingComponent } from '../../../shared/loading/themed-loading.component';
5359
import { PaginationComponent } from '../../../shared/pagination/pagination.component';
5460
import { PaginationComponentOptions } from '../../../shared/pagination/pagination-component-options.model';
@@ -153,12 +159,13 @@ export class ProcessOverviewTableComponent implements OnInit, OnDestroy {
153159

154160
constructor(protected processOverviewService: ProcessOverviewService,
155161
protected processBulkDeleteService: ProcessBulkDeleteService,
156-
protected ePersonDataService: EPersonDataService,
157-
protected dsoNameService: DSONameService,
162+
public ePersonDataService: EPersonDataService,
163+
public dsoNameService: DSONameService,
158164
protected paginationService: PaginationService,
159165
protected routeService: RouteService,
160166
protected router: Router,
161167
protected auth: AuthService,
168+
private translateService: TranslateService,
162169
@Inject(PLATFORM_ID) protected platformId: object,
163170
) {
164171
}
@@ -176,7 +183,7 @@ export class ProcessOverviewTableComponent implements OnInit, OnDestroy {
176183
// Creates an ID from the first 2 characters of the process status.
177184
// Should two process status values ever start with the same substring,
178185
// increase the number of characters until the ids are distinct.
179-
this.paginationId = this.processStatus.toLowerCase().substring(0,2);
186+
this.paginationId = this.processStatus.toLowerCase().substring(0, 2);
180187

181188
const defaultPaginationOptions = Object.assign(new PaginationComponentOptions(), {
182189
id: this.paginationId,
@@ -218,7 +225,7 @@ export class ProcessOverviewTableComponent implements OnInit, OnDestroy {
218225
// Map RemoteData<PaginatedList<Process>> to RemoteData<PaginatedList<ProcessOverviewTableEntry>>
219226
switchMap((processesRD: RemoteData<PaginatedList<Process>>) => {
220227
// Create observable emitting all processes one by one
221-
return observableFrom(processesRD.payload.page).pipe(
228+
return observableFrom(processesRD.payload.page).pipe(
222229
// Map every Process to ProcessOverviewTableEntry
223230
mergeMap((process: Process) => {
224231
return this.getEPersonName(process.userId).pipe(
@@ -243,7 +250,6 @@ export class ProcessOverviewTableComponent implements OnInit, OnDestroy {
243250
}),
244251
);
245252
}),
246-
247253
).subscribe((next: RemoteData<PaginatedList<ProcessOverviewTableEntry>>) => {
248254
this.processesRD$.next(next);
249255
}));
@@ -267,10 +273,20 @@ export class ProcessOverviewTableComponent implements OnInit, OnDestroy {
267273
* @param id ID of the EPerson
268274
*/
269275
getEPersonName(id: string): Observable<string> {
270-
return this.ePersonDataService.findById(id).pipe(
271-
getFirstSucceededRemoteDataPayload(),
272-
map((eperson: EPerson) => this.dsoNameService.getName(eperson)),
273-
);
276+
if (isNotEmpty(id)) {
277+
return this.ePersonDataService.findById(id).pipe(
278+
getFirstCompletedRemoteData(),
279+
switchMap((rd: RemoteData<EPerson>) => {
280+
if (rd.hasSucceeded) {
281+
return [this.dsoNameService.getName(rd.payload)];
282+
} else {
283+
return this.translateService.get('process.overview.unknown.user');
284+
}
285+
}),
286+
);
287+
} else {
288+
return this.translateService.get('process.overview.unknown.user');
289+
}
274290
}
275291

276292
/**

src/assets/i18n/en.json5

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3846,6 +3846,8 @@
38463846

38473847
"process.overview.delete.header": "Delete processes",
38483848

3849+
"process.overview.unknown.user": "Unknown",
3850+
38493851
"process.bulk.delete.error.head": "Error on deleteing process",
38503852

38513853
"process.bulk.delete.error.body": "The process with ID {{processId}} could not be deleted. The remaining processes will continue being deleted. ",

0 commit comments

Comments
 (0)