[PM-8379] Fix Vault Popup Items service loading (#10324)

* [PM-8379] Avoid fetching decrypted ciphers if no sync has completed

* [PM-8379] Do not emit ciphers loading when applying filters

apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts

@@ -1,11 +1,12 @@
 import { TestBed } from "@angular/core/testing";
 import { mock } from "jest-mock-extended";
-import { BehaviorSubject } from "rxjs";
+import { BehaviorSubject, firstValueFrom, timeout } from "rxjs";
 
 import { SearchService } from "@bitwarden/common/abstractions/search.service";
 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
 import { ProductTierType } from "@bitwarden/common/billing/enums";
+import { SyncService } from "@bitwarden/common/platform/sync";
 import { ObservableTracker } from "@bitwarden/common/spec";
 import { CipherId } from "@bitwarden/common/types/guid";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
@@ -37,6 +38,7 @@ describe("VaultPopupItemsService", () => {
   const searchService = mock<SearchService>();
   const collectionService = mock<CollectionService>();
   const vaultAutofillServiceMock = mock<VaultPopupAutofillService>();
+  const syncServiceMock = mock<SyncService>();
 
   beforeEach(() => {
     allCiphers = cipherFactory(10);
@@ -90,6 +92,8 @@ describe("VaultPopupItemsService", () => {
     organizationServiceMock.organizations$ = new BehaviorSubject([mockOrg]);
     collectionService.decryptedCollections$ = new BehaviorSubject(mockCollections);
 
+    syncServiceMock.getLastSync.mockResolvedValue(new Date());
+
     testBed = TestBed.configureTestingModule({
       providers: [
         { provide: CipherService, useValue: cipherServiceMock },
@@ -99,6 +103,7 @@ describe("VaultPopupItemsService", () => {
         { provide: VaultPopupListFiltersService, useValue: vaultPopupListFiltersServiceMock },
         { provide: CollectionService, useValue: collectionService },
         { provide: VaultPopupAutofillService, useValue: vaultAutofillServiceMock },
+        { provide: SyncService, useValue: syncServiceMock },
       ],
     });
 
@@ -155,6 +160,14 @@ describe("VaultPopupItemsService", () => {
     await expect(tracker.pauseUntilReceived(3)).rejects.toThrow("Timeout exceeded");
   });
 
+  it("should not emit cipher list if syncService.getLastSync returns null", async () => {
+    syncServiceMock.getLastSync.mockResolvedValue(null);
+
+    const obs$ = service.autoFillCiphers$.pipe(timeout(50));
+
+    await expect(firstValueFrom(obs$)).rejects.toThrow("Timeout has occurred");
+  });
+
   describe("autoFillCiphers$", () => {
     it("should return empty array if there is no current tab", (done) => {
       (vaultAutofillServiceMock.currentAutofillTab$ as BehaviorSubject<any>).next(null);
@@ -394,19 +407,6 @@ describe("VaultPopupItemsService", () => {
       expect(tracked.emissions[1]).toBe(true);
       expect(tracked.emissions[2]).toBe(false);
     });
-
-    it("should cycle when filters are applied", async () => {
-      // Restart tracking
-      tracked = new ObservableTracker(service.loading$);
-      service.applyFilter("test");
-
-      await trackedCiphers.pauseUntilReceived(2);
-
-      expect(tracked.emissions.length).toBe(3);
-      expect(tracked.emissions[0]).toBe(false);
-      expect(tracked.emissions[1]).toBe(true);
-      expect(tracked.emissions[2]).toBe(false);
-    });
   });
 
   describe("applyFilter", () => {

apps/browser/src/vault/popup/services/vault-popup-items.service.ts

@@ -5,6 +5,7 @@ import {
   concatMap,
   distinctUntilChanged,
   distinctUntilKeyChanged,
+  filter,
   from,
   map,
   merge,
@@ -21,6 +22,7 @@ import {
 import { SearchService } from "@bitwarden/common/abstractions/search.service";
 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
+import { SyncService } from "@bitwarden/common/platform/sync";
 import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service";
@@ -78,6 +80,8 @@ export class VaultPopupItemsService {
   ).pipe(
     runInsideAngular(inject(NgZone)), // Workaround to ensure cipher$ state provider emissions are run inside Angular
     tap(() => this._ciphersLoading$.next()),
+    switchMap(() => Utils.asyncToObservable(() => this.syncService.getLastSync())),
+    filter((lastSync) => lastSync !== null), // Only attempt to load ciphers if we performed a sync
     switchMap(() => Utils.asyncToObservable(() => this.cipherService.getAllDecrypted())),
     switchMap((ciphers) =>
       combineLatest([
@@ -108,7 +112,6 @@ export class VaultPopupItemsService {
     this._searchText$,
     this.vaultPopupListFiltersService.filterFunction$,
   ]).pipe(
-    tap(() => this._ciphersLoading$.next()),
     map(([ciphers, searchText, filterFunction]): [CipherView[], string] => [
       filterFunction(ciphers),
       searchText,
@@ -236,6 +239,7 @@ export class VaultPopupItemsService {
     private searchService: SearchService,
     private collectionService: CollectionService,
     private vaultPopupAutofillService: VaultPopupAutofillService,
+    private syncService: SyncService,
   ) {}
 
   applyFilter(newSearchText: string) {