Skip to content

Commit aea39ef

Browse files
alexandrevryghemalexandrevryghem
committed
Merge branch 'w2p-115427_fixed-delete-item-page-freezing_contribute-7.6' into w2p-115427_fixed-delete-item-page-freezing_contribute-main
 Conflicts: src/app/item-page/edit-item-page/item-delete/item-delete.component.ts src/app/item-page/full/full-item-page.component.spec.ts src/app/item-page/full/full-item-page.component.ts src/app/item-page/item-page.resolver.spec.ts src/app/item-page/item-page.resolver.ts src/app/item-page/simple/item-page.component.spec.ts src/app/item-page/simple/item-page.component.ts
2 parents 659052f + 249cac4 commit aea39ef

8 files changed

Lines changed: 99 additions & 73 deletions

‎src/app/item-page/edit-item-page/item-delete/item-delete.component.html‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,30 +6,29 @@ <h1>{{headerMessage | translate: {id: item.handle} }}</h1>
66
<p>{{descriptionMessage | translate}}</p>
77
<ds-modify-item-overview [item]="item"></ds-modify-item-overview>
88

9-
<ng-container *ngVar="(types$ | async) as types">
9+
<ng-container *ngVar="(typeDTOs$ | async) as types">
1010

1111
<div *ngIf="types && types.length > 0" class="mb-4">
1212

1313
{{'virtual-metadata.delete-item.info' | translate}}
1414

15-
<div *ngFor="let type of types" class="mb-4">
16-
17-
<div *ngVar="(isSelected(type) | async) as selected"
15+
<div *ngFor="let typeDto of types" class="mb-4">
16+
<div *ngVar="(typeDto.isSelected$ | async) as selected"
1817
class="d-flex flex-row">
1918

20-
<div class="m-2" (click)="setSelected(type, !selected)">
19+
<div class="m-2" (click)="setSelected(typeDto.relationshipType, !selected)">
2120
<label>
22-
<input type="checkbox" [checked]="selected">
21+
<input type="checkbox" [checked]="selected" [disabled]="isDeleting$ | async">
2322
</label>
2423
</div>
2524

2625
<div class="flex-column flex-grow-1">
27-
<h5 (click)="setSelected(type, !selected)">
28-
{{getRelationshipMessageKey(getLabel(type) | async) | translate}}
26+
<h5 (click)="setSelected(typeDto.relationshipType, !selected)">
27+
{{getRelationshipMessageKey(typeDto.label$ | async) | translate}}
2928
</h5>
30-
<div *ngFor="let relationship of (getRelationships(type) | async)"
29+
<div *ngFor="let relationshipDto of (typeDto.relationshipDTOs$ | async)"
3130
class="d-flex flex-row">
32-
<ng-container *ngVar="(getRelatedItem(relationship) | async) as relatedItem">
31+
<ng-container *ngVar="(relationshipDto.relatedItem$ | async) as relatedItem">
3332

3433
<ds-listable-object-component-loader
3534
*ngIf="relatedItem"
@@ -46,7 +45,7 @@ <h5 (click)="setSelected(type, !selected)">
4645
</div>
4746

4847
<ng-template #virtualMetadataModal>
49-
<div>
48+
<div class="thumb-font-1">
5049
<div class="modal-header">
5150
{{'virtual-metadata.delete-item.modal-head' | translate}}
5251
<button type="button" class="close"
@@ -60,7 +59,7 @@ <h5 (click)="setSelected(type, !selected)">
6059
[object]="relatedItem"
6160
[viewMode]="viewMode">
6261
</ds-listable-object-component-loader>
63-
<div *ngFor="let metadata of (getVirtualMetadata(relationship) | async)">
62+
<div *ngFor="let metadata of (relationshipDto.virtualMetadata$ | async)">
6463
<div>
6564
<div class="font-weight-bold">
6665
{{metadata.metadataField}}
@@ -87,10 +86,11 @@ <h5 (click)="setSelected(type, !selected)">
8786
</ng-container>
8887

