routing: improve compare navigation

tensorboard/webapp/app_routing/effects/app_routing_effects.ts

@@ -12,7 +12,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License.
 ==============================================================================*/
-import {Inject, Injectable} from '@angular/core';
+import {Injectable} from '@angular/core';
 import {Actions, createEffect, ofType} from '@ngrx/effects';
 import {Action, createAction, Store} from '@ngrx/store';
 import {merge, Observable, of} from 'rxjs';
@@ -34,13 +34,17 @@ import {
   stateRehydratedFromUrl,
 } from '../actions';
 import {AppRootProvider} from '../app_root';
-import {areRoutesEqual, getRouteId} from '../internal_utils';
+import {
+  areRoutesEqual,
+  getRouteId,
+  serializeCompareExperimentParams,
+} from '../internal_utils';
 import {Location} from '../location';
 import {ProgrammaticalNavigationModule} from '../programmatical_navigation_module';
 import {RouteConfigs} from '../route_config';
 import {RouteRegistryModule} from '../route_registry_module';
 import {getActiveRoute} from '../store/app_routing_selectors';
-import {Navigation, Route} from '../types';
+import {Navigation, Route, RouteKind, RouteParams} from '../types';
 
 /** @typehack */ import * as _typeHackNgrxEffects from '@ngrx/effects';
 /** @typehack */ import * as _typeHackModels from '@ngrx/store/src/models';
