[TLC-380] Template link, spec test, doc fixup as per review

src/app/core/browse/browse-definition-data.service.spec.ts

@@ -2,27 +2,66 @@ import { BrowseDefinitionDataService } from './browse-definition-data.service';
 import { followLink } from '../../shared/utils/follow-link-config.model';
 import { EMPTY } from 'rxjs';
 import { FindListOptions } from '../data/find-list-options.model';
+import { getMockRemoteDataBuildService } from '../../shared/mocks/remote-data-build.service.mock';
+import { RequestService } from '../data/request.service';
+import { HALEndpointServiceStub } from '../../shared/testing/hal-endpoint-service.stub';
+import { getMockObjectCacheService } from '../../shared/mocks/object-cache.service.mock';
 
 describe(`BrowseDefinitionDataService`, () => {
+  let requestService: RequestService;
   let service: BrowseDefinitionDataService;
-  const findAllDataSpy = jasmine.createSpyObj('findAllData', {
+  let findAllDataSpy;
-    findAll: EMPTY,
+  let searchDataSpy;
-  });
+  const browsesEndpointURL = 'https://rest.api/browses';
+  const halService: any = new HALEndpointServiceStub(browsesEndpointURL);
+
   const options = new FindListOptions();
   const linksToFollow = [
     followLink('entries'),
     followLink('items')
   ];
 
+  function initTestService() {
+    return new BrowseDefinitionDataService(
+      requestService,
+      getMockRemoteDataBuildService(),
+      getMockObjectCacheService(),
+      halService,
+    );
+  }
+
   beforeEach(() => {
-    service = new BrowseDefinitionDataService(null, null, null, null);
+    service = initTestService();
+    findAllDataSpy = jasmine.createSpyObj('findAllData', {
+      findAll: EMPTY,
+    });
+    searchDataSpy = jasmine.createSpyObj('searchData', {
+      searchBy: EMPTY,
+      getSearchByHref: EMPTY,
+    });
     (service as any).findAllData = findAllDataSpy;
+    (service as any).searchData = searchDataSpy;
   });
 
+  describe('findByFields', () => {
+    it(`should call searchByHref on searchData`, () => {
+      service.findByFields(['test'], true, false, ...linksToFollow);
+      expect(searchDataSpy.getSearchByHref).toHaveBeenCalled();
+    });
+  });
+  describe('searchBy', () => {
+    it(`should call searchBy on searchData`, () => {
+      service.searchBy('test', options, true, false, ...linksToFollow);
+      expect(searchDataSpy.searchBy).toHaveBeenCalledWith('test', options, true, false, ...linksToFollow);
+    });
+  });
   describe(`findAll`, () => {
     it(`should call findAll on findAllData`, () => {
       service.findAll(options, true, false, ...linksToFollow);
       expect(findAllDataSpy.findAll).toHaveBeenCalledWith(options, true, false, ...linksToFollow);
     });
   });
+
+
+
 });

src/app/core/browse/browse-definition-data.service.ts

@@ -27,19 +27,6 @@ export class BrowseDefinitionDataService extends IdentifiableDataService<BrowseD
   private findAllData: FindAllDataImpl<BrowseDefinition>;
   private searchData: SearchDataImpl<BrowseDefinition>;
 
-  public static toSearchKeyArray(metadataKey: string): string[] {
-    const keyParts = metadataKey.split('.');
-    const searchFor = [];
-    searchFor.push('*');
-    for (let i = 0; i < keyParts.length - 1; i++) {
-      const prevParts = keyParts.slice(0, i + 1);
-      const nextPart = [...prevParts, '*'].join('.');
-      searchFor.push(nextPart);
-    }
-    searchFor.push(metadataKey);
-    return searchFor;
-  }
-
   constructor(
     protected requestService: RequestService,
     protected rdbService: RemoteDataBuildService,
@@ -100,54 +87,16 @@ export class BrowseDefinitionDataService extends IdentifiableDataService<BrowseD
     return this.searchData.getSearchByHref(searchMethod, options, ...linksToFollow);
   }
 
