From e255053d37258538a6bb5faa411e15be6f735cd4 Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Tue, 16 Nov 2021 13:47:45 +0530 Subject: [PATCH 1/4] fix: common project should not depend on components project --- .../src/navigation/navigation.service.test.ts | 32 ----------- .../src/navigation/navigation.service.ts | 22 -------- .../navigation-list.service.test.ts | 56 +++++++++++++++++++ .../src/navigation/navigation-list.service.ts | 30 ++++++++++ projects/components/src/public-api.ts | 1 + .../shared/navigation/navigation.component.ts | 8 +-- 6 files changed, 91 insertions(+), 58 deletions(-) create mode 100644 projects/components/src/navigation/navigation-list.service.test.ts create mode 100644 projects/components/src/navigation/navigation-list.service.ts diff --git a/projects/common/src/navigation/navigation.service.test.ts b/projects/common/src/navigation/navigation.service.test.ts index ffe9a2c68..11fb50f46 100644 --- a/projects/common/src/navigation/navigation.service.test.ts +++ b/projects/common/src/navigation/navigation.service.test.ts @@ -2,9 +2,7 @@ import { Location } from '@angular/common'; import { Title } from '@angular/platform-browser'; import { Router, UrlSegment } from '@angular/router'; import { RouterTestingModule } from '@angular/router/testing'; -import { IconType } from '@hypertrace/assets-library'; import { APP_TITLE } from '@hypertrace/common'; -import { NavItemType } from '@hypertrace/components'; import { patchRouterNavigateForTest } from '@hypertrace/test-utils'; import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest'; import { @@ -298,36 +296,6 @@ 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'] - }); - }); - test('setting title should work as expected', () => { router.navigate(['root', 'child']); expect(spectator.inject(Title).setTitle).toHaveBeenCalledWith('defaultAppTitle | child1'); diff --git a/projects/common/src/navigation/navigation.service.ts b/projects/common/src/navigation/navigation.service.ts index e80c263ee..11a4cf84e 100644 --- a/projects/common/src/navigation/navigation.service.ts +++ b/projects/common/src/navigation/navigation.service.ts @@ -14,8 +14,6 @@ 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, tap } from 'rxjs/operators'; import { isEqualIgnoreFunctions, throwIfNil } from '../utilities/lang/lang-utils'; @@ -243,26 +241,6 @@ 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 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) || []; - } - public rootRoute(): ActivatedRoute { return this.router.routerState.root; } diff --git a/projects/components/src/navigation/navigation-list.service.test.ts b/projects/components/src/navigation/navigation-list.service.test.ts new file mode 100644 index 000000000..96c7f456d --- /dev/null +++ b/projects/components/src/navigation/navigation-list.service.test.ts @@ -0,0 +1,56 @@ +import { ActivatedRoute } from '@angular/router'; +import { IconType } from '@hypertrace/assets-library'; +import { NavigationService } from '@hypertrace/common'; +import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest'; +import { NavigationListService } from './navigation-list.service'; +import { NavItemType} from './navigation.config'; + +describe('Navigation List Service', () => { + let spectator: SpectatorService; + + const buildService = createServiceFactory({ + service: NavigationListService, + providers: [ + mockProvider(ActivatedRoute), + mockProvider(NavigationService, { getRouteConfig: jest.fn().mockReturnValue({ + path: 'root', + data: { features: ['test-feature'] }, + children: [] + })}), + ] + }); + + beforeEach(() => { + spectator = buildService(); + }); + + test('decorating navItem with features work as expected', () => { + expect( + spectator.service.decorateNavItem( + { + type: NavItemType.Header, + label: 'Label' + }, + spectator.inject(ActivatedRoute) + ) + ).toEqual({ type: NavItemType.Header, label: 'Label' }); + + expect( + spectator.service.decorateNavItem( + { + type: NavItemType.Link, + label: 'Label', + icon: IconType.None, + matchPaths: ['root'] + }, + spectator.inject(ActivatedRoute) + ) + ).toEqual({ + type: NavItemType.Link, + label: 'Label', + icon: IconType.None, + matchPaths: ['root'], + features: ['test-feature'] + }); + }); +}); diff --git a/projects/components/src/navigation/navigation-list.service.ts b/projects/components/src/navigation/navigation-list.service.ts new file mode 100644 index 000000000..c35ea3c66 --- /dev/null +++ b/projects/components/src/navigation/navigation-list.service.ts @@ -0,0 +1,30 @@ +import { Injectable } from '@angular/core'; +import { ActivatedRoute } from '@angular/router'; +import { HtRoute, NavigationService } from '@hypertrace/common'; +import { uniq } from 'lodash-es'; +import { NavItemConfig, NavItemType } from './navigation.config'; + +@Injectable({ providedIn: 'root'}) +export class NavigationListService { + public constructor(private readonly navigationService: NavigationService) {} + + public decorateNavItem(navItem: NavItemConfig, activatedRoute: ActivatedRoute): NavItemConfig { + if (navItem.type !== NavItemType.Link) { + return { ...navItem }; + } + const features = navItem.matchPaths + .map(path => this.navigationService.getRouteConfig([path], 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) || []; + } +} diff --git a/projects/components/src/public-api.ts b/projects/components/src/public-api.ts index b9f887434..374277bae 100644 --- a/projects/components/src/public-api.ts +++ b/projects/components/src/public-api.ts @@ -157,6 +157,7 @@ export * from './navigation/navigation-list.module'; export * from './navigation/nav-item/nav-item.component'; export * from './navigation/navigation.config'; export * from './navigation/navigation-list-component.service'; +export * from './navigation/navigation-list.service'; // Let async export { LetAsyncDirective } from './let-async/let-async.directive'; diff --git a/src/app/shared/navigation/navigation.component.ts b/src/app/shared/navigation/navigation.component.ts index 6c9aed540..50d05a243 100644 --- a/src/app/shared/navigation/navigation.component.ts +++ b/src/app/shared/navigation/navigation.component.ts @@ -1,8 +1,8 @@ import { ChangeDetectionStrategy, Component } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; import { IconType } from '@hypertrace/assets-library'; -import { NavigationService, PreferenceService } from '@hypertrace/common'; -import { NavItemConfig, NavItemType } from '@hypertrace/components'; +import { PreferenceService } from '@hypertrace/common'; +import { NavigationListService, NavItemConfig, NavItemType } from '@hypertrace/components'; import { ObservabilityIconType } from '@hypertrace/observability'; import { Observable } from 'rxjs'; @@ -74,12 +74,12 @@ export class NavigationComponent { ]; public constructor( - private readonly navigationService: NavigationService, + private readonly navigationListService: NavigationListService, private readonly preferenceService: PreferenceService, private readonly activatedRoute: ActivatedRoute ) { this.navItems = this.navItemDefinitions.map(definition => - this.navigationService.decorateNavItem(definition, this.activatedRoute) + this.navigationListService.decorateNavItem(definition, this.activatedRoute) ); this.isCollapsed$ = this.preferenceService.get(NavigationComponent.COLLAPSED_PREFERENCE, false); } From fac51dbdd399dd7eb86bf16232dcbe6d2283da57 Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Tue, 16 Nov 2021 13:52:03 +0530 Subject: [PATCH 2/4] fix: prettier fix --- .../src/navigation/navigation-list.service.test.ts | 8 +++++--- .../components/src/navigation/navigation-list.service.ts | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/projects/components/src/navigation/navigation-list.service.test.ts b/projects/components/src/navigation/navigation-list.service.test.ts index 96c7f456d..d2bb6be1d 100644 --- a/projects/components/src/navigation/navigation-list.service.test.ts +++ b/projects/components/src/navigation/navigation-list.service.test.ts @@ -3,7 +3,7 @@ import { IconType } from '@hypertrace/assets-library'; import { NavigationService } from '@hypertrace/common'; import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest'; import { NavigationListService } from './navigation-list.service'; -import { NavItemType} from './navigation.config'; +import { NavItemType } from './navigation.config'; describe('Navigation List Service', () => { let spectator: SpectatorService; @@ -12,11 +12,13 @@ describe('Navigation List Service', () => { service: NavigationListService, providers: [ mockProvider(ActivatedRoute), - mockProvider(NavigationService, { getRouteConfig: jest.fn().mockReturnValue({ + mockProvider(NavigationService, { + getRouteConfig: jest.fn().mockReturnValue({ path: 'root', data: { features: ['test-feature'] }, children: [] - })}), + }) + }) ] }); diff --git a/projects/components/src/navigation/navigation-list.service.ts b/projects/components/src/navigation/navigation-list.service.ts index c35ea3c66..ef1efcc29 100644 --- a/projects/components/src/navigation/navigation-list.service.ts +++ b/projects/components/src/navigation/navigation-list.service.ts @@ -4,7 +4,7 @@ import { HtRoute, NavigationService } from '@hypertrace/common'; import { uniq } from 'lodash-es'; import { NavItemConfig, NavItemType } from './navigation.config'; -@Injectable({ providedIn: 'root'}) +@Injectable({ providedIn: 'root' }) export class NavigationListService { public constructor(private readonly navigationService: NavigationService) {} From b2c67d5aa2051f261690b3b2a45d421fa1d18995 Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Tue, 16 Nov 2021 18:25:03 +0530 Subject: [PATCH 3/4] fix: test update --- src/app/shared/navigation/navigation.component.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/app/shared/navigation/navigation.component.test.ts b/src/app/shared/navigation/navigation.component.test.ts index 7ffe5b976..d60e21120 100644 --- a/src/app/shared/navigation/navigation.component.test.ts +++ b/src/app/shared/navigation/navigation.component.test.ts @@ -1,6 +1,6 @@ import { ActivatedRoute } from '@angular/router'; import { NavigationService, PreferenceService } from '@hypertrace/common'; -import { LetAsyncModule, NavigationListComponent } from '@hypertrace/components'; +import { LetAsyncModule, NavigationListComponent, NavigationListService } from '@hypertrace/components'; import { createComponentFactory, mockProvider } from '@ngneat/spectator/jest'; import { MockComponent } from 'ng-mocks'; import { BehaviorSubject, of } from 'rxjs'; @@ -19,8 +19,9 @@ describe('NavigationComponent', () => { features: ['example-feature'] } }), - decorateNavItem: jest.fn().mockImplementation(navItem => ({ ...navItem, features: ['example-feature'] })) + }), + mockProvider(NavigationListService, { decorateNavItem: jest.fn().mockImplementation(navItem => ({ ...navItem, features: ['example-feature'] }))}), mockProvider(ActivatedRoute), mockProvider(PreferenceService, { get: jest.fn().mockReturnValue(of(false)) }) ] From 6066a7337e2af858a6723ebc806fced16fbfb49b Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Tue, 16 Nov 2021 18:29:09 +0530 Subject: [PATCH 4/4] fix: prettier fix --- src/app/shared/navigation/navigation.component.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/app/shared/navigation/navigation.component.test.ts b/src/app/shared/navigation/navigation.component.test.ts index d60e21120..213d97b9c 100644 --- a/src/app/shared/navigation/navigation.component.test.ts +++ b/src/app/shared/navigation/navigation.component.test.ts @@ -18,10 +18,11 @@ describe('NavigationComponent', () => { data: { features: ['example-feature'] } - }), - + }) + }), + mockProvider(NavigationListService, { + decorateNavItem: jest.fn().mockImplementation(navItem => ({ ...navItem, features: ['example-feature'] })) }), - mockProvider(NavigationListService, { decorateNavItem: jest.fn().mockImplementation(navItem => ({ ...navItem, features: ['example-feature'] }))}), mockProvider(ActivatedRoute), mockProvider(PreferenceService, { get: jest.fn().mockReturnValue(of(false)) }) ]