[PM-8208] Fix: Product Navigation flash (#9587)

* wait until a sync is complete to render the product switcher content

* refactor unneeded observables into their own variable

* do not show product switcher button until content is loaded

* use `ReplaySubject` to ensure that `syncCompleted$` last value is always used

apps/web/src/app/layouts/product-switcher/product-switcher.component.html

@@ -4,5 +4,6 @@
   [bitMenuTriggerFor]="content?.menu"
   [buttonType]="buttonType"
   [attr.aria-label]="'switchProducts' | i18n"
+  *ngIf="products$ | async"
 ></button>
 <product-switcher-content #content></product-switcher-content>

apps/web/src/app/layouts/product-switcher/product-switcher.component.ts

@@ -1,6 +1,8 @@
 import { AfterViewInit, ChangeDetectorRef, Component, Input } from "@angular/core";
 
 import { IconButtonType } from "@bitwarden/components/src/icon-button/icon-button.component";
+
+import { ProductSwitcherService } from "./shared/product-switcher.service";
 @Component({
   selector: "product-switcher",
   templateUrl: "./product-switcher.component.html",
@@ -21,5 +23,10 @@ export class ProductSwitcherComponent implements AfterViewInit {
     this.changeDetector.detectChanges();
   }
 
-  constructor(private changeDetector: ChangeDetectorRef) {}
+  constructor(
+    private changeDetector: ChangeDetectorRef,
+    private productSwitcherService: ProductSwitcherService,
+  ) {}
+
+  protected readonly products$ = this.productSwitcherService.products$;
 }

apps/web/src/app/layouts/product-switcher/shared/product-switcher.service.spec.ts

@@ -8,6 +8,7 @@ import { OrganizationService } from "@bitwarden/common/admin-console/abstraction
 import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
 import { Provider } from "@bitwarden/common/admin-console/models/domain/provider";
+import { SyncService } from "@bitwarden/common/platform/sync";
 
 import { ProductSwitcherService } from "./product-switcher.service";
 
@@ -17,8 +18,19 @@ describe("ProductSwitcherService", () => {
   let organizationService: MockProxy<OrganizationService>;
   let providerService: MockProxy<ProviderService>;
   let activeRouteParams = convertToParamMap({ organizationId: "1234" });
+  const getLastSync = jest.fn().mockResolvedValue(new Date("2024-05-14"));
+
+  // The service is dependent on the SyncService, which is behind a `setTimeout`
+  // Most of the tests don't need to test this aspect so `advanceTimersByTime`
+  // is used to simulate the completion of the sync
+  function initiateService() {
+    service = TestBed.inject(ProductSwitcherService);
+    jest.advanceTimersByTime(201);
+  }
 
   beforeEach(() => {
+    jest.useFakeTimers();
+    getLastSync.mockResolvedValue(new Date("2024-05-14"));
     router = mock<Router>();
     organizationService = mock<OrganizationService>();
     providerService = mock<ProviderService>();
@@ -46,14 +58,40 @@ describe("ProductSwitcherService", () => {
             transform: (key: string) => key,
           },
         },
+        {
+          provide: SyncService,
+          useValue: { getLastSync },
+        },
       ],
     });
   });
 
+  afterEach(() => {
+    jest.useRealTimers();
+  });
+
+  describe("SyncService", () => {
+    it("waits until sync is complete before emitting products", (done) => {
+      getLastSync.mockResolvedValue(null);
+
+      initiateService();
+
+      // The subscription will only emit once the sync returns a value
+      service.products$.subscribe((products) => {
+        expect(products).toBeDefined();
+        done();
+      });
+
+      // Simulate sync completion & advance timers
+      getLastSync.mockResolvedValue(new Date("2024-05-15"));
+      jest.advanceTimersByTime(201);
+    });
+  });
+
   describe("product separation", () => {
     describe("Password Manager", () => {
       it("is always included", async () => {
-        service = TestBed.inject(ProductSwitcherService);
+        initiateService();
 
         const products = await firstValueFrom(service.products$);
 
@@ -63,7 +101,7 @@ describe("ProductSwitcherService", () => {
 
     describe("Secret Manager", () => {
       it("is included in other when there are no organizations with SM", async () => {
-        service = TestBed.inject(ProductSwitcherService);
+        initiateService();
 
         const products = await firstValueFrom(service.products$);
 
@@ -75,7 +113,7 @@ describe("ProductSwitcherService", () => {
           { id: "1234", canAccessSecretsManager: true, enabled: true },
         ] as Organization[]);
 
-        service = TestBed.inject(ProductSwitcherService);
+        initiateService();
 
         const products = await firstValueFrom(service.products$);
 
@@ -85,7 +123,7 @@ describe("ProductSwitcherService", () => {
 
     describe("Admin/Organizations", () => {
       it("includes Organizations in other when there are organizations", async () => {
-        service = TestBed.inject(ProductSwitcherService);
+        initiateService();
 
         const products = await firstValueFrom(service.products$);
 
@@ -96,7 +134,7 @@ describe("ProductSwitcherService", () => {
       it("includes Admin Console in bento when a user has access to it", async () => {
         organizationService.organizations$ = of([{ id: "1234", isOwner: true }] as Organization[]);
 
-        service = TestBed.inject(ProductSwitcherService);
+        initiateService();
 
         const products = await firstValueFrom(service.products$);
 
@@ -107,8 +145,7 @@ describe("ProductSwitcherService", () => {
 
     describe("Provider Portal", () => {
       it("is not included when there are no providers", async () => {
-        service = TestBed.inject(ProductSwitcherService);
-
+        initiateService();
         const products = await firstValueFrom(service.products$);
 
         expect(products.bento.find((p) => p.name === "Provider Portal")).toBeUndefined();
@@ -118,7 +155,7 @@ describe("ProductSwitcherService", () => {
       it("is included when there are providers", async () => {
         providerService.getAll.mockResolvedValue([{ id: "67899" }] as Provider[]);
 
-        service = TestBed.inject(ProductSwitcherService);
+        initiateService();
 
         const products = await firstValueFrom(service.products$);
 
@@ -129,7 +166,7 @@ describe("ProductSwitcherService", () => {
 
   describe("active product", () => {
     it("marks Password Manager as active", async () => {
-      service = TestBed.inject(ProductSwitcherService);
+      initiateService();
 
       const products = await firstValueFrom(service.products$);
 
@@ -141,7 +178,7 @@ describe("ProductSwitcherService", () => {
     it("marks Secret Manager as active", async () => {
       router.url = "/sm/";
 
-      service = TestBed.inject(ProductSwitcherService);
+      initiateService();
 
       const products = await firstValueFrom(service.products$);
 
@@ -155,7 +192,7 @@ describe("ProductSwitcherService", () => {
       activeRouteParams = convertToParamMap({ organizationId: "1" });
       router.url = "/organizations/";
 
-      service = TestBed.inject(ProductSwitcherService);
+      initiateService();
 
       const products = await firstValueFrom(service.products$);
 
@@ -168,7 +205,7 @@ describe("ProductSwitcherService", () => {
       providerService.getAll.mockResolvedValue([{ id: "67899" }] as Provider[]);
       router.url = "/providers/";
 
-      service = TestBed.inject(ProductSwitcherService);
+      initiateService();
 
       const products = await firstValueFrom(service.products$);
 
@@ -187,7 +224,7 @@ describe("ProductSwitcherService", () => {
         { id: "4243", canAccessSecretsManager: true, enabled: true, name: "Org 32" },
       ] as Organization[]);
 
-      service = TestBed.inject(ProductSwitcherService);
+      initiateService();
 
       const products = await firstValueFrom(service.products$);
 
@@ -205,7 +242,7 @@ describe("ProductSwitcherService", () => {
       { id: "4243", isOwner: true, name: "My Org" },
     ] as Organization[]);
 
-    service = TestBed.inject(ProductSwitcherService);
+    initiateService();
 
     const products = await firstValueFrom(service.products$);
 

apps/web/src/app/layouts/product-switcher/shared/product-switcher.service.ts

@@ -1,13 +1,6 @@
 import { Injectable } from "@angular/core";
-import {
-  ActivatedRoute,
-  Event,
-  NavigationEnd,
-  NavigationStart,
-  ParamMap,
-  Router,
-} from "@angular/router";
-import { combineLatest, concatMap, filter, map, Observable, startWith } from "rxjs";
+import { ActivatedRoute, NavigationEnd, NavigationStart, ParamMap, Router } from "@angular/router";
+import { combineLatest, concatMap, filter, map, Observable, ReplaySubject, startWith } from "rxjs";
 
 import { I18nPipe } from "@bitwarden/angular/platform/pipes/i18n.pipe";
 import {
@@ -16,6 +9,7 @@ import {
 } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
+import { SyncService } from "@bitwarden/common/platform/sync";
 
 export type ProductSwitcherItem = {
   /**
@@ -59,13 +53,38 @@ export type ProductSwitcherItem = {
   providedIn: "root",
 })
 export class ProductSwitcherService {
+  /**
+   * Emits when the sync service has completed a sync
+   *
+   * Without waiting for a sync to be complete, in accurate product information
+   * can be displayed to the user for a brief moment until the sync is complete
+   * and all data is available.
+   */
+  private syncCompleted$ = new ReplaySubject<void>(1);
+
+  /**
+   * Certain events should trigger an update to the `products$` observable but the values
+   * themselves are not needed. This observable is used to only trigger the update.
+   */
+  private triggerProductUpdate$: Observable<void> = combineLatest([
+    this.syncCompleted$,
+    this.router.events.pipe(
+      // Product paths need to be updated when routes change, but the router event isn't actually needed
+      startWith(null), // Start with a null event to trigger the initial combineLatest
+      filter((e) => e instanceof NavigationEnd || e instanceof NavigationStart || e === null),
+    ),
+  ]).pipe(map(() => null));
+
   constructor(
     private organizationService: OrganizationService,
     private providerService: ProviderService,
     private route: ActivatedRoute,
     private router: Router,
     private i18n: I18nPipe,
-  ) {}
+    private syncService: SyncService,
+  ) {
+    this.pollUntilSynced();
+  }
 
   products$: Observable<{
     bento: ProductSwitcherItem[];
@@ -73,13 +92,9 @@ export class ProductSwitcherService {
   }> = combineLatest([
     this.organizationService.organizations$,
     this.route.paramMap,
-    this.router.events.pipe(
-      // Product paths need to be updated when routes change, but the router event isn't actually needed
-      startWith(null), // Start with a null event to trigger the initial combineLatest
-      filter((e) => e instanceof NavigationEnd || e instanceof NavigationStart || e === null),
-    ),
+    this.triggerProductUpdate$,
   ]).pipe(
-    map(([orgs, ...rest]): [Organization[], ParamMap, Event | null] => {
+    map(([orgs, ...rest]): [Organization[], ParamMap, void] => {
       return [
         // Sort orgs by name to match the order within the sidebar
         orgs.sort((a, b) => a.name.localeCompare(b.name)),
@@ -186,4 +201,15 @@ export class ProductSwitcherService {
       };
     }),
   );
+
+  /** Poll the `syncService` until a sync is completed */
+  private pollUntilSynced() {
+    const interval = setInterval(async () => {
+      const lastSync = await this.syncService.getLastSync();
+      if (lastSync !== null) {
+        clearInterval(interval);
+        this.syncCompleted$.next();
+      }
+    }, 200);
+  }
 }