hypertrace · Christian862 · Mar 21, 2022 · Feb 8, 2022 · Feb 9, 2022 · Feb 11, 2022
@@ -3,6 +3,5 @@ import { InjectionToken } from '@angular/core';
 export const GLOBAL_HEADER_HEIGHT = new InjectionToken<string>('Global Header Height');
 
 export const enum ApplicationFeature {
-  PageTimeRange = 'ui.page-time-range',
-  NavigationRedesign = 'ui.navigation-version-2'
+  PageTimeRange = 'ui.page-time-range'
 }
@@ -1,4 +1,6 @@
 import {
+  FeatureState,
+  FeatureStateResolver,
   FixedTimeRange,
   NavigationService,
   RelativeTimeRange,
@@ -9,6 +11,7 @@ import {
 } from '@hypertrace/common';
 import { runFakeRxjs } from '@hypertrace/test-utils';
 import { createServiceFactory, mockProvider } from '@ngneat/spectator/jest';
+import { of } from 'rxjs';
 import { PageTimeRangePreferenceService } from './page-time-range-preference.service';
 
 describe('Page time range preference service', () => {
@@ -18,6 +21,9 @@ describe('Page time range preference service', () => {
     providers: [
       mockProvider(NavigationService, {
         getRouteConfig: jest.fn().mockReturnValue({ data: { defaultTimeRange: defaultPageTimeRange } })
+      }),
+      mockProvider(FeatureStateResolver, {
+        getFeatureState: jest.fn().mockReturnValue(of(FeatureState.Enabled))
       })
     ]
   });

@@ -1,4 +1,5 @@
 import { Injectable } from '@angular/core';
+import { ActivatedRoute } from '@angular/router';
 import { isNil } from 'lodash-es';
 import { combineLatest, Observable } from 'rxjs';
 import { map, shareReplay, take } from 'rxjs/operators';
@@ -18,8 +19,6 @@ export class PageTimeRangePreferenceService {
   private static readonly STORAGE_TYPE: StorageType = StorageType.Local;
   private static readonly TIME_RANGE_PREFERENCE_KEY: string = 'page-time-range';
 
-  private readonly globalDefaultTimeRange: TimeRange = new RelativeTimeRange(new TimeDuration(30, TimeUnit.Minute));
-
   private readonly pageTimeRangeStringDictionary$: Observable<PageTimeRangeStringDictionary>;
 
   public constructor(
@@ -31,26 +30,22 @@ export class PageTimeRangePreferenceService {
     this.pageTimeRangeStringDictionary$ = this.buildPageTimeRangeObservable();
   }
 
-  public getTimeRangePreferenceForPage(rootLevelPath: string): Observable<TimeRange> {
+  public getTimeRangePreferenceForPage(rootLevelPath: string): Observable<TimeRangeCallback> {
     return combineLatest([
       this.pageTimeRangeStringDictionary$,
-      this.featureStateResolver.getCombinedFeatureState([
-        ApplicationFeature.PageTimeRange,
-        ApplicationFeature.NavigationRedesign
-      ])
+      this.featureStateResolver.getFeatureState(ApplicationFeature.PageTimeRange)
     ]).pipe(
       map(([pageTimeRangeStringDictionary, featureState]) => {
         if (featureState === FeatureState.Enabled) {
           if (isNil(pageTimeRangeStringDictionary[rootLevelPath])) {
-            // Right side for when FFs are enabled but page path doesn't have 'defaultTimeRange' set on AR data
-            return this.getDefaultPageTimeRange() ?? this.globalDefaultTimeRange;
+            return this.getDefaultTimeRangeForCurrentRoute;
           }
 
-          return this.timeRangeService.timeRangeFromUrlString(pageTimeRangeStringDictionary[rootLevelPath]);
+          return () => this.timeRangeService.timeRangeFromUrlString(pageTimeRangeStringDictionary[rootLevelPath]);
         }
 
-        // When FFs are disabled
-        return this.globalDefaultTimeRange;
+        // When FF is disabled
+        return this.getGlobalDefaultTimeRange;
       })
     );
   }
@@ -83,11 +78,18 @@ export class PageTimeRangePreferenceService {
       .pipe(shareReplay(1));
   }
 
-  private getDefaultPageTimeRange(): TimeRange | undefined {
-    return this.navigationService.getCurrentActivatedRoute().snapshot.data?.defaultTimeRange;
-  }
+  public getDefaultTimeRangeForCurrentRoute = (): TimeRange => {
+    const currentRoute: ActivatedRoute = this.navigationService.getCurrentActivatedRoute();
+    // Right side for when FF is enabled but 'defaultTimeRange' is not set on AR data
+    return currentRoute.snapshot.data?.defaultTimeRange ?? this.getGlobalDefaultTimeRange();
+  };
+
+  public getGlobalDefaultTimeRange = (): TimeRange => {
+    return new RelativeTimeRange(new TimeDuration(1, TimeUnit.Hour));
+  };
 }
 
 interface PageTimeRangeStringDictionary {
   [path: string]: string;
 }
+export type TimeRangeCallback = () => TimeRange;
@@ -1,5 +1,4 @@
 import {
-  ApplicationFeature,
   FeatureState,
   FeatureStateResolver,
   NavigationService,
@@ -12,7 +11,6 @@ import { of } from 'rxjs';
 import { BreadcrumbsService } from '../../breadcrumbs/breadcrumbs.service';
 import { FeatureConfigCheckModule } from '../../feature-check/feature-config-check.module';
 import { PageTimeRangeComponent } from '../../page-time-range/page-time-range.component';
-import { TimeRangeComponent } from '../../time-range/time-range.component';
 import { PageHeaderComponent } from './page-header.component';
 
 describe('Page Header Component', () => {
@@ -21,7 +19,7 @@ describe('Page Header Component', () => {
   const createHost = createHostFactory({
     component: PageHeaderComponent,
     imports: [FeatureConfigCheckModule],
-    declarations: [MockComponent(PageTimeRangeComponent), MockComponent(TimeRangeComponent)],
+    declarations: [MockComponent(PageTimeRangeComponent)],
     shallow: true,
     providers: [
       mockProvider(NavigationService),
@@ -50,7 +48,7 @@ describe('Page Header Component', () => {
     expect(spectator.query('.beta')).not.toExist();
   });
 
-  test('should display page time range component when feature flags are enabled', () => {
+  test('should display page time range component when feature flag is enabled', () => {
     spectator = createHost('<ht-page-header></ht-page-header>', {
       providers: [
         mockProvider(FeatureStateResolver, {
@@ -60,26 +58,6 @@ describe('Page Header Component', () => {
     });
 
     expect(spectator.query(PageTimeRangeComponent)).toExist();
-    expect(spectator.query(TimeRangeComponent)).not.toExist();
-  });
-
-  test('should display time-range component when navigation feature flag is disabled and page time range is enabled', () => {
-    spectator = createHost('<ht-page-header></ht-page-header>', {
-      providers: [
-        mockProvider(FeatureStateResolver, {
-          getCombinedFeatureState: jest.fn().mockImplementation(([feature]) => {
-            if (feature === ApplicationFeature.NavigationRedesign) {
-              return of(FeatureState.Disabled);
-            }
-
-            return of(FeatureState.Enabled);
-          })
-        })
-      ]
-    });
-
-    expect(spectator.query(PageTimeRangeComponent)).not.toExist();
-    expect(spectator.query(TimeRangeComponent)).toExist();
   });
 
   test('should not display any time range if FF is disabled', () => {
@@ -92,6 +70,5 @@ describe('Page Header Component', () => {
     });
 
     expect(spectator.query(PageTimeRangeComponent)).not.toExist();
-    expect(spectator.query(TimeRangeComponent)).not.toExist();
   });
 });
@@ -35,14 +35,7 @@ import { NavigableTab } from '../../tabs/navigable/navigable-tab';
           <ng-container *ngIf="this.contentAlignment === '${PageHeaderContentAlignment.Row}'">
             <ng-container *ngTemplateOutlet="this.projectedContentTemplate"></ng-container>
           </ng-container>
-          <ht-page-time-range
-            class="time-range"
-            *htIfFeature="'${ApplicationFeature.NavigationRedesign}' | htFeature; else globalTimeRangeTemplate"
-          ></ht-page-time-range>
-
-          <ng-template #globalTimeRangeTemplate>
-            <ht-time-range></ht-time-range>
-          </ng-template>
+          <ht-page-time-range class="time-range"></ht-page-time-range>
         </div>
         <ng-container *ngIf="this.contentAlignment === '${PageHeaderContentAlignment.Column}'">
           <ng-container *ngTemplateOutlet="this.projectedContentTemplate"></ng-container>

@@ -44,7 +44,8 @@ describe('Navigation List Component Service', () => {
     service: NavigationListComponentService,
     providers: [
       mockProvider(FeatureStateResolver, {
-        getCombinedFeatureState: jest.fn().mockReturnValue(of(FeatureState.Enabled))
+        getCombinedFeatureState: jest.fn().mockReturnValue(of(FeatureState.Enabled)),
+        getFeatureState: jest.fn().mockReturnValue(of(FeatureState.Enabled))
       }),
       mockProvider(PageTimeRangePreferenceService, {
         getTimeRangePreferenceForPage: jest.fn().mockReturnValue(of(mockTimeRange))

@@ -1,8 +1,13 @@
 import { Injectable } from '@angular/core';
-import { FeatureState, FeatureStateResolver, PageTimeRangePreferenceService } from '@hypertrace/common';
+import {
+  ApplicationFeature,
+  FeatureState,
+  FeatureStateResolver,
+  PageTimeRangePreferenceService
+} from '@hypertrace/common';
 import { isEmpty } from 'lodash-es';
 import { combineLatest, Observable, of } from 'rxjs';
-import { map } from 'rxjs/operators';
+import { map, switchMap } from 'rxjs/operators';
 import { NavItemConfig, NavItemHeaderConfig, NavItemLinkConfig, NavItemType } from './navigation.config';
 
 @Injectable({ providedIn: 'root' })
@@ -29,11 +34,10 @@ export class NavigationListComponentService {
     return updatedItems;
   }
 
-  public resolveNavItemConfigTimeRanges(
-    navItems: NavItemConfig[],
-    pageLevelTimeRangeFeatureState: FeatureState
-  ): Observable<NavItemConfig[]> {
-    return combineLatest(this.getTimeRangesForNavItems(navItems, pageLevelTimeRangeFeatureState));
+  public resolveNavItemConfigTimeRanges(navItems: NavItemConfig[]): Observable<NavItemConfig[]> {
+    return this.featureStateResolver
+      .getFeatureState(ApplicationFeature.PageTimeRange)
+      .pipe(switchMap(featureState => combineLatest(this.getTimeRangesForNavItems(navItems, featureState))));
   }
 
   private getTimeRangesForNavItems(
@@ -43,9 +47,9 @@ export class NavigationListComponentService {
     return navItems.map(navItem => {
       if (navItem.type === NavItemType.Link) {
         return this.pageTimeRangePreferenceService.getTimeRangePreferenceForPage(navItem.matchPaths[0]).pipe(
-          map(timeRange => ({
+          map(resolveTimeRange => ({
             ...navItem,
-            timeRange: timeRange,
+            resolveTimeRange: resolveTimeRange,
             pageLevelTimeRangeIsEnabled: pageLevelTimeRangeFeatureState === FeatureState.Enabled
           }))
         );

@@ -1,18 +1,9 @@
 import { ChangeDetectionStrategy, Component, EventEmitter, Input, OnChanges, Output } from '@angular/core';
 import { ActivatedRoute } from '@angular/router';
 import { IconType } from '@hypertrace/assets-library';
-import {
-  ApplicationFeature,
-  FeatureStateResolver,
-  FixedTimeRange,
-  NavigationService,
-  RelativeTimeRange,
-  TimeRangeService,
-  TypedSimpleChanges
-} from '@hypertrace/common';
-import { isNil } from 'lodash-es';
-import { combineLatest, Observable } from 'rxjs';
-import { map, shareReplay, startWith, switchMap } from 'rxjs/operators';
+import { NavigationService, TimeRangeService, TypedSimpleChanges } from '@hypertrace/common';
+import { Observable } from 'rxjs';
+import { map, startWith } from 'rxjs/operators';
 import { IconSize } from '../icon/icon-size';
 import { NavigationListComponentService } from './navigation-list-component.service';
 import { FooterItemConfig, NavItemConfig, NavItemLinkConfig, NavItemType } from './navigation.config';
@@ -24,7 +15,7 @@ import { FooterItemConfig, NavItemConfig, NavItemLinkConfig, NavItemType } from
   template: `
     <nav class="navigation-list" [ngClass]="{ expanded: !this.collapsed }">
       <div class="content" *htLetAsync="this.activeItem$ as activeItem" [htLayoutChangeTrigger]="this.collapsed">
-        <ng-container *ngFor="let item of this.navItems$ | async; let id = index">
+        <ng-container *ngFor="let item of this.navItems; let id = index">
           <ng-container [ngSwitch]="item.type">
             <div *ngIf="!this.collapsed">
               <ng-container *ngSwitchCase="'${NavItemType.Header}'">
@@ -39,7 +30,7 @@ import { FooterItemConfig, NavItemConfig, NavItemLinkConfig, NavItemType } from
 
             <ng-container *ngSwitchCase="'${NavItemType.Link}'">
               <ht-nav-item
-                (click)="this.setPageTimeRangeForSelectedNavItem(item)"
+                (click)="this.navItemSelected.emit(item)"
                 [config]="item"
                 [active]="item === activeItem"
                 [collapsed]="this.collapsed"
@@ -82,46 +73,31 @@ export class NavigationListComponent implements OnChanges {
   @Output()
   public readonly collapsedChange: EventEmitter<boolean> = new EventEmitter();
 
-  public activeItem$?: Observable<NavItemLinkConfig | undefined>;
+  @Output()
+  public readonly navItemSelected: EventEmitter<NavItemLinkConfig> = new EventEmitter();
 
-  public navItems$?: Observable<NavItemConfig[]>;
+  public activeItem$?: Observable<NavItemLinkConfig | undefined>;
 
   public constructor(
     private readonly navigationService: NavigationService,
     private readonly activatedRoute: ActivatedRoute,
     private readonly navListComponentService: NavigationListComponentService,
-    private readonly timeRangeService: TimeRangeService,
-    private readonly featureStateResolver: FeatureStateResolver
+    private readonly timeRangeService: TimeRangeService
   ) {}
 
   public ngOnChanges(changes: TypedSimpleChanges<this>): void {
     if (changes.navItems) {
       this.navItems = this.navListComponentService.resolveFeaturesAndUpdateVisibilityForNavItems(this.navItems);
 
-      // Decorate the nav items with the corresponding time ranges, depending on the FF state.
-      // The time ranges in nav items are streams that get the most recent value from page time range preference service
-      this.navItems$ = this.featureStateResolver
-        .getCombinedFeatureState([ApplicationFeature.PageTimeRange, ApplicationFeature.NavigationRedesign])
-        .pipe(
-          switchMap(pageLevelTimeRangeFeature =>
-            this.navListComponentService.resolveNavItemConfigTimeRanges(this.navItems, pageLevelTimeRangeFeature)
-          ),
-          shareReplay()
-        );
-
-      // For each nav item, find the (possibly new) active nav item
-      // Time range is set on component load, when it isn't already
+      // Initialize the time range service
       // Depending on FF status, the TR will be either global or page level for the init
-      this.activeItem$ = combineLatest([
-        this.navItems$,
-        this.navigationService.navigation$.pipe(startWith(this.navigationService.getCurrentActivatedRoute()))
-      ]).pipe(
-        map(([navItems]) => {
-          const activeItem = this.findActiveItem(navItems);
-          if (!this.timeRangeService.isInitialized() && activeItem) {
-            this.timeRangeService.setDefaultTimeRange(activeItem.timeRange!);
+      this.activeItem$ = this.navigationService.navigation$.pipe(
+        startWith(this.navigationService.getCurrentActivatedRoute()),
+        map(() => {
+          const activeItem = this.findActiveItem(this.navItems);
+          if (!this.timeRangeService.isInitialized() && activeItem?.resolveTimeRange) {
+            this.timeRangeService.setDefaultTimeRange(activeItem.resolveTimeRange());
           }
-
           return activeItem;
         })
       );
@@ -139,18 +115,6 @@ export class NavigationListComponent implements OnChanges {
     return this.collapsed ? IconType.TriangleRight : IconType.TriangleLeft;
   }
 
-  public setPageTimeRangeForSelectedNavItem(navItemLink: NavItemLinkConfig): void {
-    if (!isNil(navItemLink.timeRange) && navItemLink.pageLevelTimeRangeIsEnabled) {
-      if (navItemLink.timeRange instanceof FixedTimeRange) {
-        const timeRange: FixedTimeRange = navItemLink.timeRange;
-        this.timeRangeService.setFixedRange(timeRange.startTime, timeRange.endTime);
-      } else if (navItemLink.timeRange instanceof RelativeTimeRange) {
-        const timeRange: RelativeTimeRange = navItemLink.timeRange;
-        this.timeRangeService.setRelativeRange(timeRange.duration.value, timeRange.duration.unit);
-      }
-    }
-  }
-
   private findActiveItem(navItems: NavItemConfig[]): NavItemLinkConfig | undefined {
     return navItems
       .filter((item): item is NavItemLinkConfig => item.type === NavItemType.Link)

@@ -1,4 +1,4 @@
-import { Color, FeatureState, TimeRange } from '@hypertrace/common';
+import { Color, FeatureState, TimeRangeCallback } from '@hypertrace/common';
 import { Observable } from 'rxjs';
 import { IconSize } from '../icon/icon-size';
 
@@ -16,7 +16,7 @@ export interface NavItemLinkConfig {
   trailingIcon?: string;
   trailingIconTooltip?: string;
   trailingIconColor?: Color;
-  timeRange?: TimeRange;
+  resolveTimeRange?: TimeRangeCallback;
   pageLevelTimeRangeIsEnabled?: boolean;
   featureState$?: Observable<FeatureState>;
 }

@@ -1,5 +1,5 @@
 import { ChangeDetectionStrategy, Component } from '@angular/core';
-import { ActivatedRoute, UrlSegment } from '@angular/router';
+import { UrlSegment } from '@angular/router';
 import { LoggerService, NavigationService, PageTimeRangePreferenceService, TimeRange } from '@hypertrace/common';
 import { isNil } from 'lodash-es';
 
@@ -19,13 +19,15 @@ export class PageTimeRangeComponent {
     const activatedRoute = this.navigationService.getCurrentActivatedRoute();
     const urlSegments: UrlSegment[] = activatedRoute.pathFromRoot.flatMap(activeRoute => activeRoute.snapshot.url);
 
-    if (this.shouldSavePageTimeRange(activatedRoute)) {
+    if (this.shouldSavePageTimeRange()) {
       this.savePageTimeRange(selectedTimeRange, urlSegments[0]);
     }
   }
 
-  public shouldSavePageTimeRange(route: ActivatedRoute): boolean {
-    return !isNil(route.snapshot.data?.defaultTimeRange);
+  public shouldSavePageTimeRange(): boolean {
+    const currentRoute = this.navigationService.getCurrentActivatedRoute();
+
+    return !isNil(currentRoute.snapshot.data?.defaultTimeRange);
   }
 
   public savePageTimeRange(selectedTimeRange: TimeRange, segment: UrlSegment): void {