Skip to content

Commit

Permalink
Merge pull request #3109 from alexandrevryghem/w2p-113560_edit-item-a…
Browse files Browse the repository at this point in the history
…dd-relationships-one-by-one_contribute-7_x

[Port dspace-7_x] Fixed caching & same entity type relationship with different left/rightwardtype bugs on edit item relationships
  • Loading branch information
tdonohue authored Jun 11, 2024
2 parents 2d28dfc + a0260f5 commit 1621036
Show file tree
Hide file tree
Showing 23 changed files with 533 additions and 69 deletions.
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

0 comments on commit 1621036

Please sign in to comment.