-  findByField(
-    field: string,
-    useCachedVersionIfAvailable = true,
-    reRequestOnStale = true,
-    ...linksToFollow: FollowLinkConfig<BrowseDefinition>[]
-  ): Observable<RemoteData<BrowseDefinition>> {
-    const searchParams = [];
-    searchParams.push(new RequestParam('field', field));
-
-    const hrefObs = this.getSearchByHref(
-      'byField',
-      { searchParams },
-      ...linksToFollow
-    );
-
-    return this.findByHref(
-      hrefObs,
-      useCachedVersionIfAvailable,
-      reRequestOnStale,
-      ...linksToFollow,
-    );
-  }
-
-  findAllLinked(
-    useCachedVersionIfAvailable = true,
-    reRequestOnStale = true,
-    ...linksToFollow: FollowLinkConfig<BrowseDefinition>[]
-  ): Observable<RemoteData<BrowseDefinition>> {
-    const searchParams = [];
-
-    const hrefObs = this.getSearchByHref(
-      'allLinked',
-      { searchParams },
-      ...linksToFollow
-    );
-
-    return this.findByHref(
-      hrefObs,
-      useCachedVersionIfAvailable,
-      reRequestOnStale,
-      ...linksToFollow,
-    );
-  }
-
   /**
-   * Get the browse URL by providing a list of metadata keys
+   * Get the browse URL by providing a list of metadata keys. The first matching browse index definition
-   * @param metadatumKey
+   * for any of the fields is returned. This is used in eg. item page field component, which can be configured
-   * @param linkPath
+   * with several fields for a component like 'Author', and needs to know if and how to link the values
+   * to configured browse indices.
+   *
+   * @param fields  an array of field strings, eg. ['dc.contributor.author', 'dc.creator']
+   * @param useCachedVersionIfAvailable Override the data service useCachedVersionIfAvailable parameter (default: true)
+   * @param reRequestOnStale Override the data service reRequestOnStale parameter (default: true)
+   * @param linksToFollow Override the data service linksToFollow parameter (default: empty array)
    */
   findByFields(
     fields: string[],

src/app/item-page/field-components/metadata-values/metadata-values.component.spec.ts

@@ -52,6 +52,7 @@ describe('MetadataValuesComponent', () => {
     comp.mdValues = mockMetadata;
     comp.separator = mockSeperator;
     comp.label = mockLabel;
+    comp.urlRegex = /^.*test.*$/;
     fixture.detectChanges();
   }));
 
@@ -67,4 +68,9 @@ describe('MetadataValuesComponent', () => {
     expect(separators.length).toBe(mockMetadata.length - 1);
   });
 
+  it('should correctly detect a pattern on string containing "test"', () => {
+    const mdValue = {value: "This is a test value"} as MetadataValue;
+    expect(comp.hasLink(mdValue)).toBe(true);
+  });
+
 });

src/app/item-page/field-components/metadata-values/metadata-values.component.ts

