Edit metadata redesign

src/app/collection-page/collection-page.module.ts

@@ -16,6 +16,7 @@ import { StatisticsModule } from '../statistics/statistics.module';
 import { CollectionFormModule } from './collection-form/collection-form.module';
 import { ThemedCollectionPageComponent } from './themed-collection-page.component';
 import { ComcolModule } from '../shared/comcol/comcol.module';
+import { DsoSharedModule } from '../dso-shared/dso-shared.module';
 
 @NgModule({
   imports: [
@@ -26,6 +27,7 @@ import { ComcolModule } from '../shared/comcol/comcol.module';
     EditItemPageModule,
     CollectionFormModule,
     ComcolModule,
+    DsoSharedModule,
   ],
   declarations: [
     CollectionPageComponent,

src/app/collection-page/edit-item-template-page/edit-item-template-page.component.html

@@ -3,7 +3,7 @@
     <div class="col-12" *ngVar="(itemRD$ | async) as itemRD">
       <ng-container *ngIf="itemRD?.hasSucceeded">
         <h2 class="border-bottom">{{ 'collection.edit.template.head' | translate:{ collection: collection?.name } }}</h2>
-        <ds-themed-item-metadata [updateService]="itemTemplateService" [item]="itemRD?.payload"></ds-themed-item-metadata>
+        <ds-themed-dso-edit-metadata [updateDataService]="itemTemplateService" [dso]="itemRD?.payload"></ds-themed-dso-edit-metadata>
         <button [routerLink]="getCollectionEditUrl(collection)" class="btn btn-outline-secondary">{{ 'collection.edit.template.cancel' | translate }}</button>
       </ng-container>
       <ds-themed-loading *ngIf="itemRD?.isLoading" [message]="'collection.edit.template.loading' | translate"></ds-themed-loading>

src/app/core/data/array-move-change-analyzer.service.spec.ts

@@ -41,21 +41,28 @@ describe('ArrayMoveChangeAnalyzer', () => {
     ], new MoveTest(0, 3));
 
     testMove([
+      { op: 'move', from: '/2', path: '/3' },
       { op: 'move', from: '/0', path: '/3' },
-      { op: 'move', from: '/2', path: '/1' }
     ], new MoveTest(0, 3), new MoveTest(1, 2));
 
     testMove([
+      { op: 'move', from: '/3', path: '/4' },
       { op: 'move', from: '/0', path: '/1' },
-      { op: 'move', from: '/3', path: '/4' }
     ], new MoveTest(0, 1), new MoveTest(3, 4));
 
     testMove([], new MoveTest(0, 4), new MoveTest(4, 0));
 
     testMove([
+      { op: 'move', from: '/2', path: '/3' },
       { op: 'move', from: '/0', path: '/3' },
-      { op: 'move', from: '/2', path: '/1' }
     ], new MoveTest(0, 4), new MoveTest(1, 3), new MoveTest(2, 4));
+
+    testMove([
+      { op: 'move', from: '/3', path: '/4' },
+      { op: 'move', from: '/2', path: '/4' },
+      { op: 'move', from: '/1', path: '/3' },
+      { op: 'move', from: '/0', path: '/3' },
+    ], new MoveTest(4, 1), new MoveTest(4, 2), new MoveTest(0, 3));
   });
 
   describe('when some values are undefined (index 2 and 3)', () => {

src/app/core/data/array-move-change-analyzer.service.ts

@@ -16,22 +16,31 @@ export class ArrayMoveChangeAnalyzer<T> {
    * @param array2  The custom array to compare with the original
    */
   diff(array1: T[], array2: T[]): MoveOperation[] {
-    const result = [];
-    const moved = [...array1];
-    array1.forEach((value: T, index: number) => {
-      if (hasValue(value)) {
-        const otherIndex = array2.indexOf(value);
-        const movedIndex = moved.indexOf(value);
-        if (index !== otherIndex && movedIndex !== otherIndex) {
-          moveItemInArray(moved, movedIndex, otherIndex);
-          result.push(Object.assign({
-            op: 'move',
-            from: '/' + movedIndex,
-            path: '/' + otherIndex
-          }) as MoveOperation);
-        }
+    return this.getMoves(array1, array2).map((move) => Object.assign({
+      op: 'move',
+      from: '/' + move[0],
+      path: '/' + move[1],
+    }) as MoveOperation);
+  }
+
+  /**
+   * Determine a set of moves required to transform array1 into array2
+   * The moves are returned as an array of pairs of numbers where the first number is the original index and the second
+   * is the new index
+   * It is assumed the operations are executed in the order they're returned (and not simultaneously)
+   * @param array1
+   * @param array2
+   */
+  private getMoves(array1: any[], array2: any[]): number[][] {
+    const moved = [...array2];
+
+    return array1.reduce((moves, item, index) => {
+      if (hasValue(item) && item !== moved[index]) {
+        const last = moved.lastIndexOf(item);
+        moveItemInArray(moved, last, index);
+        moves.unshift([index, last]);
       }
-    });
-    return result;
+      return moves;
+    }, []);
   }
 }

src/app/core/data/object-updates/patch-operation-service/operations/metadata/metadata-patch-move-operation.model.ts

@@ -0,0 +1,33 @@
+import { MetadataPatchOperation } from './metadata-patch-operation.model';
+import { Operation } from 'fast-json-patch';
+
+/**
+ * Wrapper object for a metadata patch move Operation
+ */
+export class MetadataPatchMoveOperation extends MetadataPatchOperation {
+  static operationType = 'move';
+
+  /**
+   * The original place of the metadata value to move
+   */
+  from: number;
+
+  /**
+   * The new place to move the metadata value to
+   */
+  to: number;
+
+  constructor(field: string, from: number, to: number) {
+    super(MetadataPatchMoveOperation.operationType, field);
+    this.from = from;
+    this.to = to;
+  }
+
+  /**
+   * Transform the MetadataPatchOperation into a fast-json-patch Operation by constructing its path and other properties
+   * using the information provided.
+   */
+  toOperation(): Operation {
+    return { op: this.op as any, from: `/metadata/${this.field}/${this.from}`, path: `/metadata/${this.field}/${this.to}` };
+  }
+}

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

@@ -10,13 +10,19 @@ import { DeleteRequest } from './request.models';
 import { RelationshipDataService } from './relationship-data.service';
 import { RequestService } from './request.service';
 import { HALEndpointServiceStub } from '../../shared/testing/hal-endpoint-service.stub';
-import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils';
+import {
+  createFailedRemoteDataObject$,
+  createSuccessfulRemoteDataObject,
+  createSuccessfulRemoteDataObject$
+} from '../../shared/remote-data.utils';
 import { getMockRemoteDataBuildServiceHrefMap } from '../../shared/mocks/remote-data-build.service.mock';
 import { getMockRequestService } from '../../shared/mocks/request.service.mock';
 import { createPaginatedList } from '../../shared/testing/utils.test';
 import { RequestEntry } from './request-entry.model';
 import { FindListOptions } from './find-list-options.model';
 import { testSearchDataImplementation } from './base/search-data.spec';
+import { MetadataValue } from '../shared/metadata.models';
+import { MetadataRepresentationType } from '../shared/metadata-representation/metadata-representation.model';
 
 describe('RelationshipDataService', () => {
   let service: RelationshipDataService;
@@ -233,4 +239,152 @@ describe('RelationshipDataService', () => {
       });
     });
   });
+
+  describe('resolveMetadataRepresentation', () => {
+    const parentItem: Item = Object.assign(new Item(), {
+      id: 'parent-item',
+      metadata: {
+        'dc.contributor.author': [
+          Object.assign(new MetadataValue(), {
+            language: null,
+            value: 'Related Author with authority',
+            authority: 'virtual::related-author',
+            place: 2
+          }),
+          Object.assign(new MetadataValue(), {
+            language: null,
+            value: 'Author without authority',
+            place: 1
+          }),
+        ],
+        'dc.creator': [
+          Object.assign(new MetadataValue(), {
+            language: null,
+            value: 'Related Creator with authority',
+            authority: 'virtual::related-creator',
+            place: 3,
+          }),
+            Object.assign(new MetadataValue(), {
+            language: null,
+            value: 'Related Creator with authority - unauthorized',
+            authority: 'virtual::related-creator-unauthorized',
+            place: 4,
+          }),
+        ],
+        'dc.title': [
+          Object.assign(new MetadataValue(), {
+            language: null,
+            value: 'Parent Item'
+          }),
+        ]
+      }
+    });
+    const relatedAuthor: Item = Object.assign(new Item(), {
+      id: 'related-author',
+      metadata: {
+        'dc.title': [
+          Object.assign(new MetadataValue(), {
+            language: null,
+            value: 'Related Author'
+          }),
+        ]
+      }
+    });
+    const relatedCreator: Item = Object.assign(new Item(), {
+      id: 'related-creator',
+      metadata: {
+        'dc.title': [
+          Object.assign(new MetadataValue(), {
+            language: null,
+            value: 'Related Creator'
+          }),
+        ],
+        'dspace.entity.type': 'Person',
+      }
+    });
+    const authorRelation: Relationship = Object.assign(new Relationship(), {
+      leftItem: createSuccessfulRemoteDataObject$(parentItem),
+      rightItem: createSuccessfulRemoteDataObject$(relatedAuthor)
+    });
+    const creatorRelation: Relationship = Object.assign(new Relationship(), {
+      leftItem: createSuccessfulRemoteDataObject$(parentItem),
+      rightItem: createSuccessfulRemoteDataObject$(relatedCreator),
+    });
+    const creatorRelationUnauthorized: Relationship = Object.assign(new Relationship(), {
+      leftItem: createSuccessfulRemoteDataObject$(parentItem),
+      rightItem: createFailedRemoteDataObject$('Unauthorized', 401),
+    });
+
+    let metadatum: MetadataValue;
+
+    beforeEach(() => {
+      service.findById = (id: string) => {
+        if (id === 'related-author') {
+          return createSuccessfulRemoteDataObject$(authorRelation);
+        }
+        if (id === 'related-creator') {
+          return createSuccessfulRemoteDataObject$(creatorRelation);
+        }
+        if (id === 'related-creator-unauthorized') {
+          return createSuccessfulRemoteDataObject$(creatorRelationUnauthorized);
+        }
+      };
+    });
+
+    describe('when the metadata isn\'t virtual', () => {
+      beforeEach(() => {
+        metadatum = parentItem.metadata['dc.contributor.author'][1];
+      });
+
+      it('should return a plain text MetadatumRepresentation', (done) => {
+        service.resolveMetadataRepresentation(metadatum, parentItem, 'Person').subscribe((result) => {
+          expect(result.representationType).toEqual(MetadataRepresentationType.PlainText);
+          done();
+        });
+      });
+    });
+
+    describe('when the metadata is a virtual author', () => {
+      beforeEach(() => {
+        metadatum = parentItem.metadata['dc.contributor.author'][0];
+      });
+
+      it('should return a ItemMetadataRepresentation with the correct value', (done) => {
+        service.resolveMetadataRepresentation(metadatum, parentItem, 'Person').subscribe((result) => {
+          expect(result.representationType).toEqual(MetadataRepresentationType.Item);
+          expect(result.getValue()).toEqual(metadatum.value);
+          expect((result as any).id).toEqual(relatedAuthor.id);
+          done();
+        });
+      });
+    });
+
+    describe('when the metadata is a virtual creator', () => {
+      beforeEach(() => {
+        metadatum = parentItem.metadata['dc.creator'][0];
+      });
+
+      it('should return a ItemMetadataRepresentation with the correct value', (done) => {
+        service.resolveMetadataRepresentation(metadatum, parentItem, 'Person').subscribe((result) => {
+          expect(result.representationType).toEqual(MetadataRepresentationType.Item);
+          expect(result.getValue()).toEqual(metadatum.value);
+          expect((result as any).id).toEqual(relatedCreator.id);
+          done();
+        });
+      });
+    });
+
+    describe('when the metadata refers to a relationship leading to an error response', () => {
+      beforeEach(() => {
+        metadatum = parentItem.metadata['dc.creator'][1];
+      });
+
+      it('should return an authority controlled MetadatumRepresentation', (done) => {
+        service.resolveMetadataRepresentation(metadatum, parentItem, 'Person').subscribe((result) => {
+          expect(result.representationType).toEqual(MetadataRepresentationType.AuthorityControlled);
+          done();
+        });
+      });
+    });
+  });
 });

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

