Open in new tab (#856)

* feature: adding open in new tab icon on security overlay

* feat: changes for open in new tab component

* fix: file rename and unti tests

* fix: lint fixes

* fix: lint fixes

* fix: pr suggested changes

* fix: pr suggested changes

* fix: lint fixes

* fix: pr suggested changes

* feat: pr suggested changes

* feat: pr suggested changes

* fix: merging main

* fix: imporving code coverage

* fix: removing unused utilities

Co-authored-by: Alok Singh <[email protected]>

Author: alok-traceable

projects/common/src/navigation/navigation.service.test.ts

@@ -3,7 +3,13 @@ import { Router, UrlSegment } from '@angular/router';
 import { RouterTestingModule } from '@angular/router/testing';
 import { patchRouterNavigateForTest } from '@hypertrace/test-utils';
 import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest';
-import { NavigationParamsType, NavigationService } from './navigation.service';
+import {
+  ExternalNavigationPathParams,
+  ExternalNavigationWindowHandling,
+  NavigationParams,
+  NavigationParamsType,
+  NavigationService
+} from './navigation.service';
 
 describe('Navigation Service', () => {
   const firstChildRouteConfig = {
@@ -234,4 +240,49 @@ describe('Navigation Service', () => {
     ]);
     expect(spectator.service.getCurrentActivatedRoute().snapshot.queryParams).toEqual({ global: 'foo' });
   });
+
+  test('construct external url in case useGlobalParams is set to true', () => {
+    const externalNavigationParams: NavigationParams = {
+      navType: NavigationParamsType.External,
+      useGlobalParams: true,
+      url: '/some/internal/path/of/app',
+      windowHandling: ExternalNavigationWindowHandling.NewWindow
+    };
+
+    spectator.service.addQueryParametersToUrl({ time: '1h', environment: 'development' });
+    spectator.service.registerGlobalQueryParamKey('time');
+    spectator.service.registerGlobalQueryParamKey('environment');
+
+    externalNavigationParams.useGlobalParams = true;
+
+    expect(Array.isArray(spectator.service.buildNavigationParams(externalNavigationParams).path)).toBe(true);
+    expect(spectator.service.buildNavigationParams(externalNavigationParams).path[1]).toHaveProperty(
+      ExternalNavigationPathParams.Url
+    );
+
+    let pathParam = spectator.service.buildNavigationParams(externalNavigationParams).path[1];
+    expect(typeof pathParam).not.toBe('string');
+
+    if (typeof pathParam !== 'string') {
+      expect(pathParam[ExternalNavigationPathParams.Url]).toBe(
+        `${externalNavigationParams.url}?time=1h&environment=development`
+      );
+    }
+
+    externalNavigationParams.url = '/some/internal/path/of/app?type=json';
+
+    expect(Array.isArray(spectator.service.buildNavigationParams(externalNavigationParams).path)).toBe(true);
+    expect(spectator.service.buildNavigationParams(externalNavigationParams).path[1]).toHaveProperty(
+      ExternalNavigationPathParams.Url
+    );
+
+    pathParam = spectator.service.buildNavigationParams(externalNavigationParams).path[1];
+    expect(typeof pathParam).not.toBe('string');
+
+    if (typeof pathParam !== 'string') {
+      expect(pathParam[ExternalNavigationPathParams.Url]).toBe(
+        `/some/internal/path/of/app?type=json&time=1h&environment=development`
+      );
+    }
+  });
 });

projects/common/src/navigation/navigation.service.ts

@@ -73,6 +73,22 @@ export class NavigationService {
     return this.currentParamMap.getAll(parameterName);
   }
 
