Skip to content

feat: move a common method into navigation service #969

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jul 7, 2021
33 changes: 33 additions & 0 deletions projects/common/src/navigation/navigation.service.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { Location } from '@angular/common';
import { Router, UrlSegment } from '@angular/router';
import { RouterTestingModule } from '@angular/router/testing';
import { IconType } from '@hypertrace/assets-library';
import { NavItemType } from '@hypertrace/components';
import { patchRouterNavigateForTest } from '@hypertrace/test-utils';
import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest';
import {
Expand Down Expand Up @@ -47,6 +49,7 @@ describe('Navigation Service', () => {
RouterTestingModule.withRoutes([
{
path: 'root',
data: { features: ['test-feature'] },
children: [firstChildRouteConfig, secondChildRouteConfig]
}
])
Expand Down Expand Up @@ -285,4 +288,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']
});
});
});
22 changes: 22 additions & 0 deletions projects/common/src/navigation/navigation.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ 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 { isEqualIgnoreFunctions, throwIfNil } from '../utilities/lang/lang-utils';
Expand Down Expand Up @@ -229,6 +231,26 @@ export class NavigationService {
return this.findRouteConfig(path, childRoutes ? childRoutes : []);
}

public decorateNavItem(navItem: NavItemConfig, activatedRoute: ActivatedRoute): NavItemConfig {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add some tests for these two methods since they are shared? Can do it in a follow up PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

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;
}
Expand Down
3 changes: 2 additions & 1 deletion src/app/shared/navigation/navigation.component.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)) })
Expand Down
27 changes: 4 additions & 23 deletions src/app/shared/navigation/navigation.component.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
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 { uniq } from 'lodash-es';
import { Observable } from 'rxjs';

@Component({
Expand Down Expand Up @@ -79,31 +78,13 @@ 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 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) || [];
}
}