8988
<div class="space-children-mr">
90-
<button (click)="performAction()"
89+
<button [disabled]="isDeleting$ | async" (click)="performAction()"
9190
class="btn btn-outline-secondary perform-action">{{confirmMessage | translate}}
9291
</button>
93-
<button [routerLink]="[itemPageRoute, 'edit']" class="btn btn-outline-secondary cancel">
92+
<button [disabled]="isDeleting$ | async" [routerLink]="[itemPageRoute, 'edit']"
93+
class="btn btn-outline-secondary cancel">
9494
{{cancelMessage| translate}}
9595
</button>
9696
</div>

‎src/app/item-page/edit-item-page/item-delete/item-delete.component.ts‎

Lines changed: 65 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// eslint-disable-next-line max-classes-per-file
12
import {
23
AsyncPipe,
34
NgForOf,
@@ -68,6 +69,34 @@ import { ModifyItemOverviewComponent } from '../modify-item-overview/modify-item
6869
import { AbstractSimpleItemActionComponent } from '../simple-item-action/abstract-simple-item-action.component';
6970
import { VirtualMetadata } from '../virtual-metadata/virtual-metadata.component';
7071

72+
/**
73+
* Data Transfer Object used to prevent the HTML template to call function returning Observables
74+
*/
75+
class RelationshipTypeDTO {
76+
77+
relationshipType: RelationshipType;
78+
79+
isSelected$: Observable<boolean>;
80+
81+
label$: Observable<string>;
82+
83+
relationshipDTOs$: Observable<RelationshipDTO[]>;
84+
85+
}
86+
87+
/**
88+
* Data Transfer Object used to prevent the HTML template to call function returning Observables
89+
*/
90+
class RelationshipDTO {
91+
92+
relationship: Relationship;
93+
94+
relatedItem$: Observable<Item>;
95+
96+
virtualMetadata$: Observable<VirtualMetadata[]>;
97+
98+
}
99+
71100
@Component({
72101
selector: 'ds-item-delete',
73102
templateUrl: '../item-delete/item-delete.component.html',
@@ -106,7 +135,7 @@ export class ItemDeleteComponent
106135
* A list of the relationship types for which this item has relations as an observable.
107136
* The list doesn't contain duplicates.
108137
*/
109-
types$: BehaviorSubject<RelationshipType[]> = new BehaviorSubject([]);
138+
typeDTOs$: BehaviorSubject<RelationshipTypeDTO[]> = new BehaviorSubject([]);
110139

111140
/**
112141
* A map which stores the relationships of this item for each type as observable lists
@@ -135,6 +164,8 @@ export class ItemDeleteComponent
135164
*/
136165
private subs: Subscription[] = [];
137166

167+
public isDeleting$: BehaviorSubject<boolean> = new BehaviorSubject(false);
168+
138169
constructor(protected route: ActivatedRoute,
139170
protected router: Router,
140171
protected notificationsService: NotificationsService,
@@ -189,13 +220,24 @@ export class ItemDeleteComponent
189220
),
190221
);
191222
}),
192-
).subscribe((types: RelationshipType[]) => this.types$.next(types)));
223+
).subscribe((types: RelationshipType[]) => this.typeDTOs$.next(types.map((relationshipType: RelationshipType) => Object.assign(new RelationshipTypeDTO(), {
224+
relationshipType: relationshipType,
225+
isSelected$: this.isSelected(relationshipType),
226+
label$: this.getLabel(relationshipType),
227+
relationshipDTOs$: this.getRelationships(relationshipType).pipe(
228+
map((relationships: Relationship[]) => relationships.map((relationship: Relationship) => Object.assign(new RelationshipDTO(), {
229+
relationship: relationship,
230+
relatedItem$: this.getRelatedItem(relationship),
231+
virtualMetadata$: this.getVirtualMetadata(relationship),
232+
} as RelationshipDTO))),
233+
),
234+
})))));
193235
}
194236

