update specs

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

@@ -211,32 +211,40 @@ describe("VaultPopupItemsService", () => {
     });
   });
 
-  it("should update cipher list when cipherService.ciphers$ emits", async () => {
+  it("should not re-emit autoFillCiphers$ when cipherService.ciphers$ emits but the resulting list is unchanged", async () => {
     const tracker = new ObservableTracker(service.autoFillCiphers$);
 
+    // Initial emission
     await tracker.expectEmission();
+    const initial = tracker.emissions[0];
 
+    // Trigger a ciphers$ emission that doesn't actually change the final list
     ciphersSubject.next({});
 
-    await tracker.expectEmission();
+    // Ensure we do NOT get a second emission
+    await expect(tracker.pauseUntilReceived(2)).rejects.toThrow("Timeout exceeded");
+    expect(tracker.emissions.length).toBe(1);
 
-    // Should only emit twice
-    expect(tracker.emissions.length).toBe(2);
-    await expect(tracker.pauseUntilReceived(3)).rejects.toThrow("Timeout exceeded");
+    // Sanity check: still the same array of ciphers
+    expect(tracker.emissions[0]).toBe(initial);
   });
 
-  it("should update cipher list when cipherService.localData$ emits", async () => {
+  it("should not re-emit autoFillCiphers$ when cipherService.localData$ emits  but the resulting list is unchanged", async () => {
     const tracker = new ObservableTracker(service.autoFillCiphers$);
 
+    // Initial emission
     await tracker.expectEmission();
+    const initial = tracker.emissions[0];
 
+    // Trigger a localData$ emission that doesn't actually change the final list
     localDataSubject.next({});
 
-    await tracker.expectEmission();
+    // Ensure we do NOT get a second emission
+    await expect(tracker.pauseUntilReceived(2)).rejects.toThrow("Timeout exceeded");
+    expect(tracker.emissions.length).toBe(1);
 
-    // Should only emit twice
-    expect(tracker.emissions.length).toBe(2);
-    await expect(tracker.pauseUntilReceived(3)).rejects.toThrow("Timeout exceeded");
+    // Sanity check: still the same array of ciphers
+    expect(tracker.emissions[0]).toBe(initial);
   });
 
   it("should not emit cipher list if syncService.getLastSync returns null", async () => {
@@ -468,13 +476,9 @@ describe("VaultPopupItemsService", () => {
 
   describe("loading$", () => {
     let tracked: ObservableTracker<boolean>;
-    let trackedCiphers: ObservableTracker<any>;
     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 () => {
@@ -482,22 +486,28 @@ describe("VaultPopupItemsService", () => {
     });
 
     it("should emit false once ciphers are available", async () => {
-      expect(tracked.emissions.length).toBe(2);
+      await tracked.pauseUntilReceived(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$);
+    it("should cycle when cipherService.ciphers$ emits even if cipher lists don't re-emit", async () => {
+      await tracked.pauseUntilReceived(2);
+
+      const before = [...tracked.emissions];
+
       ciphersSubject.next({});
 
-      await trackedCiphers.pauseUntilReceived(2);
+      await tracked.pauseUntilReceived(before.length + 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);
+      const after = tracked.emissions;
+
+      expect(after.length).toBeGreaterThan(before.length);
+
+      const tail = after.slice(before.length);
+      expect(tail[0]).toBe(true);
+      expect(tail[1]).toBe(false);
     });
   });
 

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

@@ -219,6 +219,7 @@ export class VaultPopupItemsService {
       return this.cipherService.filterCiphersForUrl(ciphers, tab.url, otherTypes);
     }),
     map((ciphers) => ciphers.sort(this.sortCiphersForAutofill.bind(this))),
+    distinctUntilChanged(ciphersEqualByIdAndOrder),
     shareReplay({ refCount: false, bufferSize: 1 }),
   );
 
@@ -231,6 +232,7 @@ export class VaultPopupItemsService {
     map(([autoFillCiphers, ciphers]) =>
       ciphers.filter((cipher) => cipher.favorite && !autoFillCiphers.includes(cipher)),
     ),
+    distinctUntilChanged(ciphersEqualByIdAndOrder),
     shareReplay({ refCount: false, bufferSize: 1 }),
   );
 
@@ -250,6 +252,7 @@ export class VaultPopupItemsService {
         (cipher) => !autoFillCiphers.includes(cipher) && !favoriteCiphers.includes(cipher),
       ),
     ),
+    distinctUntilChanged(ciphersEqualByIdAndOrder),
     shareReplay({ refCount: false, bufferSize: 1 }),
   );
 
@@ -257,8 +260,10 @@ export class VaultPopupItemsService {
    * Observable that indicates whether the service is currently loading ciphers.
    */
   loading$: Observable<boolean> = merge(
+    // Turn loading on whenever we start processing ciphers
     this._ciphersLoading$.pipe(map(() => true)),
-    this.remainingCiphers$.pipe(map(() => false)),
+    // Turn loading off whenever the decrypted cipher list finishes updating
+    this._allDecryptedCiphers$.pipe(map(() => false)),
   ).pipe(startWith(true), distinctUntilChanged(), shareReplay({ refCount: false, bufferSize: 1 }));
 
   /** Observable that indicates whether there is search text present.
@@ -398,3 +403,25 @@ export class VaultPopupItemsService {
 const waitUntilSync = <T>(syncService: SyncService): MonoTypeOperatorFunction<T> => {
   return waitUntil(syncService.activeUserLastSync$().pipe(filter((lastSync) => lastSync != null)));
 };
+
+const ciphersEqualByIdAndOrder = (a: PopupCipherViewLike[], b: PopupCipherViewLike[]): boolean => {
+  if (a === b) {
+    return true;
+  }
+
+  if (!a || !b) {
+    return false;
+  }
+
+  if (a.length !== b.length) {
+    return false;
+  }
+
+  for (let i = 0; i < a.length; i++) {
+    if (a[i]?.id !== b[i]?.id) {
+      return false;
+    }
+  }
+
+  return true;
+};