Merge pull request #2761 from ybnd/fix-versioning-button

Fix caching issues for versioning

src/app/core/data/base/identifiable-data.service.spec.ts

@@ -5,10 +5,7 @@
  *
  * http://www.dspace.org/license/
  */
-import {
-  Observable,
-  of as observableOf,
-} from 'rxjs';
+import { of as observableOf } from 'rxjs';
 import { TestScheduler } from 'rxjs/testing';
 
 import { getMockRemoteDataBuildService } from '../../../shared/mocks/remote-data-build.service.mock';
@@ -18,14 +15,14 @@ import { followLink } from '../../../shared/utils/follow-link-config.model';
 import { RemoteDataBuildService } from '../../cache/builders/remote-data-build.service';
 import { ObjectCacheService } from '../../cache/object-cache.service';
 import { HALEndpointService } from '../../shared/hal-endpoint.service';
-import { FindListOptions } from '../find-list-options.model';
 import { RemoteData } from '../remote-data';
 import { RequestService } from '../request.service';
 import { RequestEntryState } from '../request-entry-state.model';
 import { EMBED_SEPARATOR } from './base-data.service';
 import { IdentifiableDataService } from './identifiable-data.service';
 
-const endpoint = 'https://rest.api/core';
+const base = 'https://rest.api/core';
+const endpoint = 'test';
 
 class TestService extends IdentifiableDataService<any> {
   constructor(
@@ -34,11 +31,7 @@ class TestService extends IdentifiableDataService<any> {
     protected objectCache: ObjectCacheService,
     protected halService: HALEndpointService,
   ) {
-    super(undefined, requestService, rdbService, objectCache, halService);
-  }
-
-  public getBrowseEndpoint(options: FindListOptions = {}, linkPath: string = this.linkPath): Observable<string> {
-    return observableOf(endpoint);
+    super(endpoint, requestService, rdbService, objectCache, halService);
   }
 }
 
@@ -55,7 +48,7 @@ describe('IdentifiableDataService', () => {
 
   function initTestService(): TestService {
     requestService = getMockRequestService();
-    halService = new HALEndpointServiceStub('url') as any;
+    halService = new HALEndpointServiceStub(base) as any;
     rdbService = getMockRemoteDataBuildService();
     objectCache = {
 
@@ -147,4 +140,12 @@ describe('IdentifiableDataService', () => {
       expect(result).toEqual(expected);
     });
   });
+
+  describe('invalidateById', () => {
+    it('should invalidate the correct resource by href', () => {
+      spyOn(service, 'invalidateByHref').and.returnValue(observableOf(true));
+      service.invalidateById('123');
+      expect(service.invalidateByHref).toHaveBeenCalledWith(`${base}/${endpoint}/123`);
+    });
+  });
 });

src/app/core/data/base/identifiable-data.service.ts

@@ -6,7 +6,11 @@
  * http://www.dspace.org/license/
  */
 import { Observable } from 'rxjs';
-import { map } from 'rxjs/operators';
+import {
+  map,
+  switchMap,
+  take,
+} from 'rxjs/operators';
 
 import { FollowLinkConfig } from '../../../shared/utils/follow-link-config.model';
 import { RemoteDataBuildService } from '../../cache/builders/remote-data-build.service';
@@ -81,4 +85,19 @@ export class IdentifiableDataService<T extends CacheableObject> extends BaseData
     return this.getEndpoint().pipe(
       map((endpoint: string) => this.getIDHref(endpoint, resourceID, ...linksToFollow)));
   }
+
+  /**
+   * Invalidate a cached resource by its identifier
+   * @param resourceID  the identifier of the resource to invalidate
+   */
+  invalidateById(resourceID: string): Observable<boolean> {
+    const ok$ = this.getIDHrefObs(resourceID).pipe(
+      take(1),
+      switchMap((href) => this.invalidateByHref(href)),
+    );
+
+    ok$.subscribe();
+
+    return ok$;
+  }
 }

src/app/core/data/version-history-data.service.spec.ts

@@ -69,6 +69,10 @@ describe('VersionHistoryDataService', () => {
       },
     },
   });