@@ -1,7 +1,7 @@
 import { HttpHeaders } from '@angular/common/http';
 import { Inject, Injectable } from '@angular/core';
 import { MemoizedSelector, select, Store } from '@ngrx/store';
-import { combineLatest as observableCombineLatest, Observable } from 'rxjs';
+import { combineLatest as observableCombineLatest, Observable, of as observableOf } from 'rxjs';
 import { distinctUntilChanged, filter, map, mergeMap, startWith, switchMap, take, tap } from 'rxjs/operators';
 import {
   compareArraysUsingIds, PAGINATED_RELATIONS_TO_ITEMS_OPERATOR,
@@ -46,6 +46,11 @@ import { PutData, PutDataImpl } from './base/put-data';
 import { IdentifiableDataService } from './base/identifiable-data.service';
 import { dataService } from './base/data-service.decorator';
 import { itemLinksToFollow } from '../../shared/utils/relation-query.utils';
+import { MetadataValue } from '../shared/metadata.models';
+import { MetadataRepresentation } from '../shared/metadata-representation/metadata-representation.model';
+import { MetadatumRepresentation } from '../shared/metadata-representation/metadatum/metadatum-representation.model';
+import { ItemMetadataRepresentation } from '../shared/metadata-representation/item/item-metadata-representation.model';
+import { DSpaceObject } from '../shared/dspace-object.model';
 
 const relationshipListsStateSelector = (state: AppState) => state.relationshipLists;
 
@@ -550,4 +555,40 @@ export class RelationshipDataService extends IdentifiableDataService<Relationshi
   searchBy(searchMethod: string, options?: FindListOptions, useCachedVersionIfAvailable?: boolean, reRequestOnStale?: boolean, ...linksToFollow: FollowLinkConfig<Relationship>[]): Observable<RemoteData<PaginatedList<Relationship>>> {
     return this.searchData.searchBy(searchMethod, options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow);
   }
+
+  /**
+   * Resolve a {@link MetadataValue} into a {@link MetadataRepresentation} of the correct type
+   * @param metadatum   {@link MetadataValue} to resolve
+   * @param parentItem  Parent dspace object the metadata value belongs to
+   * @param itemType    The type of item this metadata value represents (will only be used when no related item can be found, as a fallback)
+   */
+  resolveMetadataRepresentation(metadatum: MetadataValue, parentItem: DSpaceObject, itemType: string): Observable<MetadataRepresentation> {
+    if (metadatum.isVirtual) {
+      return this.findById(metadatum.virtualValue, true, false, followLink('leftItem'), followLink('rightItem')).pipe(
+        getFirstSucceededRemoteData(),
+        switchMap((relRD: RemoteData<Relationship>) =>
+          observableCombineLatest(relRD.payload.leftItem, relRD.payload.rightItem).pipe(
+            filter(([leftItem, rightItem]) => leftItem.hasCompleted && rightItem.hasCompleted),
+            map(([leftItem, rightItem]) => {
+              if (!leftItem.hasSucceeded || !rightItem.hasSucceeded) {
+                return null;
+              } else if (rightItem.hasSucceeded && leftItem.payload.id === parentItem.id) {
+                return rightItem.payload;
+              } else if (rightItem.payload.id === parentItem.id) {
+                return leftItem.payload;
+              }
+            }),
+            map((item: Item) => {
+              if (hasValue(item)) {
+                return Object.assign(new ItemMetadataRepresentation(metadatum), item);
+              } else {
+                return Object.assign(new MetadatumRepresentation(itemType), metadatum);
+              }
+            })
+          )
+        ));
+    } else {
+      return observableOf(Object.assign(new MetadatumRepresentation(itemType), metadatum));
+    }
+  }
 }

