Skip to content

Commit f7a14a1

Browse files
authored
Merge pull request DSpace#3106 from alexandrevryghem/w2p-113560_edit-item-add-relationships-one-by-one_contribute-main
Fixed caching & same entity type relationship with different left/rightwardtype bugs on edit item relationships
2 parents 1de8423 + 556e04d commit f7a14a1

23 files changed

Lines changed: 566 additions & 59 deletions

src/app/core/data/relationship-data.service.spec.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ describe('RelationshipDataService', () => {
128128
const itemService = jasmine.createSpyObj('itemService', {
129129
findById: (uuid) => createSuccessfulRemoteDataObject(relatedItems.find((relatedItem) => relatedItem.id === uuid)),
130130
findByHref: createSuccessfulRemoteDataObject$(relatedItems[0]),
131+
getIDHrefObs: (uuid: string) => observableOf(`https://demo.dspace.org/server/api/core/items/${uuid}`),
131132
});
132133

133134
const getRequestEntry$ = (successful: boolean) => {
@@ -244,6 +245,16 @@ describe('RelationshipDataService', () => {
244245
});
245246
});
246247

248+
describe('searchByItemsAndType', () => {
249+
it('should call addDependency for each item to invalidate the request when one of the items is update', () => {
250+
spyOn(service as any, 'addDependency');
251+
252+
service.searchByItemsAndType(relationshipType.id, item.id, relationshipType.leftwardType, ['item-id-1', 'item-id-2']);
253+
254+
expect((service as any).addDependency).toHaveBeenCalledTimes(2);
255+
});
256+
});
257+
247258
describe('resolveMetadataRepresentation', () => {
248259
const parentItem: Item = Object.assign(new Item(), {
249260
id: 'parent-item',

src/app/core/data/relationship-data.service.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,13 +574,18 @@ export class RelationshipDataService extends IdentifiableDataService<Relationshi
574574
);
575575
});
576576

577-
return this.searchBy(
577+
const searchRD$: Observable<RemoteData<PaginatedList<Relationship>>> = this.searchBy(
578578
'byItemsAndType',
579579
{
580580
searchParams: searchParams,
581581
},
582582
) as Observable<RemoteData<PaginatedList<Relationship>>>;
583583

584+
arrayOfItemIds.forEach((itemId: string) => {
585+
this.addDependency(searchRD$, this.itemService.getIDHrefObs(encodeURIComponent(itemId)));
586+
});
587+
588+
return searchRD$;
584589
}
585590

586591
/**

src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.spec.ts

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ describe('EditItemRelationshipsService', () => {
185185

186186
expect(itemService.invalidateByHref).toHaveBeenCalledWith(currentItem.self);
187187
expect(itemService.invalidateByHref).toHaveBeenCalledWith(relationshipItem1.self);
188+
expect(itemService.invalidateByHref).toHaveBeenCalledWith(relationshipItem2.self);
188189

189190
expect(notificationsService.success).toHaveBeenCalledTimes(1);
190191
});
@@ -265,6 +266,116 @@ describe('EditItemRelationshipsService', () => {
265266
});
266267
});
267268

269+
describe('isProvidedItemTypeLeftType', () => {
270+
it('should return true if the provided item corresponds to the left type of the relationship', (done) => {
271+
const relationshipType = Object.assign(new RelationshipType(), {
272+
leftType: createSuccessfulRemoteDataObject$({ id: 'leftType' }),
273+
rightType: createSuccessfulRemoteDataObject$({ id: 'rightType' }),
274+
});
275+
const itemType = Object.assign(new ItemType(), { id: 'leftType' } );
276+
const item = Object.assign(new Item(), { uuid: 'item-uuid' });
277+
278+
const result = service.isProvidedItemTypeLeftType(relationshipType, itemType, item);
279+
result.subscribe((resultValue) => {
280+
expect(resultValue).toBeTrue();
281+
done();
282+
});
283+
});
284+
285+
it('should return false if the provided item corresponds to the right type of the relationship', (done) => {
286+
const relationshipType = Object.assign(new RelationshipType(), {
287+
leftType: createSuccessfulRemoteDataObject$({ id: 'leftType' }),
288+
rightType: createSuccessfulRemoteDataObject$({ id: 'rightType' }),
289+
});
290+
const itemType = Object.assign(new ItemType(), { id: 'rightType' } );
291+
const item = Object.assign(new Item(), { uuid: 'item-uuid' });
292+
293+
const result = service.isProvidedItemTypeLeftType(relationshipType, itemType, item);
294+
result.subscribe((resultValue) => {
295+
expect(resultValue).toBeFalse();
296+
done();
297+
});
298+
});
299+
300+
it('should return undefined if the provided item corresponds does not match any of the relationship types', (done) => {
301+
const relationshipType = Object.assign(new RelationshipType(), {
302+
leftType: createSuccessfulRemoteDataObject$({ id: 'leftType' }),
303+
rightType: createSuccessfulRemoteDataObject$({ id: 'rightType' }),
304+
});
305+
const itemType = Object.assign(new ItemType(), { id: 'something-else' } );
306+
const item = Object.assign(new Item(), { uuid: 'item-uuid' });
307+
308+
const result = service.isProvidedItemTypeLeftType(relationshipType, itemType, item);
309+
result.subscribe((resultValue) => {
310+
expect(resultValue).toBeUndefined();
311+
done();
312+
});
313+
});
314+
});
315+
316+
describe('relationshipMatchesBothSameTypes', () => {
317+
it('should return true if both left and right type of the relationship type are the same and match the provided itemtype', (done) => {
318+
const relationshipType = Object.assign(new RelationshipType(), {
319+
leftType: createSuccessfulRemoteDataObject$({ id: 'sameType' }),
320+
rightType: createSuccessfulRemoteDataObject$({ id:'sameType' }),
321+
leftwardType: 'isDepartmentOfDivision',
322+
rightwardType: 'isDivisionOfDepartment',
323+
});
324+
const itemType = Object.assign(new ItemType(), { id: 'sameType' } );
325+
326+
const result = service.shouldDisplayBothRelationshipSides(relationshipType, itemType);
327+
result.subscribe((resultValue) => {
328+
expect(resultValue).toBeTrue();
329+
done();
330+
});
331+
});
332+
it('should return false if both left and right type of the relationship type are the same and match the provided itemtype but the leftwardType & rightwardType is identical', (done) => {
333+
const relationshipType = Object.assign(new RelationshipType(), {
334+
leftType: createSuccessfulRemoteDataObject$({ id: 'sameType' }),
335+
rightType: createSuccessfulRemoteDataObject$({ id: 'sameType' }),
336+
leftwardType: 'isOrgUnitOfOrgUnit',
337+
rightwardType: 'isOrgUnitOfOrgUnit',
338+
});
339+
const itemType = Object.assign(new ItemType(), { id: 'sameType' });
340+
341+
const result = service.shouldDisplayBothRelationshipSides(relationshipType, itemType);
342+
result.subscribe((resultValue) => {
343+
expect(resultValue).toBeFalse();
344+
done();
345+
});
346+
});
347+
it('should return false if both left and right type of the relationship type are the same and do not match the provided itemtype', (done) => {
348+
const relationshipType = Object.assign(new RelationshipType(), {
349+
leftType: createSuccessfulRemoteDataObject$({ id: 'sameType' }),
350+
rightType: createSuccessfulRemoteDataObject$({ id: 'sameType' }),
351+
leftwardType: 'isDepartmentOfDivision',
352+
rightwardType: 'isDivisionOfDepartment',
353+
});
354+
const itemType = Object.assign(new ItemType(), { id: 'something-else' } );
355+
356+
const result = service.shouldDisplayBothRelationshipSides(relationshipType, itemType);
357+
result.subscribe((resultValue) => {
358+
expect(resultValue).toBeFalse();
359+
done();
360+
});
361+
});
362+
it('should return false if both left and right type of the relationship type are different', (done) => {
363+
const relationshipType = Object.assign(new RelationshipType(), {
364+
leftType: createSuccessfulRemoteDataObject$({ id: 'leftType' }),
365+
rightType: createSuccessfulRemoteDataObject$({ id: 'rightType' }),
366+
leftwardType: 'isAuthorOfPublication',
367+
rightwardType: 'isPublicationOfAuthor',
368+
});
369+
const itemType = Object.assign(new ItemType(), { id: 'leftType' } );
370+
371+
const result = service.shouldDisplayBothRelationshipSides(relationshipType, itemType);
372+
result.subscribe((resultValue) => {
373+
expect(resultValue).toBeFalse();
374+
done();
375+
});
376+
});
377+
});
378+
268379
describe('displayNotifications', () => {
269380
it('should show one success notification when multiple requests succeeded', () => {
270381
service.displayNotifications([

src/app/item-page/edit-item-page/item-relationships/edit-item-relationships.service.ts

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { NgbModal } from '@ng-bootstrap/ng-bootstrap';
33
import { TranslateService } from '@ngx-translate/core';
44
import {
55
BehaviorSubject,
6+
combineLatest as observableCombineLatest,
67
EMPTY,
78
Observable,
89
Subscription,
@@ -28,8 +29,14 @@ import { ObjectUpdatesService } from '../../../core/data/object-updates/object-u
2829
import { RelationshipDataService } from '../../../core/data/relationship-data.service';
2930
import { RemoteData } from '../../../core/data/remote-data';
3031
import { Item } from '../../../core/shared/item.model';
32+
import { ItemType } from '../../../core/shared/item-relationships/item-type.model';
3133
import { Relationship } from '../../../core/shared/item-relationships/relationship.model';
34+
import { RelationshipType } from '../../../core/shared/item-relationships/relationship-type.model';
3235
import { NoContent } from '../../../core/shared/NoContent.model';
36+
import {
37+
getFirstSucceededRemoteData,
38+
getRemoteDataPayload,
39+
} from '../../../core/shared/operators';
3340
import { hasValue } from '../../../shared/empty.util';
3441
import { NotificationsService } from '../../../shared/notifications/notifications.service';
3542

@@ -70,7 +77,17 @@ export class EditItemRelationshipsService {
7077
// process each update one by one, while waiting for the previous to finish
7178
concatMap((update: FieldUpdate) => {
7279
if (update.changeType === FieldChangeType.REMOVE) {
73-
return this.deleteRelationship(update.field as DeleteRelationship).pipe(take(1));
80+
return this.deleteRelationship(update.field as DeleteRelationship).pipe(
81+
take(1),
82+
switchMap((deleteRD: RemoteData<NoContent>) => {
83+
if (deleteRD.hasSucceeded) {
84+
return this.itemService.invalidateByHref((update.field as DeleteRelationship).relatedItem._links.self.href).pipe(
85+
map(() => deleteRD),
86+
);
87+
}
88+
return [deleteRD];
89+
}),
90+
);
7491
} else if (update.changeType === FieldChangeType.ADD) {
7592
return this.addRelationship(update.field as RelationshipIdentifiable).pipe(
7693
take(1),
@@ -181,6 +198,55 @@ export class EditItemRelationshipsService {
181198
}
182199
}
183200

201+
isProvidedItemTypeLeftType(relationshipType: RelationshipType, itemType: ItemType, item: Item): Observable<boolean> {
202+
return this.getRelationshipLeftAndRightType(relationshipType).pipe(
203+
map(([leftType, rightType]: [ItemType, ItemType]) => {
204+
if (leftType.id === itemType.id) {
205+
return true;
206+
}
207+
208+
if (rightType.id === itemType.id) {
209+
return false;
210+
}
211+
212+
// should never happen...
213+
console.warn(`The item ${item.uuid} is not on the right or the left side of relationship type ${relationshipType.uuid}`);
214+
return undefined;
215+
}),
216+
);
217+
}
218+
219+
/**
220+
* Whether both side of the relationship need to be displayed on the edit relationship page or not.
221+
*
222+
* @param relationshipType The relationship type
223+
* @param itemType The item type
224+
*/
225+
shouldDisplayBothRelationshipSides(relationshipType: RelationshipType, itemType: ItemType): Observable<boolean> {
226+
return this.getRelationshipLeftAndRightType(relationshipType).pipe(
227+
map(([leftType, rightType]: [ItemType, ItemType]) => {
228+
return leftType.id === itemType.id && rightType.id === itemType.id && relationshipType.leftwardType !== relationshipType.rightwardType;
229+
}),
230+
);
231+
}
232+
233+
protected getRelationshipLeftAndRightType(relationshipType: RelationshipType): Observable<[ItemType, ItemType]> {
234+
const leftType$: Observable<ItemType> = relationshipType.leftType.pipe(
235+
getFirstSucceededRemoteData(),
236+
getRemoteDataPayload(),
237+
);
238+
239+
const rightType$: Observable<ItemType> = relationshipType.rightType.pipe(
240+
getFirstSucceededRemoteData(),
241+
getRemoteDataPayload(),
242+
);
243+
244+
return observableCombineLatest([
245+
leftType$,
246+
rightType$,
247+
]);
248+
}
249+
184250

185251

186252
/**
@@ -197,6 +263,5 @@ export class EditItemRelationshipsService {
197263
*/
198264
getNotificationContent(key: string): string {
199265
return this.translateService.instant(this.notificationsPrefix + key + '.content');
200-
201266
}
202267
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<ng-container *ngIf="shouldDisplayBothRelationshipSides$ | async">
2+
<ds-edit-relationship-list
3+
[url]="url"
4+
[item]="item"
5+
[itemType]="itemType"
6+
[relationshipType]="relationshipType"
7+
[hasChanges]="hasChanges"
8+
[currentItemIsLeftItem$]="isLeftItem$"
9+
class="d-block mb-4"
10+
></ds-edit-relationship-list>
11+
<ds-edit-relationship-list
12+
[url]="url"
13+
[item]="item"
14+
[itemType]="itemType"
15+
[relationshipType]="relationshipType"
16+
[hasChanges]="hasChanges"
17+
[currentItemIsLeftItem$]="isRightItem$"
18+
></ds-edit-relationship-list>
19+
</ng-container>
20+
21+
<ng-container *ngIf="(shouldDisplayBothRelationshipSides$ | async) === false">
22+
<ds-edit-relationship-list
23+
[url]="url"
24+
[item]="item"
25+
[itemType]="itemType"
26+
[relationshipType]="relationshipType"
27+
[hasChanges]="hasChanges"
28+
[currentItemIsLeftItem$]="currentItemIsLeftItem$"
29+
></ds-edit-relationship-list>
30+
</ng-container>
31+

src/app/item-page/edit-item-page/item-relationships/edit-relationship-list-wrapper/edit-relationship-list-wrapper.component.scss

Whitespace-only changes.

0 commit comments

Comments
 (0)