@@ -144,7 +148,26 @@ export class AppRoutingEffects {
       return nav !== null;
     }),
     map((programmaticalNavigation) => {
-      const {routeKind, routeParams} = programmaticalNavigation!;
+      const nav = programmaticalNavigation!;
+      const routeKind = nav.routeKind;
+
+      // TODO(stephanwlee): currently, the RouteParams is ill-typed and you can
+      // currently add any property without any type error. Better type it.
+      let routeParams: RouteParams;
+      switch (nav.routeKind) {
+        case RouteKind.COMPARE_EXPERIMENT:
+          routeParams = {
+            experimentIds: serializeCompareExperimentParams(
+              nav.routeParams.aliasAndExperimentIds
+            ),
+          };
+          break;
+        default:
+          routeParams = nav.routeParams;
+      }
+      return {routeKind, routeParams};
+    }),
+    map(({routeKind, routeParams}) => {
       const routeMatch = this.routeConfigs
         ? this.routeConfigs.matchByRouteKind(routeKind, routeParams)
         : null;

tensorboard/webapp/app_routing/effects/app_routing_effects_test.ts

@@ -16,7 +16,7 @@ limitations under the License.
 import {Component} from '@angular/core';
 import {fakeAsync, flush, TestBed, tick} from '@angular/core/testing';
 import {provideMockActions} from '@ngrx/effects/testing';
-import {Action, createAction, Store} from '@ngrx/store';
+import {Action, createAction, props, Store} from '@ngrx/store';
 import {MockStore, provideMockStore} from '@ngrx/store/testing';
 import {of, ReplaySubject} from 'rxjs';
 
@@ -26,6 +26,7 @@ import {navigationRequested} from '../actions';
 import {AppRootProvider, TestableAppRootProvider} from '../app_root';
 import {Location} from '../location';
 import {
+  NavigateToCompare,
   NavigateToExperiments,
   ProgrammaticalNavigationModule,
 } from '../programmatical_navigation_module';
@@ -40,6 +41,10 @@ import {AppRoutingEffects, TEST_ONLY} from './app_routing_effects';
 class TestableComponent {}
 
 const testAction = createAction('[TEST] test actions');
+const testNavToCompareAction = createAction(
+  '[TEST] test nav to compare',
+  props<NavigateToCompare['routeParams']>()
+);
 
 describe('app_routing_effects', () => {
   let effects: AppRoutingEffects;
@@ -87,10 +92,10 @@ describe('app_routing_effects', () => {
       ];
     }
 
-    function programmaticalNavigationFactory() {
+    function programmaticalNavToListviewFactory() {
       return {
         actionCreator: testAction,
-        lambda: (action: typeof testAction) => {
+        lambda: (action: ReturnType<typeof testAction>) => {
           return {
             routeKind: RouteKind.EXPERIMENTS,
             routeParams: {},
@@ -99,11 +104,26 @@ describe('app_routing_effects', () => {
       };
     }
 
+    function programmaticalCompareNavigationFactory() {
+      return {
+        actionCreator: testNavToCompareAction,
+        lambda: (action: ReturnType<typeof testNavToCompareAction>) => {
+          return {
+            routeKind: RouteKind.COMPARE_EXPERIMENT,
+            routeParams: action,
+          } as NavigateToCompare;
+        },
+      };
+    }
+
     await TestBed.configureTestingModule({
       imports: [
         RouteRegistryModule.registerRoutes(routeFactory),
         ProgrammaticalNavigationModule.registerProgrammaticalNavigation(
-          programmaticalNavigationFactory
+          programmaticalNavToListviewFactory
+        ),
+        ProgrammaticalNavigationModule.registerProgrammaticalNavigation(
+          programmaticalCompareNavigationFactory
         ),
       ],
       providers: [
@@ -539,6 +559,33 @@ describe('app_routing_effects', () => {
           }),
         ]);
       });
+
+      it('translates programmatical compare nav correctly', () => {
+        store.overrideSelector(getActiveRoute, null);
+        store.refreshState();
+
+        action.next(
+          testNavToCompareAction({
+            aliasAndExperimentIds: [
+              {alias: 'bar', id: 'foo'},
+              {alias: 'omega', id: 'alpha'},
+            ],
+          })
+        );
+
+        expect(actualActions).toEqual([
+          actions.navigating({
+            after: buildRoute({
+              routeKind: RouteKind.COMPARE_EXPERIMENT,
+              params: {
+                experimentIds: 'bar:foo,omega:alpha',
+              },
+              pathname: '/compare',
+              queryParams: [],
+            }),
+          }),
+        ]);
+      });
     });
   });
 

tensorboard/webapp/app_routing/internal_utils.ts

@@ -54,6 +54,12 @@ export function parseCompareExperimentStr(
   });
 }
 
+export function serializeCompareExperimentParams(
+  params: Array<{alias: string; id: string}>
+): string {
+  return params.map(({alias, id}) => `${alias}:${id}`).join(',');
+}
+
 /**
  * Returns experimentIds from route parameter. For a route that does not contain
  * any experiment ids, it returns null.

tensorboard/webapp/app_routing/internal_utils_test.ts

@@ -44,6 +44,35 @@ describe('app_routing/utils', () => {
     });
   });
 
+  describe('#serializeCompareExperimentParams', () => {
+    it('serializes to empty string with an empty input', () => {
+      expect(utils.serializeCompareExperimentParams([])).toBe('');
+    });
+
+    it('serializes alias and displayName', () => {
+      const input = [
+        {alias: 'foo', id: 'bar'},
+        {alias: 'baz', id: 'baz'},
+      ];
+      expect(utils.serializeCompareExperimentParams(input)).toBe(
+        'foo:bar,baz:baz'
+      );
+    });
+
+    it('does not deduplicate or validate', () => {
+      const input = [
+        {alias: 'foo', id: 'bar'},
+        // does not deduplicate
+        {alias: 'tar', id: 'bar'},
+        {alias: 'foo', id: 'bang'},
+        {alias: '', id: ''},
+      ];
+      expect(utils.serializeCompareExperimentParams(input)).toBe(
+        'foo:bar,tar:bar,foo:bang,:'
+      );
+    });
+  });
+
   describe('#getExperimentIdsFromRouteParams', () => {
     it('returns ids from compare route', () => {
       const actual = utils.getExperimentIdsFromRouteParams(

tensorboard/webapp/app_routing/programmatical_navigation_module_test.ts

@@ -101,11 +101,18 @@ describe('programmatical navigation module test', () => {
     function provider2() {
       return {
         actionCreator: testAction,
-        lambda: (action: typeof testAction) => {
+        lambda: (action: typeof testAction): NavigateToCompare => {
           return {
             routeKind: RouteKind.COMPARE_EXPERIMENT,
-            routeParams: {experimentIds: 'foo'},
-          } as NavigateToCompare;
+            routeParams: {
+              aliasAndExperimentIds: [
+                {
+                  alias: 'Foo',
+                  id: 'foo',
+                },
+              ],
+            },
+          };
         },
       };
     }

tensorboard/webapp/app_routing/programmatical_navigation_types.ts

@@ -15,7 +15,7 @@ limitations under the License.
 import {InjectionToken} from '@angular/core';
 import {Action, ActionCreator, Creator} from '@ngrx/store';
 
-import {CompareRouteParams, ExperimentRouteParams, RouteKind} from './types';
+import {ExperimentRouteParams, RouteKind} from './types';
 
 export const NAVIGATION_PROVIDER = new InjectionToken<NavigationLambda[]>(
   '[App Routing] Programmatical Navigation Provider'
@@ -28,7 +28,9 @@ export interface NavigateToExperiment {
 
 export interface NavigateToCompare {
   routeKind: RouteKind.COMPARE_EXPERIMENT;
-  routeParams: CompareRouteParams;
+  routeParams: {
+    aliasAndExperimentIds: Array<{alias: string; id: string}>;
+  };
 }
 
 export interface NavigateToExperiments {