refactor: re-structure of page time range to be query param centric

projects/common/src/navigation/ht-route-data.ts

@@ -7,4 +7,5 @@ export interface HtRouteData {
   features?: string[];
   title?: string;
   defaultTimeRange?: TimeRange;
+  shouldSavePageTimeRange?: boolean;
 }

projects/common/src/time/page-time-range-preference-service.test.ts

@@ -21,9 +21,7 @@ describe('Page time range preference service', () => {
     service: PageTimeRangePreferenceService,
     providers: [
       mockProvider(NavigationService, {
-        getCurrentActivatedRoute: jest
-          .fn()
-          .mockReturnValue({ snapshot: { data: { defaultTimeRange: defaultPageTimeRange } } })
+        getRouteConfig: jest.fn().mockReturnValue({ data: { defaultTimeRange: defaultPageTimeRange } })
       }),
       mockProvider(FeatureStateResolver, {
         getFeatureState: jest.fn().mockReturnValue(of(FeatureState.Enabled))

projects/common/src/time/page-time-range-preference.service.ts

@@ -1,5 +1,4 @@
 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';
@@ -38,7 +37,7 @@ export class PageTimeRangePreferenceService {
       map(([pageTimeRangeStringDictionary, featureState]) => {
         if (featureState === FeatureState.Enabled) {
           if (isNil(pageTimeRangeStringDictionary[rootLevelPath])) {
-            return () => this.getDefaultTimeRangeForCurrentRoute();
+            return () => this.getDefaultTimeRangeForPath(rootLevelPath);
           }
 
           return () => this.timeRangeService.timeRangeFromUrlString(pageTimeRangeStringDictionary[rootLevelPath]);
@@ -78,11 +77,14 @@ export class PageTimeRangePreferenceService {
       .pipe(shareReplay(1));
   }
 
-  public getDefaultTimeRangeForCurrentRoute(): TimeRange {
-    const currentRoute: ActivatedRoute = this.navigationService.getCurrentActivatedRoute();
-    // Right side for when FF is enabled but 'defaultTimeRange' is not set on AR data
+  public getDefaultTimeRangeForPath(rootLevelPath: string): TimeRange {
+    const routeConfigForPath = this.navigationService.getRouteConfig(
+      [rootLevelPath],
+      this.navigationService.rootRoute()
+    );
 
-    return currentRoute.snapshot.data?.defaultTimeRange ?? this.getGlobalDefaultTimeRange();
+    // Right side for when FF is enabled but 'defaultTimeRange' is not set on AR data
+    return routeConfigForPath?.data?.defaultTimeRange ?? this.getGlobalDefaultTimeRange();
   }
 
   public getGlobalDefaultTimeRange(): TimeRange {

projects/common/src/time/time-range.service.test.ts

@@ -1,4 +1,5 @@
 import { ActivatedRoute, convertToParamMap } from '@angular/router';
+import { FeatureState, FeatureStateResolver } from '@hypertrace/common';
 import { runFakeRxjs } from '@hypertrace/test-utils';
 import { createServiceFactory, mockProvider } from '@ngneat/spectator/jest';
 import { NEVER, Observable, of } from 'rxjs';
@@ -19,11 +20,14 @@ describe('Time range service', () => {
               initialTrString =>
                 // tslint:disable-next-line: no-object-literal-type-assertion
                 ({
-                  queryParamMap: of(convertToParamMap({ time: initialTrString }))
+                  queryParamMap: of(convertToParamMap({ time: initialTrString, refresh: 'true' }))
                 } as ActivatedRoute)
             )
           );
         }
+      }),
+      mockProvider(FeatureStateResolver, {
+        getFeatureState: () => of(FeatureState.Enabled)
       })
     ]
   });
@@ -84,7 +88,9 @@ describe('Time range service', () => {
 
   test('returns custom time filter', () => {
     const spectator = buildService();
-    expect(spectator.service.toQueryParams(new Date(1642296703000), new Date(1642396703000))).toStrictEqual({
+    expect(
+      spectator.service.toQueryParams(new FixedTimeRange(new Date(1642296703000), new Date(1642396703000)))
+    ).toStrictEqual({
       ['time']: new FixedTimeRange(new Date(1642296703000), new Date(1642396703000)).toUrlString()
     });
   });

projects/common/src/time/time-range.service.ts

@@ -1,7 +1,8 @@
 import { Injectable } from '@angular/core';
-import { isEmpty, isNil } from 'lodash-es';
-import { EMPTY, ReplaySubject } from 'rxjs';
-import { catchError, filter, map, switchMap, take } from 'rxjs/operators';
+import { ParamMap } from '@angular/router';
+import { isEmpty, isNil, omit } from 'lodash-es';
+import { concat, EMPTY, Observable, ReplaySubject } from 'rxjs';
+import { catchError, distinctUntilChanged, filter, map, switchMap, take } from 'rxjs/operators';
 import { NavigationService, QueryParamObject } from '../navigation/navigation.service';
 import { ReplayObservable } from '../utilities/rxjs/rxjs-utils';
 import { FixedTimeRange } from './fixed-time-range';
@@ -16,6 +17,7 @@ import { TimeUnit } from './time-unit.type';
 })
 export class TimeRangeService {
   private static readonly TIME_RANGE_QUERY_PARAM: string = 'time';
+  private static readonly REFRESH_ON_NAVIGATION: string = 'refresh';
 
   private readonly timeRangeSubject$: ReplaySubject<TimeRange> = new ReplaySubject(1);
   private currentTimeRange?: TimeRange;
@@ -51,31 +53,67 @@ export class TimeRangeService {
   }
 
   public setRelativeRange(value: number, unit: TimeUnit): this {
-    return this.setTimeRange(TimeRangeService.toRelativeTimeRange(value, unit));
+    // return this.setTimeRange(TimeRangeService.toRelativeTimeRange(value, unit));
+    return this.setTimeRangeInUrl(TimeRangeService.toRelativeTimeRange(value, unit));
   }
 
   public setFixedRange(startTime: Date, endTime: Date): this {
-    return this.setTimeRange(TimeRangeService.toFixedTimeRange(startTime, endTime));
+    // return this.setTimeRange(TimeRangeService.toFixedTimeRange(startTime, endTime));
+    return this.setTimeRangeInUrl(TimeRangeService.toFixedTimeRange(startTime, endTime));
   }
 
   public refresh(): void {
-    this.setTimeRange(this.getCurrentTimeRange());
+    const currentStringTimeRange = this.getCurrentTimeRange().toUrlString();
+    this.applyTimeRangeChange(this.timeRangeFromUrlString(currentStringTimeRange));
+  }
+
+  private getInitialTimeRange(): Observable<TimeRange> {
+    return this.navigationService.navigation$.pipe(
+      take(1), // Wait for first navigation
+      switchMap(activatedRoute => activatedRoute.queryParamMap), // Get the params from it
+      take(1), // Only the first set of params
+      map(paramMap => paramMap.get(TimeRangeService.TIME_RANGE_QUERY_PARAM)), // Extract the time range value from it
+      filter((timeRangeString): timeRangeString is string => !isEmpty(timeRangeString)), // Only valid time ranges
+      map(timeRangeString => this.timeRangeFromUrlString(timeRangeString)),
+      catchError(() => EMPTY)
+    );
+  }
+
+  private getPageTimeRangeChanges(): Observable<TimeRange> {
+    return this.navigationService.navigation$.pipe(
+      map(activeRoute => activeRoute.snapshot.queryParamMap),
+      filter(queryParamMap => !isNil(queryParamMap.get(TimeRangeService.TIME_RANGE_QUERY_PARAM))),
+      distinctUntilChanged((prevParamMap, currParamMap) => this.shouldNotUpdateTimeRange(prevParamMap, currParamMap)),
+      map(currQueryParamMap => {
+        const timeRangeQueryParamString = currQueryParamMap.get(TimeRangeService.TIME_RANGE_QUERY_PARAM);
+
+        return this.timeRangeFromUrlString(timeRangeQueryParamString!);
+      })
+    );
   }
 
   private initializeTimeRange(): void {
-    this.navigationService.navigation$
-      .pipe(
-        take(1), // Wait for first navigation
-        switchMap(activatedRoute => activatedRoute.queryParamMap), // Get the params from it
-        take(1), // Only the first set of params
-        map(paramMap => paramMap.get(TimeRangeService.TIME_RANGE_QUERY_PARAM)), // Extract the time range value from it
-        filter((timeRangeString): timeRangeString is string => !isEmpty(timeRangeString)), // Only valid time ranges
-        map(timeRangeString => this.timeRangeFromUrlString(timeRangeString)),
-        catchError(() => EMPTY)
-      )
-      .subscribe(timeRange => {
-        this.setTimeRange(timeRange);
-      });
+    concat(this.getInitialTimeRange(), this.getPageTimeRangeChanges()).subscribe(timeRange => {
+      if (!this.timeRangeMatchesCurrentUrl(timeRange)) {
+        this.setTimeRangeInUrl(timeRange);
+      } else {
+        this.applyTimeRangeChange(timeRange);
+
+        const queryParams = this.navigationService.getCurrentActivatedRoute().snapshot.queryParams;
+        if (TimeRangeService.REFRESH_ON_NAVIGATION in queryParams) {
+          this.navigationService.replaceQueryParametersInUrl(omit(queryParams, TimeRangeService.REFRESH_ON_NAVIGATION));
+        }
+      }
+    });
+  }
+
+  private shouldNotUpdateTimeRange(prevParamMap: ParamMap, currParamMap: ParamMap): boolean {
+    const refreshQueryParam = currParamMap.get(TimeRangeService.REFRESH_ON_NAVIGATION);
+
+    return (
+      prevParamMap.get(TimeRangeService.TIME_RANGE_QUERY_PARAM) ===
+        currParamMap.get(TimeRangeService.TIME_RANGE_QUERY_PARAM) && isEmpty(refreshQueryParam)
+    );
   }
 
   public timeRangeFromUrlString(timeRangeFromUrl: string): TimeRange {
@@ -91,19 +129,24 @@ export class TimeRangeService {
     throw new Error(); // Caught in observable
   }
 
-  private setTimeRange(newTimeRange: TimeRange): this {
+  private applyTimeRangeChange(newTimeRange: TimeRange): this {
     this.currentTimeRange = newTimeRange;
     this.timeRangeSubject$.next(newTimeRange);
+
+    return this;
+  }
+
+  private setTimeRangeInUrl(timeRange: TimeRange): this {
     this.navigationService.addQueryParametersToUrl({
-      [TimeRangeService.TIME_RANGE_QUERY_PARAM]: newTimeRange.toUrlString()
+      [TimeRangeService.TIME_RANGE_QUERY_PARAM]: timeRange.toUrlString()
     });
 
     return this;
   }
 
   public setDefaultTimeRange(timeRange: TimeRange): void {
     if (!this.currentTimeRange) {
-      this.setTimeRange(timeRange);
+      this.setTimeRangeInUrl(timeRange);
     }
   }
 
@@ -115,12 +158,22 @@ export class TimeRangeService {
     return new FixedTimeRange(startTime, endTime);
   }
 
-  public toQueryParams(startTime: Date, endTime: Date): QueryParamObject {
-    const newTimeRange = new FixedTimeRange(startTime, endTime);
-
-    return {
-      [TimeRangeService.TIME_RANGE_QUERY_PARAM]: newTimeRange.toUrlString()
+  public toQueryParams(timeRange: TimeRange, refreshTimeOnNavigationParam?: boolean): QueryParamObject {
+    const queryParams: QueryParamObject = {
+      [TimeRangeService.TIME_RANGE_QUERY_PARAM]: timeRange.toUrlString()
     };
+
+    if (refreshTimeOnNavigationParam) {
+      return { ...queryParams, [TimeRangeService.REFRESH_ON_NAVIGATION]: true };
+    }
+
+    return queryParams;
+  }
+
+  private timeRangeMatchesCurrentUrl(timeRange: TimeRange): boolean {
+    return (
+      this.navigationService.getQueryParameter(TimeRangeService.TIME_RANGE_QUERY_PARAM, '') === timeRange.toUrlString()
+    );
   }
 
   public isInitialized(): boolean {

projects/components/src/navigation/nav-item/nav-item.component.test.ts

@@ -33,7 +33,8 @@ describe('Navigation Item Component', () => {
         getCurrentActivatedRoute: jest.fn().mockReturnValue(of(activatedRoute))
       }),
       mockProvider(FeatureStateResolver, {
-        getCombinedFeatureState: () => of(FeatureState.Enabled)
+        getCombinedFeatureState: () => of(FeatureState.Enabled),
+        getFeatureState: () => of(FeatureState.Enabled)
       })
     ]
   });

projects/components/src/navigation/nav-item/nav-item.component.ts

@@ -1,6 +1,6 @@
 import { ChangeDetectionStrategy, Component, Input } from '@angular/core';
 import { ActivatedRoute } from '@angular/router';
-import { FeatureState, NavigationParams, NavigationParamsType } from '@hypertrace/common';
+import { FeatureState, NavigationParams, NavigationParamsType, TimeRangeService } from '@hypertrace/common';
 import { IconSize } from '../../icon/icon-size';
 import { NavItemLinkConfig, NavViewStyle } from '../navigation.config';
 
@@ -9,7 +9,7 @@ import { NavItemLinkConfig, NavViewStyle } from '../navigation.config';
   styleUrls: ['./nav-item.component.scss'],
   changeDetection: ChangeDetectionStrategy.OnPush,
   template: `
-    <ht-link *ngIf="this.config" [paramsOrUrl]="buildNavigationParam | htMemoize: this.config">
+    <ht-link *ngIf="this.config" [paramsOrUrl]="this.buildNavigationParam(this.config)">
       <div
         *htIfFeature="this.config.featureState$ | async as featureState"
         class="nav-item"
@@ -57,12 +57,26 @@ export class NavItemComponent {
   @Input()
   public readonly navItemViewStyle?: NavViewStyle;
 
-  public buildNavigationParam = (item: NavItemLinkConfig): NavigationParams => ({
-    navType: NavigationParamsType.InApp,
-    path: item.matchPaths[0],
-    relativeTo: this.activatedRoute,
-    replaceCurrentHistory: item.replaceCurrentHistory
-  });
+  public buildNavigationParam(item: NavItemLinkConfig): NavigationParams {
+    const navParams: NavigationParams = {
+      navType: NavigationParamsType.InApp,
+      path: item.matchPaths[0],
+      relativeTo: this.activatedRoute,
+      replaceCurrentHistory: item.replaceCurrentHistory
+    };
 
-  public constructor(private readonly activatedRoute: ActivatedRoute) {}
+    if (this.config.pageLevelTimeRangeIsEnabled && this.config.timeRangeResolver) {
+      return {
+        ...navParams,
+        queryParams: this.timeRangeService.toQueryParams(this.config.timeRangeResolver(), true)
+      };
+    }
+
+    return navParams;
+  }
+
+  public constructor(
+    private readonly activatedRoute: ActivatedRoute,
+    private readonly timeRangeService: TimeRangeService
+  ) {}
 }

projects/components/src/navigation/navigation-list.component.ts

@@ -39,7 +39,6 @@ import {
             <ng-container *ngSwitchCase="'${NavItemType.Link}'">
               <ht-nav-item
                 [navItemViewStyle]="this.navViewStyle"
-                (click)="this.navItemClick.emit(item)"
                 [config]="item"
                 [active]="item === activeItem"
                 [collapsed]="this.collapsed"
@@ -95,9 +94,6 @@ export class NavigationListComponent implements OnChanges {
   @Input()
   public readonly navViewStyle?: NavViewStyle;
 
-  @Output()
-  public readonly navItemClick: EventEmitter<NavItemLinkConfig> = new EventEmitter();
-
   @Output()
   public readonly activeItemChange: EventEmitter<NavItemLinkConfig> = new EventEmitter();
 

projects/components/src/page-time-range/page-time-range.component.ts

@@ -35,7 +35,7 @@ export class PageTimeRangeComponent {
   }
 
   public shouldSavePageTimeRange(currentRoute: ActivatedRoute): boolean {
-    return !isNil(currentRoute.snapshot.data?.defaultTimeRange);
+    return currentRoute.snapshot.data?.shouldSavePageTimeRange ?? false;
   }
 
   public savePageTimeRange(selectedTimeRange: TimeRange, segment: UrlSegment): void {

projects/components/src/page-time-range/page-time-range.test.ts

@@ -18,7 +18,7 @@ describe('Page time range component', () => {
   const route = {
     snapshot: {
       data: {
-        defaultTimeRange: undefined
+        shouldSavePageTimeRange: undefined
       }
     },
     pathFromRoot: { flatMap: jest.fn().mockReturnValue(['foo']) }
@@ -52,14 +52,14 @@ describe('Page time range component', () => {
     expect(spectator.component.savePageTimeRange).not.toHaveBeenCalled();
   });
 
-  test('should not attempt to save time range when route is not a first level route', () => {
+  test('should not attempt to save time range when saving flag property is not set', () => {
     spectator = createHost(`<ht-page-time-range></ht-page-time-range>`, {
       providers: [
         mockProvider(NavigationService, {
           getCurrentActivatedRoute: jest.fn().mockReturnValue({
             snapshot: {
               data: {
-                defaultTimeRange: undefined
+                shouldSavePageTimeRange: undefined
               }
             },
             pathFromRoot: { flatMap: jest.fn().mockReturnValue(['parent-path', 'child-path']) }
@@ -77,14 +77,14 @@ describe('Page time range component', () => {
     expect(spectator.component.savePageTimeRange).not.toHaveBeenCalled();
   });
 
-  test('should save time range when route is first level, and the defaultTimeRange property is present', () => {
+  test('should save time range when route has truthy property shouldSavePageTimeRange', () => {
     spectator = createHost(`<ht-page-time-range></ht-page-time-range>`, {
       providers: [
         mockProvider(NavigationService, {
           getCurrentActivatedRoute: jest.fn().mockReturnValue({
             snapshot: {
               data: {
-                defaultTimeRange: new RelativeTimeRange(new TimeDuration(30, TimeUnit.Minute))
+                shouldSavePageTimeRange: true
               }
             },
             pathFromRoot: { flatMap: () => [{ path: 'parent-path' }] as UrlSegment[] }