Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Port dspace-7_x] Fixed caching & same entity type relationship with different left/rightwardtype bugs on edit item relationships #3109

13 changes: 12 additions & 1 deletion src/app/core/data/relationship-data.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ describe('RelationshipDataService', () => {

const itemService = jasmine.createSpyObj('itemService', {
findById: (uuid) => createSuccessfulRemoteDataObject(relatedItems.find((relatedItem) => relatedItem.id === uuid)),
findByHref: createSuccessfulRemoteDataObject$(relatedItems[0])
findByHref: createSuccessfulRemoteDataObject$(relatedItems[0]),
getIDHrefObs: (uuid: string) => observableOf(`https://demo.dspace.org/server/api/core/items/${uuid}`),
});

function initTestService() {
Expand Down Expand Up @@ -240,6 +241,16 @@ describe('RelationshipDataService', () => {
});
});

describe('searchByItemsAndType', () => {
it('should call addDependency for each item to invalidate the request when one of the items is update', () => {
spyOn(service as any, 'addDependency');

service.searchByItemsAndType(relationshipType.id, item.id, relationshipType.leftwardType, ['item-id-1', 'item-id-2']);

expect((service as any).addDependency).toHaveBeenCalledTimes(2);
});
});

describe('resolveMetadataRepresentation', () => {
const parentItem: Item = Object.assign(new Item(), {
id: 'parent-item',
Expand Down
7 changes: 6 additions & 1 deletion src/app/core/data/relationship-data.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -533,13 +533,18 @@ export class RelationshipDataService extends IdentifiableDataService<Relationshi
);
});

return this.searchBy(
const searchRD$: Observable<RemoteData<PaginatedList<Relationship>>> = this.searchBy(
'byItemsAndType',
{
searchParams: searchParams
},
) as Observable<RemoteData<PaginatedList<Relationship>>>;

arrayOfItemIds.forEach((itemId: string) => {
this.addDependency(searchRD$, this.itemService.getIDHrefObs(encodeURIComponent(itemId)));
});

return searchRD$;
}

/**
Expand Down
4 changes: 4 additions & 0 deletions src/app/item-page/edit-item-page/edit-item-page.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ import { ResultsBackButtonModule } from '../../shared/results-back-button/result
import {
AccessControlFormModule
} from '../../shared/access-control-form-container/access-control-form.module';
import {
EditRelationshipListWrapperComponent
} from './item-relationships/edit-relationship-list-wrapper/edit-relationship-list-wrapper.component';

/**
* Module that contains all components related to the Edit Item page administrator functionality
Expand Down Expand Up @@ -94,6 +97,7 @@ import {
ItemRegisterDoiComponent,
ItemCurateComponent,
ItemAccessControlComponent,
EditRelationshipListWrapperComponent,
],
providers: [
BundleDataService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ describe('EditItemRelationshipsService', () => {

expect(itemService.invalidateByHref).toHaveBeenCalledWith(currentItem.self);
expect(itemService.invalidateByHref).toHaveBeenCalledWith(relationshipItem1.self);
expect(itemService.invalidateByHref).toHaveBeenCalledWith(relationshipItem2.self);

expect(notificationsService.success).toHaveBeenCalledTimes(1);
});
Expand Down Expand Up @@ -265,6 +266,116 @@ describe('EditItemRelationshipsService', () => {
});
});

describe('isProvidedItemTypeLeftType', () => {
it('should return true if the provided item corresponds to the left type of the relationship', (done) => {
const relationshipType = Object.assign(new RelationshipType(), {
leftType: createSuccessfulRemoteDataObject$({id: 'leftType'}),
rightType: createSuccessfulRemoteDataObject$({id: 'rightType'}),
});
const itemType = Object.assign(new ItemType(), {id: 'leftType'} );
const item = Object.assign(new Item(), {uuid: 'item-uuid'});

const result = service.isProvidedItemTypeLeftType(relationshipType, itemType, item);
result.subscribe((resultValue) => {
expect(resultValue).toBeTrue();
done();
});
});

it('should return false if the provided item corresponds to the right type of the relationship', (done) => {
const relationshipType = Object.assign(new RelationshipType(), {
leftType: createSuccessfulRemoteDataObject$({id: 'leftType'}),
rightType: createSuccessfulRemoteDataObject$({id: 'rightType'}),
});
const itemType = Object.assign(new ItemType(), {id: 'rightType'} );
const item = Object.assign(new Item(), {uuid: 'item-uuid'});

const result = service.isProvidedItemTypeLeftType(relationshipType, itemType, item);
result.subscribe((resultValue) => {
expect(resultValue).toBeFalse();
done();
});
});

it('should return undefined if the provided item corresponds does not match any of the relationship types', (done) => {
const relationshipType = Object.assign(new RelationshipType(), {
leftType: createSuccessfulRemoteDataObject$({id: 'leftType'}),
rightType: createSuccessfulRemoteDataObject$({id: 'rightType'}),
});
const itemType = Object.assign(new ItemType(), {id: 'something-else'} );
const item = Object.assign(new Item(), {uuid: 'item-uuid'});

const result = service.isProvidedItemTypeLeftType(relationshipType, itemType, item);
result.subscribe((resultValue) => {
expect(resultValue).toBeUndefined();
done();
});
});
});

describe('relationshipMatchesBothSameTypes', () => {
it('should return true if both left and right type of the relationship type are the same and match the provided itemtype', (done) => {
const relationshipType = Object.assign(new RelationshipType(), {
leftType: createSuccessfulRemoteDataObject$({id: 'sameType'}),
rightType: createSuccessfulRemoteDataObject$({id:'sameType'}),
leftwardType: 'isDepartmentOfDivision',
rightwardType: 'isDivisionOfDepartment',
});
const itemType = Object.assign(new ItemType(), {id: 'sameType'} );

const result = service.shouldDisplayBothRelationshipSides(relationshipType, itemType);
result.subscribe((resultValue) => {
expect(resultValue).toBeTrue();
done();
});
});
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) => {
const relationshipType = Object.assign(new RelationshipType(), {
leftType: createSuccessfulRemoteDataObject$({ id: 'sameType' }),
rightType: createSuccessfulRemoteDataObject$({ id: 'sameType' }),
leftwardType: 'isOrgUnitOfOrgUnit',
rightwardType: 'isOrgUnitOfOrgUnit',
});
const itemType = Object.assign(new ItemType(), { id: 'sameType' });

const result = service.shouldDisplayBothRelationshipSides(relationshipType, itemType);
result.subscribe((resultValue) => {
expect(resultValue).toBeFalse();
done();
});
});
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) => {
const relationshipType = Object.assign(new RelationshipType(), {
leftType: createSuccessfulRemoteDataObject$({id: 'sameType'}),
rightType: createSuccessfulRemoteDataObject$({id: 'sameType'}),
leftwardType: 'isDepartmentOfDivision',
rightwardType: 'isDivisionOfDepartment',
});
const itemType = Object.assign(new ItemType(), {id: 'something-else'} );

const result = service.shouldDisplayBothRelationshipSides(relationshipType, itemType);
result.subscribe((resultValue) => {
expect(resultValue).toBeFalse();
done();
});
});
it('should return false if both left and right type of the relationship type are different', (done) => {
const relationshipType = Object.assign(new RelationshipType(), {
leftType: createSuccessfulRemoteDataObject$({id: 'leftType'}),
rightType: createSuccessfulRemoteDataObject$({id: 'rightType'}),
leftwardType: 'isAuthorOfPublication',
rightwardType: 'isPublicationOfAuthor',
});
const itemType = Object.assign(new ItemType(), {id: 'leftType'} );

const result = service.shouldDisplayBothRelationshipSides(relationshipType, itemType);
result.subscribe((resultValue) => {
expect(resultValue).toBeFalse();
done();
});
});
});

describe('displayNotifications', () => {
it('should show one success notification when multiple requests succeeded', () => {
service.displayNotifications([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
} from '../../../core/data/object-updates/object-updates.reducer';
import { RemoteData } from '../../../core/data/remote-data';
import { Relationship } from '../../../core/shared/item-relationships/relationship.model';
import { EMPTY, Observable, BehaviorSubject, Subscription } from 'rxjs';
import { EMPTY, Observable, BehaviorSubject, Subscription, combineLatest as observableCombineLatest } from 'rxjs';
import { ObjectUpdatesService } from '../../../core/data/object-updates/object-updates.service';
import { ItemDataService } from '../../../core/data/item-data.service';
import { Item } from '../../../core/shared/item.model';
Expand All @@ -20,6 +20,9 @@ import { NgbModal } from '@ng-bootstrap/ng-bootstrap';
import { RelationshipDataService } from '../../../core/data/relationship-data.service';
import { EntityTypeDataService } from '../../../core/data/entity-type-data.service';
import { TranslateService } from '@ngx-translate/core';
import { ItemType } from '../../../core/shared/item-relationships/item-type.model';
import { getFirstSucceededRemoteData, getRemoteDataPayload } from '../../../core/shared/operators';
import { RelationshipType } from '../../../core/shared/item-relationships/relationship-type.model';

@Injectable({
providedIn: 'root'
Expand Down Expand Up @@ -58,7 +61,17 @@ export class EditItemRelationshipsService {
// process each update one by one, while waiting for the previous to finish
concatMap((update: FieldUpdate) => {
if (update.changeType === FieldChangeType.REMOVE) {
return this.deleteRelationship(update.field as DeleteRelationship).pipe(take(1));
return this.deleteRelationship(update.field as DeleteRelationship).pipe(
take(1),
switchMap((deleteRD: RemoteData<NoContent>) => {
if (deleteRD.hasSucceeded) {
return this.itemService.invalidateByHref((update.field as DeleteRelationship).relatedItem._links.self.href).pipe(
map(() => deleteRD),
);
}
return [deleteRD];
}),
);
} else if (update.changeType === FieldChangeType.ADD) {
return this.addRelationship(update.field as RelationshipIdentifiable).pipe(
take(1),
Expand Down Expand Up @@ -169,6 +182,55 @@ export class EditItemRelationshipsService {
}
}

isProvidedItemTypeLeftType(relationshipType: RelationshipType, itemType: ItemType, item: Item): Observable<boolean> {
return this.getRelationshipLeftAndRightType(relationshipType).pipe(
map(([leftType, rightType]: [ItemType, ItemType]) => {
if (leftType.id === itemType.id) {
return true;
}

if (rightType.id === itemType.id) {
return false;
}

// should never happen...
console.warn(`The item ${item.uuid} is not on the right or the left side of relationship type ${relationshipType.uuid}`);
return undefined;
})
);
}

/**
* Whether both side of the relationship need to be displayed on the edit relationship page or not.
*
* @param relationshipType The relationship type
* @param itemType The item type
*/
shouldDisplayBothRelationshipSides(relationshipType: RelationshipType, itemType: ItemType): Observable<boolean> {
return this.getRelationshipLeftAndRightType(relationshipType).pipe(
map(([leftType, rightType]: [ItemType, ItemType]) => {
return leftType.id === itemType.id && rightType.id === itemType.id && relationshipType.leftwardType !== relationshipType.rightwardType;
}),
);
}

protected getRelationshipLeftAndRightType(relationshipType: RelationshipType): Observable<[ItemType, ItemType]> {
const leftType$: Observable<ItemType> = relationshipType.leftType.pipe(
getFirstSucceededRemoteData(),
getRemoteDataPayload(),
);

const rightType$: Observable<ItemType> = relationshipType.rightType.pipe(
getFirstSucceededRemoteData(),
getRemoteDataPayload(),
);

return observableCombineLatest([
leftType$,
rightType$,
]);
}



/**
Expand All @@ -185,6 +247,5 @@ export class EditItemRelationshipsService {
*/
getNotificationContent(key: string): string {
return this.translateService.instant(this.notificationsPrefix + key + '.content');

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<ng-container *ngIf="shouldDisplayBothRelationshipSides$ | async">
<ds-edit-relationship-list
[url]="url"
[item]="item"
[itemType]="itemType"
[relationshipType]="relationshipType"
[hasChanges]="hasChanges"
[currentItemIsLeftItem$]="isLeftItem$"
class="d-block mb-4"
></ds-edit-relationship-list>
<ds-edit-relationship-list
[url]="url"
[item]="item"
[itemType]="itemType"
[relationshipType]="relationshipType"
[hasChanges]="hasChanges"
[currentItemIsLeftItem$]="isRightItem$"
></ds-edit-relationship-list>
</ng-container>

<ng-container *ngIf="(shouldDisplayBothRelationshipSides$ | async) === false">
<ds-edit-relationship-list
[url]="url"
[item]="item"
[itemType]="itemType"
[relationshipType]="relationshipType"
[hasChanges]="hasChanges"
[currentItemIsLeftItem$]="currentItemIsLeftItem$"
></ds-edit-relationship-list>
</ng-container>

Loading
Loading