src/app/core/shared/operators.ts

@@ -226,7 +226,7 @@ export const metadataFieldsToString = () =>
             map((schema: MetadataSchema) => ({ field, schema }))
           );
         });
-        return observableCombineLatest(fieldSchemaArray);
+        return isNotEmpty(fieldSchemaArray) ? observableCombineLatest(fieldSchemaArray) : [[]];
       }),
       map((fieldSchemaArray: { field: MetadataField, schema: MetadataSchema }[]): string[] => {
         return fieldSchemaArray.map((fieldSchema: { field: MetadataField, schema: MetadataSchema }) => fieldSchema.schema.prefix + '.' + fieldSchema.field.toString());

src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-field-values/dso-edit-metadata-field-values.component.html

@@ -0,0 +1,15 @@
+<div class="flex-grow-1 ds-drop-list h-100" [class.disabled]="(draggingMdField$ | async) && (draggingMdField$ | async) !== mdField" cdkDropList (cdkDropListDropped)="drop($event)" role="table">
+  <ds-dso-edit-metadata-value-headers role="presentation" [dsoType]="dsoType"></ds-dso-edit-metadata-value-headers>
+  <ds-dso-edit-metadata-value *ngFor="let mdValue of form.fields[mdField]; let idx = index" role="presentation"
+                              [dso]="dso"
+                              [mdValue]="mdValue"
+                              [dsoType]="dsoType"
+                              [saving$]="saving$"
+                              [isOnlyValue]="form.fields[mdField].length === 1"
+                              (edit)="mdValue.editing = true"
+                              (confirm)="mdValue.confirmChanges($event); form.resetReinstatable(); valueSaved.emit()"
+                              (remove)="mdValue.change === DsoEditMetadataChangeTypeEnum.ADD ? form.remove(mdField, idx) : mdValue.change = DsoEditMetadataChangeTypeEnum.REMOVE; form.resetReinstatable(); valueSaved.emit()"
+                              (undo)="mdValue.change === DsoEditMetadataChangeTypeEnum.ADD ? form.remove(mdField, idx) : mdValue.discard(); valueSaved.emit()"
+                              (dragging)="$event ? draggingMdField$.next(mdField) : draggingMdField$.next(null)">
+  </ds-dso-edit-metadata-value>
+</div>