From cc862af2498885257a0e6187a2e3b761c0b33450 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 2 Apr 2019 17:36:51 +0200 Subject: [PATCH 01/25] 61142: Item relations initial edit page --- resources/i18n/en.json | 4 + .../edit-item-page/edit-item-page.module.ts | 6 +- .../edit-item-page.routing.module.ts | 6 + .../edit-in-place-relationship.component.html | 19 ++ .../edit-in-place-relationship.component.scss | 15 ++ ...it-in-place-relationship.component.spec.ts | 0 .../edit-in-place-relationship.component.ts | 64 +++++ .../item-relationships.component.html | 33 +++ .../item-relationships.component.scss | 22 ++ .../item-relationships.component.spec.ts | 0 .../item-relationships.component.ts | 228 ++++++++++++++++++ .../object-updates/object-updates.service.ts | 15 ++ 12 files changed, 411 insertions(+), 1 deletion(-) create mode 100644 src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.html create mode 100644 src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.scss create mode 100644 src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.spec.ts create mode 100644 src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.ts create mode 100644 src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html create mode 100644 src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.scss create mode 100644 src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts create mode 100644 src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts diff --git a/resources/i18n/en.json b/resources/i18n/en.json index 1952e345d8..8b7d15abfd 100644 --- a/resources/i18n/en.json +++ b/resources/i18n/en.json @@ -175,6 +175,10 @@ "head": "Item Metadata", "title": "Item Edit - Metadata" }, + "relationships": { + "head": "Item Relationships", + "title": "Item Edit - Relationships" + }, "view": { "head": "View Item", "title": "Item Edit - View" diff --git a/src/app/+item-page/edit-item-page/edit-item-page.module.ts b/src/app/+item-page/edit-item-page/edit-item-page.module.ts index 0c1de642ce..079f065d5f 100644 --- a/src/app/+item-page/edit-item-page/edit-item-page.module.ts +++ b/src/app/+item-page/edit-item-page/edit-item-page.module.ts @@ -15,6 +15,8 @@ import { ItemDeleteComponent } from './item-delete/item-delete.component'; import { ItemMetadataComponent } from './item-metadata/item-metadata.component'; import { EditInPlaceFieldComponent } from './item-metadata/edit-in-place-field/edit-in-place-field.component'; import { ItemBitstreamsComponent } from './item-bitstreams/item-bitstreams.component'; +import { ItemRelationshipsComponent } from './item-relationships/item-relationships.component'; +import { EditInPlaceRelationshipComponent } from './item-relationships/edit-in-place-relationship/edit-in-place-relationship.component'; /** * Module that contains all components related to the Edit Item page administrator functionality @@ -37,8 +39,10 @@ import { ItemBitstreamsComponent } from './item-bitstreams/item-bitstreams.compo ItemDeleteComponent, ItemStatusComponent, ItemMetadataComponent, + ItemRelationshipsComponent, ItemBitstreamsComponent, - EditInPlaceFieldComponent + EditInPlaceFieldComponent, + EditInPlaceRelationshipComponent ] }) export class EditItemPageModule { diff --git a/src/app/+item-page/edit-item-page/edit-item-page.routing.module.ts b/src/app/+item-page/edit-item-page/edit-item-page.routing.module.ts index 223b5f7c8e..55c5bcb747 100644 --- a/src/app/+item-page/edit-item-page/edit-item-page.routing.module.ts +++ b/src/app/+item-page/edit-item-page/edit-item-page.routing.module.ts @@ -10,6 +10,7 @@ import { ItemDeleteComponent } from './item-delete/item-delete.component'; import { ItemStatusComponent } from './item-status/item-status.component'; import { ItemMetadataComponent } from './item-metadata/item-metadata.component'; import { ItemBitstreamsComponent } from './item-bitstreams/item-bitstreams.component'; +import { ItemRelationshipsComponent } from './item-relationships/item-relationships.component'; const ITEM_EDIT_WITHDRAW_PATH = 'withdraw'; const ITEM_EDIT_REINSTATE_PATH = 'reinstate'; @@ -49,6 +50,11 @@ const ITEM_EDIT_DELETE_PATH = 'delete'; component: ItemMetadataComponent, data: { title: 'item.edit.tabs.metadata.title' } }, + { + path: 'relationships', + component: ItemRelationshipsComponent, + data: { title: 'item.edit.tabs.relationships.title' } + }, { path: 'view', /* TODO - change when view page exists */ diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.html b/src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.html new file mode 100644 index 0000000000..84c645d1a8 --- /dev/null +++ b/src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.html @@ -0,0 +1,19 @@ +
+
+ +
+
+
+ + +
+
+
diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.scss b/src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.scss new file mode 100644 index 0000000000..808a8344ba --- /dev/null +++ b/src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.scss @@ -0,0 +1,15 @@ +@import '../../../../../styles/variables.scss'; + +.btn[disabled] { + color: $gray-600; + border-color: $gray-600; + z-index: 0; // prevent border colors jumping on hover +} + +.relationship-action-buttons { + margin: 0; + position: absolute; + top: 50%; + left: 50%; + transform: translate(-50%, -50%); +} diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.spec.ts b/src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.spec.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.ts b/src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.ts new file mode 100644 index 0000000000..bb29f7a889 --- /dev/null +++ b/src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.ts @@ -0,0 +1,64 @@ +import { Component, Input, OnChanges } from '@angular/core'; +import { FieldUpdate } from '../../../../core/data/object-updates/object-updates.reducer'; +import { cloneDeep } from 'lodash'; +import { Item } from '../../../../core/shared/item.model'; +import { VIEW_MODE_ELEMENT } from '../../../simple/related-items/related-items-component'; +import { ObjectUpdatesService } from '../../../../core/data/object-updates/object-updates.service'; +import { FieldChangeType } from '../../../../core/data/object-updates/object-updates.actions'; +import { Observable } from 'rxjs/internal/Observable'; +import { map } from 'rxjs/operators'; + +@Component({ + // tslint:disable-next-line:component-selector + selector: '[ds-edit-in-place-relationship]', + styleUrls: ['./edit-in-place-relationship.component.scss'], + templateUrl: './edit-in-place-relationship.component.html', +}) +export class EditInPlaceRelationshipComponent implements OnChanges { + /** + * The current field, value and state of the relationship + */ + @Input() fieldUpdate: FieldUpdate; + + /** + * The current url of this page + */ + @Input() url: string; + + /** + * The related item of this relationship + */ + item: Item; + + /** + * The view-mode we're currently on + */ + viewMode = VIEW_MODE_ELEMENT; + + constructor(private objectUpdatesService: ObjectUpdatesService) { + } + + /** + * Sets the current relationship based on the fieldUpdate input field + */ + ngOnChanges(): void { + this.item = cloneDeep(this.fieldUpdate.field) as Item; + } + + remove(): void { + this.objectUpdatesService.saveRemoveFieldUpdate(this.url, this.item); + } + + undo(): void { + this.objectUpdatesService.removeSingleFieldUpdate(this.url, this.item.uuid); + } + + canRemove(): boolean { + return this.fieldUpdate.changeType !== FieldChangeType.REMOVE; + } + + canUndo(): boolean { + return this.fieldUpdate.changeType >= 0; + } + +} diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html new file mode 100644 index 0000000000..c95812b4bb --- /dev/null +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html @@ -0,0 +1,33 @@ +
+
+ + + +
+
+
{{label}}
+
+
+
+
diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.scss b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.scss new file mode 100644 index 0000000000..898533a9f0 --- /dev/null +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.scss @@ -0,0 +1,22 @@ +@import '../../../../styles/variables.scss'; + +.button-row { + .btn { + margin-right: 0.5 * $spacer; + + &:last-child { + margin-right: 0; + } + + @media screen and (min-width: map-get($grid-breakpoints, sm)) { + min-width: $edit-item-button-min-width; + } + } + + &.top .btn { + margin-top: $spacer/2; + margin-bottom: $spacer/2; + } + + +} diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts new file mode 100644 index 0000000000..0bd1635ae1 --- /dev/null +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -0,0 +1,228 @@ +import { Component, Inject, OnInit } from '@angular/core'; +import { Item } from '../../../core/shared/item.model'; +import { FieldUpdate, FieldUpdates } from '../../../core/data/object-updates/object-updates.reducer'; +import { Observable } from 'rxjs/internal/Observable'; +import { ActivatedRoute, Router } from '@angular/router'; +import { ObjectUpdatesService } from '../../../core/data/object-updates/object-updates.service'; +import { distinctUntilChanged, filter, first, flatMap, map, startWith, switchMap, tap } from 'rxjs/operators'; +import { zip as observableZip } from 'rxjs'; +import { RemoteData } from '../../../core/data/remote-data'; +import { PaginatedList } from '../../../core/data/paginated-list'; +import { Relationship } from '../../../core/shared/item-relationships/relationship.model'; +import { hasValue, hasValueOperator } from '../../../shared/empty.util'; +import { getRemoteDataPayload, getSucceededRemoteData } from '../../../core/shared/operators'; +import { NotificationsService } from '../../../shared/notifications/notifications.service'; +import { RelationshipType } from '../../../core/shared/item-relationships/relationship-type.model'; +import { + compareArraysUsingIds, + filterRelationsByTypeLabel, + relationsToItems +} from '../../simple/item-types/shared/item.component'; +import { ItemDataService } from '../../../core/data/item-data.service'; +import { combineLatest as observableCombineLatest } from 'rxjs/internal/observable/combineLatest'; +import { TranslateService } from '@ngx-translate/core'; +import { GLOBAL_CONFIG, GlobalConfig } from '../../../../config'; + +@Component({ + selector: 'ds-item-relationships', + styleUrls: ['./item-relationships.component.scss'], + templateUrl: './item-relationships.component.html', +}) +export class ItemRelationshipsComponent implements OnInit { + + /** + * The item to display the edit page for + */ + item: Item; + /** + * The current values and updates for all this item's metadata fields + */ + updates$: Observable; + /** + * The current url of this page + */ + url: string; + /** + * Prefix for this component's notification translate keys + */ + private notificationsPrefix = 'item.edit.metadata.notifications.'; + + /** + * The labels of all different relations within this item + */ + relationLabels$: Observable; + + /** + * Resolved relationships and types together in one observable + */ + resolvedRelsAndTypes$: Observable<[Relationship[], RelationshipType[]]>; + /** + * The time span for being able to undo discarding changes + */ + private discardTimeOut: number; + + constructor(private route: ActivatedRoute, + private router: Router, + private translateService: TranslateService, + @Inject(GLOBAL_CONFIG) protected EnvConfig: GlobalConfig, + private objectUpdatesService: ObjectUpdatesService, + private notificationsService: NotificationsService, + private itemDataService: ItemDataService) { + } + + ngOnInit(): void { + this.route.parent.data.pipe(map((data) => data.item)) + .pipe( + first(), + map((data: RemoteData) => data.payload) + ).subscribe((item: Item) => { + this.item = item; + }); + this.discardTimeOut = this.EnvConfig.item.edit.undoTimeout; + this.url = this.router.url; + if (this.url.indexOf('?') > 0) { + this.url = this.url.substr(0, this.url.indexOf('?')); + } + this.hasChanges().pipe(first()).subscribe((hasChanges) => { + if (!hasChanges) { + this.initializeOriginalFields(); + } else { + this.checkLastModified(); + } + }); + this.updates$ = this.getRelationships().pipe( + relationsToItems(this.item.id, this.itemDataService), + switchMap((items: Item[]) => this.objectUpdatesService.getFieldUpdates(this.url, items)) + ); + this.initRelationshipObservables(); + } + + initRelationshipObservables() { + const relationships$ = this.getRelationships(); + + const relationshipTypes$ = relationships$.pipe( + flatMap((rels: Relationship[]) => + observableZip(...rels.map((rel: Relationship) => rel.relationshipType)).pipe( + map(([...arr]: Array>) => arr.map((d: RemoteData) => d.payload).filter((type) => hasValue(type))) + ) + ), + distinctUntilChanged(compareArraysUsingIds()) + ); + + this.resolvedRelsAndTypes$ = observableCombineLatest( + relationships$, + relationshipTypes$ + ); + this.relationLabels$ = relationshipTypes$.pipe( + map((types: RelationshipType[]) => Array.from(new Set(types.map((type) => type.leftLabel)))) + ); + } + + /** + * Prevent unnecessary rerendering so fields don't lose focus + */ + trackUpdate(index, update: FieldUpdate) { + return update && update.field ? update.field.uuid : undefined; + } + + /** + * Checks whether or not there are currently updates for this item + */ + hasChanges(): Observable { + return this.objectUpdatesService.hasUpdates(this.url); + } + + /** + * Checks whether or not the item is currently reinstatable + */ + isReinstatable(): Observable { + return this.objectUpdatesService.isReinstatable(this.url); + } + + discard(): void { + const undoNotification = this.notificationsService.info(this.getNotificationTitle('discarded'), this.getNotificationContent('discarded'), { timeOut: this.discardTimeOut }); + this.objectUpdatesService.discardFieldUpdates(this.url, undoNotification); + } + + /** + * Request the object updates service to undo discarding all changes to this item + */ + reinstate() { + this.objectUpdatesService.reinstateFieldUpdates(this.url); + } + + submit(): void { + const updatedItems$ = this.getRelationships().pipe( + first(), + relationsToItems(this.item.id, this.itemDataService), + switchMap((items: Item[]) => this.objectUpdatesService.getUpdatedFields(this.url, items) as Observable) + ); + // TODO: Delete relationships + } + + private initializeOriginalFields() { + this.getRelationships().pipe( + first(), + relationsToItems(this.item.id, this.itemDataService) + ).subscribe((items: Item[]) => { + this.objectUpdatesService.initialize(this.url, items, this.item.lastModified); + }); + } + + /** + * Checks if the current item is still in sync with the version in the store + * If it's not, a notification is shown and the changes are removed + */ + private checkLastModified() { + const currentVersion = this.item.lastModified; + this.objectUpdatesService.getLastModified(this.url).pipe(first()).subscribe( + (updateVersion: Date) => { + if (updateVersion.getDate() !== currentVersion.getDate()) { + this.notificationsService.warning(this.getNotificationTitle('outdated'), this.getNotificationContent('outdated')); + this.initializeOriginalFields(); + } + } + ); + } + + public getRelationships(): Observable { + return this.item.relationships.pipe( + getSucceededRemoteData(), + getRemoteDataPayload(), + map((rels: PaginatedList) => rels.page), + hasValueOperator(), + distinctUntilChanged(compareArraysUsingIds()) + ); + } + + public getRelatedItemsByLabel(label: string): Observable { + return this.resolvedRelsAndTypes$.pipe( + filterRelationsByTypeLabel(label), + relationsToItems(this.item.id, this.itemDataService) + ); + } + + public getUpdatesByLabel(label: string): Observable { + return this.getRelatedItemsByLabel(label).pipe( + switchMap((items: Item[]) => this.objectUpdatesService.getFieldUpdatesExclusive(this.url, items)) + ) + } + + /** + * Get translated notification title + * @param key + */ + private getNotificationTitle(key: string) { + return this.translateService.instant(this.notificationsPrefix + key + '.title'); + } + + /** + * Get translated notification content + * @param key + */ + private getNotificationContent(key: string) { + return this.translateService.instant(this.notificationsPrefix + key + '.content'); + + } + +} diff --git a/src/app/core/data/object-updates/object-updates.service.ts b/src/app/core/data/object-updates/object-updates.service.ts index a13fb9487b..6ef3ca91ca 100644 --- a/src/app/core/data/object-updates/object-updates.service.ts +++ b/src/app/core/data/object-updates/object-updates.service.ts @@ -103,6 +103,21 @@ export class ObjectUpdatesService { })) } + getFieldUpdatesExclusive(url: string, initialFields: Identifiable[]): Observable { + const objectUpdates = this.getObjectEntry(url); + return objectUpdates.pipe(map((objectEntry) => { + const fieldUpdates: FieldUpdates = {}; + for (const object of initialFields) { + let fieldUpdate = objectEntry.fieldUpdates[object.uuid]; + if (isEmpty(fieldUpdate)) { + fieldUpdate = { field: object, changeType: undefined }; + } + fieldUpdates[object.uuid] = fieldUpdate; + } + return fieldUpdates; + })) + } + /** * Method to check if a specific field is currently editable in the store * @param url The URL of the page on which the field resides From 4a749cf91de89223b93e301d89bb685f01615c20 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 3 Apr 2019 11:58:45 +0200 Subject: [PATCH 02/25] 61142: AbstractItemUpdate component and refactoring of item-metadata and item-relationships --- resources/i18n/en.json | 25 +++ .../abstract-item-update.component.ts | 174 ++++++++++++++++++ .../item-metadata/item-metadata.component.ts | 152 ++------------- .../item-relationships.component.html | 2 +- .../item-relationships.component.ts | 159 ++++------------ 5 files changed, 254 insertions(+), 258 deletions(-) create mode 100644 src/app/+item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts diff --git a/resources/i18n/en.json b/resources/i18n/en.json index 8b7d15abfd..b64edd42d5 100644 --- a/resources/i18n/en.json +++ b/resources/i18n/en.json @@ -273,6 +273,31 @@ "content": "Your changes to this item's metadata were saved." } } + }, + "relationships": { + "discard-button": "Discard", + "reinstate-button": "Undo", + "save-button": "Save", + "edit": { + "buttons": { + "remove": "Remove", + "undo": "Undo changes" + } + }, + "notifications": { + "outdated": { + "title": "Changed outdated", + "content": "The item you're currently working on has been changed by another user. Your current changes are discarded to prevent conflicts" + }, + "discarded": { + "title": "Changed discarded", + "content": "Your changes were discarded. To reinstate your changes click the 'Undo' button" + }, + "saved": { + "title": "Relationships saved", + "content": "Your changes to this item's relationships were saved." + } + } } } }, diff --git a/src/app/+item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts b/src/app/+item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts new file mode 100644 index 0000000000..3cc2a5ed84 --- /dev/null +++ b/src/app/+item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts @@ -0,0 +1,174 @@ +import { Component, Inject, OnInit } from '@angular/core'; +import { FieldUpdate, FieldUpdates } from '../../../core/data/object-updates/object-updates.reducer'; +import { Observable } from 'rxjs/internal/Observable'; +import { Item } from '../../../core/shared/item.model'; +import { ItemDataService } from '../../../core/data/item-data.service'; +import { ObjectUpdatesService } from '../../../core/data/object-updates/object-updates.service'; +import { ActivatedRoute, Router } from '@angular/router'; +import { NotificationsService } from '../../../shared/notifications/notifications.service'; +import { TranslateService } from '@ngx-translate/core'; +import { GLOBAL_CONFIG, GlobalConfig } from '../../../../config'; +import { first, map } from 'rxjs/operators'; +import { RemoteData } from '../../../core/data/remote-data'; + +@Component({ + selector: 'ds-abstract-item-update', + template: ``, +}) +/** + * Abstract component for managing object updates of an item + */ +export abstract class AbstractItemUpdateComponent implements OnInit { + /** + * The item to display the edit page for + */ + protected item: Item; + /** + * The current values and updates for all this item's metadata fields + */ + protected updates$: Observable; + /** + * The current url of this page + */ + protected url: string; + /** + * Prefix for this component's notification translate keys + */ + protected notificationsPrefix; + /** + * The time span for being able to undo discarding changes + */ + protected discardTimeOut: number; + + constructor( + protected itemService: ItemDataService, + protected objectUpdatesService: ObjectUpdatesService, + protected router: Router, + protected notificationsService: NotificationsService, + protected translateService: TranslateService, + @Inject(GLOBAL_CONFIG) protected EnvConfig: GlobalConfig, + protected route: ActivatedRoute + ) { + + } + + /** + * Initialize common properties between item-update components + */ + ngOnInit(): void { + this.route.parent.data.pipe(map((data) => data.item)) + .pipe( + first(), + map((data: RemoteData) => data.payload) + ).subscribe((item: Item) => { + this.item = item; + }); + + this.discardTimeOut = this.EnvConfig.item.edit.undoTimeout; + this.url = this.router.url; + if (this.url.indexOf('?') > 0) { + this.url = this.url.substr(0, this.url.indexOf('?')); + } + this.hasChanges().pipe(first()).subscribe((hasChanges) => { + if (!hasChanges) { + this.initializeOriginalFields(); + } else { + this.checkLastModified(); + } + }); + + this.initializeNotificationsPrefix(); + } + + /** + * Initialize the prefix for notification messages + */ + abstract initializeNotificationsPrefix(): void; + + /** + * Sends all initial values of this item to the object updates service + */ + abstract initializeOriginalFields(): void; + + /** + * Prevent unnecessary rerendering so fields don't lose focus + */ + trackUpdate(index, update: FieldUpdate) { + return update && update.field ? update.field.uuid : undefined; + } + + /** + * Checks whether or not there are currently updates for this item + */ + hasChanges(): Observable { + return this.objectUpdatesService.hasUpdates(this.url); + } + + /** + * Check if the current page is entirely valid + */ + protected isValid() { + return this.objectUpdatesService.isValidPage(this.url); + } + + /** + * Checks if the current item is still in sync with the version in the store + * If it's not, a notification is shown and the changes are removed + */ + private checkLastModified() { + const currentVersion = this.item.lastModified; + this.objectUpdatesService.getLastModified(this.url).pipe(first()).subscribe( + (updateVersion: Date) => { + if (updateVersion.getDate() !== currentVersion.getDate()) { + this.notificationsService.warning(this.getNotificationTitle('outdated'), this.getNotificationContent('outdated')); + this.initializeOriginalFields(); + } + } + ); + } + + /** + * Submit the current changes + */ + abstract submit(): void; + + /** + * Request the object updates service to discard all current changes to this item + * Shows a notification to remind the user that they can undo this + */ + discard() { + const undoNotification = this.notificationsService.info(this.getNotificationTitle('discarded'), this.getNotificationContent('discarded'), { timeOut: this.discardTimeOut }); + this.objectUpdatesService.discardFieldUpdates(this.url, undoNotification); + } + + /** + * Request the object updates service to undo discarding all changes to this item + */ + reinstate() { + this.objectUpdatesService.reinstateFieldUpdates(this.url); + } + + /** + * Checks whether or not the item is currently reinstatable + */ + isReinstatable(): Observable { + return this.objectUpdatesService.isReinstatable(this.url); + } + + /** + * Get translated notification title + * @param key + */ + protected getNotificationTitle(key: string) { + return this.translateService.instant(this.notificationsPrefix + key + '.title'); + } + + /** + * Get translated notification content + * @param key + */ + protected getNotificationContent(key: string) { + return this.translateService.instant(this.notificationsPrefix + key + '.content'); + + } +} diff --git a/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.ts b/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.ts index 6b3e05c818..7c9202c3b9 100644 --- a/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.ts +++ b/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.ts @@ -6,8 +6,6 @@ import { ActivatedRoute, Router } from '@angular/router'; import { cloneDeep } from 'lodash'; import { Observable } from 'rxjs'; import { - FieldUpdate, - FieldUpdates, Identifiable } from '../../../core/data/object-updates/object-updates.reducer'; import { first, map, switchMap, take, tap } from 'rxjs/operators'; @@ -20,6 +18,7 @@ import { RegistryService } from '../../../core/registry/registry.service'; import { MetadataField } from '../../../core/metadata/metadatafield.model'; import { MetadatumViewModel } from '../../../core/shared/metadata.models'; import { Metadata } from '../../../core/shared/metadata.utils'; +import { AbstractItemUpdateComponent } from '../abstract-item-update/abstract-item-update.component'; @Component({ selector: 'ds-item-metadata', @@ -29,28 +28,7 @@ import { Metadata } from '../../../core/shared/metadata.utils'; /** * Component for displaying an item's metadata edit page */ -export class ItemMetadataComponent implements OnInit { - - /** - * The item to display the edit page for - */ - item: Item; - /** - * The current values and updates for all this item's metadata fields - */ - updates$: Observable; - /** - * The current url of this page - */ - url: string; - /** - * The time span for being able to undo discarding changes - */ - private discardTimeOut: number; - /** - * Prefix for this component's notification translate keys - */ - private notificationsPrefix = 'item.edit.metadata.notifications.'; +export class ItemMetadataComponent extends AbstractItemUpdateComponent { /** * Observable with a list of strings with all existing metadata field keys @@ -58,90 +36,54 @@ export class ItemMetadataComponent implements OnInit { metadataFields$: Observable; constructor( - private itemService: ItemDataService, - private objectUpdatesService: ObjectUpdatesService, - private router: Router, - private notificationsService: NotificationsService, - private translateService: TranslateService, + protected itemService: ItemDataService, + protected objectUpdatesService: ObjectUpdatesService, + protected router: Router, + protected notificationsService: NotificationsService, + protected translateService: TranslateService, @Inject(GLOBAL_CONFIG) protected EnvConfig: GlobalConfig, - private route: ActivatedRoute, - private metadataFieldService: RegistryService, + protected route: ActivatedRoute, + protected metadataFieldService: RegistryService, ) { - + super(itemService, objectUpdatesService, router, notificationsService, translateService, EnvConfig, route); } /** * Set up and initialize all fields */ ngOnInit(): void { + super.ngOnInit(); this.metadataFields$ = this.findMetadataFields(); - this.route.parent.data.pipe(map((data) => data.item)) - .pipe( - first(), - map((data: RemoteData) => data.payload) - ).subscribe((item: Item) => { - this.item = item; - }); - - this.discardTimeOut = this.EnvConfig.item.edit.undoTimeout; - this.url = this.router.url; - if (this.url.indexOf('?') > 0) { - this.url = this.url.substr(0, this.url.indexOf('?')); - } - this.hasChanges().pipe(first()).subscribe((hasChanges) => { - if (!hasChanges) { - this.initializeOriginalFields(); - } else { - this.checkLastModified(); - } - }); this.updates$ = this.objectUpdatesService.getFieldUpdates(this.url, this.item.metadataAsList); } + /** + * Initialize the prefix for notification messages + */ + public initializeNotificationsPrefix(): void { + this.notificationsPrefix = 'item.edit.metadata.notifications.'; + } + /** * Sends a new add update for a field to the object updates service * @param metadata The metadata to add, if no parameter is supplied, create a new Metadatum */ add(metadata: MetadatumViewModel = new MetadatumViewModel()) { this.objectUpdatesService.saveAddFieldUpdate(this.url, metadata); - - } - - /** - * Request the object updates service to discard all current changes to this item - * Shows a notification to remind the user that they can undo this - */ - discard() { - const undoNotification = this.notificationsService.info(this.getNotificationTitle('discarded'), this.getNotificationContent('discarded'), { timeOut: this.discardTimeOut }); - this.objectUpdatesService.discardFieldUpdates(this.url, undoNotification); - } - - /** - * Request the object updates service to undo discarding all changes to this item - */ - reinstate() { - this.objectUpdatesService.reinstateFieldUpdates(this.url); } /** * Sends all initial values of this item to the object updates service */ - private initializeOriginalFields() { + public initializeOriginalFields() { this.objectUpdatesService.initialize(this.url, this.item.metadataAsList, this.item.lastModified); } - /** - * Prevent unnecessary rerendering so fields don't lose focus - */ - trackUpdate(index, update: FieldUpdate) { - return update && update.field ? update.field.uuid : undefined; - } - /** * Requests all current metadata for this item and requests the item service to update the item * Makes sure the new version of the item is rendered on the page */ - submit() { + public submit() { this.isValid().pipe(first()).subscribe((isValid) => { if (isValid) { const metadata$: Observable = this.objectUpdatesService.getUpdatedFields(this.url, this.item.metadataAsList) as Observable; @@ -167,60 +109,6 @@ export class ItemMetadataComponent implements OnInit { }); } - /** - * Checks whether or not there are currently updates for this item - */ - hasChanges(): Observable { - return this.objectUpdatesService.hasUpdates(this.url); - } - - /** - * Checks whether or not the item is currently reinstatable - */ - isReinstatable(): Observable { - return this.objectUpdatesService.isReinstatable(this.url); - } - - /** - * Checks if the current item is still in sync with the version in the store - * If it's not, a notification is shown and the changes are removed - */ - private checkLastModified() { - const currentVersion = this.item.lastModified; - this.objectUpdatesService.getLastModified(this.url).pipe(first()).subscribe( - (updateVersion: Date) => { - if (updateVersion.getDate() !== currentVersion.getDate()) { - this.notificationsService.warning(this.getNotificationTitle('outdated'), this.getNotificationContent('outdated')); - this.initializeOriginalFields(); - } - } - ); - } - - /** - * Check if the current page is entirely valid - */ - private isValid() { - return this.objectUpdatesService.isValidPage(this.url); - } - - /** - * Get translated notification title - * @param key - */ - private getNotificationTitle(key: string) { - return this.translateService.instant(this.notificationsPrefix + key + '.title'); - } - - /** - * Get translated notification content - * @param key - */ - private getNotificationContent(key: string) { - return this.translateService.instant(this.notificationsPrefix + key + '.content'); - - } - /** * Method to request all metadata fields and convert them to a list of strings */ diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html index c95812b4bb..cfa3b8f415 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html @@ -19,7 +19,7 @@
{{label}}
-
; - /** - * The current url of this page - */ - url: string; - /** - * Prefix for this component's notification translate keys - */ - private notificationsPrefix = 'item.edit.metadata.notifications.'; +/** + * Component for displaying an item's relationships edit page + */ +export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { /** * The labels of all different relations within this item @@ -56,47 +37,20 @@ export class ItemRelationshipsComponent implements OnInit { * Resolved relationships and types together in one observable */ resolvedRelsAndTypes$: Observable<[Relationship[], RelationshipType[]]>; - /** - * The time span for being able to undo discarding changes - */ - private discardTimeOut: number; - - constructor(private route: ActivatedRoute, - private router: Router, - private translateService: TranslateService, - @Inject(GLOBAL_CONFIG) protected EnvConfig: GlobalConfig, - private objectUpdatesService: ObjectUpdatesService, - private notificationsService: NotificationsService, - private itemDataService: ItemDataService) { - } ngOnInit(): void { - this.route.parent.data.pipe(map((data) => data.item)) - .pipe( - first(), - map((data: RemoteData) => data.payload) - ).subscribe((item: Item) => { - this.item = item; - }); - this.discardTimeOut = this.EnvConfig.item.edit.undoTimeout; - this.url = this.router.url; - if (this.url.indexOf('?') > 0) { - this.url = this.url.substr(0, this.url.indexOf('?')); - } - this.hasChanges().pipe(first()).subscribe((hasChanges) => { - if (!hasChanges) { - this.initializeOriginalFields(); - } else { - this.checkLastModified(); - } - }); + super.ngOnInit(); + this.updates$ = this.getRelationships().pipe( - relationsToItems(this.item.id, this.itemDataService), + relationsToItems(this.item.id, this.itemService), switchMap((items: Item[]) => this.objectUpdatesService.getFieldUpdates(this.url, items)) ); this.initRelationshipObservables(); } + /** + * Initialize the item's relationship observables for easier access across the component + */ initRelationshipObservables() { const relationships$ = this.getRelationships(); @@ -119,72 +73,36 @@ export class ItemRelationshipsComponent implements OnInit { } /** - * Prevent unnecessary rerendering so fields don't lose focus + * Initialize the prefix for notification messages */ - trackUpdate(index, update: FieldUpdate) { - return update && update.field ? update.field.uuid : undefined; + public initializeNotificationsPrefix(): void { + this.notificationsPrefix = 'item.edit.relationships.notifications.'; } - /** - * Checks whether or not there are currently updates for this item - */ - hasChanges(): Observable { - return this.objectUpdatesService.hasUpdates(this.url); - } - - /** - * Checks whether or not the item is currently reinstatable - */ - isReinstatable(): Observable { - return this.objectUpdatesService.isReinstatable(this.url); - } - - discard(): void { - const undoNotification = this.notificationsService.info(this.getNotificationTitle('discarded'), this.getNotificationContent('discarded'), { timeOut: this.discardTimeOut }); - this.objectUpdatesService.discardFieldUpdates(this.url, undoNotification); - } - - /** - * Request the object updates service to undo discarding all changes to this item - */ - reinstate() { - this.objectUpdatesService.reinstateFieldUpdates(this.url); - } - - submit(): void { + public submit(): void { const updatedItems$ = this.getRelationships().pipe( first(), - relationsToItems(this.item.id, this.itemDataService), + relationsToItems(this.item.id, this.itemService), switchMap((items: Item[]) => this.objectUpdatesService.getUpdatedFields(this.url, items) as Observable) ); // TODO: Delete relationships } - private initializeOriginalFields() { + /** + * Sends all initial values of this item to the object updates service + */ + public initializeOriginalFields() { this.getRelationships().pipe( first(), - relationsToItems(this.item.id, this.itemDataService) + relationsToItems(this.item.id, this.itemService) ).subscribe((items: Item[]) => { this.objectUpdatesService.initialize(this.url, items, this.item.lastModified); }); } /** - * Checks if the current item is still in sync with the version in the store - * If it's not, a notification is shown and the changes are removed + * Fetch all the relationships of the item */ - private checkLastModified() { - const currentVersion = this.item.lastModified; - this.objectUpdatesService.getLastModified(this.url).pipe(first()).subscribe( - (updateVersion: Date) => { - if (updateVersion.getDate() !== currentVersion.getDate()) { - this.notificationsService.warning(this.getNotificationTitle('outdated'), this.getNotificationContent('outdated')); - this.initializeOriginalFields(); - } - } - ); - } - public getRelationships(): Observable { return this.item.relationships.pipe( getSucceededRemoteData(), @@ -195,34 +113,25 @@ export class ItemRelationshipsComponent implements OnInit { ); } + /** + * Transform the item's relationships of a specific type into related items + * @param label The relationship type's label + */ public getRelatedItemsByLabel(label: string): Observable { return this.resolvedRelsAndTypes$.pipe( filterRelationsByTypeLabel(label), - relationsToItems(this.item.id, this.itemDataService) + relationsToItems(this.item.id, this.itemService) ); } + /** + * Get FieldUpdates for the relationships of a specific type + * @param label The relationship type's label + */ public getUpdatesByLabel(label: string): Observable { return this.getRelatedItemsByLabel(label).pipe( switchMap((items: Item[]) => this.objectUpdatesService.getFieldUpdatesExclusive(this.url, items)) ) } - /** - * Get translated notification title - * @param key - */ - private getNotificationTitle(key: string) { - return this.translateService.instant(this.notificationsPrefix + key + '.title'); - } - - /** - * Get translated notification content - * @param key - */ - private getNotificationContent(key: string) { - return this.translateService.instant(this.notificationsPrefix + key + '.content'); - - } - } From a99fa4d4a26c1143e386cc0791d7ba93d3b1235a Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 3 Apr 2019 13:05:43 +0200 Subject: [PATCH 03/25] 61142: Renaming and refactoring item-relationships --- .../abstract-item-update.component.ts | 10 ++++++++- .../edit-item-page/edit-item-page.module.ts | 4 ++-- .../item-metadata/item-metadata.component.ts | 6 +++++ .../edit-relationship.component.html} | 0 .../edit-relationship.component.scss} | 0 .../edit-relationship.component.spec.ts} | 0 .../edit-relationship.component.ts} | 22 ++++++++++++++----- .../item-relationships.component.html | 2 +- .../item-relationships.component.ts | 18 ++++++++++----- 9 files changed, 47 insertions(+), 15 deletions(-) rename src/app/+item-page/edit-item-page/item-relationships/{edit-in-place-relationship/edit-in-place-relationship.component.html => edit-relationship/edit-relationship.component.html} (100%) rename src/app/+item-page/edit-item-page/item-relationships/{edit-in-place-relationship/edit-in-place-relationship.component.scss => edit-relationship/edit-relationship.component.scss} (100%) rename src/app/+item-page/edit-item-page/item-relationships/{edit-in-place-relationship/edit-in-place-relationship.component.spec.ts => edit-relationship/edit-relationship.component.spec.ts} (100%) rename src/app/+item-page/edit-item-page/item-relationships/{edit-in-place-relationship/edit-in-place-relationship.component.ts => edit-relationship/edit-relationship.component.ts} (74%) diff --git a/src/app/+item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts b/src/app/+item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts index 3cc2a5ed84..76e6eb9446 100644 --- a/src/app/+item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts +++ b/src/app/+item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts @@ -24,7 +24,8 @@ export abstract class AbstractItemUpdateComponent implements OnInit { */ protected item: Item; /** - * The current values and updates for all this item's metadata fields + * The current values and updates for all this item's fields + * Should be initialized in the initializeUpdates method of the child component */ protected updates$: Observable; /** @@ -33,6 +34,7 @@ export abstract class AbstractItemUpdateComponent implements OnInit { protected url: string; /** * Prefix for this component's notification translate keys + * Should be initialized in the initializeNotificationsPrefix method of the child component */ protected notificationsPrefix; /** @@ -78,8 +80,14 @@ export abstract class AbstractItemUpdateComponent implements OnInit { }); this.initializeNotificationsPrefix(); + this.initializeUpdates(); } + /** + * Initialize the values and updates of the current item's fields + */ + abstract initializeUpdates(): void; + /** * Initialize the prefix for notification messages */ diff --git a/src/app/+item-page/edit-item-page/edit-item-page.module.ts b/src/app/+item-page/edit-item-page/edit-item-page.module.ts index 079f065d5f..db7557b43c 100644 --- a/src/app/+item-page/edit-item-page/edit-item-page.module.ts +++ b/src/app/+item-page/edit-item-page/edit-item-page.module.ts @@ -16,7 +16,7 @@ import { ItemMetadataComponent } from './item-metadata/item-metadata.component'; import { EditInPlaceFieldComponent } from './item-metadata/edit-in-place-field/edit-in-place-field.component'; import { ItemBitstreamsComponent } from './item-bitstreams/item-bitstreams.component'; import { ItemRelationshipsComponent } from './item-relationships/item-relationships.component'; -import { EditInPlaceRelationshipComponent } from './item-relationships/edit-in-place-relationship/edit-in-place-relationship.component'; +import { EditRelationshipComponent } from './item-relationships/edit-relationship/edit-relationship.component'; /** * Module that contains all components related to the Edit Item page administrator functionality @@ -42,7 +42,7 @@ import { EditInPlaceRelationshipComponent } from './item-relationships/edit-in-p ItemRelationshipsComponent, ItemBitstreamsComponent, EditInPlaceFieldComponent, - EditInPlaceRelationshipComponent + EditRelationshipComponent ] }) export class EditItemPageModule { diff --git a/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.ts b/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.ts index 7c9202c3b9..6e8be0efb6 100644 --- a/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.ts +++ b/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.ts @@ -54,6 +54,12 @@ export class ItemMetadataComponent extends AbstractItemUpdateComponent { ngOnInit(): void { super.ngOnInit(); this.metadataFields$ = this.findMetadataFields(); + } + + /** + * Initialize the values and updates of the current item's metadata fields + */ + public initializeUpdates(): void { this.updates$ = this.objectUpdatesService.getFieldUpdates(this.url, this.item.metadataAsList); } diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.html b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.html similarity index 100% rename from src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.html rename to src/app/+item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.html diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.scss b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.scss similarity index 100% rename from src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.scss rename to src/app/+item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.scss diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.spec.ts b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.spec.ts similarity index 100% rename from src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.spec.ts rename to src/app/+item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.spec.ts diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.ts b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.ts similarity index 74% rename from src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.ts rename to src/app/+item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.ts index bb29f7a889..b7ca15f211 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/edit-in-place-relationship/edit-in-place-relationship.component.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.ts @@ -5,16 +5,14 @@ import { Item } from '../../../../core/shared/item.model'; import { VIEW_MODE_ELEMENT } from '../../../simple/related-items/related-items-component'; import { ObjectUpdatesService } from '../../../../core/data/object-updates/object-updates.service'; import { FieldChangeType } from '../../../../core/data/object-updates/object-updates.actions'; -import { Observable } from 'rxjs/internal/Observable'; -import { map } from 'rxjs/operators'; @Component({ // tslint:disable-next-line:component-selector - selector: '[ds-edit-in-place-relationship]', - styleUrls: ['./edit-in-place-relationship.component.scss'], - templateUrl: './edit-in-place-relationship.component.html', + selector: '[ds-edit-relationship]', + styleUrls: ['./edit-relationship.component.scss'], + templateUrl: './edit-relationship.component.html', }) -export class EditInPlaceRelationshipComponent implements OnChanges { +export class EditRelationshipComponent implements OnChanges { /** * The current field, value and state of the relationship */ @@ -45,18 +43,30 @@ export class EditInPlaceRelationshipComponent implements OnChanges { this.item = cloneDeep(this.fieldUpdate.field) as Item; } + /** + * Sends a new remove update for this field to the object updates service + */ remove(): void { this.objectUpdatesService.saveRemoveFieldUpdate(this.url, this.item); } + /** + * Cancels the current update for this field in the object updates service + */ undo(): void { this.objectUpdatesService.removeSingleFieldUpdate(this.url, this.item.uuid); } + /** + * Check if a user should be allowed to remove this field + */ canRemove(): boolean { return this.fieldUpdate.changeType !== FieldChangeType.REMOVE; } + /** + * Check if a user should be allowed to cancel the update to this field + */ canUndo(): boolean { return this.fieldUpdate.changeType >= 0; } diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html index cfa3b8f415..6d7f82a3da 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html @@ -20,7 +20,7 @@
{{label}}
-
+
diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html index 6d7f82a3da..50b64fed16 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html @@ -17,17 +17,34 @@  {{"item.edit.metadata.save-button" | translate}}
-
-
{{label}}
+
+
{{getRelationshipMessageKey(label) | translate}}
+ [ngClass]="{'alert alert-danger': updateValue.changeType === 2}"> +
+
+
+
+ + +
diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.scss b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.scss index 898533a9f0..cbedd42280 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.scss +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.scss @@ -20,3 +20,14 @@ } + +.relationship-row:not(.alert-danger) { + padding: $alert-padding-y 0; +} + +.relationship-row.alert-danger { + margin-left: -$alert-padding-x; + margin-right: -$alert-padding-x; + margin-top: -1px; + margin-bottom: -1px; +} diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts index adcf0abcad..56bac6c478 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -1,8 +1,8 @@ import { Component, Inject } from '@angular/core'; import { Item } from '../../../core/shared/item.model'; -import { FieldUpdates } from '../../../core/data/object-updates/object-updates.reducer'; +import { FieldUpdate, FieldUpdates } from '../../../core/data/object-updates/object-updates.reducer'; import { Observable } from 'rxjs/internal/Observable'; -import { switchMap, take } from 'rxjs/operators'; +import { distinctUntilChanged, switchMap, take } from 'rxjs/operators'; import { AbstractItemUpdateComponent } from '../abstract-item-update/abstract-item-update.component'; import { ItemDataService } from '../../../core/data/item-data.service'; import { ObjectUpdatesService } from '../../../core/data/object-updates/object-updates.service'; @@ -98,4 +98,16 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { ) } + /** + * Get the i18n message key for a relationship + * @param label The relationship type's label + */ + public getRelationshipMessageKey(label: string): string { + if (label.indexOf('Of') > -1) { + return `relationships.${label.substring(0, label.indexOf('Of') + 2)}` + } else { + return label; + } + } + } From 1e31fadb70e016008927cbb1f9c473870f88c6dd Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 4 Apr 2019 12:39:25 +0200 Subject: [PATCH 06/25] 61142: Submit intermediate commit --- resources/i18n/en.json | 1 + .../item-relationships.component.html | 23 +++++++----- .../item-relationships.component.ts | 35 ++++++++++++++++--- src/app/core/data/relationship.service.ts | 13 +++---- 4 files changed, 51 insertions(+), 21 deletions(-) diff --git a/resources/i18n/en.json b/resources/i18n/en.json index b64edd42d5..284fab6c82 100644 --- a/resources/i18n/en.json +++ b/resources/i18n/en.json @@ -733,6 +733,7 @@ "sub-communities": "Loading sub-communities...", "recent-submissions": "Loading recent submissions...", "item": "Loading item...", + "items": "Loading items...", "objects": "Loading...", "search-results": "Loading search results...", "browse-by": "Loading items...", diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html index 50b64fed16..be400649c4 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html @@ -18,14 +18,21 @@
-
{{getRelationshipMessageKey(label) | translate}}
-
-
+ +
+
{{getRelationshipMessageKey(label) | translate}}
+ +
+
+ + +
+
diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts index 56bac6c478..963db2a67e 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -2,7 +2,8 @@ import { Component, Inject } from '@angular/core'; import { Item } from '../../../core/shared/item.model'; import { FieldUpdate, FieldUpdates } from '../../../core/data/object-updates/object-updates.reducer'; import { Observable } from 'rxjs/internal/Observable'; -import { distinctUntilChanged, switchMap, take } from 'rxjs/operators'; +import { map, switchMap, take, tap } from 'rxjs/operators'; +import { combineLatest as observableCombineLatest, zip as observableZip } from 'rxjs'; import { AbstractItemUpdateComponent } from '../abstract-item-update/abstract-item-update.component'; import { ItemDataService } from '../../../core/data/item-data.service'; import { ObjectUpdatesService } from '../../../core/data/object-updates/object-updates.service'; @@ -11,6 +12,10 @@ import { NotificationsService } from '../../../shared/notifications/notification import { TranslateService } from '@ngx-translate/core'; import { GLOBAL_CONFIG, GlobalConfig } from '../../../../config'; import { RelationshipService } from '../../../core/data/relationship.service'; +import { FieldChangeType } from '../../../core/data/object-updates/object-updates.actions'; +import { Relationship } from '../../../core/shared/item-relationships/relationship.model'; +import { RestResponse } from '../../../core/cache/response.models'; +import { isNotEmptyOperator } from '../../../shared/empty.util'; @Component({ selector: 'ds-item-relationships', @@ -65,10 +70,32 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { } public submit(): void { - const updatedItems$ = this.relationshipService.getRelatedItems(this.item).pipe( - switchMap((items: Item[]) => this.objectUpdatesService.getUpdatedFields(this.url, items) as Observable) + const removedItemIds$ = this.relationshipService.getRelatedItems(this.item).pipe( + switchMap((items: Item[]) => this.objectUpdatesService.getFieldUpdatesExclusive(this.url, items) as Observable), + map((fieldUpdates: FieldUpdates) => Object.values(fieldUpdates).filter((fieldUpdate: FieldUpdate) => fieldUpdate.changeType === FieldChangeType.REMOVE)), + map((fieldUpdates: FieldUpdate[]) => fieldUpdates.map((fieldUpdate: FieldUpdate) => fieldUpdate.field.uuid) as string[]), + isNotEmptyOperator() ); - // TODO: Delete relationships + const allRelationshipsAndRemovedItemIds$ = observableCombineLatest( + this.relationshipService.getItemRelationshipsArray(this.item), + removedItemIds$ + ); + const removedRelationshipIds$ = allRelationshipsAndRemovedItemIds$.pipe( + map(([relationships, itemIds]) => + relationships + .filter((relationship: Relationship) => itemIds.indexOf(relationship.leftId) > -1 || itemIds.indexOf(relationship.rightId) > -1) + .map((relationship: Relationship) => relationship.id)) + ); + removedRelationshipIds$.pipe( + take(1), + switchMap((removedIds: string[]) => observableZip(removedIds.map((uuid: string) => this.relationshipService.deleteRelationship(uuid)))), + map((responses: RestResponse[]) => responses.filter((response: RestResponse) => response.isSuccessful)) + ).subscribe((responses: RestResponse[]) => { + console.log(responses); + this.initializeOriginalFields(); + this.initializeUpdates(); + this.notificationsService.success(this.getNotificationTitle('saved'), this.getNotificationContent('saved')); + }); } /** diff --git a/src/app/core/data/relationship.service.ts b/src/app/core/data/relationship.service.ts index 6e30696325..c6b8e8319c 100644 --- a/src/app/core/data/relationship.service.ts +++ b/src/app/core/data/relationship.service.ts @@ -3,7 +3,7 @@ import { RequestService } from './request.service'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { hasValue, hasValueOperator, isNotEmptyOperator } from '../../shared/empty.util'; -import { distinctUntilChanged, flatMap, map, take } from 'rxjs/operators'; +import { distinctUntilChanged, flatMap, map, switchMap, take, tap } from 'rxjs/operators'; import { configureRequest, filterSuccessfulResponses, @@ -46,17 +46,12 @@ export class RelationshipService { } deleteRelationship(uuid: string): Observable { - const requestUuid = this.requestService.generateRequestId(); - - this.getRelationshipEndpoint(uuid).pipe( + return this.getRelationshipEndpoint(uuid).pipe( isNotEmptyOperator(), distinctUntilChanged(), - map((endpointURL: string) => new DeleteRequest(requestUuid, endpointURL)), + map((endpointURL: string) => new DeleteRequest(this.requestService.generateRequestId(), endpointURL)), configureRequest(this.requestService), - take(1) - ).subscribe(); - - return this.requestService.getByUUID(requestUuid).pipe( + switchMap((restRequest: RestRequest) => this.requestService.getByUUID(restRequest.uuid)), filterSuccessfulResponses() ); } From bb683734894bfd6f79e57fd3bedaa966cb008f8a Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 4 Apr 2019 15:19:28 +0200 Subject: [PATCH 07/25] 61142: Working submit (except reloading lists) and JSDocs --- .../item-relationships.component.ts | 12 +++++-- .../object-updates/object-updates.service.ts | 6 ++++ src/app/core/data/relationship.service.ts | 32 +++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts index 963db2a67e..90bee12187 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -69,7 +69,12 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { this.notificationsPrefix = 'item.edit.relationships.notifications.'; } + /** + * Resolve the currently selected related items back to relationships and send a delete request + * Make sure the lists are refreshed afterwards + */ public submit(): void { + // Get all IDs of related items of which their relationship with the current item is about to be removed const removedItemIds$ = this.relationshipService.getRelatedItems(this.item).pipe( switchMap((items: Item[]) => this.objectUpdatesService.getFieldUpdatesExclusive(this.url, items) as Observable), map((fieldUpdates: FieldUpdates) => Object.values(fieldUpdates).filter((fieldUpdate: FieldUpdate) => fieldUpdate.changeType === FieldChangeType.REMOVE)), @@ -80,18 +85,21 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { this.relationshipService.getItemRelationshipsArray(this.item), removedItemIds$ ); + // Get all IDs of the relationships that should be removed const removedRelationshipIds$ = allRelationshipsAndRemovedItemIds$.pipe( map(([relationships, itemIds]) => relationships .filter((relationship: Relationship) => itemIds.indexOf(relationship.leftId) > -1 || itemIds.indexOf(relationship.rightId) > -1) .map((relationship: Relationship) => relationship.id)) ); + // Request a delete for every relationship found in the observable created above removedRelationshipIds$.pipe( take(1), - switchMap((removedIds: string[]) => observableZip(removedIds.map((uuid: string) => this.relationshipService.deleteRelationship(uuid)))), + switchMap((removedIds: string[]) => observableZip(...removedIds.map((uuid: string) => this.relationshipService.deleteRelationship(uuid)))), map((responses: RestResponse[]) => responses.filter((response: RestResponse) => response.isSuccessful)) ).subscribe((responses: RestResponse[]) => { - console.log(responses); + // Make sure the lists are up-to-date and send a notification that the removal was successful + // TODO: Fix lists refreshing correctly this.initializeOriginalFields(); this.initializeUpdates(); this.notificationsService.success(this.getNotificationTitle('saved'), this.getNotificationContent('saved')); diff --git a/src/app/core/data/object-updates/object-updates.service.ts b/src/app/core/data/object-updates/object-updates.service.ts index 6ef3ca91ca..c5c44fe36c 100644 --- a/src/app/core/data/object-updates/object-updates.service.ts +++ b/src/app/core/data/object-updates/object-updates.service.ts @@ -103,6 +103,12 @@ export class ObjectUpdatesService { })) } + /** + * Method that combines the state's updates (excluding updates that aren't part of the initialFields) with + * the initial values (when there's no update) to create a FieldUpdates object + * @param url The URL of the page for which the FieldUpdates should be requested + * @param initialFields The initial values of the fields + */ getFieldUpdatesExclusive(url: string, initialFields: Identifiable[]): Observable { const objectUpdates = this.getObjectEntry(url); return objectUpdates.pipe(map((objectEntry) => { diff --git a/src/app/core/data/relationship.service.ts b/src/app/core/data/relationship.service.ts index c6b8e8319c..d0308dead7 100644 --- a/src/app/core/data/relationship.service.ts +++ b/src/app/core/data/relationship.service.ts @@ -39,12 +39,20 @@ export class RelationshipService { protected itemService: ItemDataService) { } + /** + * Get the endpoint for a relationship by ID + * @param uuid + */ getRelationshipEndpoint(uuid: string) { return this.halService.getEndpoint(this.linkPath).pipe( map((href: string) => `${href}/${uuid}`) ); } + /** + * Send a delete request for a relationship by ID + * @param uuid + */ deleteRelationship(uuid: string): Observable { return this.getRelationshipEndpoint(uuid).pipe( isNotEmptyOperator(), @@ -56,6 +64,11 @@ export class RelationshipService { ); } + /** + * Get a combined observable containing an array of all relationships in an item, as well as an array of the relationships their types + * This is used for easier access of a relationship's type because they exist as observables + * @param item + */ getItemResolvedRelsAndTypes(item: Item): Observable<[Relationship[], RelationshipType[]]> { const relationships$ = this.getItemRelationshipsArray(item); @@ -74,6 +87,10 @@ export class RelationshipService { ); } + /** + * Get an item their relationships in the form of an array + * @param item + */ getItemRelationshipsArray(item: Item): Observable { return item.relationships.pipe( getSucceededRemoteData(), @@ -84,6 +101,11 @@ export class RelationshipService { ); } + /** + * Get an array of an item their unique relationship type's labels + * The array doesn't contain any duplicate labels + * @param item + */ getItemRelationshipLabels(item: Item): Observable { return this.getItemResolvedRelsAndTypes(item).pipe( map(([relsCurrentPage, relTypesCurrentPage]) => { @@ -100,12 +122,22 @@ export class RelationshipService { ) } + /** + * Resolve a given item's relationships into related items and return the items as an array + * @param item + */ getRelatedItems(item: Item): Observable { return this.getItemRelationshipsArray(item).pipe( relationsToItems(item.uuid, this.itemService) ); } + /** + * Resolve a given item's relationships into related items, filtered by a relationship label + * and return the items as an array + * @param item + * @param label + */ getRelatedItemsByLabel(item: Item, label: string): Observable { return this.getItemResolvedRelsAndTypes(item).pipe( filterRelationsByTypeLabel(label), From 0cf4cdc1c5a80d249792fa06c53d99d88640f7dd Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 4 Apr 2019 17:43:40 +0200 Subject: [PATCH 08/25] 61142: ItemRelationshipComponent test + item de-caching --- .../item-metadata.component.spec.ts | 2 +- .../item-relationships.component.spec.ts | 234 ++++++++++++++++++ .../item-relationships.component.ts | 8 +- 3 files changed, 242 insertions(+), 2 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.spec.ts b/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.spec.ts index f2cd74fc2f..2b50b75a2a 100644 --- a/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.spec.ts +++ b/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.spec.ts @@ -28,7 +28,7 @@ import { MetadataSchema } from '../../../core/metadata/metadataschema.model'; import { MetadataField } from '../../../core/metadata/metadatafield.model'; import { Metadata } from '../../../core/shared/metadata.utils'; -let comp: ItemMetadataComponent; +let comp: any; let fixture: ComponentFixture; let de: DebugElement; let el: HTMLElement; diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts index e69de29bb2..8579d25fdf 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts @@ -0,0 +1,234 @@ +import { async, ComponentFixture, TestBed } from '@angular/core/testing'; +import { ItemRelationshipsComponent } from './item-relationships.component'; +import { DebugElement, NO_ERRORS_SCHEMA } from '@angular/core'; +import { INotification, Notification } from '../../../shared/notifications/models/notification.model'; +import { NotificationType } from '../../../shared/notifications/models/notification-type'; +import { RouterStub } from '../../../shared/testing/router-stub'; +import { TestScheduler } from 'rxjs/testing'; +import { SharedModule } from '../../../shared/shared.module'; +import { TranslateModule } from '@ngx-translate/core'; +import { ItemDataService } from '../../../core/data/item-data.service'; +import { ObjectUpdatesService } from '../../../core/data/object-updates/object-updates.service'; +import { ActivatedRoute, Router } from '@angular/router'; +import { NotificationsService } from '../../../shared/notifications/notifications.service'; +import { GLOBAL_CONFIG } from '../../../../config'; +import { RelationshipType } from '../../../core/shared/item-relationships/relationship-type.model'; +import { ResourceType } from '../../../core/shared/resource-type'; +import { Relationship } from '../../../core/shared/item-relationships/relationship.model'; +import { of as observableOf } from 'rxjs'; +import { RemoteData } from '../../../core/data/remote-data'; +import { Item } from '../../../core/shared/item.model'; +import { PaginatedList } from '../../../core/data/paginated-list'; +import { PageInfo } from '../../../core/shared/page-info.model'; +import { FieldChangeType } from '../../../core/data/object-updates/object-updates.actions'; +import { RelationshipService } from '../../../core/data/relationship.service'; +import { ObjectCacheService } from '../../../core/cache/object-cache.service'; +import { getTestScheduler } from 'jasmine-marbles'; +import { By } from '@angular/platform-browser'; +import { RestResponse } from '../../../core/cache/response.models'; + +let comp: any; +let fixture: ComponentFixture; +let de: DebugElement; +let el: HTMLElement; +let objectUpdatesService; +let relationshipService; +let objectCache; +const infoNotification: INotification = new Notification('id', NotificationType.Info, 'info'); +const warningNotification: INotification = new Notification('id', NotificationType.Warning, 'warning'); +const successNotification: INotification = new Notification('id', NotificationType.Success, 'success'); +const notificationsService = jasmine.createSpyObj('notificationsService', + { + info: infoNotification, + warning: warningNotification, + success: successNotification + } +); +const router = new RouterStub(); +let routeStub; +let itemService; + +const url = 'http://test-url.com/test-url'; +router.url = url; + +let scheduler: TestScheduler; +let item; +let author1; +let author2; +let fieldUpdate1; +let fieldUpdate2; +let relationships; +let relationshipType; + +describe('ItemRelationshipsComponent', () => { + beforeEach(async(() => { + const date = new Date(); + + relationshipType = Object.assign(new RelationshipType(), { + type: ResourceType.RelationshipType, + id: '1', + uuid: '1', + leftLabel: 'isAuthorOfPublication', + rightLabel: 'isPublicationOfAuthor' + }); + + relationships = [ + Object.assign(new Relationship(), { + self: url + '/2', + id: '2', + uuid: '2', + leftId: 'author1', + rightId: 'publication', + relationshipType: observableOf(new RemoteData(false, false, true, undefined, relationshipType)) + }), + Object.assign(new Relationship(), { + self: url + '/3', + id: '3', + uuid: '3', + leftId: 'author2', + rightId: 'publication', + relationshipType: observableOf(new RemoteData(false, false, true, undefined, relationshipType)) + }) + ]; + + item = Object.assign(new Item(), { + self: 'fake-item-url/publication', + id: 'publication', + uuid: 'publication', + relationships: observableOf(new RemoteData(false, false, true, undefined, new PaginatedList(new PageInfo(), relationships))), + lastModified: date + }); + + author1 = Object.assign(new Item(), { + id: 'author1', + uuid: 'author1' + }); + author2 = Object.assign(new Item(), { + id: 'author2', + uuid: 'author2' + }); + + fieldUpdate1 = { + field: author1, + changeType: undefined + }; + fieldUpdate2 = { + field: author2, + changeType: FieldChangeType.REMOVE + }; + + itemService = jasmine.createSpyObj('itemService', { + findById: observableOf(new RemoteData(false, false, true, undefined, item)) + }); + routeStub = { + parent: { + data: observableOf({ item: new RemoteData(false, false, true, null, item) }) + } + }; + + objectUpdatesService = jasmine.createSpyObj('objectUpdatesService', + { + getFieldUpdates: observableOf({ + [author1.uuid]: fieldUpdate1, + [author2.uuid]: fieldUpdate2 + }), + getFieldUpdatesExclusive: observableOf({ + [author1.uuid]: fieldUpdate1, + [author2.uuid]: fieldUpdate2 + }), + saveAddFieldUpdate: {}, + discardFieldUpdates: {}, + reinstateFieldUpdates: observableOf(true), + initialize: {}, + getUpdatedFields: observableOf([author1, author2]), + getLastModified: observableOf(date), + hasUpdates: observableOf(true), + isReinstatable: observableOf(false), // should always return something --> its in ngOnInit + isValidPage: observableOf(true) + } + ); + + relationshipService = jasmine.createSpyObj('relationshipService', + { + getItemRelationshipLabels: observableOf(['isAuthorOfPublication']), + getRelatedItems: observableOf([author1, author2]), + getRelatedItemsByLabel: observableOf([author1, author2]), + getItemRelationshipsArray: observableOf(relationships), + deleteRelationship: observableOf(new RestResponse(true, 200, 'OK')) + } + ); + + objectCache = jasmine.createSpyObj('objectCache', { + remove: undefined + }); + + scheduler = getTestScheduler(); + TestBed.configureTestingModule({ + imports: [SharedModule, TranslateModule.forRoot()], + declarations: [ItemRelationshipsComponent], + providers: [ + { provide: ItemDataService, useValue: itemService }, + { provide: ObjectUpdatesService, useValue: objectUpdatesService }, + { provide: Router, useValue: router }, + { provide: ActivatedRoute, useValue: routeStub }, + { provide: NotificationsService, useValue: notificationsService }, + { provide: GLOBAL_CONFIG, useValue: { item: { edit: { undoTimeout: 10 } } } as any }, + { provide: RelationshipService, useValue: relationshipService }, + { provide: ObjectCacheService, useValue: objectCache } + ], schemas: [ + NO_ERRORS_SCHEMA + ] + }).compileComponents(); + })); + + beforeEach(() => { + fixture = TestBed.createComponent(ItemRelationshipsComponent); + comp = fixture.componentInstance; + de = fixture.debugElement; + el = de.nativeElement; + comp.url = url; + fixture.detectChanges(); + }); + + describe('discard', () => { + beforeEach(() => { + comp.discard(); + }); + + it('it should call discardFieldUpdates on the objectUpdatesService with the correct url and notification', () => { + expect(objectUpdatesService.discardFieldUpdates).toHaveBeenCalledWith(url, infoNotification); + }); + }); + + describe('reinstate', () => { + beforeEach(() => { + comp.reinstate(); + }); + + it('it should call reinstateFieldUpdates on the objectUpdatesService with the correct url', () => { + expect(objectUpdatesService.reinstateFieldUpdates).toHaveBeenCalledWith(url); + }); + }); + + describe('changeType is REMOVE', () => { + beforeEach(() => { + fieldUpdate1.changeType = FieldChangeType.REMOVE; + fixture.detectChanges(); + }); + it('the div should have class alert-danger', () => { + const element = de.queryAll(By.css('.relationship-row'))[1].nativeElement; + expect(element.classList).toContain('alert-danger'); + }); + }); + + describe('submit', () => { + beforeEach(() => { + comp.submit(); + }); + + it('it should delete the correct relationship and de-cache the current item', () => { + expect(relationshipService.deleteRelationship).toHaveBeenCalledWith(relationships[1].uuid); + expect(objectCache.remove).toHaveBeenCalledWith(item.self); + }); + }); +}); diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts index 90bee12187..bcf43bf364 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -16,6 +16,9 @@ import { FieldChangeType } from '../../../core/data/object-updates/object-update import { Relationship } from '../../../core/shared/item-relationships/relationship.model'; import { RestResponse } from '../../../core/cache/response.models'; import { isNotEmptyOperator } from '../../../shared/empty.util'; +import { RemoteData } from '../../../core/data/remote-data'; +import { ObjectCacheService } from '../../../core/cache/object-cache.service'; +import { getSucceededRemoteData } from '../../../core/shared/operators'; @Component({ selector: 'ds-item-relationships', @@ -40,7 +43,8 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { protected translateService: TranslateService, @Inject(GLOBAL_CONFIG) protected EnvConfig: GlobalConfig, protected route: ActivatedRoute, - protected relationshipService: RelationshipService + protected relationshipService: RelationshipService, + protected objectCache: ObjectCacheService ) { super(itemService, objectUpdatesService, router, notificationsService, translateService, EnvConfig, route); } @@ -100,6 +104,8 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { ).subscribe((responses: RestResponse[]) => { // Make sure the lists are up-to-date and send a notification that the removal was successful // TODO: Fix lists refreshing correctly + this.objectCache.remove(this.item.self); + this.itemService.findById(this.item.id).pipe(getSucceededRemoteData(), take(1)).subscribe((itemRD: RemoteData) => this.item = itemRD.payload); this.initializeOriginalFields(); this.initializeUpdates(); this.notificationsService.success(this.getNotificationTitle('saved'), this.getNotificationContent('saved')); From b78b2e5b8227ebd828bc6861f99c04e6efec6b71 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Fri, 5 Apr 2019 13:46:48 +0200 Subject: [PATCH 09/25] 61142: EditRelationshipComponent tests + de-caching of requests in ItemRelationships --- .../edit-relationship.component.spec.ts | 180 ++++++++++++++++++ .../item-relationships.component.ts | 5 +- src/app/core/cache/object-cache.service.ts | 4 +- 3 files changed, 186 insertions(+), 3 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.spec.ts b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.spec.ts index e69de29bb2..fc6c999a1c 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.spec.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.spec.ts @@ -0,0 +1,180 @@ +import { async, TestBed } from '@angular/core/testing'; +import { of as observableOf } from 'rxjs/internal/observable/of'; +import { TranslateModule } from '@ngx-translate/core'; +import { ObjectUpdatesService } from '../../../../core/data/object-updates/object-updates.service'; +import { NO_ERRORS_SCHEMA } from '@angular/core'; +import { EditRelationshipComponent } from './edit-relationship.component'; +import { RelationshipType } from '../../../../core/shared/item-relationships/relationship-type.model'; +import { ResourceType } from '../../../../core/shared/resource-type'; +import { Relationship } from '../../../../core/shared/item-relationships/relationship.model'; +import { RemoteData } from '../../../../core/data/remote-data'; +import { Item } from '../../../../core/shared/item.model'; +import { PaginatedList } from '../../../../core/data/paginated-list'; +import { PageInfo } from '../../../../core/shared/page-info.model'; +import { FieldChangeType } from '../../../../core/data/object-updates/object-updates.actions'; + +let objectUpdatesService: ObjectUpdatesService; +const url = 'http://test-url.com/test-url'; + +let item; +let author1; +let author2; +let fieldUpdate1; +let fieldUpdate2; +let relationships; +let relationshipType; + +let fixture; +let comp: EditRelationshipComponent; +let de; +let el; + +describe('EditRelationshipComponent', () => { + beforeEach(async(() => { + relationshipType = Object.assign(new RelationshipType(), { + type: ResourceType.RelationshipType, + id: '1', + uuid: '1', + leftLabel: 'isAuthorOfPublication', + rightLabel: 'isPublicationOfAuthor' + }); + + relationships = [ + Object.assign(new Relationship(), { + self: url + '/2', + id: '2', + uuid: '2', + leftId: 'author1', + rightId: 'publication', + relationshipType: observableOf(new RemoteData(false, false, true, undefined, relationshipType)) + }), + Object.assign(new Relationship(), { + self: url + '/3', + id: '3', + uuid: '3', + leftId: 'author2', + rightId: 'publication', + relationshipType: observableOf(new RemoteData(false, false, true, undefined, relationshipType)) + }) + ]; + + item = Object.assign(new Item(), { + self: 'fake-item-url/publication', + id: 'publication', + uuid: 'publication', + relationships: observableOf(new RemoteData(false, false, true, undefined, new PaginatedList(new PageInfo(), relationships))) + }); + + author1 = Object.assign(new Item(), { + id: 'author1', + uuid: 'author1' + }); + author2 = Object.assign(new Item(), { + id: 'author2', + uuid: 'author2' + }); + + fieldUpdate1 = { + field: author1, + changeType: undefined + }; + fieldUpdate2 = { + field: author2, + changeType: FieldChangeType.REMOVE + }; + + objectUpdatesService = jasmine.createSpyObj('objectUpdatesService', + { + saveChangeFieldUpdate: {}, + saveRemoveFieldUpdate: {}, + setEditableFieldUpdate: {}, + setValidFieldUpdate: {}, + removeSingleFieldUpdate: {}, + isEditable: observableOf(false), // should always return something --> its in ngOnInit + isValid: observableOf(true) // should always return something --> its in ngOnInit + } + ); + + TestBed.configureTestingModule({ + imports: [TranslateModule.forRoot()], + declarations: [EditRelationshipComponent], + providers: [ + { provide: ObjectUpdatesService, useValue: objectUpdatesService } + ], schemas: [ + NO_ERRORS_SCHEMA + ] + }).compileComponents(); + })); + + beforeEach(() => { + fixture = TestBed.createComponent(EditRelationshipComponent); + comp = fixture.componentInstance; + de = fixture.debugElement; + el = de.nativeElement; + + comp.url = url; + comp.fieldUpdate = fieldUpdate1; + comp.item = item; + + fixture.detectChanges(); + }); + + describe('when fieldUpdate has no changeType', () => { + beforeEach(() => { + comp.fieldUpdate = fieldUpdate1; + fixture.detectChanges(); + }); + + describe('canRemove', () => { + it('should return true', () => { + expect(comp.canRemove()).toBe(true); + }); + }); + + describe('canUndo', () => { + it('should return false', () => { + expect(comp.canUndo()).toBe(false); + }); + }); + }); + + describe('when fieldUpdate has DELETE as changeType', () => { + beforeEach(() => { + comp.fieldUpdate = fieldUpdate2; + fixture.detectChanges(); + }); + + describe('canRemove', () => { + it('should return false', () => { + expect(comp.canRemove()).toBe(false); + }); + }); + + describe('canUndo', () => { + it('should return true', () => { + expect(comp.canUndo()).toBe(true); + }); + }); + }); + + describe('remove', () => { + beforeEach(() => { + comp.remove(); + }); + + it('should call saveRemoveFieldUpdate with the correct arguments', () => { + expect(objectUpdatesService.saveRemoveFieldUpdate).toHaveBeenCalledWith(url, item); + }); + }); + + describe('undo', () => { + beforeEach(() => { + comp.undo(); + }); + + it('should call removeSingleFieldUpdate with the correct arguments', () => { + expect(objectUpdatesService.removeSingleFieldUpdate).toHaveBeenCalledWith(url, item.uuid); + }); + }); + +}); diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts index bcf43bf364..9d348c6e13 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -19,6 +19,7 @@ import { isNotEmptyOperator } from '../../../shared/empty.util'; import { RemoteData } from '../../../core/data/remote-data'; import { ObjectCacheService } from '../../../core/cache/object-cache.service'; import { getSucceededRemoteData } from '../../../core/shared/operators'; +import { RequestService } from '../../../core/data/request.service'; @Component({ selector: 'ds-item-relationships', @@ -44,7 +45,8 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { @Inject(GLOBAL_CONFIG) protected EnvConfig: GlobalConfig, protected route: ActivatedRoute, protected relationshipService: RelationshipService, - protected objectCache: ObjectCacheService + protected objectCache: ObjectCacheService, + protected requestService: RequestService ) { super(itemService, objectUpdatesService, router, notificationsService, translateService, EnvConfig, route); } @@ -105,6 +107,7 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { // Make sure the lists are up-to-date and send a notification that the removal was successful // TODO: Fix lists refreshing correctly this.objectCache.remove(this.item.self); + this.requestService.removeByHrefSubstring(this.item.self); this.itemService.findById(this.item.id).pipe(getSucceededRemoteData(), take(1)).subscribe((itemRD: RemoteData) => this.item = itemRD.payload); this.initializeOriginalFields(); this.initializeUpdates(); diff --git a/src/app/core/cache/object-cache.service.ts b/src/app/core/cache/object-cache.service.ts index 483de65b98..d415da50b3 100644 --- a/src/app/core/cache/object-cache.service.ts +++ b/src/app/core/cache/object-cache.service.ts @@ -68,8 +68,8 @@ export class ObjectCacheService { * @param href * The unique href of the object to be removed */ - remove(uuid: string): void { - this.store.dispatch(new RemoveFromObjectCacheAction(uuid)); + remove(href: string): void { + this.store.dispatch(new RemoveFromObjectCacheAction(href)); } /** From 9f27a89dc4398b80303cd4ffc5d4f4ba0974aae8 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Fri, 5 Apr 2019 15:21:10 +0200 Subject: [PATCH 10/25] 61142: RelationshipService tests --- .../core/data/relationship.service.spec.ts | 137 ++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 src/app/core/data/relationship.service.spec.ts diff --git a/src/app/core/data/relationship.service.spec.ts b/src/app/core/data/relationship.service.spec.ts new file mode 100644 index 0000000000..0e417b7ffe --- /dev/null +++ b/src/app/core/data/relationship.service.spec.ts @@ -0,0 +1,137 @@ +import { RelationshipService } from './relationship.service'; +import { RequestService } from './request.service'; +import { HALEndpointServiceStub } from '../../shared/testing/hal-endpoint-service-stub'; +import { getMockRemoteDataBuildService } from '../../shared/mocks/mock-remote-data-build.service'; +import { of as observableOf } from 'rxjs/internal/observable/of'; +import { RequestEntry } from './request.reducer'; +import { RelationshipType } from '../shared/item-relationships/relationship-type.model'; +import { ResourceType } from '../shared/resource-type'; +import { Relationship } from '../shared/item-relationships/relationship.model'; +import { RemoteData } from './remote-data'; +import { getMockRequestService } from '../../shared/mocks/mock-request.service'; +import { Item } from '../shared/item.model'; +import { PaginatedList } from './paginated-list'; +import { PageInfo } from '../shared/page-info.model'; +import { DeleteRequest } from './request.models'; + +describe('RelationshipService', () => { + let service: RelationshipService; + let requestService: RequestService; + + const restEndpointURL = 'https://rest.api/'; + const relationshipsEndpointURL = `${restEndpointURL}/relationships`; + const halService: any = new HALEndpointServiceStub(restEndpointURL); + const rdbService = getMockRemoteDataBuildService(); + + const relationshipType = Object.assign(new RelationshipType(), { + type: ResourceType.RelationshipType, + id: '1', + uuid: '1', + leftLabel: 'isAuthorOfPublication', + rightLabel: 'isPublicationOfAuthor' + }); + + const relationships = [ + Object.assign(new Relationship(), { + self: relationshipsEndpointURL + '/2', + id: '2', + uuid: '2', + leftId: 'author1', + rightId: 'publication', + relationshipType: observableOf(new RemoteData(false, false, true, undefined, relationshipType)) + }), + Object.assign(new Relationship(), { + self: relationshipsEndpointURL + '/3', + id: '3', + uuid: '3', + leftId: 'author2', + rightId: 'publication', + relationshipType: observableOf(new RemoteData(false, false, true, undefined, relationshipType)) + }) + ]; + + const item = Object.assign(new Item(), { + self: 'fake-item-url/publication', + id: 'publication', + uuid: 'publication', + relationships: observableOf(new RemoteData(false, false, true, undefined, new PaginatedList(new PageInfo(), relationships))) + }); + + const relatedItem1 = Object.assign(new Item(), { + id: 'author1', + uuid: 'author1' + }); + const relatedItem2 = Object.assign(new Item(), { + id: 'author2', + uuid: 'author2' + }); + const relatedItems = [relatedItem1, relatedItem2]; + + const itemService = jasmine.createSpyObj('itemService', { + findById: (uuid) => new RemoteData(false, false, true, undefined, relatedItems.filter((item) => item.id === uuid)[0]) + }); + + function initTestService() { + return new RelationshipService( + requestService, + halService, + rdbService, + itemService + ); + } + + const getRequestEntry$ = (successful: boolean) => { + return observableOf({ + response: { isSuccessful: successful, payload: relationships } as any + } as RequestEntry) + }; + + beforeEach(() => { + requestService = getMockRequestService(getRequestEntry$(true)); + service = initTestService(); + }); + + describe('deleteRelationship', () => { + beforeEach(() => { + service.deleteRelationship(relationships[0].uuid).subscribe(); + }); + + it('should send a DeleteRequest', () => { + const expected = new DeleteRequest(requestService.generateRequestId(), relationshipsEndpointURL + '/' + relationships[0].uuid); + expect(requestService.configure).toHaveBeenCalledWith(expected, undefined); + }); + }); + + describe('getItemRelationshipsArray', () => { + it('should return the item\'s relationships in the form of an array', () => { + service.getItemRelationshipsArray(item).subscribe((result) => { + expect(result).toEqual(relationships); + }); + }); + }); + + describe('getItemRelationshipLabels', () => { + it('should return the correct labels', () => { + service.getItemRelationshipLabels(item).subscribe((result) => { + expect(result).toEqual([relationshipType.rightLabel]); + }); + }); + }); + + describe('getRelatedItems', () => { + it('should return the related items', () => { + service.getRelatedItems(item).subscribe((result) => { + expect(result).toEqual(relatedItems); + }); + }); + }); + + describe('getRelatedItemsByLabel', () => { + it('should return the related items by label', () => { + service.getRelatedItemsByLabel(item, relationshipType.rightLabel).subscribe((result) => { + expect(result).toEqual(relatedItems); + }); + }); + }) + +}); From 01b60dbf3459a6542bdfeb36ad5118d135117f36 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Fri, 5 Apr 2019 17:28:20 +0200 Subject: [PATCH 11/25] 61142: RemoveByHrefSubstring fix --- .../item-relationships/item-relationships.component.ts | 2 +- src/app/core/data/request.service.ts | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts index 9d348c6e13..8e8a31f896 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -108,7 +108,7 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { // TODO: Fix lists refreshing correctly this.objectCache.remove(this.item.self); this.requestService.removeByHrefSubstring(this.item.self); - this.itemService.findById(this.item.id).pipe(getSucceededRemoteData(), take(1)).subscribe((itemRD: RemoteData) => this.item = itemRD.payload); + // this.itemService.findById(this.item.id).pipe(getSucceededRemoteData(), take(1)).subscribe((itemRD: RemoteData) => this.item = itemRD.payload); this.initializeOriginalFields(); this.initializeUpdates(); this.notificationsService.success(this.getNotificationTitle('saved'), this.getNotificationContent('saved')); diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index fd463047f1..82c2a47491 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -2,7 +2,7 @@ import { Injectable } from '@angular/core'; import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store'; import { Observable, race as observableRace } from 'rxjs'; -import { filter, mergeMap, take } from 'rxjs/operators'; +import { filter, map, mergeMap, take, tap } from 'rxjs/operators'; import { AppState } from '../../app.reducer'; import { hasValue, isNotEmpty } from '../../shared/empty.util'; @@ -64,8 +64,7 @@ const uuidsFromHrefSubstringSelector = const getUuidsFromHrefSubstring = (state: IndexState, href: string): string[] => { let result = []; if (isNotEmpty(state)) { - result = Object.values(state) - .filter((value: string) => value.startsWith(href)); + result = Object.keys(state).filter((key) => key.startsWith(href)).map((key) => state[key]); } return result; }; From bbbd6959a8b01f02707cbb2520a3b649e0e4d7ec Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Mon, 8 Apr 2019 15:28:18 +0200 Subject: [PATCH 12/25] 61142: EditRelationshipList component to proper reload relationships and fix performance issues --- .../edit-item-page/edit-item-page.module.ts | 4 +- .../edit-relationship-list.component.html | 15 ++ .../edit-relationship-list.component.scss | 12 ++ .../edit-relationship-list.component.spec.ts | 137 ++++++++++++++++++ .../edit-relationship-list.component.ts | 99 +++++++++++++ .../item-relationships.component.html | 16 +- .../item-relationships.component.scss | 11 -- .../item-relationships.component.spec.ts | 28 ++-- .../item-relationships.component.ts | 60 ++++---- src/app/core/cache/object-cache.service.ts | 14 +- src/app/core/data/request.service.ts | 11 ++ 11 files changed, 332 insertions(+), 75 deletions(-) create mode 100644 src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.html create mode 100644 src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.scss create mode 100644 src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts create mode 100644 src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts diff --git a/src/app/+item-page/edit-item-page/edit-item-page.module.ts b/src/app/+item-page/edit-item-page/edit-item-page.module.ts index db7557b43c..1542d12ce5 100644 --- a/src/app/+item-page/edit-item-page/edit-item-page.module.ts +++ b/src/app/+item-page/edit-item-page/edit-item-page.module.ts @@ -17,6 +17,7 @@ import { EditInPlaceFieldComponent } from './item-metadata/edit-in-place-field/e import { ItemBitstreamsComponent } from './item-bitstreams/item-bitstreams.component'; import { ItemRelationshipsComponent } from './item-relationships/item-relationships.component'; import { EditRelationshipComponent } from './item-relationships/edit-relationship/edit-relationship.component'; +import { EditRelationshipListComponent } from './item-relationships/edit-relationship-list/edit-relationship-list.component'; /** * Module that contains all components related to the Edit Item page administrator functionality @@ -42,7 +43,8 @@ import { EditRelationshipComponent } from './item-relationships/edit-relationshi ItemRelationshipsComponent, ItemBitstreamsComponent, EditInPlaceFieldComponent, - EditRelationshipComponent + EditRelationshipComponent, + EditRelationshipListComponent ] }) export class EditItemPageModule { diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.html b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.html new file mode 100644 index 0000000000..ba5164e81a --- /dev/null +++ b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.html @@ -0,0 +1,15 @@ + +
+
{{getRelationshipMessageKey(relationshipLabel) | translate}}
+ +
+
+ + +
+ diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.scss b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.scss new file mode 100644 index 0000000000..ec6cd2ba78 --- /dev/null +++ b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.scss @@ -0,0 +1,12 @@ +@import '../../../../../styles/variables.scss'; + +.relationship-row:not(.alert-danger) { + padding: $alert-padding-y 0; +} + +.relationship-row.alert-danger { + margin-left: -$alert-padding-x; + margin-right: -$alert-padding-x; + margin-top: -1px; + margin-bottom: -1px; +} diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts new file mode 100644 index 0000000000..bd5f5f2e5c --- /dev/null +++ b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts @@ -0,0 +1,137 @@ +import { EditRelationshipListComponent } from './edit-relationship-list.component'; +import { async, ComponentFixture, TestBed } from '@angular/core/testing'; +import { RelationshipType } from '../../../../core/shared/item-relationships/relationship-type.model'; +import { ResourceType } from '../../../../core/shared/resource-type'; +import { Relationship } from '../../../../core/shared/item-relationships/relationship.model'; +import { of as observableOf } from 'rxjs/internal/observable/of'; +import { RemoteData } from '../../../../core/data/remote-data'; +import { Item } from '../../../../core/shared/item.model'; +import { PaginatedList } from '../../../../core/data/paginated-list'; +import { PageInfo } from '../../../../core/shared/page-info.model'; +import { FieldChangeType } from '../../../../core/data/object-updates/object-updates.actions'; +import { SharedModule } from '../../../../shared/shared.module'; +import { TranslateModule } from '@ngx-translate/core'; +import { ObjectUpdatesService } from '../../../../core/data/object-updates/object-updates.service'; +import { RelationshipService } from '../../../../core/data/relationship.service'; +import { DebugElement, NO_ERRORS_SCHEMA } from '@angular/core'; +import { By } from '@angular/platform-browser'; + +let comp: EditRelationshipListComponent; +let fixture: ComponentFixture; +let de: DebugElement; + +let objectUpdatesService; +let relationshipService; + +const url = 'http://test-url.com/test-url'; + +let item; +let author1; +let author2; +let fieldUpdate1; +let fieldUpdate2; +let relationships; +let relationshipType; + +describe('EditRelationshipListComponent', () => { + beforeEach(async(() => { + relationshipType = Object.assign(new RelationshipType(), { + type: ResourceType.RelationshipType, + id: '1', + uuid: '1', + leftLabel: 'isAuthorOfPublication', + rightLabel: 'isPublicationOfAuthor' + }); + + relationships = [ + Object.assign(new Relationship(), { + self: url + '/2', + id: '2', + uuid: '2', + leftId: 'author1', + rightId: 'publication', + relationshipType: observableOf(new RemoteData(false, false, true, undefined, relationshipType)) + }), + Object.assign(new Relationship(), { + self: url + '/3', + id: '3', + uuid: '3', + leftId: 'author2', + rightId: 'publication', + relationshipType: observableOf(new RemoteData(false, false, true, undefined, relationshipType)) + }) + ]; + + item = Object.assign(new Item(), { + self: 'fake-item-url/publication', + id: 'publication', + uuid: 'publication', + relationships: observableOf(new RemoteData(false, false, true, undefined, new PaginatedList(new PageInfo(), relationships))) + }); + + author1 = Object.assign(new Item(), { + id: 'author1', + uuid: 'author1' + }); + author2 = Object.assign(new Item(), { + id: 'author2', + uuid: 'author2' + }); + + fieldUpdate1 = { + field: author1, + changeType: undefined + }; + fieldUpdate2 = { + field: author2, + changeType: FieldChangeType.REMOVE + }; + + objectUpdatesService = jasmine.createSpyObj('objectUpdatesService', + { + getFieldUpdatesExclusive: observableOf({ + [author1.uuid]: fieldUpdate1, + [author2.uuid]: fieldUpdate2 + }) + } + ); + + relationshipService = jasmine.createSpyObj('relationshipService', + { + getRelatedItemsByLabel: observableOf([author1, author2]), + } + ); + + TestBed.configureTestingModule({ + imports: [SharedModule, TranslateModule.forRoot()], + declarations: [EditRelationshipListComponent], + providers: [ + { provide: ObjectUpdatesService, useValue: objectUpdatesService }, + { provide: RelationshipService, useValue: relationshipService } + ], schemas: [ + NO_ERRORS_SCHEMA + ] + }).compileComponents(); + })); + + beforeEach(() => { + fixture = TestBed.createComponent(EditRelationshipListComponent); + comp = fixture.componentInstance; + de = fixture.debugElement; + comp.item = item; + comp.url = url; + comp.relationshipLabel = relationshipType.leftLabel; + fixture.detectChanges(); + }); + + describe('changeType is REMOVE', () => { + beforeEach(() => { + fieldUpdate1.changeType = FieldChangeType.REMOVE; + fixture.detectChanges(); + }); + it('the div should have class alert-danger', () => { + const element = de.queryAll(By.css('.relationship-row'))[1].nativeElement; + expect(element.classList).toContain('alert-danger'); + }); + }); +}); diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts new file mode 100644 index 0000000000..765d6484d4 --- /dev/null +++ b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts @@ -0,0 +1,99 @@ +import { Component, Input, OnChanges, OnInit, SimpleChanges } from '@angular/core'; +import { ObjectUpdatesService } from '../../../../core/data/object-updates/object-updates.service'; +import { Observable } from 'rxjs/internal/Observable'; +import { FieldUpdate, FieldUpdates } from '../../../../core/data/object-updates/object-updates.reducer'; +import { RelationshipService } from '../../../../core/data/relationship.service'; +import { Item } from '../../../../core/shared/item.model'; +import { switchMap } from 'rxjs/operators'; +import { hasValue } from '../../../../shared/empty.util'; + +@Component({ + selector: 'ds-edit-relationship-list', + styleUrls: ['./edit-relationship-list.component.scss'], + templateUrl: './edit-relationship-list.component.html', +}) +/** + * A component creating a list of editable relationships of a certain type + * The relationships are rendered as a list of related items + */ +export class EditRelationshipListComponent implements OnInit, OnChanges { + /** + * The item to display related items for + */ + @Input() item: Item; + + /** + * The URL to the current page + * Used to fetch updates for the current item from the store + */ + @Input() url: string; + + /** + * The label of the relationship-type we're rendering a list for + */ + @Input() relationshipLabel: string; + + /** + * The FieldUpdates for the relationships in question + */ + updates$: Observable; + + constructor( + protected objectUpdatesService: ObjectUpdatesService, + protected relationshipService: RelationshipService + ) { + } + + ngOnInit(): void { + this.initUpdates(); + } + + ngOnChanges(changes: SimpleChanges): void { + this.initUpdates(); + } + + /** + * Initialize the FieldUpdates using the related items + */ + initUpdates() { + this.updates$ = this.getUpdatesByLabel(this.relationshipLabel); + } + + /** + * Transform the item's relationships of a specific type into related items + * @param label The relationship type's label + */ + public getRelatedItemsByLabel(label: string): Observable { + return this.relationshipService.getRelatedItemsByLabel(this.item, label); + } + + /** + * Get FieldUpdates for the relationships of a specific type + * @param label The relationship type's label + */ + public getUpdatesByLabel(label: string): Observable { + return this.getRelatedItemsByLabel(label).pipe( + switchMap((items: Item[]) => this.objectUpdatesService.getFieldUpdatesExclusive(this.url, items)) + ) + } + + /** + * Get the i18n message key for a relationship + * @param label The relationship type's label + */ + public getRelationshipMessageKey(label: string): string { + if (hasValue(label) && label.indexOf('Of') > -1) { + return `relationships.${label.substring(0, label.indexOf('Of') + 2)}` + } else { + return label; + } + } + + /** + * Prevent unnecessary rerendering so fields don't lose focus + */ + trackUpdate(index, update: FieldUpdate) { + return update && update.field ? update.field.uuid : undefined; + } + +} diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html index be400649c4..4bd0b3df2c 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html @@ -18,21 +18,7 @@
- -
-
{{getRelationshipMessageKey(label) | translate}}
- -
-
- - -
- +
diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.scss b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.scss index cbedd42280..898533a9f0 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.scss +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.scss @@ -20,14 +20,3 @@ } - -.relationship-row:not(.alert-danger) { - padding: $alert-padding-y 0; -} - -.relationship-row.alert-danger { - margin-left: -$alert-padding-x; - margin-right: -$alert-padding-x; - margin-top: -1px; - margin-bottom: -1px; -} diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts index 8579d25fdf..2439eb4c63 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts @@ -1,6 +1,6 @@ import { async, ComponentFixture, TestBed } from '@angular/core/testing'; import { ItemRelationshipsComponent } from './item-relationships.component'; -import { DebugElement, NO_ERRORS_SCHEMA } from '@angular/core'; +import { ChangeDetectorRef, DebugElement, NO_ERRORS_SCHEMA } from '@angular/core'; import { INotification, Notification } from '../../../shared/notifications/models/notification.model'; import { NotificationType } from '../../../shared/notifications/models/notification-type'; import { RouterStub } from '../../../shared/testing/router-stub'; @@ -24,8 +24,8 @@ import { FieldChangeType } from '../../../core/data/object-updates/object-update import { RelationshipService } from '../../../core/data/relationship.service'; import { ObjectCacheService } from '../../../core/cache/object-cache.service'; import { getTestScheduler } from 'jasmine-marbles'; -import { By } from '@angular/platform-browser'; import { RestResponse } from '../../../core/cache/response.models'; +import { RequestService } from '../../../core/data/request.service'; let comp: any; let fixture: ComponentFixture; @@ -33,6 +33,7 @@ let de: DebugElement; let el: HTMLElement; let objectUpdatesService; let relationshipService; +let requestService; let objectCache; const infoNotification: INotification = new Notification('id', NotificationType.Info, 'info'); const warningNotification: INotification = new Notification('id', NotificationType.Warning, 'warning'); @@ -158,6 +159,13 @@ describe('ItemRelationshipsComponent', () => { } ); + requestService = jasmine.createSpyObj('requestService', + { + removeByHrefSubstring: {}, + hasByHrefObservable: observableOf(false) + } + ); + objectCache = jasmine.createSpyObj('objectCache', { remove: undefined }); @@ -174,7 +182,9 @@ describe('ItemRelationshipsComponent', () => { { provide: NotificationsService, useValue: notificationsService }, { provide: GLOBAL_CONFIG, useValue: { item: { edit: { undoTimeout: 10 } } } as any }, { provide: RelationshipService, useValue: relationshipService }, - { provide: ObjectCacheService, useValue: objectCache } + { provide: ObjectCacheService, useValue: objectCache }, + { provide: RequestService, useValue: requestService }, + ChangeDetectorRef ], schemas: [ NO_ERRORS_SCHEMA ] @@ -210,17 +220,6 @@ describe('ItemRelationshipsComponent', () => { }); }); - describe('changeType is REMOVE', () => { - beforeEach(() => { - fieldUpdate1.changeType = FieldChangeType.REMOVE; - fixture.detectChanges(); - }); - it('the div should have class alert-danger', () => { - const element = de.queryAll(By.css('.relationship-row'))[1].nativeElement; - expect(element.classList).toContain('alert-danger'); - }); - }); - describe('submit', () => { beforeEach(() => { comp.submit(); @@ -229,6 +228,7 @@ describe('ItemRelationshipsComponent', () => { it('it should delete the correct relationship and de-cache the current item', () => { expect(relationshipService.deleteRelationship).toHaveBeenCalledWith(relationships[1].uuid); expect(objectCache.remove).toHaveBeenCalledWith(item.self); + expect(requestService.removeByHrefSubstring).toHaveBeenCalledWith(item.self); }); }); }); diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts index 8e8a31f896..7db8756c78 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -1,8 +1,8 @@ -import { Component, Inject } from '@angular/core'; +import { ChangeDetectorRef, Component, Inject, OnDestroy } from '@angular/core'; import { Item } from '../../../core/shared/item.model'; import { FieldUpdate, FieldUpdates } from '../../../core/data/object-updates/object-updates.reducer'; import { Observable } from 'rxjs/internal/Observable'; -import { map, switchMap, take, tap } from 'rxjs/operators'; +import { filter, map, switchMap, take } from 'rxjs/operators'; import { combineLatest as observableCombineLatest, zip as observableZip } from 'rxjs'; import { AbstractItemUpdateComponent } from '../abstract-item-update/abstract-item-update.component'; import { ItemDataService } from '../../../core/data/item-data.service'; @@ -20,6 +20,7 @@ import { RemoteData } from '../../../core/data/remote-data'; import { ObjectCacheService } from '../../../core/cache/object-cache.service'; import { getSucceededRemoteData } from '../../../core/shared/operators'; import { RequestService } from '../../../core/data/request.service'; +import { Subscription } from 'rxjs/internal/Subscription'; @Component({ selector: 'ds-item-relationships', @@ -29,13 +30,19 @@ import { RequestService } from '../../../core/data/request.service'; /** * Component for displaying an item's relationships edit page */ -export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { +export class ItemRelationshipsComponent extends AbstractItemUpdateComponent implements OnDestroy { /** * The labels of all different relations within this item */ relationLabels$: Observable; + /** + * A subscription that checks when the item is deleted in cache and reloads the item by sending a new request + * This is used to update the item in cache after relationships are deleted + */ + itemUpdateSubscription: Subscription; + constructor( protected itemService: ItemDataService, protected objectUpdatesService: ObjectUpdatesService, @@ -46,7 +53,8 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { protected route: ActivatedRoute, protected relationshipService: RelationshipService, protected objectCache: ObjectCacheService, - protected requestService: RequestService + protected requestService: RequestService, + protected cdRef: ChangeDetectorRef ) { super(itemService, objectUpdatesService, router, notificationsService, translateService, EnvConfig, route); } @@ -57,6 +65,16 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { ngOnInit(): void { super.ngOnInit(); this.relationLabels$ = this.relationshipService.getItemRelationshipLabels(this.item); + + // Update the item (and view) when it's removed in the request cache + this.itemUpdateSubscription = this.requestService.hasByHrefObservable(this.item.self).pipe( + filter((exists: boolean) => !exists), + switchMap(() => this.itemService.findById(this.item.uuid)), + getSucceededRemoteData(), + ).subscribe((itemRD: RemoteData) => { + this.item = itemRD.payload; + this.cdRef.detectChanges(); + }); } /** @@ -104,13 +122,12 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { switchMap((removedIds: string[]) => observableZip(...removedIds.map((uuid: string) => this.relationshipService.deleteRelationship(uuid)))), map((responses: RestResponse[]) => responses.filter((response: RestResponse) => response.isSuccessful)) ).subscribe((responses: RestResponse[]) => { - // Make sure the lists are up-to-date and send a notification that the removal was successful - // TODO: Fix lists refreshing correctly + // Remove the item's cache to make sure the lists are reloaded with the newest values this.objectCache.remove(this.item.self); this.requestService.removeByHrefSubstring(this.item.self); - // this.itemService.findById(this.item.id).pipe(getSucceededRemoteData(), take(1)).subscribe((itemRD: RemoteData) => this.item = itemRD.payload); this.initializeOriginalFields(); this.initializeUpdates(); + // Send a notification that the removal was successful this.notificationsService.success(this.getNotificationTitle('saved'), this.getNotificationContent('saved')); }); } @@ -125,33 +142,10 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent { } /** - * Transform the item's relationships of a specific type into related items - * @param label The relationship type's label + * Unsubscribe from the item update when the component is destroyed */ - public getRelatedItemsByLabel(label: string): Observable { - return this.relationshipService.getRelatedItemsByLabel(this.item, label); - } - - /** - * Get FieldUpdates for the relationships of a specific type - * @param label The relationship type's label - */ - public getUpdatesByLabel(label: string): Observable { - return this.getRelatedItemsByLabel(label).pipe( - switchMap((items: Item[]) => this.objectUpdatesService.getFieldUpdatesExclusive(this.url, items)) - ) - } - - /** - * Get the i18n message key for a relationship - * @param label The relationship type's label - */ - public getRelationshipMessageKey(label: string): string { - if (label.indexOf('Of') > -1) { - return `relationships.${label.substring(0, label.indexOf('Of') + 2)}` - } else { - return label; - } + ngOnDestroy(): void { + this.itemUpdateSubscription.unsubscribe(); } } diff --git a/src/app/core/cache/object-cache.service.ts b/src/app/core/cache/object-cache.service.ts index d415da50b3..c1a78225b8 100644 --- a/src/app/core/cache/object-cache.service.ts +++ b/src/app/core/cache/object-cache.service.ts @@ -3,7 +3,7 @@ import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store'; import { applyPatch, Operation } from 'fast-json-patch'; import { combineLatest as observableCombineLatest, Observable } from 'rxjs'; -import { distinctUntilChanged, filter, map, mergeMap, take, } from 'rxjs/operators'; +import { distinctUntilChanged, filter, map, mergeMap, take, tap, } from 'rxjs/operators'; import { hasNoValue, hasValue, isNotEmpty } from '../../shared/empty.util'; import { CoreState } from '../core.reducers'; import { coreSelector } from '../core.selectors'; @@ -224,6 +224,18 @@ export class ObjectCacheService { return result; } + /** + * Create an observable that emits a new value whenever the availability of the cached object changes. + * The value it emits is a boolean stating if the object exists in cache or not. + * @param selfLink The self link of the object to observe + */ + hasBySelfLinkObservable(selfLink: string): Observable { + return this.store.pipe( + select(entryFromSelfLinkSelector(selfLink)), + map((entry: ObjectCacheEntry) => this.isValid(entry)) + ); + } + /** * Check whether an ObjectCacheEntry should still be cached * diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index 82c2a47491..f15a60d5ec 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -265,4 +265,15 @@ export class RequestService { return result; } + /** + * Create an observable that emits a new value whenever the availability of the cached request changes. + * The value it emits is a boolean stating if the request exists in cache or not. + * @param href The href of the request to observe + */ + hasByHrefObservable(href: string): Observable { + return this.getByHref(href).pipe( + map((requestEntry: RequestEntry) => this.isValid(requestEntry)) + ); + } + } From eeb2c790c186da27d8254264b6658b97c8fad30a Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Mon, 8 Apr 2019 16:16:55 +0200 Subject: [PATCH 13/25] 61142: Fixed AoT build errors --- .../abstract-item-update.component.ts | 17 +++++++---------- .../item-metadata/item-metadata.component.ts | 14 +++++++++----- src/app/core/data/relationship.service.spec.ts | 2 +- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/app/+item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts b/src/app/+item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts index 76e6eb9446..c49def3dd2 100644 --- a/src/app/+item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts +++ b/src/app/+item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts @@ -1,4 +1,4 @@ -import { Component, Inject, OnInit } from '@angular/core'; +import { Component, Inject, Injectable, OnInit } from '@angular/core'; import { FieldUpdate, FieldUpdates } from '../../../core/data/object-updates/object-updates.reducer'; import { Observable } from 'rxjs/internal/Observable'; import { Item } from '../../../core/shared/item.model'; @@ -11,10 +11,7 @@ import { GLOBAL_CONFIG, GlobalConfig } from '../../../../config'; import { first, map } from 'rxjs/operators'; import { RemoteData } from '../../../core/data/remote-data'; -@Component({ - selector: 'ds-abstract-item-update', - template: ``, -}) +@Injectable() /** * Abstract component for managing object updates of an item */ @@ -22,25 +19,25 @@ export abstract class AbstractItemUpdateComponent implements OnInit { /** * The item to display the edit page for */ - protected item: Item; + item: Item; /** * The current values and updates for all this item's fields * Should be initialized in the initializeUpdates method of the child component */ - protected updates$: Observable; + updates$: Observable; /** * The current url of this page */ - protected url: string; + url: string; /** * Prefix for this component's notification translate keys * Should be initialized in the initializeNotificationsPrefix method of the child component */ - protected notificationsPrefix; + notificationsPrefix; /** * The time span for being able to undo discarding changes */ - protected discardTimeOut: number; + discardTimeOut: number; constructor( protected itemService: ItemDataService, diff --git a/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.ts b/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.ts index 6e8be0efb6..dbbcebfd00 100644 --- a/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.ts +++ b/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.ts @@ -1,4 +1,4 @@ -import { Component, Inject, Input, OnInit } from '@angular/core'; +import { Component, Inject } from '@angular/core'; import { Item } from '../../../core/shared/item.model'; import { ItemDataService } from '../../../core/data/item-data.service'; import { ObjectUpdatesService } from '../../../core/data/object-updates/object-updates.service'; @@ -60,7 +60,7 @@ export class ItemMetadataComponent extends AbstractItemUpdateComponent { * Initialize the values and updates of the current item's metadata fields */ public initializeUpdates(): void { - this.updates$ = this.objectUpdatesService.getFieldUpdates(this.url, this.item.metadataAsList); + this.updates$ = this.objectUpdatesService.getFieldUpdates(this.url, this.getMetadataAsListExcludingRelationships()); } /** @@ -82,7 +82,7 @@ export class ItemMetadataComponent extends AbstractItemUpdateComponent { * Sends all initial values of this item to the object updates service */ public initializeOriginalFields() { - this.objectUpdatesService.initialize(this.url, this.item.metadataAsList, this.item.lastModified); + this.objectUpdatesService.initialize(this.url, this.getMetadataAsListExcludingRelationships(), this.item.lastModified); } /** @@ -92,7 +92,7 @@ export class ItemMetadataComponent extends AbstractItemUpdateComponent { public submit() { this.isValid().pipe(first()).subscribe((isValid) => { if (isValid) { - const metadata$: Observable = this.objectUpdatesService.getUpdatedFields(this.url, this.item.metadataAsList) as Observable; + const metadata$: Observable = this.objectUpdatesService.getUpdatedFields(this.url, this.getMetadataAsListExcludingRelationships()) as Observable; metadata$.pipe( first(), switchMap((metadata: MetadatumViewModel[]) => { @@ -105,7 +105,7 @@ export class ItemMetadataComponent extends AbstractItemUpdateComponent { (rd: RemoteData) => { this.item = rd.payload; this.initializeOriginalFields(); - this.updates$ = this.objectUpdatesService.getFieldUpdates(this.url, this.item.metadataAsList); + this.updates$ = this.objectUpdatesService.getFieldUpdates(this.url, this.getMetadataAsListExcludingRelationships()); this.notificationsService.success(this.getNotificationTitle('saved'), this.getNotificationContent('saved')); } ) @@ -124,4 +124,8 @@ export class ItemMetadataComponent extends AbstractItemUpdateComponent { take(1), map((remoteData$) => remoteData$.payload.page.map((field: MetadataField) => field.toString()))); } + + getMetadataAsListExcludingRelationships(): MetadatumViewModel[] { + return this.item.metadataAsList.filter((metadata: MetadatumViewModel) => !metadata.key.startsWith('relation.') && !metadata.key.startsWith('relationship.')); + } } diff --git a/src/app/core/data/relationship.service.spec.ts b/src/app/core/data/relationship.service.spec.ts index 0e417b7ffe..ce2b169eef 100644 --- a/src/app/core/data/relationship.service.spec.ts +++ b/src/app/core/data/relationship.service.spec.ts @@ -68,7 +68,7 @@ describe('RelationshipService', () => { const relatedItems = [relatedItem1, relatedItem2]; const itemService = jasmine.createSpyObj('itemService', { - findById: (uuid) => new RemoteData(false, false, true, undefined, relatedItems.filter((item) => item.id === uuid)[0]) + findById: (uuid) => new RemoteData(false, false, true, undefined, relatedItems.filter((relatedItem) => relatedItem.id === uuid)[0]) }); function initTestService() { From dacf8136764c5070ee59bc8bd6c15c2dce9200f6 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 18 Apr 2019 17:24:24 +0200 Subject: [PATCH 14/25] 61142: Notifications on failed relationship delete requests --- resources/i18n/en.json | 3 ++ .../item-relationships.component.ts | 29 ++++++++++++------- src/app/core/data/relationship.service.ts | 4 +-- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/resources/i18n/en.json b/resources/i18n/en.json index 284fab6c82..eb6f600e21 100644 --- a/resources/i18n/en.json +++ b/resources/i18n/en.json @@ -296,6 +296,9 @@ "saved": { "title": "Relationships saved", "content": "Your changes to this item's relationships were saved." + }, + "failed": { + "title": "Error deleting relationship" } } } diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts index 7db8756c78..3e74794866 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -2,7 +2,7 @@ import { ChangeDetectorRef, Component, Inject, OnDestroy } from '@angular/core'; import { Item } from '../../../core/shared/item.model'; import { FieldUpdate, FieldUpdates } from '../../../core/data/object-updates/object-updates.reducer'; import { Observable } from 'rxjs/internal/Observable'; -import { filter, map, switchMap, take } from 'rxjs/operators'; +import { filter, map, switchMap, take, tap } from 'rxjs/operators'; import { combineLatest as observableCombineLatest, zip as observableZip } from 'rxjs'; import { AbstractItemUpdateComponent } from '../abstract-item-update/abstract-item-update.component'; import { ItemDataService } from '../../../core/data/item-data.service'; @@ -14,7 +14,7 @@ import { GLOBAL_CONFIG, GlobalConfig } from '../../../../config'; import { RelationshipService } from '../../../core/data/relationship.service'; import { FieldChangeType } from '../../../core/data/object-updates/object-updates.actions'; import { Relationship } from '../../../core/shared/item-relationships/relationship.model'; -import { RestResponse } from '../../../core/cache/response.models'; +import { ErrorResponse, RestResponse } from '../../../core/cache/response.models'; import { isNotEmptyOperator } from '../../../shared/empty.util'; import { RemoteData } from '../../../core/data/remote-data'; import { ObjectCacheService } from '../../../core/cache/object-cache.service'; @@ -95,7 +95,7 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent impl /** * Resolve the currently selected related items back to relationships and send a delete request - * Make sure the lists are refreshed afterwards + * Make sure the lists are refreshed afterwards and notifications are sent for success and errors */ public submit(): void { // Get all IDs of related items of which their relationship with the current item is about to be removed @@ -119,16 +119,25 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent impl // Request a delete for every relationship found in the observable created above removedRelationshipIds$.pipe( take(1), - switchMap((removedIds: string[]) => observableZip(...removedIds.map((uuid: string) => this.relationshipService.deleteRelationship(uuid)))), - map((responses: RestResponse[]) => responses.filter((response: RestResponse) => response.isSuccessful)) + switchMap((removedIds: string[]) => observableZip(...removedIds.map((uuid: string) => this.relationshipService.deleteRelationship(uuid)))) ).subscribe((responses: RestResponse[]) => { - // Remove the item's cache to make sure the lists are reloaded with the newest values - this.objectCache.remove(this.item.self); - this.requestService.removeByHrefSubstring(this.item.self); + const failedResponses = responses.filter((response: RestResponse) => !response.isSuccessful); + const successfulResponses = responses.filter((response: RestResponse) => response.isSuccessful); + + // Display an error notification for each failed request + failedResponses.forEach((response: ErrorResponse) => { + this.notificationsService.error(this.getNotificationTitle('failed'), response.errorMessage); + }); + if (successfulResponses.length > 0) { + // Remove the item's cache to make sure the lists are reloaded with the newest values + this.objectCache.remove(this.item.self); + this.requestService.removeByHrefSubstring(this.item.self); + // Send a notification that the removal was successful + this.notificationsService.success(this.getNotificationTitle('saved'), this.getNotificationContent('saved')); + } + // Reset the state of editing relationships this.initializeOriginalFields(); this.initializeUpdates(); - // Send a notification that the removal was successful - this.notificationsService.success(this.getNotificationTitle('saved'), this.getNotificationContent('saved')); }); } diff --git a/src/app/core/data/relationship.service.ts b/src/app/core/data/relationship.service.ts index d0308dead7..ba77fb4f5a 100644 --- a/src/app/core/data/relationship.service.ts +++ b/src/app/core/data/relationship.service.ts @@ -7,7 +7,7 @@ import { distinctUntilChanged, flatMap, map, switchMap, take, tap } from 'rxjs/o import { configureRequest, filterSuccessfulResponses, - getRemoteDataPayload, + getRemoteDataPayload, getResponseFromEntry, getSucceededRemoteData } from '../shared/operators'; import { DeleteRequest, RestRequest } from './request.models'; @@ -60,7 +60,7 @@ export class RelationshipService { map((endpointURL: string) => new DeleteRequest(this.requestService.generateRequestId(), endpointURL)), configureRequest(this.requestService), switchMap((restRequest: RestRequest) => this.requestService.getByUUID(restRequest.uuid)), - filterSuccessfulResponses() + getResponseFromEntry() ); } From 5a06c9195e34b62ff734fefdf3bae89f44cd8da3 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 18 Apr 2019 17:42:38 +0200 Subject: [PATCH 15/25] 61142: Author seperator in publication list elements --- .../publication/publication-list-element.component.html | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/shared/object-list/item-list-element/item-types/publication/publication-list-element.component.html b/src/app/shared/object-list/item-list-element/item-types/publication/publication-list-element.component.html index aff19aec1d..d467edfb21 100644 --- a/src/app/shared/object-list/item-list-element/item-types/publication/publication-list-element.component.html +++ b/src/app/shared/object-list/item-list-element/item-types/publication/publication-list-element.component.html @@ -12,6 +12,7 @@ class="item-list-authors"> + ; From 15ed0cc8fa5ce4ff87403e5d2eee3cb8902fd4ee Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 23 Apr 2019 14:32:11 +0200 Subject: [PATCH 16/25] 61947: Thumbnail display fix --- src/app/core/shared/item.model.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/core/shared/item.model.ts b/src/app/core/shared/item.model.ts index 645b50d5db..7bd69131c6 100644 --- a/src/app/core/shared/item.model.ts +++ b/src/app/core/shared/item.model.ts @@ -8,6 +8,7 @@ import { Bitstream } from './bitstream.model'; import { hasValue, isNotEmpty } from '../../shared/empty.util'; import { PaginatedList } from '../data/paginated-list'; import { Relationship } from './item-relationships/relationship.model'; +import { getSucceededRemoteData } from './operators'; export class Item extends DSpaceObject { @@ -95,7 +96,7 @@ export class Item extends DSpaceObject { */ getBitstreamsByBundleName(bundleName: string): Observable { return this.bitstreams.pipe( - filter((rd: RemoteData>) => !rd.isResponsePending), + getSucceededRemoteData(), map((rd: RemoteData>) => rd.payload.page), filter((bitstreams: Bitstream[]) => hasValue(bitstreams)), take(1), From 14d7437da927d6087a98721660e9800e23c1f4f4 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 26 Jun 2019 17:47:44 +0200 Subject: [PATCH 17/25] 63184: Edit-Relationships left/right Item refactoring --- .../item-relationships.component.ts | 68 +++++++------ .../shared/item-relationships-utils.ts | 17 +++- src/app/core/data/relationship.service.ts | 98 +++++++++++++++---- 3 files changed, 133 insertions(+), 50 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts index 3e74794866..36b2e212ba 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -21,6 +21,7 @@ import { ObjectCacheService } from '../../../core/cache/object-cache.service'; import { getSucceededRemoteData } from '../../../core/shared/operators'; import { RequestService } from '../../../core/data/request.service'; import { Subscription } from 'rxjs/internal/Subscription'; +import { getRelationsByRelatedItemIds } from '../../simple/item-types/shared/item-relationships-utils'; @Component({ selector: 'ds-item-relationships', @@ -94,7 +95,7 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent impl } /** - * Resolve the currently selected related items back to relationships and send a delete request + * Resolve the currently selected related items back to relationships and send a delete request for each of the relationships found * Make sure the lists are refreshed afterwards and notifications are sent for success and errors */ public submit(): void { @@ -105,42 +106,51 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent impl map((fieldUpdates: FieldUpdate[]) => fieldUpdates.map((fieldUpdate: FieldUpdate) => fieldUpdate.field.uuid) as string[]), isNotEmptyOperator() ); - const allRelationshipsAndRemovedItemIds$ = observableCombineLatest( - this.relationshipService.getItemRelationshipsArray(this.item), - removedItemIds$ - ); - // Get all IDs of the relationships that should be removed - const removedRelationshipIds$ = allRelationshipsAndRemovedItemIds$.pipe( - map(([relationships, itemIds]) => - relationships - .filter((relationship: Relationship) => itemIds.indexOf(relationship.leftId) > -1 || itemIds.indexOf(relationship.rightId) > -1) - .map((relationship: Relationship) => relationship.id)) + // Get all the relationships that should be removed + const removedRelationships$ = removedItemIds$.pipe( + getRelationsByRelatedItemIds(this.item, this.relationshipService) ); // Request a delete for every relationship found in the observable created above - removedRelationshipIds$.pipe( + removedRelationships$.pipe( take(1), + map((removedRelationships: Relationship[]) => removedRelationships.map((rel: Relationship) => rel.id)), switchMap((removedIds: string[]) => observableZip(...removedIds.map((uuid: string) => this.relationshipService.deleteRelationship(uuid)))) ).subscribe((responses: RestResponse[]) => { - const failedResponses = responses.filter((response: RestResponse) => !response.isSuccessful); - const successfulResponses = responses.filter((response: RestResponse) => response.isSuccessful); - - // Display an error notification for each failed request - failedResponses.forEach((response: ErrorResponse) => { - this.notificationsService.error(this.getNotificationTitle('failed'), response.errorMessage); - }); - if (successfulResponses.length > 0) { - // Remove the item's cache to make sure the lists are reloaded with the newest values - this.objectCache.remove(this.item.self); - this.requestService.removeByHrefSubstring(this.item.self); - // Send a notification that the removal was successful - this.notificationsService.success(this.getNotificationTitle('saved'), this.getNotificationContent('saved')); - } - // Reset the state of editing relationships - this.initializeOriginalFields(); - this.initializeUpdates(); + this.displayNotifications(responses); + this.reset(); }); } + /** + * Display notifications + * - Error notification for each failed response with their message + * - Success notification in case there's at least one successful response + * @param responses + */ + displayNotifications(responses: RestResponse[]) { + const failedResponses = responses.filter((response: RestResponse) => !response.isSuccessful); + const successfulResponses = responses.filter((response: RestResponse) => response.isSuccessful); + + failedResponses.forEach((response: ErrorResponse) => { + this.notificationsService.error(this.getNotificationTitle('failed'), response.errorMessage); + }); + if (successfulResponses.length > 0) { + // Remove the item's cache to make sure the lists are reloaded with the newest values + this.objectCache.remove(this.item.self); + this.requestService.removeByHrefSubstring(this.item.self); + // Send a notification that the removal was successful + this.notificationsService.success(this.getNotificationTitle('saved'), this.getNotificationContent('saved')); + } + } + + /** + * Reset the state of editing relationships + */ + reset() { + this.initializeOriginalFields(); + this.initializeUpdates(); + } + /** * Sends all initial values of this item to the object updates service */ diff --git a/src/app/+item-page/simple/item-types/shared/item-relationships-utils.ts b/src/app/+item-page/simple/item-types/shared/item-relationships-utils.ts index 91f7c52bb8..eaea3d5d3e 100644 --- a/src/app/+item-page/simple/item-types/shared/item-relationships-utils.ts +++ b/src/app/+item-page/simple/item-types/shared/item-relationships-utils.ts @@ -7,11 +7,12 @@ import { hasValue } from '../../../../shared/empty.util'; import { Observable } from 'rxjs/internal/Observable'; import { Relationship } from '../../../../core/shared/item-relationships/relationship.model'; import { RelationshipType } from '../../../../core/shared/item-relationships/relationship-type.model'; -import { distinctUntilChanged, flatMap, map } from 'rxjs/operators'; +import { distinctUntilChanged, filter, flatMap, map, tap } from 'rxjs/operators'; import { of as observableOf, zip as observableZip, combineLatest as observableCombineLatest } from 'rxjs'; import { ItemDataService } from '../../../../core/data/item-data.service'; import { Item } from '../../../../core/shared/item.model'; import { RemoteData } from '../../../../core/data/remote-data'; +import { RelationshipService } from '../../../../core/data/relationship.service'; /** * Operator for comparing arrays using a mapping function @@ -120,3 +121,17 @@ export const relationsToRepresentations = (parentId: string, itemType: string, m ) ) ); + +/** + * Operator for fetching an item's relationships, but filtered by related item IDs (essentially performing a reverse lookup) + * Only relationships where leftItem or rightItem's ID is present in the list provided will be returned + * @param item + * @param relationshipService + */ +export const getRelationsByRelatedItemIds = (item: Item, relationshipService: RelationshipService) => + (source: Observable): Observable => + source.pipe( + flatMap((relatedItemIds: string[]) => relationshipService.getItemResolvedRelatedItemsAndRelationships(item).pipe( + map(([leftItems, rightItems, rels]) => rels.filter((rel: Relationship, index: number) => relatedItemIds.indexOf(leftItems[index].uuid) > -1 || relatedItemIds.indexOf(rightItems[index].uuid) > -1)) + )) + ); diff --git a/src/app/core/data/relationship.service.ts b/src/app/core/data/relationship.service.ts index 000faaf2c3..fca5074a88 100644 --- a/src/app/core/data/relationship.service.ts +++ b/src/app/core/data/relationship.service.ts @@ -3,7 +3,7 @@ import { RequestService } from './request.service'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { hasValue, hasValueOperator, isNotEmptyOperator } from '../../shared/empty.util'; -import { distinctUntilChanged, flatMap, map, switchMap, take, tap } from 'rxjs/operators'; +import { distinctUntilChanged, filter, flatMap, map, switchMap, take, tap } from 'rxjs/operators'; import { configureRequest, filterSuccessfulResponses, @@ -70,20 +70,35 @@ export class RelationshipService { * @param item */ getItemResolvedRelsAndTypes(item: Item): Observable<[Relationship[], RelationshipType[]]> { - const relationships$ = this.getItemRelationshipsArray(item); - - const relationshipTypes$ = relationships$.pipe( - flatMap((rels: Relationship[]) => - observableZip(...rels.map((rel: Relationship) => rel.relationshipType)).pipe( - map(([...arr]: Array>) => arr.map((d: RemoteData) => d.payload).filter((type) => hasValue(type))) - ) - ), - distinctUntilChanged(compareArraysUsingIds()) - ); - return observableCombineLatest( - relationships$, - relationshipTypes$ + this.getItemRelationshipsArray(item), + this.getItemRelationshipTypesArray(item) + ); + } + + /** + * Get a combined observable containing an array of all the item's relationship's left- and right-side items, as well as an array of the relationships their types + * This is used for easier access of a relationship's type and left and right items because they exist as observables + * @param item + */ + getItemResolvedRelatedItemsAndTypes(item: Item): Observable<[Item[], Item[], RelationshipType[]]> { + return observableCombineLatest( + this.getItemLeftRelatedItemArray(item), + this.getItemRightRelatedItemArray(item), + this.getItemRelationshipTypesArray(item) + ); + } + + /** + * Get a combined observable containing an array of all the item's relationship's left- and right-side items, as well as an array of the relationships themselves + * This is used for easier access of the relationship and their left and right items because they exist as observables + * @param item + */ + getItemResolvedRelatedItemsAndRelationships(item: Item): Observable<[Item[], Item[], Relationship[]]> { + return observableCombineLatest( + this.getItemLeftRelatedItemArray(item), + this.getItemRightRelatedItemArray(item), + this.getItemRelationshipsArray(item) ); } @@ -101,17 +116,60 @@ export class RelationshipService { ); } + /** + * Get an item their relationship types in the form of an array + * @param item + */ + getItemRelationshipTypesArray(item: Item): Observable { + return this.getItemRelationshipsArray(item).pipe( + flatMap((rels: Relationship[]) => + observableZip(...rels.map((rel: Relationship) => rel.relationshipType)).pipe( + map(([...arr]: Array>) => arr.map((d: RemoteData) => d.payload).filter((type) => hasValue(type))), + filter((arr) => arr.length === rels.length) + ) + ), + distinctUntilChanged(compareArraysUsingIds()) + ); + } + + /** + * Get an item his relationship's left-side related items in the form of an array + * @param item + */ + getItemLeftRelatedItemArray(item: Item): Observable { + return this.getItemRelationshipsArray(item).pipe( + flatMap((rels: Relationship[]) => observableZip(...rels.map((rel: Relationship) => rel.leftItem)).pipe( + map(([...arr]: Array>) => arr.map((rd: RemoteData) => rd.payload).filter((i) => hasValue(i))), + filter((arr) => arr.length === rels.length) + )), + distinctUntilChanged(compareArraysUsingIds()) + ); + } + + /** + * Get an item his relationship's right-side related items in the form of an array + * @param item + */ + getItemRightRelatedItemArray(item: Item): Observable { + return this.getItemRelationshipsArray(item).pipe( + flatMap((rels: Relationship[]) => observableZip(...rels.map((rel: Relationship) => rel.rightItem)).pipe( + map(([...arr]: Array>) => arr.map((rd: RemoteData) => rd.payload).filter((i) => hasValue(i))), + filter((arr) => arr.length === rels.length) + )), + distinctUntilChanged(compareArraysUsingIds()) + ); + } + /** * Get an array of an item their unique relationship type's labels * The array doesn't contain any duplicate labels * @param item */ getItemRelationshipLabels(item: Item): Observable { - return this.getItemResolvedRelsAndTypes(item).pipe( - map(([relsCurrentPage, relTypesCurrentPage]) => { + return this.getItemResolvedRelatedItemsAndTypes(item).pipe( + map(([leftItems, rightItems, relTypesCurrentPage]) => { return relTypesCurrentPage.map((type, index) => { - const relationship = relsCurrentPage[index]; - if (relationship.leftId === item.uuid) { + if (leftItems[index].uuid === item.uuid) { return type.leftLabel; } else { return type.rightLabel; @@ -128,7 +186,7 @@ export class RelationshipService { */ getRelatedItems(item: Item): Observable { return this.getItemRelationshipsArray(item).pipe( - relationsToItems(item.uuid, this.itemService) + relationsToItems(item.uuid) ); } @@ -141,7 +199,7 @@ export class RelationshipService { getRelatedItemsByLabel(item: Item, label: string): Observable { return this.getItemResolvedRelsAndTypes(item).pipe( filterRelationsByTypeLabel(label), - relationsToItems(item.uuid, this.itemService) + relationsToItems(item.uuid) ); } From 2f6c0fb1264da0a2c4c5791ba06dbaab8230e5d1 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 27 Jun 2019 10:19:53 +0200 Subject: [PATCH 18/25] 63184: Fixed test cases --- .../item-relationships.component.spec.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts index 2439eb4c63..1d3d59201b 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts @@ -15,7 +15,7 @@ import { GLOBAL_CONFIG } from '../../../../config'; import { RelationshipType } from '../../../core/shared/item-relationships/relationship-type.model'; import { ResourceType } from '../../../core/shared/resource-type'; import { Relationship } from '../../../core/shared/item-relationships/relationship.model'; -import { of as observableOf } from 'rxjs'; +import { of as observableOf, combineLatest as observableCombineLatest } from 'rxjs'; import { RemoteData } from '../../../core/data/remote-data'; import { Item } from '../../../core/shared/item.model'; import { PaginatedList } from '../../../core/data/paginated-list'; @@ -78,16 +78,12 @@ describe('ItemRelationshipsComponent', () => { self: url + '/2', id: '2', uuid: '2', - leftId: 'author1', - rightId: 'publication', relationshipType: observableOf(new RemoteData(false, false, true, undefined, relationshipType)) }), Object.assign(new Relationship(), { self: url + '/3', id: '3', uuid: '3', - leftId: 'author2', - rightId: 'publication', relationshipType: observableOf(new RemoteData(false, false, true, undefined, relationshipType)) }) ]; @@ -109,6 +105,11 @@ describe('ItemRelationshipsComponent', () => { uuid: 'author2' }); + relationships[0].leftItem = observableOf(new RemoteData(false, false, true, undefined, author1)); + relationships[0].rightItem = observableOf(new RemoteData(false, false, true, undefined, item)); + relationships[1].leftItem = observableOf(new RemoteData(false, false, true, undefined, author2)); + relationships[1].rightItem = observableOf(new RemoteData(false, false, true, undefined, item)); + fieldUpdate1 = { field: author1, changeType: undefined @@ -155,7 +156,8 @@ describe('ItemRelationshipsComponent', () => { getRelatedItems: observableOf([author1, author2]), getRelatedItemsByLabel: observableOf([author1, author2]), getItemRelationshipsArray: observableOf(relationships), - deleteRelationship: observableOf(new RestResponse(true, 200, 'OK')) + deleteRelationship: observableOf(new RestResponse(true, 200, 'OK')), + getItemResolvedRelatedItemsAndRelationships: observableCombineLatest(observableOf([author1, author2]), observableOf([item, item]), observableOf(relationships)) } ); From 417f79ab6a8108f6f18745cf25cbff3c8625d1db Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 27 Jun 2019 11:55:10 +0200 Subject: [PATCH 19/25] 63184: Relationship list refreshing fix - Re-initializing itemUpdateSubscription --- .../item-relationships/item-relationships.component.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts index 36b2e212ba..087dea656b 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -66,8 +66,13 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent impl ngOnInit(): void { super.ngOnInit(); this.relationLabels$ = this.relationshipService.getItemRelationshipLabels(this.item); + this.initializeItemUpdate(); + } - // Update the item (and view) when it's removed in the request cache + /** + * Update the item (and view) when it's removed in the request cache + */ + public initializeItemUpdate(): void { this.itemUpdateSubscription = this.requestService.hasByHrefObservable(this.item.self).pipe( filter((exists: boolean) => !exists), switchMap(() => this.itemService.findById(this.item.uuid)), @@ -144,11 +149,12 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent impl } /** - * Reset the state of editing relationships + * Re-initialize fields and subscriptions */ reset() { this.initializeOriginalFields(); this.initializeUpdates(); + this.initializeItemUpdate(); } /** From d734ed108ef960283672ac27dd0a0fb7f2edf946 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 4 Jul 2019 17:52:14 +0200 Subject: [PATCH 20/25] 63469: Intermediate commit --- .../item-relationships.component.ts | 4 --- src/app/core/data/relationship.service.ts | 25 +++++++++++++++++-- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts index 087dea656b..e8f34bc70e 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -140,10 +140,6 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent impl this.notificationsService.error(this.getNotificationTitle('failed'), response.errorMessage); }); if (successfulResponses.length > 0) { - // Remove the item's cache to make sure the lists are reloaded with the newest values - this.objectCache.remove(this.item.self); - this.requestService.removeByHrefSubstring(this.item.self); - // Send a notification that the removal was successful this.notificationsService.success(this.getNotificationTitle('saved'), this.getNotificationContent('saved')); } } diff --git a/src/app/core/data/relationship.service.ts b/src/app/core/data/relationship.service.ts index fca5074a88..735674afc3 100644 --- a/src/app/core/data/relationship.service.ts +++ b/src/app/core/data/relationship.service.ts @@ -25,6 +25,7 @@ import { compareArraysUsingIds, filterRelationsByTypeLabel, relationsToItems } from '../../+item-page/simple/item-types/shared/item-relationships-utils'; +import { ObjectCacheService } from '../cache/object-cache.service'; /** * The service handling all relationship requests @@ -36,7 +37,8 @@ export class RelationshipService { constructor(protected requestService: RequestService, protected halService: HALEndpointService, protected rdbService: RemoteDataBuildService, - protected itemService: ItemDataService) { + protected itemService: ItemDataService, + protected objectCache: ObjectCacheService) { } /** @@ -49,6 +51,11 @@ export class RelationshipService { ); } + findById(uuid: string): Observable> { + const href$ = this.getRelationshipEndpoint(uuid); + return this.rdbService.buildSingle(href$); + } + /** * Send a delete request for a relationship by ID * @param uuid @@ -60,7 +67,8 @@ export class RelationshipService { map((endpointURL: string) => new DeleteRequest(this.requestService.generateRequestId(), endpointURL)), configureRequest(this.requestService), switchMap((restRequest: RestRequest) => this.requestService.getByUUID(restRequest.uuid)), - getResponseFromEntry() + getResponseFromEntry(), + tap(() => this.clearRelatedCache(uuid)) ); } @@ -203,4 +211,17 @@ export class RelationshipService { ); } + clearRelatedCache(uuid: string) { + this.findById(uuid).pipe( + getSucceededRemoteData(), + flatMap((rd: RemoteData) => observableCombineLatest(rd.payload.leftItem.pipe(getSucceededRemoteData()), rd.payload.rightItem.pipe(getSucceededRemoteData()))), + take(1) + ).subscribe(([leftItem, rightItem]) => { + this.objectCache.remove(leftItem.payload.self); + this.objectCache.remove(rightItem.payload.self); + this.requestService.removeByHrefSubstring(leftItem.payload.self); + this.requestService.removeByHrefSubstring(rightItem.payload.self); + }); + } + } From 8265942f18349bd8c6cb4d358f03a7209c9c968f Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Mon, 8 Jul 2019 13:41:20 +0200 Subject: [PATCH 21/25] 63469: Properly reload de-cached items --- .../+search-page/search-service/search.service.ts | 13 +++++++------ src/app/core/shared/operators.ts | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/app/+search-page/search-service/search.service.ts b/src/app/+search-page/search-service/search.service.ts index 598657a1b2..79164b50c3 100644 --- a/src/app/+search-page/search-service/search.service.ts +++ b/src/app/+search-page/search-service/search.service.ts @@ -1,7 +1,7 @@ -import { combineLatest as observableCombineLatest, Observable, of as observableOf } from 'rxjs'; +import { combineLatest as observableCombineLatest, Observable, of as observableOf, zip as observableZip } from 'rxjs'; import { Injectable, OnDestroy } from '@angular/core'; import { NavigationExtras, PRIMARY_OUTLET, Router, UrlSegmentGroup } from '@angular/router'; -import { first, map, switchMap } from 'rxjs/operators'; +import { first, map, switchMap, tap } from 'rxjs/operators'; import { RemoteDataBuildService } from '../../core/cache/builders/remote-data-build.service'; import { FacetConfigSuccessResponse, @@ -23,7 +23,7 @@ import { getSucceededRemoteData } from '../../core/shared/operators'; import { URLCombiner } from '../../core/url-combiner/url-combiner'; -import { hasValue, isEmpty, isNotEmpty, isNotUndefined } from '../../shared/empty.util'; +import { hasValue, hasValueOperator, isEmpty, isNotEmpty, isNotUndefined } from '../../shared/empty.util'; import { NormalizedSearchResult } from '../normalized-search-result.model'; import { SearchOptions } from '../search-options.model'; import { SearchResult } from '../search-result.model'; @@ -137,10 +137,11 @@ export class SearchService implements OnDestroy { map((sqr: SearchQueryResponse) => { return sqr.objects .filter((nsr: NormalizedSearchResult) => isNotUndefined(nsr.indexableObject)) - .map((nsr: NormalizedSearchResult) => { - return this.rdb.buildSingle(nsr.indexableObject); - }) + .map((nsr: NormalizedSearchResult) => new GetRequest(this.requestService.generateRequestId(), nsr.indexableObject)) }), + // Send a request for each item to ensure fresh cache + tap((reqs: RestRequest[]) => reqs.forEach((req: RestRequest) => this.requestService.configure(req))), + map((reqs: RestRequest[]) => reqs.map((req: RestRequest) => this.rdb.buildSingle(req.href))), switchMap((input: Array>>) => this.rdb.aggregate(input)), ); diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index ae46691e39..d46c688e68 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -91,7 +91,7 @@ export const toDSpaceObjectListRD = () => source.pipe( filter((rd: RemoteData>>) => rd.hasSucceeded), map((rd: RemoteData>>) => { - const dsoPage: T[] = rd.payload.page.map((searchResult: SearchResult) => searchResult.indexableObject); + const dsoPage: T[] = rd.payload.page.filter((result) => hasValue(result)).map((searchResult: SearchResult) => searchResult.indexableObject); const payload = Object.assign(rd.payload, { page: dsoPage }) as PaginatedList; return Object.assign(rd, { payload: payload }); }) From 34e75a46e5919f4aaa436e5c5c49b9b6a8cfc889 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Mon, 8 Jul 2019 14:30:19 +0200 Subject: [PATCH 22/25] 63469: JSDocs + tests --- .../item-relationships.component.spec.ts | 4 +- .../core/data/relationship.service.spec.ts | 61 +++++++++++++------ src/app/core/data/relationship.service.ts | 8 +++ 3 files changed, 50 insertions(+), 23 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts index 1d3d59201b..51394ef9e5 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts @@ -227,10 +227,8 @@ describe('ItemRelationshipsComponent', () => { comp.submit(); }); - it('it should delete the correct relationship and de-cache the current item', () => { + it('it should delete the correct relationship', () => { expect(relationshipService.deleteRelationship).toHaveBeenCalledWith(relationships[1].uuid); - expect(objectCache.remove).toHaveBeenCalledWith(item.self); - expect(requestService.removeByHrefSubstring).toHaveBeenCalledWith(item.self); }); }); }); diff --git a/src/app/core/data/relationship.service.spec.ts b/src/app/core/data/relationship.service.spec.ts index ce2b169eef..88da4a5496 100644 --- a/src/app/core/data/relationship.service.spec.ts +++ b/src/app/core/data/relationship.service.spec.ts @@ -13,6 +13,8 @@ import { Item } from '../shared/item.model'; import { PaginatedList } from './paginated-list'; import { PageInfo } from '../shared/page-info.model'; import { DeleteRequest } from './request.models'; +import { ObjectCacheService } from '../cache/object-cache.service'; +import { Observable } from 'rxjs/internal/Observable'; describe('RelationshipService', () => { let service: RelationshipService; @@ -22,6 +24,11 @@ describe('RelationshipService', () => { const relationshipsEndpointURL = `${restEndpointURL}/relationships`; const halService: any = new HALEndpointServiceStub(restEndpointURL); const rdbService = getMockRemoteDataBuildService(); + const objectCache = Object.assign({ + /* tslint:disable:no-empty */ + remove: () => {} + /* tslint:enable:no-empty */ + }) as ObjectCacheService; const relationshipType = Object.assign(new RelationshipType(), { type: ResourceType.RelationshipType, @@ -31,24 +38,20 @@ describe('RelationshipService', () => { rightLabel: 'isPublicationOfAuthor' }); - const relationships = [ - Object.assign(new Relationship(), { - self: relationshipsEndpointURL + '/2', - id: '2', - uuid: '2', - leftId: 'author1', - rightId: 'publication', - relationshipType: observableOf(new RemoteData(false, false, true, undefined, relationshipType)) - }), - Object.assign(new Relationship(), { - self: relationshipsEndpointURL + '/3', - id: '3', - uuid: '3', - leftId: 'author2', - rightId: 'publication', - relationshipType: observableOf(new RemoteData(false, false, true, undefined, relationshipType)) - }) - ]; + const relationship1 = Object.assign(new Relationship(), { + self: relationshipsEndpointURL + '/2', + id: '2', + uuid: '2', + relationshipType: observableOf(new RemoteData(false, false, true, undefined, relationshipType)) + }); + const relationship2 = Object.assign(new Relationship(), { + self: relationshipsEndpointURL + '/3', + id: '3', + uuid: '3', + relationshipType: observableOf(new RemoteData(false, false, true, undefined, relationshipType)) + }); + + const relationships = [ relationship1, relationship2 ]; const item = Object.assign(new Item(), { self: 'fake-item-url/publication', @@ -65,6 +68,10 @@ describe('RelationshipService', () => { id: 'author2', uuid: 'author2' }); + relationship1.leftItem = getRemotedataObservable(relatedItem1); + relationship1.rightItem = getRemotedataObservable(item); + relationship2.leftItem = getRemotedataObservable(relatedItem2); + relationship2.rightItem = getRemotedataObservable(item); const relatedItems = [relatedItem1, relatedItem2]; const itemService = jasmine.createSpyObj('itemService', { @@ -76,7 +83,8 @@ describe('RelationshipService', () => { requestService, halService, rdbService, - itemService + itemService, + objectCache ); } @@ -93,13 +101,22 @@ describe('RelationshipService', () => { describe('deleteRelationship', () => { beforeEach(() => { + spyOn(service, 'findById').and.returnValue(getRemotedataObservable(relationship1)); + spyOn(objectCache, 'remove'); service.deleteRelationship(relationships[0].uuid).subscribe(); }); it('should send a DeleteRequest', () => { - const expected = new DeleteRequest(requestService.generateRequestId(), relationshipsEndpointURL + '/' + relationships[0].uuid); + const expected = new DeleteRequest(requestService.generateRequestId(), relationshipsEndpointURL + '/' + relationship1.uuid); expect(requestService.configure).toHaveBeenCalledWith(expected, undefined); }); + + it('should clear the related items their cache', () => { + expect(objectCache.remove).toHaveBeenCalledWith(relatedItem1.self); + expect(objectCache.remove).toHaveBeenCalledWith(item.self); + expect(requestService.removeByHrefSubstring).toHaveBeenCalledWith(relatedItem1.self); + expect(requestService.removeByHrefSubstring).toHaveBeenCalledWith(item.self); + }); }); describe('getItemRelationshipsArray', () => { @@ -135,3 +152,7 @@ describe('RelationshipService', () => { }) }); + +function getRemotedataObservable(obj: any): Observable> { + return observableOf(new RemoteData(false, false, true, undefined, obj)); +} diff --git a/src/app/core/data/relationship.service.ts b/src/app/core/data/relationship.service.ts index 735674afc3..1699b6a27d 100644 --- a/src/app/core/data/relationship.service.ts +++ b/src/app/core/data/relationship.service.ts @@ -51,6 +51,10 @@ export class RelationshipService { ); } + /** + * Find a relationship by its UUID + * @param uuid + */ findById(uuid: string): Observable> { const href$ = this.getRelationshipEndpoint(uuid); return this.rdbService.buildSingle(href$); @@ -211,6 +215,10 @@ export class RelationshipService { ); } + /** + * Clear object and request caches of the items related to a relationship (left and right items) + * @param uuid + */ clearRelatedCache(uuid: string) { this.findById(uuid).pipe( getSucceededRemoteData(), From e05f44d079f406cad7fbc31c5bc4e14bde143e72 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 13 Aug 2019 12:30:47 +0200 Subject: [PATCH 23/25] 64225: Import fix --- .../edit-item-page/item-metadata/item-metadata.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.ts b/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.ts index dbbcebfd00..be657d71dc 100644 --- a/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.ts +++ b/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.ts @@ -15,10 +15,10 @@ import { NotificationsService } from '../../../shared/notifications/notification import { GLOBAL_CONFIG, GlobalConfig } from '../../../../config'; import { TranslateService } from '@ngx-translate/core'; import { RegistryService } from '../../../core/registry/registry.service'; -import { MetadataField } from '../../../core/metadata/metadatafield.model'; import { MetadatumViewModel } from '../../../core/shared/metadata.models'; import { Metadata } from '../../../core/shared/metadata.utils'; import { AbstractItemUpdateComponent } from '../abstract-item-update/abstract-item-update.component'; +import { MetadataField } from '../../../core/metadata/metadata-field.model'; @Component({ selector: 'ds-item-metadata', From fc21dc2019ee79f887b81b0006b1a1f238161e18 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 13 Aug 2019 14:46:10 +0200 Subject: [PATCH 24/25] 64225: Search results reloading correctly after object deletion --- src/app/+search-page/search-page.component.ts | 2 +- .../search-service/search.service.ts | 24 +++++++++++++++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/app/+search-page/search-page.component.ts b/src/app/+search-page/search-page.component.ts index 03433d1da1..c268c5b7f6 100644 --- a/src/app/+search-page/search-page.component.ts +++ b/src/app/+search-page/search-page.component.ts @@ -109,7 +109,7 @@ export class SearchPageComponent implements OnInit { ngOnInit(): void { this.searchOptions$ = this.getSearchOptions(); this.sub = this.searchOptions$.pipe( - switchMap((options) => this.service.search(options).pipe(getSucceededRemoteData(), startWith(observableOf(undefined))))) + switchMap((options) => this.service.search(options).pipe(getSucceededRemoteData(), startWith(undefined)))) .subscribe((results) => { this.resultsRD$.next(results); }); diff --git a/src/app/+search-page/search-service/search.service.ts b/src/app/+search-page/search-service/search.service.ts index 4e67fda864..be95ed096e 100644 --- a/src/app/+search-page/search-service/search.service.ts +++ b/src/app/+search-page/search-service/search.service.ts @@ -103,11 +103,18 @@ export class SearchService implements OnDestroy { * @returns {Observable>>>} Emits a paginated list with all search results found */ search(searchOptions?: PaginatedSearchOptions): Observable>>> { - const requestObs = this.halService.getEndpoint(this.searchLinkPath).pipe( + const hrefObs = this.halService.getEndpoint(this.searchLinkPath).pipe( map((url: string) => { if (hasValue(searchOptions)) { - url = (searchOptions as PaginatedSearchOptions).toRestUrl(url); + return (searchOptions as PaginatedSearchOptions).toRestUrl(url); + } else { + return url; } + }) + ); + + const requestObs = hrefObs.pipe( + map((url: string) => { const request = new this.request(this.requestService.generateRequestId(), url); const getResponseParserFn: () => GenericConstructor = () => { @@ -169,11 +176,20 @@ export class SearchService implements OnDestroy { const payloadObs = observableCombineLatest(tDomainListObs, pageInfoObs).pipe( map(([tDomainList, pageInfo]) => { - return new PaginatedList(pageInfo, tDomainList); + return new PaginatedList(pageInfo, tDomainList.filter((obj) => hasValue(obj))); }) ); - return this.rdb.toRemoteDataObservable(requestEntryObs, payloadObs); + return observableCombineLatest(hrefObs, tDomainListObs, requestEntryObs).pipe( + switchMap(([href, tDomainList, requestEntry]) => { + if (tDomainList.indexOf(undefined) > -1 && requestEntry && requestEntry.completed) { + this.requestService.removeByHrefSubstring(href); + return this.search(searchOptions) + } else { + return this.rdb.toRemoteDataObservable(requestEntryObs, payloadObs); + } + }) + ); } /** From 7d0439b006c259056f359f2ac111d2baa7534c06 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 14 Aug 2019 16:00:26 +0200 Subject: [PATCH 25/25] 64225: Fixed AoT build errors --- .../edit-relationship-list.component.spec.ts | 1 - .../edit-relationship/edit-relationship.component.spec.ts | 1 - .../item-relationships/item-relationships.component.spec.ts | 1 - src/app/core/data/relationship.service.spec.ts | 1 - 4 files changed, 4 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts index bd5f5f2e5c..3748ebca9d 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts @@ -36,7 +36,6 @@ let relationshipType; describe('EditRelationshipListComponent', () => { beforeEach(async(() => { relationshipType = Object.assign(new RelationshipType(), { - type: ResourceType.RelationshipType, id: '1', uuid: '1', leftLabel: 'isAuthorOfPublication', diff --git a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.spec.ts b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.spec.ts index fc6c999a1c..3306d8eb01 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.spec.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/edit-relationship/edit-relationship.component.spec.ts @@ -32,7 +32,6 @@ let el; describe('EditRelationshipComponent', () => { beforeEach(async(() => { relationshipType = Object.assign(new RelationshipType(), { - type: ResourceType.RelationshipType, id: '1', uuid: '1', leftLabel: 'isAuthorOfPublication', diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts index 51394ef9e5..b1a4e11371 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts @@ -66,7 +66,6 @@ describe('ItemRelationshipsComponent', () => { const date = new Date(); relationshipType = Object.assign(new RelationshipType(), { - type: ResourceType.RelationshipType, id: '1', uuid: '1', leftLabel: 'isAuthorOfPublication', diff --git a/src/app/core/data/relationship.service.spec.ts b/src/app/core/data/relationship.service.spec.ts index 88da4a5496..0ced517d74 100644 --- a/src/app/core/data/relationship.service.spec.ts +++ b/src/app/core/data/relationship.service.spec.ts @@ -31,7 +31,6 @@ describe('RelationshipService', () => { }) as ObjectCacheService; const relationshipType = Object.assign(new RelationshipType(), { - type: ResourceType.RelationshipType, id: '1', uuid: '1', leftLabel: 'isAuthorOfPublication',