From ad863b62a74447bd06da36c695d411ca37d8e3e4 Mon Sep 17 00:00:00 2001
From: Yana De Pauw <yana@atmire.com>
Date: Thu, 8 Aug 2019 15:44:54 +0200
Subject: [PATCH] 63838: Fix feedback

Add a title to the move page
Disable submit button when no target is selected
Add operation in progress indicator
Use updated inputSuggestion to fix bug with value selection
---
 resources/i18n/en.json                        |  2 +
 .../item-move/item-move.component.html        | 23 +++---
 .../item-move/item-move.component.spec.ts     | 30 +++-----
 .../item-move/item-move.component.ts          | 72 +++++++++++--------
 .../item-operation.component.html             |  2 +-
 .../item-operation.component.spec.ts          | 13 ++--
 6 files changed, 75 insertions(+), 67 deletions(-)

diff --git a/resources/i18n/en.json b/resources/i18n/en.json
index a93e863132..8c5bb9fa1a 100644
--- a/resources/i18n/en.json
+++ b/resources/i18n/en.json
@@ -186,6 +186,7 @@
       },
       "move": {
         "head":"Move item: {{id}}",
+        "title":"Move item",
         "description": "Select the collection you wish to move this item to. To narrow down the list of displayed collections, you can enter a search query in the box.",
         "search.placeholder": "Enter a search query to look for collections",
         "inheritpolicies": {
@@ -193,6 +194,7 @@
           "checkbox": "Inherit policies"
         },
         "move": "Move",
+        "processing": "Moving...",
         "cancel": "Cancel",
         "success": "The item has been moved succesfully",
         "error": "An error occured when attempting to move the item"
diff --git a/src/app/+item-page/edit-item-page/item-move/item-move.component.html b/src/app/+item-page/edit-item-page/item-move/item-move.component.html
index 063028c719..cf5ada77cf 100644
--- a/src/app/+item-page/edit-item-page/item-move/item-move.component.html
+++ b/src/app/+item-page/edit-item-page/item-move/item-move.component.html
@@ -1,29 +1,31 @@
 <div class="container">
     <div class="row">
         <div class="col-12">
-            <h2>{{'item.edit.move.head' | translate: { id: (itemRD$ | async)?.payload?.handle} }}</h2>
+            <h2>{{'item.edit.move.head' | translate: {id: (itemRD$ | async)?.payload?.handle} }}</h2>
             <p>{{'item.edit.move.description' | translate}}</p>
             <div class="row">
                 <div class="col-12">
-                    <ds-input-suggestions #f id="search-form"
+                    <ds-dso-input-suggestions #f id="search-form"
                                           [suggestions]="(collectionSearchResults | async)"
                                           [placeholder]="'item.edit.move.search.placeholder'| translate"
                                           [action]="getCurrentUrl()"
                                           [name]="'item-move'"
-                                          [(ngModel)]="selectedCollection"
+                                          [(ngModel)]="selectedCollectionName"
                                           (clickSuggestion)="onClick($event)"
+                                          (typeSuggestion)="resetCollection($event)"
                                           (findSuggestions)="findSuggestions($event)"
                                           (click)="f.open()"
                                           ngDefaultControl>
-                    </ds-input-suggestions>
+                    </ds-dso-input-suggestions>
+
                 </div>
-            </div>
+            </div>                                           
             <div class="row">
                 <div class="col-12">
                     <p>
                         <input type="checkbox" name="tc" [(ngModel)]="inheritPolicies" id="inheritPoliciesCheckbox">
                         <label for="inheritPoliciesCheckbox">{{'item.edit.move.inheritpolicies.checkbox' |
-                            translate}}</label>
+                                translate}}</label>
                     </p>
                     <p>
                         {{'item.edit.move.inheritpolicies.description' | translate}}
@@ -31,9 +33,14 @@
                 </div>
             </div>
 
-            <button (click)="moveCollection()" class="btn btn-outline-secondary">{{'item.edit.move.move' | translate}}
+            <button (click)="moveCollection()" class="btn btn-primary" [disabled]=!canSubmit>
+                <span *ngIf="!processing"> {{'item.edit.move.move' | translate}}</span>
+                <span *ngIf="processing"><i class='fas fa-circle-notch fa-spin'></i>
+                    {{'item.edit.move.processing' | translate}}
+                </span>
             </button>
-            <button [routerLink]="['/items/', (itemRD$ | async)?.payload?.id, 'edit']" class="btn btn-outline-secondary">
+            <button [routerLink]="['/items/', (itemRD$ | async)?.payload?.id, 'edit']"
+                    class="btn btn-outline-secondary">
                 {{'item.edit.move.cancel' | translate}}
             </button>
         </div>
