71380: drag-and-drop-list customOrder to avoid elements hopping back after drop + freeze fix

src/app/+item-page/edit-item-page/edit-item-page.module.ts

@@ -32,6 +32,7 @@ import { ItemVersionHistoryComponent } from './item-version-history/item-version
 import { ItemAuthorizationsComponent } from './item-authorizations/item-authorizations.component';
 import { ResourcePolicyEditComponent } from '../../shared/resource-policies/edit/resource-policy-edit.component';
 import { ResourcePolicyCreateComponent } from '../../shared/resource-policies/create/resource-policy-create.component';
+import { ObjectValuesPipe } from '../../shared/utils/object-values-pipe';
 
 /**
  * Module that contains all components related to the Edit Item page administrator functionality
@@ -75,7 +76,8 @@ import { ResourcePolicyCreateComponent } from '../../shared/resource-policies/cr
     ResourcePolicyCreateComponent,
   ],
   providers: [
-    BundleDataService
+    BundleDataService,
+    ObjectValuesPipe
   ]
 })
 export class EditItemPageModule {

src/app/+item-page/edit-item-page/item-bitstreams/item-bitstreams.component.ts

@@ -1,4 +1,4 @@
-import { ChangeDetectorRef, Component, OnDestroy } from '@angular/core';
+import { ChangeDetectorRef, Component, NgZone, OnDestroy } from '@angular/core';
 import { AbstractItemUpdateComponent } from '../abstract-item-update/abstract-item-update.component';
 import { filter, map, switchMap, take } from 'rxjs/operators';
 import { Observable } from 'rxjs/internal/Observable';
@@ -88,7 +88,8 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme
     public objectCache: ObjectCacheService,
     public requestService: RequestService,
     public cdRef: ChangeDetectorRef,
-    public bundleService: BundleDataService
+    public bundleService: BundleDataService,
+    public zone: NgZone
   ) {
     super(itemService, objectUpdatesService, router, notificationsService, translateService, route);
   }
@@ -187,6 +188,7 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme
    * @param event   The event containing the index the bitstream came from and was dropped to
    */
   dropBitstream(bundle: Bundle, event: any) {
+    this.zone.runOutsideAngular(() => {
       if (hasValue(event) && hasValue(event.fromIndex) && hasValue(event.toIndex) && hasValue(event.finish)) {
         const moveOperation = Object.assign({
           op: 'move',
@@ -196,9 +198,10 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme
         this.bundleService.patch(bundle, [moveOperation]).pipe(take(1)).subscribe((response: RestResponse) => {
           this.displayNotifications('item.edit.bitstreams.notifications.move', [response]);
           this.requestService.removeByHrefSubstring(bundle.self);
-        event.finish();
+          this.zone.run(() => event.finish());
         });
       }
+    });
   }
 
   /**

src/app/+item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/paginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component.html

@@ -7,18 +7,20 @@
                [collectionSize]="(objectsRD$ | async)?.payload?.totalElements"
                [disableRouteParameterUpdate]="true"
                (pageChange)="switchPage($event)">
+  <ng-container *ngIf="!(loading$ | async)">
     <div [id]="bundle.id" class="bundle-bitstreams-list"
          [ngClass]="{'mb-3': (objectsRD$ | async)?.payload?.totalElements > pageSize}"
-       *ngVar="((updates$ | async) | dsObjectValues) as updateValues" cdkDropList (cdkDropListDropped)="drop($event)">
+         *ngVar="(updates$ | async) as updates" cdkDropList (cdkDropListDropped)="drop($event)">
-    <div class="row bitstream-row" *ngFor="let updateValue of updateValues" cdkDrag
+      <ng-container *ngIf="updates">
-         [id]="updateValue.field.uuid"
+        <div class="row bitstream-row" *ngFor="let uuid of customOrder" cdkDrag
+             [id]="uuid"
              [ngClass]="{
-                  'table-warning': updateValue.changeType === 0,
+                  'table-warning': updates[uuid].changeType === 0,
-                  'table-danger': updateValue.changeType === 2,
+                  'table-danger': updates[uuid].changeType === 2,
-                  'table-success': updateValue.changeType === 1,
+                  'table-success': updates[uuid].changeType === 1,
-                  'bg-white': updateValue.changeType === undefined
+                  'bg-white': updates[uuid].changeType === undefined
                 }">
-      <ds-item-edit-bitstream [fieldUpdate]="updateValue"
+          <ds-item-edit-bitstream [fieldUpdate]="updates[uuid]"
                                   [bundleUrl]="bundle.self"
                                   [columnSizes]="columnSizes">
             <div class="d-flex align-items-center" slot="drag-handle" cdkDragHandle>
@@ -26,5 +28,8 @@
             </div>
           </ds-item-edit-bitstream>
         </div>
+      </ng-container>
     </div>
+  </ng-container>
+  <ds-loading *ngIf="(loading$ | async)" [message]="'loading.bitstreams' | translate"></ds-loading>
 </ds-pagination>

src/app/+item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/paginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component.spec.ts

@@ -22,6 +22,7 @@ describe('PaginatedDragAndDropBitstreamListComponent', () => {
   let fixture: ComponentFixture<PaginatedDragAndDropBitstreamListComponent>;
   let objectUpdatesService: ObjectUpdatesService;
   let bundleService: BundleDataService;
+  let objectValuesPipe: ObjectValuesPipe;
 
   const columnSizes = new ResponsiveTableSizes([
     new ResponsiveColumnSizes(2, 2, 3, 4, 4),
@@ -100,12 +101,15 @@ describe('PaginatedDragAndDropBitstreamListComponent', () => {
       getBitstreams: createSuccessfulRemoteDataObject$(createPaginatedList([bitstream1, bitstream2]))
     });
 
+    objectValuesPipe = new ObjectValuesPipe();
+
     TestBed.configureTestingModule({
       imports: [TranslateModule.forRoot()],
-      declarations: [PaginatedDragAndDropBitstreamListComponent, VarDirective, ObjectValuesPipe],
+      declarations: [PaginatedDragAndDropBitstreamListComponent, VarDirective],
       providers: [
         { provide: ObjectUpdatesService, useValue: objectUpdatesService },
-        { provide: BundleDataService, useValue: bundleService }
+        { provide: BundleDataService, useValue: bundleService },
+        { provide: ObjectValuesPipe, useValue: objectValuesPipe }
       ], schemas: [
         NO_ERRORS_SCHEMA
       ]

src/app/+item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/paginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component.ts

@@ -8,6 +8,7 @@ import { switchMap } from 'rxjs/operators';
 import { PaginatedSearchOptions } from '../../../../../shared/search/paginated-search-options.model';
 import { ResponsiveTableSizes } from '../../../../../shared/responsive-table-sizes/responsive-table-sizes';
 import { followLink } from '../../../../../shared/utils/follow-link-config.model';
+import { ObjectValuesPipe } from '../../../../../shared/utils/object-values-pipe';
 
 @Component({
   selector: 'ds-paginated-drag-and-drop-bitstream-list',
@@ -33,8 +34,9 @@ export class PaginatedDragAndDropBitstreamListComponent extends AbstractPaginate
 
   constructor(protected objectUpdatesService: ObjectUpdatesService,
               protected elRef: ElementRef,
+              protected objectValuesPipe: ObjectValuesPipe,
               protected bundleService: BundleDataService) {
-    super(objectUpdatesService, elRef);
+    super(objectUpdatesService, elRef, objectValuesPipe);
   }
 
   ngOnInit() {

src/app/core/data/object-updates/object-updates.service.ts

@@ -24,7 +24,7 @@ import {
   SetValidFieldUpdateAction
 } from './object-updates.actions';
 import { distinctUntilChanged, filter, map, switchMap } from 'rxjs/operators';
-import { hasNoValue, hasValue, isEmpty, isNotEmpty } from '../../../shared/empty.util';
+import { hasNoValue, hasValue, isEmpty, isNotEmpty, isNotEmptyOperator } from '../../../shared/empty.util';
 import { INotification } from '../../../shared/notifications/models/notification.model';
 
 function objectUpdatesStateSelector(): MemoizedSelector<CoreState, ObjectUpdatesState> {
@@ -125,7 +125,7 @@ export class ObjectUpdatesService {
    */
   getFieldUpdatesExclusive(url: string, initialFields: Identifiable[]): Observable<FieldUpdates> {
     const objectUpdates = this.getObjectEntry(url);
-    return objectUpdates.pipe(map((objectEntry) => {
+    return objectUpdates.pipe(isNotEmptyOperator(), map((objectEntry) => {
       const fieldUpdates: FieldUpdates = {};
       for (const object of initialFields) {
         let fieldUpdate = objectEntry.fieldUpdates[object.uuid];

src/app/shared/pagination-drag-and-drop/abstract-paginated-drag-and-drop-list.component.spec.ts

@@ -12,14 +12,16 @@ import { take } from 'rxjs/operators';
 import { PaginationComponent } from '../pagination/pagination.component';
 import { createSuccessfulRemoteDataObject } from '../remote-data.utils';
 import { createPaginatedList } from '../testing/utils.test';
+import { ObjectValuesPipe } from '../utils/object-values-pipe';
 
 class MockAbstractPaginatedDragAndDropListComponent extends AbstractPaginatedDragAndDropListComponent<DSpaceObject> {
 
   constructor(protected objectUpdatesService: ObjectUpdatesService,
               protected elRef: ElementRef,
+              protected objectValuesPipe: ObjectValuesPipe,
               protected mockUrl: string,
               protected mockObjectsRD$: Observable<RemoteData<PaginatedList<DSpaceObject>>>) {
-    super(objectUpdatesService, elRef);
+    super(objectUpdatesService, elRef, objectValuesPipe);
   }
 
   initializeObjectsRD(): void {
@@ -35,6 +37,7 @@ describe('AbstractPaginatedDragAndDropListComponent', () => {
   let component: MockAbstractPaginatedDragAndDropListComponent;
   let objectUpdatesService: ObjectUpdatesService;
   let elRef: ElementRef;
+  let objectValuesPipe: ObjectValuesPipe;
 
   const url = 'mock-abstract-paginated-drag-and-drop-list-component';
 
@@ -60,11 +63,12 @@ describe('AbstractPaginatedDragAndDropListComponent', () => {
         querySelector: {}
       })
     };
+    objectValuesPipe = new ObjectValuesPipe();
     paginationComponent = jasmine.createSpyObj('paginationComponent', {
       doPageChange: {}
     });
     objectsRD$ = new BehaviorSubject(objectsRD);
-    component = new MockAbstractPaginatedDragAndDropListComponent(objectUpdatesService, elRef, url, objectsRD$);
+    component = new MockAbstractPaginatedDragAndDropListComponent(objectUpdatesService, elRef, objectValuesPipe, url, objectsRD$);
     component.paginationComponent = paginationComponent;
     component.ngOnInit();
   });

src/app/shared/pagination-drag-and-drop/abstract-paginated-drag-and-drop-list.component.ts

@@ -1,17 +1,26 @@
-import { FieldUpdates } from '../../core/data/object-updates/object-updates.reducer';
+import { FieldUpdate, FieldUpdates, Identifiable } from '../../core/data/object-updates/object-updates.reducer';
 import { Observable } from 'rxjs/internal/Observable';
 import { RemoteData } from '../../core/data/remote-data';
 import { PaginatedList } from '../../core/data/paginated-list';
 import { PaginationComponentOptions } from '../pagination/pagination-component-options.model';
 import { BehaviorSubject } from 'rxjs/internal/BehaviorSubject';
 import { ObjectUpdatesService } from '../../core/data/object-updates/object-updates.service';
-import { switchMap, take } from 'rxjs/operators';
+import { distinctUntilChanged, map, switchMap, take } from 'rxjs/operators';
 import { hasValue } from '../empty.util';
 import { paginatedListToArray } from '../../core/shared/operators';
 import { DSpaceObject } from '../../core/shared/dspace-object.model';
-import { CdkDragDrop } from '@angular/cdk/drag-drop';
+import { CdkDragDrop, moveItemInArray } from '@angular/cdk/drag-drop';
-import { ElementRef, EventEmitter, Output, ViewChild } from '@angular/core';
+import { ElementRef, EventEmitter, OnDestroy, Output, ViewChild } from '@angular/core';
 import { PaginationComponent } from '../pagination/pagination.component';
+import { ObjectValuesPipe } from '../utils/object-values-pipe';
+import { compareArraysUsing } from '../../+item-page/simple/item-types/shared/item-relationships-utils';
+import { Subscription } from 'rxjs/internal/Subscription';
+
+/**
+ * Operator used for comparing {@link FieldUpdate}s by their field's UUID
+ */
+export const compareArraysUsingFieldUuids = () =>
+  compareArraysUsing((fieldUpdate: FieldUpdate) => (hasValue(fieldUpdate) && hasValue(fieldUpdate.field)) ? fieldUpdate.field.uuid : undefined);
 
 /**
  * An abstract component containing general methods and logic to be able to drag and drop objects within a paginated
@@ -29,7 +38,7 @@ import { PaginationComponent } from '../pagination/pagination.component';
  *
  * An example component extending from this abstract component: PaginatedDragAndDropBitstreamListComponent
  */
-export abstract class AbstractPaginatedDragAndDropListComponent<T extends DSpaceObject> {
+export abstract class AbstractPaginatedDragAndDropListComponent<T extends DSpaceObject> implements OnDestroy {
   /**
    * A view on the child pagination component
    */
@@ -57,10 +66,16 @@ export abstract class AbstractPaginatedDragAndDropListComponent<T extends DSpace
    */
   updates$: Observable<FieldUpdates>;
 
+  /**
+   * A list of object UUIDs
+   * This is the order the objects will be displayed in
+   */
+  customOrder: string[];
+
   /**
    * The amount of objects to display per page
    */
-  pageSize = 10;
+  pageSize = 3;
 
   /**
    * The page options to use for fetching the objects
@@ -77,8 +92,21 @@ export abstract class AbstractPaginatedDragAndDropListComponent<T extends DSpace
    */
   currentPage$ = new BehaviorSubject<number>(1);
 
+  /**
+   * Whether or not we should display a loading animation
+   * This is used to display a loading page when the user drops a bitstream onto a new page. The loading animation
+   * should stop once the bitstream has moved to the new page and the new page's response has loaded
+   */
+  loading$: BehaviorSubject<boolean> = new BehaviorSubject<boolean>(false);
+
+  /**
+   * List of subscriptions
+   */
+  subs: Subscription[] = [];
+
   protected constructor(protected objectUpdatesService: ObjectUpdatesService,
-                        protected elRef: ElementRef) {
+                        protected elRef: ElementRef,
+                        protected objectValuesPipe: ObjectValuesPipe) {
   }
 
   /**
@@ -114,6 +142,14 @@ export abstract class AbstractPaginatedDragAndDropListComponent<T extends DSpace
       paginatedListToArray(),
       switchMap((objects: T[]) => this.objectUpdatesService.getFieldUpdatesExclusive(this.url, objects))
     );
+    this.subs.push(
+      this.updates$.pipe(
+        map((fieldUpdates) => this.objectValuesPipe.transform(fieldUpdates)),
+        distinctUntilChanged(compareArraysUsingFieldUuids())
+      ).subscribe((updateValues) => {
+        this.customOrder = updateValues.map((fieldUpdate) => fieldUpdate.field.uuid);
+      })
+    );
   }
 
   /**
@@ -148,19 +184,40 @@ export abstract class AbstractPaginatedDragAndDropListComponent<T extends DSpace
       }
     }
 
+    const isNewPage = dragPage !== dropPage;
+    // Move the object in the custom order array if the drop happened within the same page
+    // This allows us to instantly display a change in the order, instead of waiting for the REST API's response first
+    if (!isNewPage && dragIndex !== dropIndex) {
+      moveItemInArray(this.customOrder, dragIndex, dropIndex);
+    }
+
     const redirectPage = dropPage + 1;
     const fromIndex = (dragPage * this.pageSize) + dragIndex;
     const toIndex = (dropPage * this.pageSize) + dropIndex;
-    // Send out a drop event when the field exists and the "from" and "to" indexes are different from each other
+    // Send out a drop event (and navigate to the new page) when the "from" and "to" indexes are different from each other
     if (fromIndex !== toIndex) {
+      if (isNewPage) {
+        this.customOrder = [];
+        this.paginationComponent.doPageChange(redirectPage);
+        this.loading$.next(true);
+      }
       this.dropObject.emit(Object.assign({
         fromIndex,
         toIndex,
         finish: () => {
+          if (isNewPage) {
             this.currentPage$.next(redirectPage);
-          this.paginationComponent.doPageChange(redirectPage);
+            this.loading$.next(false);
+          }
         }
       }));
     }
   }
+
+  /**
+   * unsub all subscriptions
+   */
+  ngOnDestroy(): void {
+    this.subs.filter((sub) => hasValue(sub)).forEach((sub) => sub.unsubscribe());
+  }
 }