@@ -67,9 +67,9 @@ export class MetadataValuesComponent implements OnChanges {
 
   /**
    * Does this metadata value have a valid URL that should be rendered as a link?
-   * @param value
+   * @param value A MetadataValue being displayed
    */
-  hasLink(value): boolean {
+  hasLink(value: MetadataValue): boolean {
     if (hasValue(this.urlRegex)) {
       const pattern = new RegExp(this.urlRegex);
       return pattern.test(value.value);

src/app/item-page/simple/field-components/specific-field/generic/generic-item-page-field.component.ts

@@ -43,7 +43,7 @@ export class GenericItemPageFieldComponent extends ItemPageFieldComponent {
   /**
    * Whether any valid HTTP(S) URL should be rendered as a link
    */
-  @Input() urlRegex?;
+  @Input() urlRegex?: string;
 
 
 }

src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.html

@@ -1,7 +1,8 @@
 <div>
   <a *ngIf="(metadataRepresentation.representationType=='browse_link')"
      target="_blank" class="dont-break-out"
-     href="/browse/{{metadataRepresentation.browseDefinition.id}}?{{metadataRepresentation.browseDefinition.metadataBrowse?'value':'startsWith'}}={{metadataRepresentation.getValue()}}">
+     [routerLink]="['/browse/', metadataRepresentation.browseDefinition.id]"
+     [queryParams]="getQueryParams()">
     {{metadataRepresentation.getValue()}}
   </a>
   <b>(new browse link page)</b>

src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.spec.ts

@@ -8,6 +8,11 @@ const mockMetadataRepresentation = Object.assign(new MetadatumRepresentation('ty
   value: 'Test Author'
 });
 
+const mockMetadataRepresentationWithUrl = Object.assign(new MetadatumRepresentation('type'), {
+  key: 'dc.subject',
+  value: 'http://purl.org/test/subject'
+});
+
 describe('BrowseLinkMetadataListElementComponent', () => {
   let comp: BrowseLinkMetadataListElementComponent;
   let fixture: ComponentFixture<BrowseLinkMetadataListElementComponent>;
@@ -33,6 +38,25 @@ describe('BrowseLinkMetadataListElementComponent', () => {
     it('should contain the value as a browse link', () => {
       expect(fixture.debugElement.nativeElement.textContent).toContain(mockMetadataRepresentation.value);
     });
+    it('should NOT match isLink', () => {
+      expect(comp.isLink).toBe(false);
+    })
+  });
+
+  beforeEach(waitForAsync(() => {
+    fixture = TestBed.createComponent(BrowseLinkMetadataListElementComponent);
+    comp = fixture.componentInstance;
+    comp.metadataRepresentation = mockMetadataRepresentationWithUrl;
+    fixture.detectChanges();
+  }));
+
+  waitForAsync(() => {
+    it('should contain the value expected', () => {
+      expect(fixture.debugElement.nativeElement.textContent).toContain(mockMetadataRepresentationWithUrl.value);
+    });
+    it('should match isLink', () => {
+      expect(comp.isLink).toBe(true);
+    })
   });
 
 });

src/app/shared/object-list/metadata-representation-list-element/browse-link/browse-link-metadata-list-element.component.ts

@@ -15,4 +15,15 @@ import { metadataRepresentationComponent } from '../../../metadata-representatio
  * It will simply use the value retrieved from MetadataRepresentation.getValue() to display as plain text
  */
 export class BrowseLinkMetadataListElementComponent extends MetadataRepresentationListElementComponent {
+  /**
+   * Get the appropriate query parameters for this browse link, depending on whether the browse definition
+   * expects 'startsWith' (eg browse by date) or 'value' (eg browse by title)
+   */
+  getQueryParams() {
+    let queryParams = {startsWith: this.metadataRepresentation.getValue()};
+    if (this.metadataRepresentation.browseDefinition.metadataBrowse) {
+      return {value: this.metadataRepresentation.getValue()};
+    }
+    return queryParams;
+  }
 }

src/app/shared/object-list/metadata-representation-list-element/metadata-representation-list-element.component.ts

@@ -14,6 +14,9 @@ export class MetadataRepresentationListElementComponent {
    */
   metadataRepresentation: MetadataRepresentation;
 
+  /**
+   * Returns true if this component's value matches a basic regex "Is this an HTTP URL" test
+   */
   isLink(): boolean {
     // Match any http:// or https://
     const linkPattern = /^https?\/\//;

src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.html

@@ -4,13 +4,14 @@
     {{metadataRepresentation.getValue()}}
   </span>
   <a *ngIf="(metadataRepresentation.representationType=='plain_text') && isLink()" class="dont-break-out"
-  target="_blank" href="{{metadataRepresentation.getValue()}}">
+  target="_blank" [href]="metadataRepresentation.getValue()">
     {{metadataRepresentation.getValue()}}
   </a>
   <span *ngIf="(metadataRepresentation.representationType=='authority_controlled')" class="dont-break-out">{{metadataRepresentation.getValue()}}</span>
   <a *ngIf="(metadataRepresentation.representationType=='browse_link')"
      class="dont-break-out ds-browse-link"
-     href="/browse/{{metadataRepresentation.browseDefinition.id}}?{{metadataRepresentation.browseDefinition.metadataBrowse?'value':'startsWith'}}={{metadataRepresentation.getValue()}}&bbm.page=1">
+     [routerLink]="['/browse/', metadataRepresentation.browseDefinition.id]"
+     [queryParams]="getQueryParams()">
     {{metadataRepresentation.getValue()}}
   </a>
 </div>

src/app/shared/object-list/metadata-representation-list-element/plain-text/plain-text-metadata-list-element.component.ts

@@ -15,4 +15,15 @@ import { metadataRepresentationComponent } from '../../../metadata-representatio
  * It will simply use the value retrieved from MetadataRepresentation.getValue() to display as plain text
  */
 export class PlainTextMetadataListElementComponent extends MetadataRepresentationListElementComponent {
+  /**
+   * Get the appropriate query parameters for this browse link, depending on whether the browse definition
+   * expects 'startsWith' (eg browse by date) or 'value' (eg browse by title)
+   */
+  getQueryParams() {
+    let queryParams = {startsWith: this.metadataRepresentation.getValue()};
+    if (this.metadataRepresentation.browseDefinition.metadataBrowse) {
+      return {value: this.metadataRepresentation.getValue()};
+    }
+    return queryParams;
+  }
 }

src/app/shared/testing/browse-definition-data-service.stub.ts

@@ -48,8 +48,8 @@ export const BrowseDefinitionDataServiceStub: any = {
 
   /**
    * Get the browse URL by providing a list of metadata keys
-   * @param metadatumKey
+   *
-   * @param linkPath
+   * @param metadataKeys  a list of fields eg. ['dc.contributor.author', 'dc.creator']
    */
   findByFields(metadataKeys: string[]): Observable<RemoteData<BrowseDefinition>> {
     let searchKeyArray: string[] = [];