Merge pull request #1064 from atmire/w2p-77742_Collection-item-template-bugfixes

Bugs on Collection Item Template fix

src/app/+collection-page/collection-page-routing-paths.ts

@@ -24,6 +24,10 @@ export function getCollectionEditRolesRoute(id) {
   return new URLCombiner(getCollectionPageRoute(id), COLLECTION_EDIT_PATH, COLLECTION_EDIT_ROLES_PATH).toString();
 }
 
+export function getCollectionItemTemplateRoute(id) {
+  return new URLCombiner(getCollectionPageRoute(id), ITEMTEMPLATE_PATH).toString();
+}
+
 export const COLLECTION_CREATE_PATH = 'create';
 export const COLLECTION_EDIT_PATH = 'edit';
 export const COLLECTION_EDIT_ROLES_PATH = 'roles';

src/app/+collection-page/edit-collection-page/collection-metadata/collection-metadata.component.spec.ts

@@ -15,6 +15,7 @@ import { Collection } from '../../../core/shared/collection.model';
 import { ObjectCacheService } from '../../../core/cache/object-cache.service';
 import { RequestService } from '../../../core/data/request.service';
 import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils';
+import { getCollectionItemTemplateRoute } from '../../collection-page-routing-paths';
 
 describe('CollectionMetadataComponent', () => {
   let comp: CollectionMetadataComponent;
@@ -35,11 +36,13 @@ describe('CollectionMetadataComponent', () => {
       self: { href: 'collection-selflink' }
     }
   });
+  const collectionTemplateHref = 'rest/api/test/collections/template';
 
-  const itemTemplateServiceStub = Object.assign({
+  const itemTemplateServiceStub = jasmine.createSpyObj('itemTemplateService', {
-    findByCollectionID: () => createSuccessfulRemoteDataObject$(template),
+    findByCollectionID: createSuccessfulRemoteDataObject$(template),
-    create: () => createSuccessfulRemoteDataObject$(template),
+    create: createSuccessfulRemoteDataObject$(template),
-    deleteByCollectionID: () => observableOf(true)
+    deleteByCollectionID: observableOf(true),
+    getCollectionEndpoint: observableOf(collectionTemplateHref),
   });
 
   const notificationsService = jasmine.createSpyObj('notificationsService', {
@@ -50,7 +53,7 @@ describe('CollectionMetadataComponent', () => {
     remove: {}
   });
   const requestService = jasmine.createSpyObj('requestService', {
-    removeByHrefSubstring: {}
+    setStaleByHrefSubstring: {}
   });
 
   beforeEach(waitForAsync(() => {
@@ -87,14 +90,14 @@ describe('CollectionMetadataComponent', () => {
     it('should navigate to the collection\'s itemtemplate page', () => {
       spyOn(router, 'navigate');
       comp.addItemTemplate();
-      expect(router.navigate).toHaveBeenCalledWith(['collections', collection.uuid, 'itemtemplate']);
+      expect(router.navigate).toHaveBeenCalledWith([getCollectionItemTemplateRoute(collection.uuid)]);
     });
   });
 
   describe('deleteItemTemplate', () => {
     describe('when delete returns a success', () => {
       beforeEach(() => {
-        spyOn(itemTemplateService, 'deleteByCollectionID').and.returnValue(observableOf(true));
+        (itemTemplateService.deleteByCollectionID as jasmine.Spy).and.returnValue(observableOf(true));
         comp.deleteItemTemplate();
       });
 
@@ -103,14 +106,15 @@ describe('CollectionMetadataComponent', () => {
       });
 
       it('should reset related object and request cache', () => {
-        expect(objectCache.remove).toHaveBeenCalledWith(template.self);
+        expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith(collectionTemplateHref);
-        expect(requestService.removeByHrefSubstring).toHaveBeenCalledWith(collection.self);
+        expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith(template.self);
+        expect(requestService.setStaleByHrefSubstring).toHaveBeenCalledWith(collection.self);
       });
     });
 
     describe('when delete returns a failure', () => {
       beforeEach(() => {
-        spyOn(itemTemplateService, 'deleteByCollectionID').and.returnValue(observableOf(false));
+        (itemTemplateService.deleteByCollectionID as jasmine.Spy).and.returnValue(observableOf(false));
         comp.deleteItemTemplate();
       });
 

src/app/+collection-page/edit-collection-page/collection-metadata/collection-metadata.component.ts

@@ -7,12 +7,13 @@ import { ItemTemplateDataService } from '../../../core/data/item-template-data.s
 import { combineLatest as combineLatestObservable, Observable } from 'rxjs';
 import { RemoteData } from '../../../core/data/remote-data';
 import { Item } from '../../../core/shared/item.model';
-import { getRemoteDataPayload, getFirstSucceededRemoteData } from '../../../core/shared/operators';
+import { getFirstSucceededRemoteDataPayload } from '../../../core/shared/operators';
-import { switchMap, take } from 'rxjs/operators';
+import { switchMap, tap } from 'rxjs/operators';
 import { NotificationsService } from '../../../shared/notifications/notifications.service';
 import { TranslateService } from '@ngx-translate/core';
 import { ObjectCacheService } from '../../../core/cache/object-cache.service';
 import { RequestService } from '../../../core/data/request.service';
+import { getCollectionItemTemplateRoute } from '../../collection-page-routing-paths';
 
 /**
  * Component for editing a collection's metadata
@@ -53,8 +54,7 @@ export class CollectionMetadataComponent extends ComcolMetadataComponent<Collect
    */
   initTemplateItem() {
     this.itemTemplateRD$ = this.dsoRD$.pipe(
-      getFirstSucceededRemoteData(),
+      getFirstSucceededRemoteDataPayload(),
-      getRemoteDataPayload(),
       switchMap((collection: Collection) => this.itemTemplateService.findByCollectionID(collection.uuid))
     );
   }
@@ -64,19 +64,20 @@ export class CollectionMetadataComponent extends ComcolMetadataComponent<Collect
    */
   addItemTemplate() {
     const collection$ = this.dsoRD$.pipe(
-      getFirstSucceededRemoteData(),
+      getFirstSucceededRemoteDataPayload(),
-      getRemoteDataPayload(),
-      take(1)
     );
     const template$ = collection$.pipe(
-      switchMap((collection: Collection) => this.itemTemplateService.create(new Item(), collection.uuid)),
+      switchMap((collection: Collection) => this.itemTemplateService.create(new Item(), collection.uuid).pipe(
-      getFirstSucceededRemoteData(),
+        getFirstSucceededRemoteDataPayload(),
-      getRemoteDataPayload(),
+      )),
-      take(1)
+    );
+    const templateHref$ = collection$.pipe(
+      switchMap((collection) => this.itemTemplateService.getCollectionEndpoint(collection.id)),
     );
 
-    combineLatestObservable(collection$, template$).subscribe(([collection, template]) => {
+    combineLatestObservable(collection$, template$, templateHref$).subscribe(([collection, template, templateHref]) => {
-      this.router.navigate(['collections', collection.uuid, 'itemtemplate']);
+      this.requestService.setStaleByHrefSubstring(templateHref);
+      this.router.navigate([getCollectionItemTemplateRoute(collection.uuid)]);
     });
   }
 
@@ -85,23 +86,30 @@ export class CollectionMetadataComponent extends ComcolMetadataComponent<Collect
    */
   deleteItemTemplate() {
     const collection$ = this.dsoRD$.pipe(
-      getFirstSucceededRemoteData(),
+      getFirstSucceededRemoteDataPayload(),
-      getRemoteDataPayload(),
-      take(1)
     );
     const template$ = collection$.pipe(
-      switchMap((collection: Collection) => this.itemTemplateService.findByCollectionID(collection.uuid)),
+      switchMap((collection: Collection) => this.itemTemplateService.findByCollectionID(collection.uuid).pipe(
-      getFirstSucceededRemoteData(),
+        getFirstSucceededRemoteDataPayload(),
-      getRemoteDataPayload(),
+      )),
-      take(1)
+    );
+    const templateHref$ = collection$.pipe(
+      switchMap((collection) => this.itemTemplateService.getCollectionEndpoint(collection.id)),
     );
 
-    combineLatestObservable(collection$, template$).pipe(
+    combineLatestObservable(collection$, template$, templateHref$).pipe(
-      switchMap(([collection, template]) => {
+      switchMap(([collection, template, templateHref]) => {
-        const success$ = this.itemTemplateService.deleteByCollectionID(template, collection.uuid);
+        return this.itemTemplateService.deleteByCollectionID(template, collection.uuid).pipe(
-        this.objectCache.remove(template.self);
+          tap((success: boolean) => {
-        this.requestService.removeByHrefSubstring(collection.self);
+            if (success) {
-        return success$;
+              this.objectCache.remove(templateHref);
+              this.objectCache.remove(template.self);
+              this.requestService.setStaleByHrefSubstring(template.self);
+              this.requestService.setStaleByHrefSubstring(templateHref);
+              this.requestService.setStaleByHrefSubstring(collection.self);
+            }
+          })
+        );
       })
     ).subscribe((success: boolean) => {
       if (success) {

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

@@ -1,9 +1,13 @@
 <div class="container" *ngVar="(collectionRD$ | async)?.payload as collection">
   <div class="row">
-    <div class="col-12">
+    <div class="col-12" *ngVar="(itemRD$ | async) as itemRD">
-      <h2 class="border-bottom">{{ 'collection.edit.template.head' | translate:{ collection: collection?.name } }}</h2>
+      <ng-container *ngIf="itemRD?.hasSucceeded">
-      <ds-item-metadata [updateService]="itemTemplateService"></ds-item-metadata>
+        <h2 class="border-bottom">{{ 'collection.edit.template.head' | translate:{ collection: collection?.name } }}</h2>
-      <button [routerLink]="getCollectionEditUrl(collection)" class="btn btn-outline-secondary">{{ 'collection.edit.template.cancel' | translate }}</button>
+        <ds-item-metadata [updateService]="itemTemplateService" [item]="itemRD?.payload"></ds-item-metadata>
+        <button [routerLink]="getCollectionEditUrl(collection)" class="btn btn-outline-secondary">{{ 'collection.edit.template.cancel' | translate }}</button>
+      </ng-container>
+      <ds-loading *ngIf="itemRD?.isLoading" [message]="'collection.edit.template.loading' | translate"></ds-loading>
+      <ds-alert *ngIf="itemRD?.hasFailed" [type]="AlertTypeEnum.Error" [content]="'collection.edit.template.error' | translate"></ds-alert>
     </div>
   </div>
 </div>

src/app/+collection-page/edit-item-template-page/edit-item-template-page.component.spec.ts

@@ -9,7 +9,7 @@ import { ActivatedRoute } from '@angular/router';
 import { of as observableOf } from 'rxjs';
 import { Collection } from '../../core/shared/collection.model';
 import { NO_ERRORS_SCHEMA } from '@angular/core';
-import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils';
+import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils';
 import { getCollectionEditRoute } from '../collection-page-routing-paths';
 
 describe('EditItemTemplatePageComponent', () => {
@@ -24,11 +24,14 @@ describe('EditItemTemplatePageComponent', () => {
       id: 'collection-id',
       name: 'Fake Collection'
     });
+    itemTemplateService = jasmine.createSpyObj('itemTemplateService', {
+      findByCollectionID: createSuccessfulRemoteDataObject$({})
+    });
     TestBed.configureTestingModule({
       imports: [TranslateModule.forRoot(), SharedModule, CommonModule, RouterTestingModule],
       declarations: [EditItemTemplatePageComponent],
       providers: [
-        { provide: ItemTemplateDataService, useValue: {} },
+        { provide: ItemTemplateDataService, useValue: itemTemplateService },
         { provide: ActivatedRoute, useValue: { parent: { data: observableOf({ dso: createSuccessfulRemoteDataObject(collection) }) } } }
       ],
       schemas: [NO_ERRORS_SCHEMA]
@@ -38,7 +41,6 @@ describe('EditItemTemplatePageComponent', () => {
   beforeEach(() => {
     fixture = TestBed.createComponent(EditItemTemplatePageComponent);
     comp = fixture.componentInstance;
-    itemTemplateService = (comp as any).itemTemplateService;
     fixture.detectChanges();
   });
 

src/app/+collection-page/edit-item-template-page/edit-item-template-page.component.ts

@@ -3,9 +3,12 @@ import { Observable } from 'rxjs';
 import { RemoteData } from '../../core/data/remote-data';
 import { Collection } from '../../core/shared/collection.model';
 import { ActivatedRoute } from '@angular/router';
-import { first, map } from 'rxjs/operators';
+import { first, map, switchMap } from 'rxjs/operators';
 import { ItemTemplateDataService } from '../../core/data/item-template-data.service';
 import { getCollectionEditRoute } from '../collection-page-routing-paths';
+import { Item } from '../../core/shared/item.model';
+import { getFirstSucceededRemoteDataPayload } from '../../core/shared/operators';
+import { AlertType } from '../../shared/alert/aletr-type';
 
 @Component({
   selector: 'ds-edit-item-template-page',
@@ -21,12 +24,27 @@ export class EditItemTemplatePageComponent implements OnInit {
    */
   collectionRD$: Observable<RemoteData<Collection>>;
 
+  /**
+   * The template item
+   */
+  itemRD$: Observable<RemoteData<Item>>;
+
+  /**
+   * The AlertType enumeration
+   * @type {AlertType}
+   */
+  AlertTypeEnum = AlertType;
+
   constructor(protected route: ActivatedRoute,
               public itemTemplateService: ItemTemplateDataService) {
   }
 
   ngOnInit(): void {
     this.collectionRD$ = this.route.parent.data.pipe(first(), map((data) => data.dso));
+    this.itemRD$ = this.collectionRD$.pipe(
+      getFirstSucceededRemoteDataPayload(),
+      switchMap((collection) => this.itemTemplateService.findByCollectionID(collection.id)),
+    );
   }
 
   /**

src/app/+item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts

@@ -1,4 +1,4 @@
-import { Component, OnInit, OnDestroy } from '@angular/core';
+import { Component, OnInit, OnDestroy, Input } from '@angular/core';
 import {
   FieldUpdate,
   FieldUpdates
@@ -30,7 +30,7 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl
   /**
    * The item to display the edit page for
    */
-  item: Item;
+  @Input() item: Item;
   /**
    * The current values and updates for all this item's fields
    * Should be initialized in the initializeUpdates method of the child component
@@ -63,22 +63,24 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl
    * Initialize common properties between item-update components
    */
   ngOnInit(): void {
-    this.itemUpdateSubscription = observableCombineLatest([this.route.data, this.route.parent.data]).pipe(
+    if (hasValue(this.item)) {
-      map(([data, parentData]: [Data, Data]) => Object.assign({}, data, parentData)),
+      this.setItem(this.item);
-      map((data: any) => data.dso),
+    } else {
-      tap((rd: RemoteData<Item>) => {
+      // The item wasn't provided through an input, retrieve it from the route instead.
-        this.item = rd.payload;
+      this.itemUpdateSubscription = observableCombineLatest([this.route.data, this.route.parent.data]).pipe(
-      }),
+        map(([data, parentData]: [Data, Data]) => Object.assign({}, data, parentData)),
-      switchMap((rd: RemoteData<Item>) => {
+        map((data: any) => data.dso),
-        return this.itemService.findByHref(rd.payload._links.self.href, true, true, ...ITEM_PAGE_LINKS_TO_FOLLOW);
+        tap((rd: RemoteData<Item>) => {
-      }),
+          this.item = rd.payload;
-      getAllSucceededRemoteData()
+        }),
-    ).subscribe((rd: RemoteData<Item>) => {
+        switchMap((rd: RemoteData<Item>) => {
-      this.item = rd.payload;
+          return this.itemService.findByHref(rd.payload._links.self.href, true, true, ...ITEM_PAGE_LINKS_TO_FOLLOW);
-      this.itemPageRoute = getItemPageRoute(this.item);
+        }),
-      this.postItemInit();
+        getAllSucceededRemoteData()
-      this.initializeUpdates();
+      ).subscribe((rd: RemoteData<Item>) => {
-    });
+        this.setItem(rd.payload);
+      });
+    }
 
     this.discardTimeOut = environment.item.edit.undoTimeout;
     this.url = this.router.url;
@@ -97,6 +99,13 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl
     this.initializeUpdates();
   }
 
+  setItem(item: Item) {
+    this.item = item;
+    this.itemPageRoute = getItemPageRoute(this.item);
+    this.postItemInit();
+    this.initializeUpdates();
+  }
+
   ngOnDestroy() {
     if (hasValue(this.itemUpdateSubscription)) {
       this.itemUpdateSubscription.unsubscribe();

src/app/core/data/item-template-data.service.ts

@@ -22,6 +22,7 @@ import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model';
 import { NoContent } from '../shared/NoContent.model';
 import { hasValue } from '../../shared/empty.util';
 import { Operation } from 'fast-json-patch';
+import { getFirstCompletedRemoteData } from '../shared/operators';
 
 /* tslint:disable:max-classes-per-file */
 /**
@@ -57,15 +58,23 @@ class DataServiceImpl extends ItemDataService {
     super(requestService, rdbService, store, bs, objectCache, halService, notificationsService, http, comparator, bundleService);
   }
 
+  /**
+   * Get the endpoint based on a collection
+   * @param collectionID  The ID of the collection to base the endpoint on
+   */
+  public getCollectionEndpoint(collectionID: string): Observable<string> {
+    return this.collectionService.getIDHrefObs(collectionID).pipe(
+      switchMap((href: string) => this.halService.getEndpoint(this.collectionLinkPath, href))
+    );
+  }
+
   /**
    * Set the endpoint to be based on a collection
    * @param collectionID  The ID of the collection to base the endpoint on
    */
   private setCollectionEndpoint(collectionID: string) {
     this.collectionEndpoint = true;
-    this.endpoint$ = this.collectionService.getIDHrefObs(collectionID).pipe(
+    this.endpoint$ = this.getCollectionEndpoint(collectionID);
-      switchMap((href: string) => this.halService.getEndpoint(this.collectionLinkPath, href))
-    );
   }
 
   /**
@@ -130,6 +139,7 @@ class DataServiceImpl extends ItemDataService {
   deleteByCollectionID(item: Item, collectionID: string): Observable<boolean> {
     this.setRegularEndpoint();
     return super.delete(item.uuid).pipe(
+      getFirstCompletedRemoteData(),
       map((response: RemoteData<NoContent>) => hasValue(response) && response.hasSucceeded)
     );
   }
@@ -209,5 +219,13 @@ export class ItemTemplateDataService implements UpdateDataService<Item> {
   deleteByCollectionID(item: Item, collectionID: string): Observable<boolean> {
     return this.dataService.deleteByCollectionID(item, collectionID);
   }
+
+  /**
+   * Get the endpoint based on a collection
+   * @param collectionID  The ID of the collection to base the endpoint on
+   */
+  getCollectionEndpoint(collectionID: string): Observable<string> {
+    return this.dataService.getCollectionEndpoint(collectionID);
+  }
 }
 /* tslint:enable:max-classes-per-file */

src/assets/i18n/en.json5

@@ -777,10 +777,14 @@
 
   "collection.edit.template.edit-button": "Edit",
 
+  "collection.edit.template.error": "An error occurred retrieving the template item",
+
   "collection.edit.template.head": "Edit Template Item for Collection \"{{ collection }}\"",
 
   "collection.edit.template.label": "Template item",
 
+  "collection.edit.template.loading": "Loading template item...",
+
   "collection.edit.template.notifications.delete.error": "Failed to delete the item template",
 
   "collection.edit.template.notifications.delete.success": "Successfully deleted the item template",