195-
this.subs.push(this.types$.pipe(
237+
this.subs.push(this.typeDTOs$.pipe(
196238
take(1),
197-
).subscribe((types) =>
198-
this.objectUpdatesService.initialize(this.url, types, this.item.lastModified),
239+
).subscribe((types: RelationshipTypeDTO[]) =>
240+
this.objectUpdatesService.initialize(this.url, types.map((relationshipTypeDto: RelationshipTypeDTO) => relationshipTypeDto.relationshipType), this.item.lastModified),
199241
));
200242
}
201243

@@ -368,34 +410,33 @@ export class ItemDeleteComponent
368410
* @param selected whether the type should be selected
369411
*/
370412
setSelected(type: RelationshipType, selected: boolean): void {
371-
this.objectUpdatesService.setSelectedVirtualMetadata(this.url, this.item.uuid, type.uuid, selected);
413+
if (this.isDeleting$.value === false) {
414+
this.objectUpdatesService.setSelectedVirtualMetadata(this.url, this.item.uuid, type.uuid, selected);
415+
}
372416
}
373417

374418
/**
375419
* Perform the delete operation
376420
*/
377-
performAction() {
378-
379-
this.subs.push(this.types$.pipe(
380-
switchMap((types) =>
421+
performAction(): void {
422+
this.isDeleting$.next(true);
423+
this.subs.push(this.typeDTOs$.pipe(
424+
switchMap((types: RelationshipTypeDTO[]) =>
381425
combineLatest(
382-
types.map((type) => this.isSelected(type)),
426+
types.map((type: RelationshipTypeDTO) => type.isSelected$),
383427
).pipe(
384428
defaultIfEmpty([]),
385-
map((selection) => types.filter(
386-
(type, index) => selection[index],
429+
map((selection: boolean[]) => types.filter(
430+
(type: RelationshipTypeDTO, index: number) => selection[index],
387431
)),
388-
map((selectedTypes) => selectedTypes.map((type) => type.id)),
432+
map((selectedDtoTypes: RelationshipTypeDTO[]) => selectedDtoTypes.map((typeDto: RelationshipTypeDTO) => typeDto.relationshipType.id)),
389433
),
390434
),
391-
switchMap((types) =>
392-
this.itemDataService.delete(this.item.id, types).pipe(getFirstCompletedRemoteData()),
393-
),
394-
).subscribe(
395-
(rd: RemoteData<NoContent>) => {
396-
this.notify(rd.hasSucceeded);
397-
},
398-
));
435+
switchMap((types: string[]) => this.itemDataService.delete(this.item.id, types)),
436+
getFirstCompletedRemoteData(),
437+
).subscribe((rd: RemoteData<NoContent>) => {
438+
this.notify(rd.hasSucceeded);
439+
}));
399440
}
400441

401442
/**
@@ -405,10 +446,10 @@ export class ItemDeleteComponent
405446
notify(succeeded: boolean) {
406447
if (succeeded) {
407448
this.notificationsService.success(this.translateService.get('item.edit.' + this.messageKey + '.success'));
408-
this.router.navigate(['']);
449+
void this.router.navigate(['']);
409450
} else {
410451
this.notificationsService.error(this.translateService.get('item.edit.' + this.messageKey + '.error'));
411-
this.router.navigate([getItemEditRoute(this.item)]);
452+
void this.router.navigate([getItemEditRoute(this.item)]);
412453
}
413454
}
414455

‎src/app/item-page/full/full-item-page.component.spec.ts‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import {
2222
of as observableOf,
2323
} from 'rxjs';
2424

25-
import { AuthService } from '../../core/auth/auth.service';
2625
import { NotifyInfoService } from '../../core/coar-notify/notify-info/notify-info.service';
2726
import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
2827
import { ItemDataService } from '../../core/data/item-data.service';
@@ -79,7 +78,6 @@ describe('FullItemPageComponent', () => {
7978
let comp: FullItemPageComponent;
8079
let fixture: ComponentFixture<FullItemPageComponent>;
8180

82-
let authService: AuthService;
8381
let routeStub: ActivatedRouteStub;
8482
let routeData;
8583
let authorizationDataService: AuthorizationDataService;
@@ -102,11 +100,6 @@ describe('FullItemPageComponent', () => {
102100
};
103101

104102
beforeEach(waitForAsync(() => {
105-
authService = jasmine.createSpyObj('authService', {
106-
isAuthenticated: observableOf(true),
107-
setRedirectUrl: {},
108-
});
109-
110103
routeData = {
111104
dso: createSuccessfulRemoteDataObject(mockItem),
112105
};
@@ -151,7 +144,6 @@ describe('FullItemPageComponent', () => {
151144
{ provide: ActivatedRoute, useValue: routeStub },
152145
{ provide: ItemDataService, useValue: {} },
153146
{ provide: HeadTagService, useValue: headTagService },
154-
{ provide: AuthService, useValue: authService },
155147
{ provide: AuthorizationDataService, useValue: authorizationDataService },
156148
{ provide: ServerResponseService, useValue: serverResponseService },
157149
{ provide: SignpostingDataService, useValue: signpostingDataService },

‎src/app/item-page/full/full-item-page.component.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import {
2929
map,
3030
} from 'rxjs/operators';
3131

32-
import { AuthService } from '../../core/auth/auth.service';
3332
import { NotifyInfoService } from '../../core/coar-notify/notify-info/notify-info.service';
3433
import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
3534
import { ItemDataService } from '../../core/data/item-data.service';
@@ -103,7 +102,6 @@ export class FullItemPageComponent extends ItemPageComponent implements OnInit,
103102
protected route: ActivatedRoute,
104103
protected router: Router,
105104
protected items: ItemDataService,
106-
protected authService: AuthService,
107105
protected authorizationService: AuthorizationDataService,
108106
protected _location: Location,
109107
protected responseService: ServerResponseService,
@@ -112,7 +110,7 @@ export class FullItemPageComponent extends ItemPageComponent implements OnInit,
112110
protected notifyInfoService: NotifyInfoService,
113111
@Inject(PLATFORM_ID) protected platformId: string,
114112
) {
115-
super(route, router, items, authService, authorizationService, responseService, signpostingDataService, linkHeadService, notifyInfoService, platformId);
113+
super(route, router, items, authorizationService, responseService, signpostingDataService, linkHeadService, notifyInfoService, platformId);
116114
}
117115

118116
/*** AoT inheritance fix, will hopefully be resolved in the near future **/

‎src/app/item-page/item-page.resolver.spec.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,20 @@
11
import { TestBed } from '@angular/core/testing';
2-
import { Router } from '@angular/router';
3-
import { RouterTestingModule } from '@angular/router/testing';
2+
import {
3+
Router,
4+
RouterModule,
5+
} from '@angular/router';
46
import { first } from 'rxjs/operators';
57

68
import { DSpaceObject } from '../core/shared/dspace-object.model';
79
import { MetadataValueFilter } from '../core/shared/metadata.models';
810
import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils';
11+
import { AuthServiceStub } from '../shared/testing/auth-service.stub';
912
import { itemPageResolver } from './item-page.resolver';
1013

1114
describe('itemPageResolver', () => {
1215
beforeEach(() => {
1316
TestBed.configureTestingModule({
14-
imports: [RouterTestingModule.withRoutes([{
17+
imports: [RouterModule.forRoot([{
1518
path: 'entities/:entity-type/:id',
1619
component: {} as any,
1720
}])],
@@ -22,7 +25,8 @@ describe('itemPageResolver', () => {
2225
let resolver: any;
2326
let itemService: any;
2427
let store: any;
25-
let router: any;
28+
let router: Router;
29+
let authService: AuthServiceStub;
2630

2731
const uuid = '1234-65487-12354-1235';
2832
let item: DSpaceObject;
@@ -42,6 +46,7 @@ describe('itemPageResolver', () => {
4246
store = jasmine.createSpyObj('store', {
4347
dispatch: {},
4448
});
49+
authService = new AuthServiceStub();
4550
resolver = itemPageResolver;
4651
});
4752

@@ -54,6 +59,7 @@ describe('itemPageResolver', () => {
5459
router,
5560
itemService,
5661
store,
62+
authService,
5763
).pipe(first())
5864
.subscribe(
5965
() => {
@@ -73,6 +79,7 @@ describe('itemPageResolver', () => {
7379
router,
7480
itemService,
7581
store,
82+
authService,
7683
).pipe(first())
7784
.subscribe(
7885
() => {

‎src/app/item-page/item-page.resolver.ts‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@ import { Observable } from 'rxjs';
1010
import { map } from 'rxjs/operators';
1111

1212
import { AppState } from '../app.reducer';
13+
import { AuthService } from '../core/auth/auth.service';
1314
import { ItemDataService } from '../core/data/item-data.service';
1415
import { RemoteData } from '../core/data/remote-data';
1516
import { ResolvedAction } from '../core/resolving/resolver.actions';
17+
import { redirectOn4xx } from '../core/shared/authorized.operators';
1618
import { Item } from '../core/shared/item.model';
1719
import { getFirstCompletedRemoteData } from '../core/shared/operators';
1820
import { hasValue } from '../shared/empty.util';
@@ -26,6 +28,7 @@ import { getItemPageRoute } from './item-page-routing-paths';
2628
* @param {Router} router
2729
* @param {ItemDataService} itemService
2830
* @param {Store<AppState>} store
31+
* @param {AuthService} authService
2932
* @returns Observable<<RemoteData<Item>> Emits the found item based on the parameters in the current route,
3033
* or an error if something went wrong
3134
*/
@@ -35,22 +38,18 @@ export const itemPageResolver: ResolveFn<RemoteData<Item>> = (
3538
router: Router = inject(Router),
3639
itemService: ItemDataService = inject(ItemDataService),
3740
store: Store<AppState> = inject(Store<AppState>),
41+
authService: AuthService = inject(AuthService),
3842
): Observable<RemoteData<Item>> => {
39-
const itemRD$ = itemService.findById(
43+
return itemService.findById(
4044
route.params.id,
4145
true,
4246
false,
4347
...ITEM_PAGE_LINKS_TO_FOLLOW,
4448
).pipe(
4549
getFirstCompletedRemoteData(),
46-
);
47-
48-
itemRD$.subscribe((itemRD: RemoteData<Item>) => {
49-
store.dispatch(new ResolvedAction(state.url, itemRD.payload));
50-
});
51-
52-
return itemRD$.pipe(
50+
redirectOn4xx(router, authService),
5351
map((rd: RemoteData<Item>) => {
52+
store.dispatch(new ResolvedAction(state.url, rd.payload));
5453
if (rd.hasSucceeded && hasValue(rd.payload)) {
5554
const thisRoute = state.url;
5655

@@ -63,7 +62,7 @@ export const itemPageResolver: ResolveFn<RemoteData<Item>> = (
6362
if (!thisRoute.startsWith(itemRoute)) {
6463
const itemId = rd.payload.uuid;
6564
const subRoute = thisRoute.substring(thisRoute.indexOf(itemId) + itemId.length, thisRoute.length);
66-
router.navigateByUrl(itemRoute + subRoute);
65+
void router.navigateByUrl(itemRoute + subRoute);
6766
}
6867
}
6968
return rd;

0 commit comments

Comments
 (0)