+  const version1WithDraft = Object.assign(new Version(), {
+    ...version1,
+    versionhistory: createSuccessfulRemoteDataObject$(versionHistoryDraft),
+  });
   const versions = [version1, version2];
   versionHistory.versions = createSuccessfulRemoteDataObject$(createPaginatedList(versions));
   const item1 = Object.assign(new Item(), {
@@ -190,21 +194,18 @@ describe('VersionHistoryDataService', () => {
   });
 
   describe('hasDraftVersion$', () => {
-    beforeEach(waitForAsync(() => {
-      versionService.findByHref.and.returnValue(createSuccessfulRemoteDataObject$<Version>(version1));
-    }));
     it('should return false if draftVersion is false', fakeAsync(() => {
-      versionService.getHistoryFromVersion.and.returnValue(of(versionHistory));
+      versionService.findByHref.and.returnValue(createSuccessfulRemoteDataObject$<Version>(version1));
       service.hasDraftVersion$('href').subscribe((res) => {
         expect(res).toBeFalse();
       });
     }));
+
     it('should return true if draftVersion is true', fakeAsync(() => {
-      versionService.getHistoryFromVersion.and.returnValue(of(versionHistoryDraft));
+      versionService.findByHref.and.returnValue(createSuccessfulRemoteDataObject$<Version>(version1WithDraft));
       service.hasDraftVersion$('href').subscribe((res) => {
         expect(res).toBeTrue();
       });
     }));
   });
-
 });

src/app/core/data/version-history-data.service.ts

@@ -1,17 +1,22 @@
 import { HttpHeaders } from '@angular/common/http';
 import { Injectable } from '@angular/core';
 import {
+  combineLatest,
   Observable,
-  of,
+  of as observableOf,
 } from 'rxjs';
 import {
   filter,
+  find,
   map,
   switchMap,
   take,
 } from 'rxjs/operators';
 