diff --git a/src/app/+item-page/edit-item-page/item-move/item-move.component.spec.ts b/src/app/+item-page/edit-item-page/item-move/item-move.component.spec.ts
index 3d947fdabe..e73b4b6f9a 100644
--- a/src/app/+item-page/edit-item-page/item-move/item-move.component.spec.ts
+++ b/src/app/+item-page/edit-item-page/item-move/item-move.component.spec.ts
@@ -103,20 +103,8 @@ describe('ItemMoveComponent', () => {
     });
     it('should load suggestions', () => {
       const expected = [
-        {
-          displayValue: 'Test collection 1',
-          value: {
-            name: 'Test collection 1',
-            object: collection1,
-          }
-        },
-        {
-          displayValue: 'Test collection 2',
-          value: {
-            name: 'Test collection 2',
-            object: collection2,
-          }
-        }
+        collection1,
+        collection2
       ];
 
       comp.collectionSearchResults.subscribe((value) => {
@@ -128,20 +116,18 @@ describe('ItemMoveComponent', () => {
       expect(comp.getCurrentUrl()).toEqual('fake-url/fake-id/edit');
     });
     it('should on click select the correct collection name and id', () => {
-      const data = {
-        name: 'Test collection 1',
-        object: collection1,
-      };
+      const data = collection1;
+
       comp.onClick(data);
 
-      expect(comp.selectedCollection).toEqual('Test collection 1');
-      expect(comp.selectedCollectionObject).toEqual(collection1);
+      expect(comp.selectedCollectionName).toEqual('Test collection 1');
+      expect(comp.selectedCollection).toEqual(collection1);
     });
     describe('moveCollection', () => {
       it('should call itemDataService.moveToCollection', () => {
         comp.itemId = 'item-id';
-        comp.selectedCollection = 'selected-collection-id';
-        comp.selectedCollectionObject = collection1;
+        comp.selectedCollectionName = 'selected-collection-id';
+        comp.selectedCollection = collection1;
         comp.moveCollection();
 
         expect(mockItemDataService.moveToCollection).toHaveBeenCalledWith('item-id', collection1);
diff --git a/src/app/+item-page/edit-item-page/item-move/item-move.component.ts b/src/app/+item-page/edit-item-page/item-move/item-move.component.ts
index 4cca0cd3a4..d964c940f7 100644
--- a/src/app/+item-page/edit-item-page/item-move/item-move.component.ts
+++ b/src/app/+item-page/edit-item-page/item-move/item-move.component.ts
@@ -1,23 +1,23 @@
-import {Component, OnInit} from '@angular/core';
-import {SearchService} from '../../../+search-page/search-service/search.service';
-import {first, map} from 'rxjs/operators';
-import {DSpaceObjectType} from '../../../core/shared/dspace-object-type.model';
-import {SearchOptions} from '../../../+search-page/search-options.model';
-import {RemoteData} from '../../../core/data/remote-data';
-import {DSpaceObject} from '../../../core/shared/dspace-object.model';
-import {PaginatedList} from '../../../core/data/paginated-list';
-import {SearchResult} from '../../../+search-page/search-result.model';
-import {Item} from '../../../core/shared/item.model';
-import {ActivatedRoute, Router} from '@angular/router';
-import {NotificationsService} from '../../../shared/notifications/notifications.service';
-import {TranslateService} from '@ngx-translate/core';
-import {getSucceededRemoteData} from '../../../core/shared/operators';
-import {ItemDataService} from '../../../core/data/item-data.service';
-import {getItemEditPath} from '../../item-page-routing.module';
-import {Observable} from 'rxjs';
-import {of as observableOf} from 'rxjs';
+import { Component, OnInit } from '@angular/core';
+import { SearchService } from '../../../+search-page/search-service/search.service';
+import { first, map } from 'rxjs/operators';
+import { DSpaceObjectType } from '../../../core/shared/dspace-object-type.model';
+import { SearchOptions } from '../../../+search-page/search-options.model';
+import { RemoteData } from '../../../core/data/remote-data';
+import { DSpaceObject } from '../../../core/shared/dspace-object.model';
+import { PaginatedList } from '../../../core/data/paginated-list';
+import { SearchResult } from '../../../+search-page/search-result.model';
+import { Item } from '../../../core/shared/item.model';
+import { ActivatedRoute, Router } from '@angular/router';
+import { NotificationsService } from '../../../shared/notifications/notifications.service';
+import { TranslateService } from '@ngx-translate/core';
+import { getSucceededRemoteData } from '../../../core/shared/operators';
+import { ItemDataService } from '../../../core/data/item-data.service';
+import { getItemEditPath } from '../../item-page-routing.module';
+import { Observable, of as observableOf } from 'rxjs';
 import { RestResponse } from '../../../core/cache/response.models';
 import { Collection } from '../../../core/shared/collection.model';
+import { tap } from 'rxjs/internal/operators/tap';
 
 @Component({
   selector: 'ds-item-move',
@@ -31,13 +31,18 @@ export class ItemMoveComponent implements OnInit {
    * TODO: There is currently no backend support to change the owningCollection and inherit policies,
    * TODO: when this is added, the inherit policies option should be used.
    */
+
+  selectorType = DSpaceObjectType.COLLECTION;
+
   inheritPolicies = false;
   itemRD$: Observable<RemoteData<Item>>;
   collectionSearchResults: Observable<any[]> = observableOf([]);
-  selectedCollection: string;
-  selectedCollectionObject: Collection;
+  selectedCollectionName: string;
+  selectedCollection: Collection;
+  canSubmit = false;
 
   itemId: string;
+  processing = false;
 
   constructor(private route: ActivatedRoute,
               private router: Router,
@@ -48,7 +53,7 @@ export class ItemMoveComponent implements OnInit {
   }
 
   ngOnInit(): void {
-    this.itemRD$ = this.route.data.pipe(map((data) => data.item),getSucceededRemoteData()) as Observable<RemoteData<Item>>;
+    this.itemRD$ = this.route.data.pipe(map((data) => data.item), getSucceededRemoteData()) as Observable<RemoteData<Item>>;
     this.itemRD$.subscribe((rd) => {
         this.itemId = rd.payload.id;
       }
@@ -76,12 +81,9 @@ export class ItemMoveComponent implements OnInit {
       first(),
       map((rd: RemoteData<PaginatedList<SearchResult<DSpaceObject>>>) => {
         return rd.payload.page.map((searchResult) => {
-          return {
-            displayValue: searchResult.indexableObject.name,
-            value: {name: searchResult.indexableObject.name, object: searchResult.indexableObject}
-          };
-        });
-      })
+          return searchResult.indexableObject
+        })
+      }) ,
     );
 
   }
@@ -91,8 +93,9 @@ export class ItemMoveComponent implements OnInit {
    * @param data - obtained from the ds-input-suggestions component
    */
   onClick(data: any): void {
-    this.selectedCollection = data.name;
-    this.selectedCollectionObject = data.object;
+    this.selectedCollection = data;
+    this.selectedCollectionName = data.name;
+    this.canSubmit = true;
   }
 
   /**
@@ -106,7 +109,8 @@ export class ItemMoveComponent implements OnInit {
    * Moves the item to a new collection based on the selected collection
    */
   moveCollection() {
-    this.itemDataService.moveToCollection(this.itemId, this.selectedCollectionObject).pipe(first()).subscribe(
+    this.processing = true;
+    this.itemDataService.moveToCollection(this.itemId, this.selectedCollection).pipe(first()).subscribe(
       (response: RestResponse) => {
         this.router.navigate([getItemEditPath(this.itemId)]);
         if (response.isSuccessful) {
@@ -114,8 +118,16 @@ export class ItemMoveComponent implements OnInit {
         } else {
           this.notificationsService.error(this.translateService.get('item.edit.move.error'));
         }
+        this.processing = false;
       }
     );
+  }
 
+  /**
+   * Resets the can submit when the user changes the content of the input field
+   * @param data
+   */
+  resetCollection(data: any) {
+    this.canSubmit = false;
   }
 }
diff --git a/src/app/+item-page/edit-item-page/item-operation/item-operation.component.html b/src/app/+item-page/edit-item-page/item-operation/item-operation.component.html
index 4623195437..3a52fd0d12 100644
--- a/src/app/+item-page/edit-item-page/item-operation/item-operation.component.html
+++ b/src/app/+item-page/edit-item-page/item-operation/item-operation.component.html
@@ -4,7 +4,7 @@
       </span>
 </div>
 <div *ngIf="!operation.disabled" class="col-9 float-left action-button">
-  <a class="btn btn-outline-secondary" href="{{operation.operationUrl}}">
+  <a class="btn btn-outline-secondary" [routerLink]="operation.operationUrl">
     {{'item.edit.tabs.status.buttons.' + operation.operationKey + '.button' | translate}}
   </a>
 </div>
diff --git a/src/app/+item-page/edit-item-page/item-operation/item-operation.component.spec.ts b/src/app/+item-page/edit-item-page/item-operation/item-operation.component.spec.ts
index 1901bf5fb4..7122dbaf42 100644
--- a/src/app/+item-page/edit-item-page/item-operation/item-operation.component.spec.ts
+++ b/src/app/+item-page/edit-item-page/item-operation/item-operation.component.spec.ts
@@ -1,8 +1,9 @@
-import {ItemOperation} from './itemOperation.model';
-import {async, TestBed} from '@angular/core/testing';
-import {ItemOperationComponent} from './item-operation.component';
-import {TranslateModule} from '@ngx-translate/core';
-import {By} from '@angular/platform-browser';
+import { ItemOperation } from './itemOperation.model';
+import { async, TestBed } from '@angular/core/testing';
+import { ItemOperationComponent } from './item-operation.component';
+import { TranslateModule } from '@ngx-translate/core';
+import { By } from '@angular/platform-browser';
+import { RouterTestingModule } from '@angular/router/testing';
 
 describe('ItemOperationComponent', () => {
   let itemOperation: ItemOperation;
@@ -12,7 +13,7 @@ describe('ItemOperationComponent', () => {
 
   beforeEach(async(() => {
     TestBed.configureTestingModule({
-      imports: [TranslateModule.forRoot()],
+      imports: [TranslateModule.forRoot(), RouterTestingModule.withRoutes([])],
       declarations: [ItemOperationComponent]
     }).compileComponents();
   }));
-- 
GitLab