Clarin9/Fix "latest version" notice link ignoring the /repository base href (#876)

src/app/item-page/versions/notice/item-versions-notice.component.spec.ts

@@ -1,3 +1,7 @@
+import {
+  APP_BASE_HREF,
+  Location,
+} from '@angular/common';
 import { NO_ERRORS_SCHEMA } from '@angular/core';
 import {
   ComponentFixture,
@@ -7,7 +11,10 @@ import {
 import { By } from '@angular/platform-browser';
 import { NoopAnimationsModule } from '@angular/platform-browser/animations';
 import { RouterTestingModule } from '@angular/router/testing';
-import { TranslateModule } from '@ngx-translate/core';
+import {
+  TranslateModule,
+  TranslateService,
+} from '@ngx-translate/core';
 import { of } from 'rxjs';
 import { take } from 'rxjs/operators';
 
@@ -61,6 +68,7 @@ describe('ItemVersionsNoticeComponent', () => {
   const versionHistoryServiceSpy = jasmine.createSpyObj('versionHistoryService',
     ['getVersions', 'getLatestVersionFromHistory$', 'isLatest$' ],
   );
+  const locationStub = jasmine.createSpyObj('location', ['prepareExternalUrl']);
 
   beforeEach(waitForAsync(() => {
 
@@ -73,6 +81,7 @@ describe('ItemVersionsNoticeComponent', () => {
       ],
       providers: [
         { provide: VersionHistoryDataService, useValue: versionHistoryServiceSpy },
+        { provide: Location, useValue: locationStub },
       ],
       schemas: [NO_ERRORS_SCHEMA],
     }).compileComponents();
@@ -84,6 +93,8 @@ describe('ItemVersionsNoticeComponent', () => {
     versionHistoryServiceSpy.getVersions.and.returnValue(createSuccessfulRemoteDataObject$(createPaginatedList(versions)));
     versionHistoryServiceSpy.getLatestVersionFromHistory$.and.returnValue(of(latestVersion));
     versionHistoryServiceSpy.isLatest$.and.callFake(isLatestFcn);
+    // Simulate a UI deployed under a sub-path namespace, i.e. <base href="/repository/">
+    locationStub.prepareExternalUrl.and.callFake((url: string) => `/repository${url}`);
   }));
 
   describe('when the item is the latest version', () => {
@@ -121,6 +132,87 @@ describe('ItemVersionsNoticeComponent', () => {
     });
   });
 
+  describe('getItemPage', () => {
+    beforeEach(() => {
+      initComponentWithItem(firstItem);
+    });
+
+    it('should resolve the latest version item page url against the base href', () => {
+      locationStub.prepareExternalUrl.calls.reset();
+
+      expect(component.getItemPage(latestItem)).toEqual('/repository/items/latest_item_id');
+      // The plain router path must be what is handed to Location, otherwise the prefix would be applied twice
+      expect(locationStub.prepareExternalUrl).toHaveBeenCalledOnceWith('/items/latest_item_id');
+    });
+
+    it('should not resolve a url when no item is provided', () => {
+      locationStub.prepareExternalUrl.calls.reset();
+
+      expect(component.getItemPage(undefined)).toBeUndefined();
+      expect(locationStub.prepareExternalUrl).not.toHaveBeenCalled();
+    });
+
+    it('should render the notice anchor with the base href applied', () => {
+      const translate = TestBed.inject(TranslateService);
+      translate.setTranslation('en', {
+        'item.version.notice': 'The latest version can be found <a href=\'{{destination}}\'>here</a>.',
+      }, true);
+      translate.use('en');
+      fixture.detectChanges();
+
+      const anchor = fixture.debugElement.query(By.css('ds-alert a'));
+      expect(anchor).not.toBeNull();
+      expect(anchor.nativeElement.getAttribute('href')).toEqual('/repository/items/latest_item_id');
+    });
+  });
+
+  describe('getItemPage with the real Location', () => {
+    // the real PathLocationStrategy, so the "no-op for NAMESPACE=/" claim is actually covered
+    [
+      { baseHref: '/', expected: '/items/latest_item_id' },
+      { baseHref: '/repository/', expected: '/repository/items/latest_item_id' },
+      // the form express hands to the SSR platform injector (req.baseUrl, no trailing slash)
+      { baseHref: '/repository', expected: '/repository/items/latest_item_id' },
+    ].forEach(({ baseHref, expected }) => {
+      it(`should resolve the item page url against base href '${baseHref}'`, () => {
+        expect(createComponentWithBaseHref(baseHref).getItemPage(latestItem)).toEqual(expected);
+      });
+    });
+
+    it('should keep the entity route shape and only add the prefix', () => {
+      const entityItem = Object.assign(new Item(), {
+        id: 'entity_item_id',
+        uuid: 'entity_item_id',
+        metadata: { 'dspace.entity.type': [{ value: 'Publication' }] },
+      });
+
+      expect(createComponentWithBaseHref('/repository/').getItemPage(entityItem))
+        .toEqual('/repository/entities/publication/entity_item_id');
+    });
+
+    function createComponentWithBaseHref(baseHref: string): ItemVersionsNoticeComponent {
+      TestBed.resetTestingModule();
+      TestBed.configureTestingModule({
+        // no RouterTestingModule on purpose: its SpyLocation/MockLocationStrategy ignore APP_BASE_HREF
+        imports: [
+          TranslateModule.forRoot(),
+          ItemVersionsNoticeComponent,
+          NoopAnimationsModule,
+        ],
+        providers: [
+          { provide: VersionHistoryDataService, useValue: versionHistoryServiceSpy },
+          { provide: APP_BASE_HREF, useValue: baseHref },
+        ],
+        schemas: [NO_ERRORS_SCHEMA],
+      });
+
+      const realLocationFixture = TestBed.createComponent(ItemVersionsNoticeComponent);
+      realLocationFixture.componentInstance.item = firstItem;
+      realLocationFixture.detectChanges();
+      return realLocationFixture.componentInstance;
+    }
+  });
+
   function initComponentWithItem(item: Item) {
     fixture = TestBed.createComponent(ItemVersionsNoticeComponent);
     component = fixture.componentInstance;

src/app/item-page/versions/notice/item-versions-notice.component.ts

@@ -1,4 +1,7 @@
-import { AsyncPipe } from '@angular/common';
+import {
+  AsyncPipe,
+  Location,
+} from '@angular/common';
 import {
   Component,
   Input,
@@ -84,7 +87,10 @@ export class ItemVersionsNoticeComponent implements OnInit {
    */
   public AlertTypeEnum = AlertType;
 
-  constructor(private versionHistoryService: VersionHistoryDataService) {
+  constructor(
+    private versionHistoryService: VersionHistoryDataService,
+    private location: Location,
+  ) {
   }
 
   /**
@@ -128,12 +134,17 @@ export class ItemVersionsNoticeComponent implements OnInit {
   }
 
   /**
-   * Get the item page url
+   * Get the item page url, resolved against the base href. The url lands in the raw `<a href>` of the
+   * `item.version.notice` translation, so the browser resolves it and not the router - a plain
+   * `/items/<uuid>` would ignore `<base href="/repository/">`. No-op when the base href is `/`.
+   *
+   * Undefined while the latest version is still loading.
+   *
    * @param item The item for which the url is requested
    */
-  getItemPage(item: Item): string {
+  getItemPage(item: Item | undefined): string | undefined {
     if (hasValue(item)) {
-      return getItemPageRoute(item);
+      return this.location.prepareExternalUrl(getItemPageRoute(item));
     }
   }
 }