+  public constructExternalUrl(urlString: string): string {
+    const inputUrlTree: UrlTree = this.router.parseUrl(urlString);
+    const globalQueryParams: Params = {};
+
+    this.globalQueryParams.forEach(key => {
+      const paramValue = this.getQueryParameter(key, '');
+      if (paramValue !== '') {
+        globalQueryParams[key] = paramValue;
+      }
+    });
+
+    inputUrlTree.queryParams = { ...inputUrlTree.queryParams, ...globalQueryParams };
+
+    return this.router.serializeUrl(inputUrlTree);
+  }
+
   public buildNavigationParams(
     paramsOrUrl: NavigationParams | string
   ): { path: NavigationPath; extras?: NavigationExtras } {
@@ -84,7 +100,9 @@ export class NavigationService {
         path: [
           '/external',
           {
-            [ExternalNavigationPathParams.Url]: params.url,
+            [ExternalNavigationPathParams.Url]: params.useGlobalParams
+              ? this.constructExternalUrl(params.url)
+              : params.url,
             [ExternalNavigationPathParams.WindowHandling]: params.windowHandling
           }
         ],
@@ -304,7 +322,7 @@ export interface QueryParamObject extends Params {
 
 export type NavigationPath = string | (string | Dictionary<string>)[];
 
-export type NavigationParams = InAppNavigationParams | ExternalNavigationParamsNew;
+export type NavigationParams = InAppNavigationParams | ExternalNavigationParams;
 export interface InAppNavigationParams {
   navType: NavigationParamsType.InApp;
   path: NavigationPath;
@@ -314,11 +332,11 @@ export interface InAppNavigationParams {
   relativeTo?: ActivatedRoute;
 }
 
-export interface ExternalNavigationParamsNew {
+export interface ExternalNavigationParams {
   navType: NavigationParamsType.External;
   url: string;
   windowHandling: ExternalNavigationWindowHandling; // Currently an enum called NavigationType
-  queryParams?: QueryParamObject;
+  useGlobalParams?: boolean;
 }
 
 export const enum ExternalNavigationPathParams {

projects/components/src/open-in-new-tab/open-in-new-tab.component.test.ts

@@ -1,13 +1,10 @@
 import { fakeAsync } from '@angular/core/testing';
-import {
-  ExternalNavigationWindowHandling,
-  NavigationParamsType,
-  NavigationService,
-  TimeRangeService
-} from '@hypertrace/common';
+import { NavigationService, TimeRangeService } from '@hypertrace/common';
 import { createHostFactory, mockProvider, Spectator } from '@ngneat/spectator/jest';
 import { MockComponent } from 'ng-mocks';
-import { ButtonComponent } from '../button/button.component';
+import { IconSize } from '../icon/icon-size';
+import { IconComponent } from '../icon/icon.component';
+import { LinkComponent } from '../link/link.component';
 import { OpenInNewTabComponent } from './open-in-new-tab.component';
 
 describe('Open in new tab component', () => {
@@ -16,42 +13,60 @@ describe('Open in new tab component', () => {
   const createHost = createHostFactory({
     shallow: true,
     component: OpenInNewTabComponent,
-    declarations: [MockComponent(ButtonComponent)],
+    declarations: [MockComponent(LinkComponent), MockComponent(IconComponent)],
     providers: [
       mockProvider(TimeRangeService, {
         getShareableCurrentUrl: () => 'url-from-timerangeservice'
       }),
-      mockProvider(NavigationService)
+      mockProvider(NavigationService, {
+        buildNavigationParams: jest.fn().mockReturnValue({
+          path: [
+            '/external',
+            {
+              url: 'http://test.hypertrace.ai',
+              navType: 'same_window'
+            }
+          ],
+          extras: { skipLocationChange: true }
+        })
+      })
     ]
   });
 
-  test('should call navigate as expected when URL input is not specified', fakeAsync(() => {
-    spectator = createHost('<ht-open-in-new-tab></ht-open-in-new-tab>');
-
-    spectator.click('.open-in-new-tab-button');
-    spectator.tick();
-
-    expect(spectator.inject(NavigationService).navigate).toHaveBeenCalledWith({
-      navType: NavigationParamsType.External,
-      windowHandling: ExternalNavigationWindowHandling.NewWindow,
-      url: 'url-from-timerangeservice'
+  test('Open in new tab component should not be displayed if paramsOrUrl is undefined', () => {
+    spectator = createHost(`<ht-open-in-new-tab [paramsOrUrl]="paramsOrUrl"></ht-open-in-new-tab>`, {
+      hostProps: {
+        paramsOrUrl: undefined
+      }
     });
-  }));
+    expect(spectator.query('.open-in-new-tab')).not.toExist();
+  });
 
-  test('should call navigate as expected when URL input is specified', fakeAsync(() => {
-    spectator = createHost('<ht-open-in-new-tab [url]="url"></ht-open-in-new-tab>', {
+  test(`Open in new tab component should exist if paramsOrUrl is not undefined`, fakeAsync(() => {
+    spectator = createHost(`<ht-open-in-new-tab [paramsOrUrl]="paramsOrUrl"></ht-open-in-new-tab>`, {
       hostProps: {
-        url: 'input-url'
+        paramsOrUrl: {}
       }
     });
+    expect(spectator.query('.open-in-new-tab')).toExist();
+    expect(spectator.query('ht-link')).toExist();
+    // Default value of icon size
+    expect(spectator.component.iconSize).toBe(IconSize.Medium);
+  }));
 
-    spectator.click('.open-in-new-tab-button');
-    spectator.tick();
-
-    expect(spectator.inject(NavigationService).navigate).toHaveBeenCalledWith({
-      navType: NavigationParamsType.External,
-      windowHandling: ExternalNavigationWindowHandling.NewWindow,
-      url: 'input-url'
-    });
+  test(`Open in new tab component should contain icon of passed size`, fakeAsync(() => {
+    spectator = createHost(
+      `<ht-open-in-new-tab [paramsOrUrl]="paramsOrUrl" [iconSize]="iconSize" ></ht-open-in-new-tab>`,
+      {
+        hostProps: {
+          paramsOrUrl: {},
+          iconSize: IconSize.Small
+        }
+      }
+    );
+    expect(spectator.query('.open-in-new-tab')).toExist();
+    expect(spectator.query('ht-link')).toExist();
+    // Expected value of icon size if pass
+    expect(spectator.component.iconSize).toBe(IconSize.Small);
   }));
 });

projects/components/src/open-in-new-tab/open-in-new-tab.component.ts

@@ -1,46 +1,23 @@
 import { ChangeDetectionStrategy, Component, Input } from '@angular/core';
 import { IconType } from '@hypertrace/assets-library';
-import {
-  ExternalNavigationWindowHandling,
-  NavigationParamsType,
-  NavigationService,
-  TimeRangeService
-} from '@hypertrace/common';
-import { ButtonSize, ButtonStyle } from '../button/button';
+import { ExternalNavigationParams } from '@hypertrace/common';
+import { IconSize } from '../icon/icon-size';
 
 @Component({
   selector: 'ht-open-in-new-tab',
   changeDetection: ChangeDetectionStrategy.OnPush,
   template: `
-    <div class="open-in-new-tab" htTooltip="Open in a new tab">
-      <ht-button
-        class="open-in-new-tab-button"
-        display="${ButtonStyle.Outlined}"
-        [size]="this.size"
-        icon="${IconType.OpenInNewTab}"
-        (click)="this.openInNewTab()"
-      ></ht-button>
+    <div *ngIf="this.paramsOrUrl" class="open-in-new-tab" htTooltip="Open in a new tab">
+      <ht-link [paramsOrUrl]="this.paramsOrUrl">
+        <ht-icon icon="${IconType.OpenInNewTab}" [size]="this.iconSize"></ht-icon>
+      </ht-link>
     </div>
   `
 })
 export class OpenInNewTabComponent {
   @Input()
-  public size?: ButtonSize = ButtonSize.Small;
+  public paramsOrUrl?: ExternalNavigationParams | string;
 
   @Input()
-  public url?: string;
-
-  public constructor(
-    private readonly navigationService: NavigationService,
-    private readonly timeRangeService: TimeRangeService
-  ) {}
-
-  public openInNewTab(): void {
-    this.navigationService.navigate({
-      navType: NavigationParamsType.External,
-      windowHandling: ExternalNavigationWindowHandling.NewWindow,
-      // Use input url if available. Else construct a shareable URL for the page
-      url: this.url ?? this.timeRangeService.getShareableCurrentUrl()
-    });
-  }
+  public iconSize: IconSize = IconSize.Medium;
 }

projects/components/src/open-in-new-tab/open-in-new-tab.module.ts

@@ -1,12 +1,13 @@
 import { CommonModule } from '@angular/common';
 import { NgModule } from '@angular/core';
-import { ButtonModule } from '../button/button.module';
+import { IconModule } from '../icon/icon.module';
+import { LinkModule } from '../link/link.module';
 import { TooltipModule } from '../tooltip/tooltip.module';
 import { OpenInNewTabComponent } from './open-in-new-tab.component';
 
 @NgModule({
   declarations: [OpenInNewTabComponent],
   exports: [OpenInNewTabComponent],
-  imports: [CommonModule, ButtonModule, TooltipModule]
+  imports: [CommonModule, TooltipModule, LinkModule, IconModule]
 })
 export class OpenInNewTabModule {}