-import { hasValueOperator } from '../../shared/empty.util';
+import {
+  hasValue,
+  hasValueOperator,
+} from '../../shared/empty.util';
 import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model';
 import { PaginatedSearchOptions } from '../../shared/search/models/paginated-search-options.model';
 import {
@@ -29,7 +34,6 @@ import {
   getFirstSucceededRemoteDataPayload,
   getRemoteDataPayload,
 } from '../shared/operators';
-import { sendRequest } from '../shared/request.operators';
 import { Version } from '../shared/version.model';
 import { VersionHistory } from '../shared/version-history.model';
 import { IdentifiableDataService } from './base/identifiable-data.service';
@@ -38,7 +42,6 @@ import { PaginatedList } from './paginated-list.model';
 import { RemoteData } from './remote-data';
 import { PostRequest } from './request.models';
 import { RequestService } from './request.service';
-import { RestRequest } from './rest-request.model';
 import { VersionDataService } from './version-data.service';
 
 /**
@@ -100,19 +103,31 @@ export class VersionHistoryDataService extends IdentifiableDataService<VersionHi
    * @param summary the summary of the new version
    */
   createVersion(itemHref: string, summary: string): Observable<RemoteData<Version>> {
+    const requestId = this.requestService.generateRequestId();
     const requestOptions: HttpOptions = Object.create({});
     let requestHeaders = new HttpHeaders();
     requestHeaders = requestHeaders.append('Content-Type', 'text/uri-list');
     requestOptions.headers = requestHeaders;
 
-    return this.halService.getEndpoint(this.versionsEndpoint).pipe(
+    this.halService.getEndpoint(this.versionsEndpoint).pipe(
       take(1),
       map((endpointUrl: string) => (summary?.length > 0) ? `${endpointUrl}?summary=${summary}` : `${endpointUrl}`),
-      map((endpointURL: string) => new PostRequest(this.requestService.generateRequestId(), endpointURL, itemHref, requestOptions)),
-      sendRequest(this.requestService),
-      switchMap((restRequest: RestRequest) => this.rdbService.buildFromRequestUUID(restRequest.uuid)),
+      find((href: string) => hasValue(href)),
+    ).subscribe((href) => {
+      const request = new PostRequest(requestId, href, itemHref, requestOptions);
+      if (hasValue(this.responseMsToLive)) {
+        request.responseMsToLive = this.responseMsToLive;
+      }
+
+      this.requestService.send(request);
+    });
+
+    return this.rdbService.buildFromRequestUUIDAndAwait<Version>(requestId, (versionRD) => combineLatest([
+      this.requestService.setStaleByHrefSubstring(versionRD.payload._links.self.href),
+      this.requestService.setStaleByHrefSubstring(versionRD.payload._links.versionhistory.href),
+    ])).pipe(
       getFirstCompletedRemoteData(),
-    ) as Observable<RemoteData<Version>>;
+    );
   }
 
   /**
@@ -151,7 +166,7 @@ export class VersionHistoryDataService extends IdentifiableDataService<VersionHi
       switchMap((res) => res.versionhistory),
       getFirstSucceededRemoteDataPayload(),
       switchMap((versionHistoryRD) => this.getLatestVersionFromHistory$(versionHistoryRD)),
-    ) : of(null);
+    ) : observableOf(null);
   }
 
   /**
@@ -162,8 +177,8 @@ export class VersionHistoryDataService extends IdentifiableDataService<VersionHi
   isLatest$(version: Version): Observable<boolean> {
     return version ? this.getLatestVersion$(version).pipe(
       take(1),
-      switchMap((latestVersion) => of(version.version === latestVersion.version)),
-    ) : of(null);
+      switchMap((latestVersion) => observableOf(version.version === latestVersion.version)),
+    ) : observableOf(null);
   }
 
   /**
@@ -172,17 +187,22 @@ export class VersionHistoryDataService extends IdentifiableDataService<VersionHi
    * @returns `true` if a workspace item exists, `false` otherwise, or `null` if a version history does not exist
    */
   hasDraftVersion$(versionHref: string): Observable<boolean> {
-    return this.versionDataService.findByHref(versionHref, true, true, followLink('versionhistory')).pipe(
+    return this.versionDataService.findByHref(versionHref, false, true, followLink('versionhistory')).pipe(
       getFirstCompletedRemoteData(),
-      switchMap((res) => {
-        if (res.hasSucceeded && !res.hasNoContent) {
-          return of(res).pipe(
-            getFirstSucceededRemoteDataPayload(),
-            switchMap((version) => this.versionDataService.getHistoryFromVersion(version)),
-            map((versionHistory) => versionHistory ? versionHistory.draftVersion : false),
+      switchMap((versionRD: RemoteData<Version>) => {
+        if (versionRD.hasSucceeded && !versionRD.hasNoContent) {
+          return versionRD.payload.versionhistory.pipe(
+            getFirstCompletedRemoteData(),
+            map((versionHistoryRD: RemoteData<VersionHistory>) => {
+              if (versionHistoryRD.hasSucceeded && !versionHistoryRD.hasNoContent) {
+                return versionHistoryRD.payload.draftVersion;
+              } else {
+                return false;
+              }
+            }),
           );
         } else {
-          return of(false);
+          return observableOf(false);
         }
       }),
     );

src/app/core/submission/workflowitem-data.service.ts

@@ -29,14 +29,12 @@ import { HALEndpointService } from '../shared/hal-endpoint.service';
 import { NoContent } from '../shared/NoContent.model';
 import { getFirstCompletedRemoteData } from '../shared/operators';
 import { WorkflowItem } from './models/workflowitem.model';
-import { WorkspaceItem } from './models/workspaceitem.model';
 
 /**
  * A service that provides methods to make REST requests with workflow items endpoint.
  */
 @Injectable({ providedIn: 'root' })
 export class WorkflowItemDataService extends IdentifiableDataService<WorkflowItem> implements SearchData<WorkflowItem>, DeleteData<WorkflowItem> {
-  protected linkPath = 'workflowitems';
   protected searchByItemLinkPath = 'item';
   protected responseMsToLive = 10 * 1000;
 
@@ -50,7 +48,7 @@ export class WorkflowItemDataService extends IdentifiableDataService<WorkflowIte
     protected halService: HALEndpointService,
     protected notificationsService: NotificationsService,
   ) {
-    super('workspaceitems', requestService, rdbService, objectCache, halService);
+    super('workflowitems', requestService, rdbService, objectCache, halService);
 
     this.searchData = new SearchDataImpl(this.linkPath, requestService, rdbService, objectCache, halService, this.responseMsToLive);
     this.deleteData = new DeleteDataImpl(this.linkPath, requestService, rdbService, objectCache, halService, notificationsService, this.responseMsToLive, this.constructIdEndpoint);
@@ -113,7 +111,7 @@ export class WorkflowItemDataService extends IdentifiableDataService<WorkflowIte
    * @param options        The {@link FindListOptions} object
    * @param linksToFollow  List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved
    */
-  public findByItem(uuid: string, useCachedVersionIfAvailable = false, reRequestOnStale = true, options: FindListOptions = {}, ...linksToFollow: FollowLinkConfig<WorkspaceItem>[]): Observable<RemoteData<WorkspaceItem>> {
+  public findByItem(uuid: string, useCachedVersionIfAvailable = false, reRequestOnStale = true, options: FindListOptions = {}, ...linksToFollow: FollowLinkConfig<WorkflowItem>[]): Observable<RemoteData<WorkflowItem>> {
     const findListOptions = new FindListOptions();
     findListOptions.searchParams = [new RequestParam('uuid', uuid)];
     const href$ = this.searchData.getSearchByHref(this.searchByItemLinkPath, findListOptions, ...linksToFollow);
@@ -134,7 +132,7 @@ export class WorkflowItemDataService extends IdentifiableDataService<WorkflowIte
    * @return {Observable<RemoteData<PaginatedList<T>>}
    *    Return an observable that emits response from the server
    */
-  public searchBy(searchMethod: string, options?: FindListOptions, useCachedVersionIfAvailable?: boolean, reRequestOnStale?: boolean, ...linksToFollow: FollowLinkConfig<WorkspaceItem>[]): Observable<RemoteData<PaginatedList<WorkspaceItem>>> {
+  public searchBy(searchMethod: string, options?: FindListOptions, useCachedVersionIfAvailable?: boolean, reRequestOnStale?: boolean, ...linksToFollow: FollowLinkConfig<WorkflowItem>[]): Observable<RemoteData<PaginatedList<WorkflowItem>>> {
     return this.searchData.searchBy(searchMethod, options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow);
   }
 

src/app/shared/dso-page/dso-versioning-modal-service/dso-versioning-modal.service.ts

@@ -7,7 +7,6 @@ import {
 } from 'rxjs';
 import {
   map,
-  startWith,
   switchMap,
   tap,
 } from 'rxjs/operators';
@@ -96,7 +95,6 @@ export class DsoVersioningModalService {
       // button is disabled if hasDraftVersion = true, and enabled if hasDraftVersion = false or null
       // (hasDraftVersion is null when a version history does not exist)
       map((res) => Boolean(res)),
-      startWith(true),
     );
   }
 

src/app/shared/mydspace-actions/claimed-task/approve/claimed-task-actions-approve.component.spec.ts

@@ -14,13 +14,11 @@ import {
   TranslateLoader,
   TranslateModule,
 } from '@ngx-translate/core';
-import {
-  of as observableOf,
-  of,
-} from 'rxjs';
+import { of as observableOf } from 'rxjs';
 
 import { RequestService } from '../../../../core/data/request.service';
 import { SearchService } from '../../../../core/shared/search/search.service';
+import { WorkflowItemDataService } from '../../../../core/submission/workflowitem-data.service';
 import { ClaimedTaskDataService } from '../../../../core/tasks/claimed-task-data.service';
 import { ClaimedTask } from '../../../../core/tasks/models/claimed-task-object.model';
 import { ProcessTaskResponse } from '../../../../core/tasks/models/process-task-response';
@@ -41,6 +39,7 @@ const searchService = getMockSearchService();
 const requestService = getMockRequestService();
 
 let mockPoolTaskDataService: PoolTaskDataService;
+let mockWorkflowItemDataService: WorkflowItemDataService;
 
 describe('ClaimedTaskActionsApproveComponent', () => {
   const object = Object.assign(new ClaimedTask(), { id: 'claimed-task-1' });
@@ -50,6 +49,10 @@ describe('ClaimedTaskActionsApproveComponent', () => {
 
   beforeEach(waitForAsync(() => {
     mockPoolTaskDataService = new PoolTaskDataService(null, null, null, null);
+    mockWorkflowItemDataService = jasmine.createSpyObj('WorkflowItemDataService', {
+      'invalidateByHref': observableOf(false),
+    });
+
     TestBed.configureTestingModule({
       imports: [
         TranslateModule.forRoot({
@@ -68,6 +71,7 @@ describe('ClaimedTaskActionsApproveComponent', () => {
         { provide: SearchService, useValue: searchService },
         { provide: RequestService, useValue: requestService },
         { provide: PoolTaskDataService, useValue: mockPoolTaskDataService },
+        { provide: WorkflowItemDataService, useValue: mockWorkflowItemDataService },
       ],
       schemas: [NO_ERRORS_SCHEMA],
     }).overrideComponent(ClaimedTaskActionsApproveComponent, {
@@ -103,7 +107,7 @@ describe('ClaimedTaskActionsApproveComponent', () => {
 
     beforeEach(() => {
       spyOn(component.processCompleted, 'emit');
-      spyOn(component, 'startActionExecution').and.returnValue(of(null));
+      spyOn(component, 'startActionExecution').and.returnValue(observableOf(null));
 
       expectedBody = {
         [component.option]: 'true',