From 19f2d2aefc3cd411511638f6c80c4695580b030e Mon Sep 17 00:00:00 2001 From: Shane Melton Date: Mon, 10 Jun 2024 09:55:12 -0700 Subject: [PATCH] [PM-8379] Update vault popup items service to track loading state (#9528) --- .../vault-popup-items.service.spec.ts | 48 +++++++++++++++++++ .../services/vault-popup-items.service.ts | 35 ++++++++++---- libs/common/spec/observable-tracker.ts | 9 ++-- 3 files changed, 78 insertions(+), 14 deletions(-) diff --git a/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts index b7091eb87b..f08f4e836e 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts @@ -379,6 +379,54 @@ describe("VaultPopupItemsService", () => { }); }); + describe("loading$", () => { + let tracked: ObservableTracker; + let trackedCiphers: ObservableTracker; + beforeEach(() => { + // Start tracking loading$ emissions + tracked = new ObservableTracker(service.loading$); + + // Track remainingCiphers$ to make cipher observables active + trackedCiphers = new ObservableTracker(service.remainingCiphers$); + }); + + it("should initialize with true first", async () => { + expect(tracked.emissions[0]).toBe(true); + }); + + it("should emit false once ciphers are available", async () => { + expect(tracked.emissions.length).toBe(2); + expect(tracked.emissions[0]).toBe(true); + expect(tracked.emissions[1]).toBe(false); + }); + + it("should cycle when cipherService.ciphers$ emits", async () => { + // Restart tracking + tracked = new ObservableTracker(service.loading$); + (cipherServiceMock.ciphers$ as BehaviorSubject).next(null); + + 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); + }); + + 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", () => { it("should call search Service with the new search term", (done) => { const searchText = "Hello"; diff --git a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts index 189ce2c09f..f96bb095b9 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts @@ -2,6 +2,7 @@ import { inject, Injectable, NgZone } from "@angular/core"; import { BehaviorSubject, combineLatest, + distinctUntilChanged, distinctUntilKeyChanged, from, map, @@ -12,6 +13,8 @@ import { startWith, Subject, switchMap, + tap, + withLatestFrom, } from "rxjs"; import { SearchService } from "@bitwarden/common/abstractions/search.service"; @@ -40,6 +43,13 @@ import { MY_VAULT_ID, VaultPopupListFiltersService } from "./vault-popup-list-fi export class VaultPopupItemsService { private _refreshCurrentTab$ = new Subject(); private _searchText$ = new BehaviorSubject(""); + + /** + * Subject that emits whenever new ciphers are being processed/filtered. + * @private + */ + private _ciphersLoading$ = new Subject(); + latestSearchText$: Observable = this._searchText$.asObservable(); /** @@ -84,6 +94,7 @@ export class VaultPopupItemsService { this.cipherService.localData$, ).pipe( runInsideAngular(inject(NgZone)), // Workaround to ensure cipher$ state provider emissions are run inside Angular + tap(() => this._ciphersLoading$.next()), switchMap(() => Utils.asyncToObservable(() => this.cipherService.getAllDecrypted())), switchMap((ciphers) => combineLatest([ @@ -112,6 +123,7 @@ export class VaultPopupItemsService { this._searchText$, this.vaultPopupListFiltersService.filterFunction$, ]).pipe( + tap(() => this._ciphersLoading$.next()), map(([ciphers, searchText, filterFunction]): [CipherView[], string] => [ filterFunction(ciphers), searchText, @@ -148,10 +160,8 @@ export class VaultPopupItemsService { * List of favorite ciphers that are not currently suggested for autofill. * Ciphers are sorted by last used date, then by name. */ - favoriteCiphers$: Observable = combineLatest([ - this.autoFillCiphers$, - this._filteredCipherList$, - ]).pipe( + favoriteCiphers$: Observable = this.autoFillCiphers$.pipe( + withLatestFrom(this._filteredCipherList$), map(([autoFillCiphers, ciphers]) => ciphers.filter((cipher) => cipher.favorite && !autoFillCiphers.includes(cipher)), ), @@ -165,12 +175,9 @@ export class VaultPopupItemsService { * List of all remaining ciphers that are not currently suggested for autofill or marked as favorite. * Ciphers are sorted by name. */ - remainingCiphers$: Observable = combineLatest([ - this.autoFillCiphers$, - this.favoriteCiphers$, - this._filteredCipherList$, - ]).pipe( - map(([autoFillCiphers, favoriteCiphers, ciphers]) => + remainingCiphers$: Observable = this.favoriteCiphers$.pipe( + withLatestFrom(this._filteredCipherList$, this.autoFillCiphers$), + map(([favoriteCiphers, ciphers, autoFillCiphers]) => ciphers.filter( (cipher) => !autoFillCiphers.includes(cipher) && !favoriteCiphers.includes(cipher), ), @@ -179,6 +186,14 @@ export class VaultPopupItemsService { shareReplay({ refCount: false, bufferSize: 1 }), ); + /** + * Observable that indicates whether the service is currently loading ciphers. + */ + loading$: Observable = merge( + this._ciphersLoading$.pipe(map(() => true)), + this.remainingCiphers$.pipe(map(() => false)), + ).pipe(startWith(true), distinctUntilChanged(), shareReplay({ refCount: false, bufferSize: 1 })); + /** * Observable that indicates whether a filter is currently applied to the ciphers. */ diff --git a/libs/common/spec/observable-tracker.ts b/libs/common/spec/observable-tracker.ts index 9bf0475bee..dfb4983593 100644 --- a/libs/common/spec/observable-tracker.ts +++ b/libs/common/spec/observable-tracker.ts @@ -1,4 +1,4 @@ -import { Observable, Subject, Subscription, firstValueFrom, throwError, timeout } from "rxjs"; +import { firstValueFrom, Observable, Subject, Subscription, throwError, timeout } from "rxjs"; /** Test class to enable async awaiting of observable emissions */ export class ObservableTracker { @@ -43,6 +43,9 @@ export class ObservableTracker { private trackEmissions(observable: Observable): T[] { const emissions: T[] = []; + this.emissionReceived.subscribe((value) => { + emissions.push(value); + }); this.subscription = observable.subscribe((value) => { if (value == null) { this.emissionReceived.next(null); @@ -64,9 +67,7 @@ export class ObservableTracker { } } }); - this.emissionReceived.subscribe((value) => { - emissions.push(value); - }); + return emissions; } }