Skip to content
53 changes: 52 additions & 1 deletion projects/common/src/navigation/navigation.service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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`
);
}
});
});
26 changes: 22 additions & 4 deletions projects/common/src/navigation/navigation.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,22 @@ export class NavigationService {
return this.currentParamMap.getAll(parameterName);
}

public constructExternalUrl(urlString: string): string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! looks good - with these changes, do we still need the changes in time range service or the new url util?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope, I already reverted those changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh right, the TR ones are gone - one of the url utils looks like it's still there being unused though

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 } {
Expand All @@ -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
}
],
Expand Down Expand Up @@ -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;
Expand All @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions projects/common/src/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,6 @@ export * from './time/time';

// Validators
export * from './utilities/validators/email-validator';

// URL Utilities
export * from './utilities/url/url-utilities';
13 changes: 13 additions & 0 deletions projects/common/src/utilities/url/url-utilities.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { Params } from '@angular/router';
import { getQueryParamStringFromObject } from './url-utilities';

describe('getQueryParamStringFromObject', () => {
test('should return string from a string dictionary', () => {
const params: Params = {};
expect(getQueryParamStringFromObject(params)).toBe('');
params.a = 'value_1';
expect(getQueryParamStringFromObject(params)).toBe('a=value_1');
params.b = 'value_2';
expect(getQueryParamStringFromObject(params)).toBe(`a=value_1&b=value_2`);
});
});
5 changes: 5 additions & 0 deletions projects/common/src/utilities/url/url-utilities.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { HttpParams } from '@angular/common/http';
import { Params } from '@angular/router';

export const getQueryParamStringFromObject = (params: Params): string =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed or should we use the below function directly?

Copy link
Contributor Author

@alok-traceable alok-traceable May 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I will remove this function as it is not being used anywhere, but It is better to have a separate function like this, firstly consumers didn't have to write a new instance of HttpParams every time, and secondly if later we want to standardize queryParams key-value pair we need to change this function only.

new HttpParams({ fromObject: params }).toString();
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand All @@ -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);
}));
});
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
@@ -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 {}