fix: common project should not depend on components project (#1264)

* fix: common project should not depend on components project

Author: arjunlalb

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');

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;
   }

projects/components/src/navigation/navigation-list.service.test.ts

@@ -0,0 +1,58 @@
+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<NavigationListService>;
+
+  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']
+    });
+  });
+});

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) || [];
+  }
+}

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';

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';
@@ -18,7 +18,9 @@ describe('NavigationComponent', () => {
           data: {
             features: ['example-feature']
           }
-        }),
+        })
+      }),
+      mockProvider(NavigationListService, {
         decorateNavItem: jest.fn().mockImplementation(navItem => ({ ...navItem, features: ['example-feature'] }))
       }),
       mockProvider(ActivatedRoute),

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);
   }