From 9b856f405e5198922c81e63871e3bc0be52860af Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Mon, 5 Jul 2021 13:09:05 +0530 Subject: [PATCH 1/7] feat: move a common method into navigation service --- .../src/navigation/navigation.service.ts | 22 ++++++++++++++++ .../navigation/navigation.component.test.ts | 3 ++- .../shared/navigation/navigation.component.ts | 25 ++----------------- 3 files changed, 26 insertions(+), 24 deletions(-) diff --git a/projects/common/src/navigation/navigation.service.ts b/projects/common/src/navigation/navigation.service.ts index 3311abe95..75ae973fa 100644 --- a/projects/common/src/navigation/navigation.service.ts +++ b/projects/common/src/navigation/navigation.service.ts @@ -13,8 +13,10 @@ import { UrlSegment, UrlTree } from '@angular/router'; +import { uniq } from 'lodash-es'; import { from, Observable, of } from 'rxjs'; import { distinctUntilChanged, filter, map, share, skip, startWith, switchMap, take } from 'rxjs/operators'; +import { NavItemConfig, NavItemType } from '../../../components/src/navigation/navigation-list.component'; import { isEqualIgnoreFunctions, throwIfNil } from '../utilities/lang/lang-utils'; import { Dictionary } from '../utilities/types/types'; import { TraceRoute } from './trace-route'; @@ -229,6 +231,26 @@ export class NavigationService { return this.findRouteConfig(path, childRoutes ? childRoutes : []); } + public decorateNavItem(navItem: NavItemConfig, activatedRoute: ActivatedRoute): NavItemConfig { + if (navItem.type !== NavItemType.Link) { + return { ...navItem }; + } + const features = navItem.matchPaths + .map(path => this.getRouteConfig([path], activatedRoute)) + .filter((maybeRoute): maybeRoute is TraceRoute => maybeRoute !== undefined) + .flatMap(route => this.getFeaturesForRoute(route)) + .concat(navItem.features || []); + + return { + ...navItem, + features: uniq(features) + }; + } + + private getFeaturesForRoute(route: TraceRoute): string[] { + return (route.data && route.data.features) || []; + } + public rootRoute(): ActivatedRoute { return this.router.routerState.root; } diff --git a/src/app/shared/navigation/navigation.component.test.ts b/src/app/shared/navigation/navigation.component.test.ts index 873ca3618..7ffe5b976 100644 --- a/src/app/shared/navigation/navigation.component.test.ts +++ b/src/app/shared/navigation/navigation.component.test.ts @@ -18,7 +18,8 @@ describe('NavigationComponent', () => { data: { features: ['example-feature'] } - }) + }), + decorateNavItem: jest.fn().mockImplementation(navItem => ({ ...navItem, features: ['example-feature'] })) }), mockProvider(ActivatedRoute), mockProvider(PreferenceService, { get: jest.fn().mockReturnValue(of(false)) }) diff --git a/src/app/shared/navigation/navigation.component.ts b/src/app/shared/navigation/navigation.component.ts index e4b9a8cf9..443fd2e18 100644 --- a/src/app/shared/navigation/navigation.component.ts +++ b/src/app/shared/navigation/navigation.component.ts @@ -1,10 +1,9 @@ import { ChangeDetectionStrategy, Component } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; import { IconType } from '@hypertrace/assets-library'; -import { NavigationService, PreferenceService, TraceRoute } from '@hypertrace/common'; +import { NavigationService, PreferenceService } from '@hypertrace/common'; import { NavItemConfig, NavItemType } from '@hypertrace/components'; import { ObservabilityIconType } from '@hypertrace/observability'; -import { uniq } from 'lodash-es'; import { Observable } from 'rxjs'; @Component({ @@ -79,31 +78,11 @@ export class NavigationComponent { private readonly preferenceService: PreferenceService, private readonly activatedRoute: ActivatedRoute ) { - this.navItems = this.navItemDefinitions.map(definition => this.decorateNavItem(definition)); + this.navItems = this.navItemDefinitions.map(definition => this.navigationService.decorateNavItem(definition, this.activatedRoute)); this.isCollapsed$ = this.preferenceService.get(NavigationComponent.COLLAPSED_PREFERENCE, false); } public onViewToggle(collapsed: boolean): void { this.preferenceService.set(NavigationComponent.COLLAPSED_PREFERENCE, collapsed); } - - private decorateNavItem(navItem: NavItemConfig): NavItemConfig { - if (navItem.type !== NavItemType.Link) { - return { ...navItem }; - } - const features = navItem.matchPaths - .map(path => this.navigationService.getRouteConfig([path], this.activatedRoute)) - .filter((maybeRoute): maybeRoute is TraceRoute => maybeRoute !== undefined) - .flatMap(route => this.getFeaturesForRoute(route)) - .concat(navItem.features || []); - - return { - ...navItem, - features: uniq(features) - }; - } - - private getFeaturesForRoute(route: TraceRoute): string[] { - return (route.data && route.data.features) || []; - } } From 1842a357e8ebe9f75a181a360884ab35c3954e08 Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Tue, 6 Jul 2021 20:18:22 +0530 Subject: [PATCH 2/7] fix: fix lint issues --- projects/common/src/navigation/navigation.service.ts | 2 +- src/app/shared/navigation/navigation.component.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/projects/common/src/navigation/navigation.service.ts b/projects/common/src/navigation/navigation.service.ts index 75ae973fa..9c2f6e0bf 100644 --- a/projects/common/src/navigation/navigation.service.ts +++ b/projects/common/src/navigation/navigation.service.ts @@ -13,10 +13,10 @@ import { UrlSegment, UrlTree } from '@angular/router'; +import { NavItemConfig, NavItemType } from '@hypertrace/components'; import { uniq } from 'lodash-es'; import { from, Observable, of } from 'rxjs'; import { distinctUntilChanged, filter, map, share, skip, startWith, switchMap, take } from 'rxjs/operators'; -import { NavItemConfig, NavItemType } from '../../../components/src/navigation/navigation-list.component'; import { isEqualIgnoreFunctions, throwIfNil } from '../utilities/lang/lang-utils'; import { Dictionary } from '../utilities/types/types'; import { TraceRoute } from './trace-route'; diff --git a/src/app/shared/navigation/navigation.component.ts b/src/app/shared/navigation/navigation.component.ts index 443fd2e18..6c9aed540 100644 --- a/src/app/shared/navigation/navigation.component.ts +++ b/src/app/shared/navigation/navigation.component.ts @@ -78,7 +78,9 @@ export class NavigationComponent { private readonly preferenceService: PreferenceService, private readonly activatedRoute: ActivatedRoute ) { - this.navItems = this.navItemDefinitions.map(definition => this.navigationService.decorateNavItem(definition, this.activatedRoute)); + this.navItems = this.navItemDefinitions.map(definition => + this.navigationService.decorateNavItem(definition, this.activatedRoute) + ); this.isCollapsed$ = this.preferenceService.get(NavigationComponent.COLLAPSED_PREFERENCE, false); } From 051c23ed82a3ac53623a23bc9ca3fe5c4af282f3 Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Wed, 7 Jul 2021 10:59:05 +0530 Subject: [PATCH 3/7] fix: conflict resolution --- projects/common/src/navigation/navigation.service.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/projects/common/src/navigation/navigation.service.ts b/projects/common/src/navigation/navigation.service.ts index f630c3df4..1414a74f4 100644 --- a/projects/common/src/navigation/navigation.service.ts +++ b/projects/common/src/navigation/navigation.service.ts @@ -237,7 +237,7 @@ export class NavigationService { } const features = navItem.matchPaths .map(path => this.getRouteConfig([path], activatedRoute)) - .filter((maybeRoute): maybeRoute is TraceRoute => maybeRoute !== undefined) + .filter((maybeRoute): maybeRoute is HtRoute => maybeRoute !== undefined) .flatMap(route => this.getFeaturesForRoute(route)) .concat(navItem.features || []); @@ -247,7 +247,7 @@ export class NavigationService { }; } - private getFeaturesForRoute(route: TraceRoute): string[] { + private getFeaturesForRoute(route: HtRoute): string[] { return (route.data && route.data.features) || []; } From 94424e4d86707f61eaf68a92bbe0dd50a203af95 Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Wed, 7 Jul 2021 11:55:28 +0530 Subject: [PATCH 4/7] fix: add test for decorateNavItem method --- .../src/navigation/navigation.service.test.ts | 36 ++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/projects/common/src/navigation/navigation.service.test.ts b/projects/common/src/navigation/navigation.service.test.ts index 90ca857e2..daf473100 100644 --- a/projects/common/src/navigation/navigation.service.test.ts +++ b/projects/common/src/navigation/navigation.service.test.ts @@ -1,6 +1,9 @@ import { Location } from '@angular/common'; import { Router, UrlSegment } from '@angular/router'; import { RouterTestingModule } from '@angular/router/testing'; +import { IconType } from '@hypertrace/assets-library'; +import { HtRoute } from '@hypertrace/common'; +import { NavItemType } from '@hypertrace/components'; import { patchRouterNavigateForTest } from '@hypertrace/test-utils'; import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest'; import { @@ -12,7 +15,7 @@ import { } from './navigation.service'; describe('Navigation Service', () => { - const firstChildRouteConfig = { + const firstChildRouteConfig: HtRoute = { path: 'child', children: [] }; @@ -47,6 +50,7 @@ describe('Navigation Service', () => { RouterTestingModule.withRoutes([ { path: 'root', + data: { features: ['test-feature'] }, children: [firstChildRouteConfig, secondChildRouteConfig] } ]) @@ -285,4 +289,34 @@ describe('Navigation Service', () => { ); } }); + + test('decorating navItem with features work as expected', () => { + expect( + spectator.service.decorateNavItem( + { + type: NavItemType.Header, + label: 'Label' + }, + spectator.service.getCurrentActivatedRoute() + ) + ).toEqual({ type: NavItemType.Header, label: 'Label' }); + + expect( + spectator.service.decorateNavItem( + { + type: NavItemType.Link, + label: 'Label', + icon: IconType.None, + matchPaths: ['root'], + }, + spectator.service.rootRoute() + ) + ).toEqual({ + type: NavItemType.Link, + label: 'Label', + icon: IconType.None, + matchPaths: ['root'], + features: ['test-feature'] + }); + }); }); From 5dce37971d0652b51d190479018416d3de232ff8 Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Wed, 7 Jul 2021 12:06:12 +0530 Subject: [PATCH 5/7] fix: prettier fix --- projects/common/src/navigation/navigation.service.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/common/src/navigation/navigation.service.test.ts b/projects/common/src/navigation/navigation.service.test.ts index daf473100..5162230e1 100644 --- a/projects/common/src/navigation/navigation.service.test.ts +++ b/projects/common/src/navigation/navigation.service.test.ts @@ -307,7 +307,7 @@ describe('Navigation Service', () => { type: NavItemType.Link, label: 'Label', icon: IconType.None, - matchPaths: ['root'], + matchPaths: ['root'] }, spectator.service.rootRoute() ) From adcb9027f3240452605e45b2f05aa3b1ba0a783e Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Wed, 7 Jul 2021 12:29:14 +0530 Subject: [PATCH 6/7] fix: remove unused method --- .../shared/navigation/navigation.component.ts | 22 +------------------ 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/src/app/shared/navigation/navigation.component.ts b/src/app/shared/navigation/navigation.component.ts index bd909b91a..6c9aed540 100644 --- a/src/app/shared/navigation/navigation.component.ts +++ b/src/app/shared/navigation/navigation.component.ts @@ -1,7 +1,7 @@ import { ChangeDetectionStrategy, Component } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; import { IconType } from '@hypertrace/assets-library'; -import { HtRoute, NavigationService, PreferenceService } from '@hypertrace/common'; +import { NavigationService, PreferenceService } from '@hypertrace/common'; import { NavItemConfig, NavItemType } from '@hypertrace/components'; import { ObservabilityIconType } from '@hypertrace/observability'; import { Observable } from 'rxjs'; @@ -87,24 +87,4 @@ export class NavigationComponent { public onViewToggle(collapsed: boolean): void { this.preferenceService.set(NavigationComponent.COLLAPSED_PREFERENCE, collapsed); } - - private decorateNavItem(navItem: NavItemConfig): NavItemConfig { - if (navItem.type !== NavItemType.Link) { - return { ...navItem }; - } - const features = navItem.matchPaths - .map(path => this.navigationService.getRouteConfig([path], this.activatedRoute)) - .filter((maybeRoute): maybeRoute is HtRoute => maybeRoute !== undefined) - .flatMap(route => this.getFeaturesForRoute(route)) - .concat(navItem.features || []); - - return { - ...navItem, - features: uniq(features) - }; - } - - private getFeaturesForRoute(route: HtRoute): string[] { - return (route.data && route.data.features) || []; - } } From f28ccc5a2e04632cc4bee087ed6a7304ea0b47c3 Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Wed, 7 Jul 2021 12:45:46 +0530 Subject: [PATCH 7/7] fix: update test --- projects/common/src/navigation/navigation.service.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/projects/common/src/navigation/navigation.service.test.ts b/projects/common/src/navigation/navigation.service.test.ts index 5162230e1..2e08eb5b6 100644 --- a/projects/common/src/navigation/navigation.service.test.ts +++ b/projects/common/src/navigation/navigation.service.test.ts @@ -2,7 +2,6 @@ import { Location } from '@angular/common'; import { Router, UrlSegment } from '@angular/router'; import { RouterTestingModule } from '@angular/router/testing'; import { IconType } from '@hypertrace/assets-library'; -import { HtRoute } from '@hypertrace/common'; import { NavItemType } from '@hypertrace/components'; import { patchRouterNavigateForTest } from '@hypertrace/test-utils'; import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest'; @@ -15,7 +14,7 @@ import { } from './navigation.service'; describe('Navigation Service', () => { - const firstChildRouteConfig: HtRoute = { + const firstChildRouteConfig = { path: 'child', children: [] };