From 81d7f319f63ab13cc1e1c5264867f6702ab3b641 Mon Sep 17 00:00:00 2001
From: Shane Melton <smelton@bitwarden.com>
Date: Thu, 24 Oct 2024 14:11:48 -0700
Subject: [PATCH] [PM-13896] Avoid sorting when a search term is applied
 (#11661)

---
 .../vault-popup-items.service.spec.ts         | 16 +++++++++--
 .../services/vault-popup-items.service.ts     | 28 +++++++++++--------
 2 files changed, 31 insertions(+), 13 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 bd683bf041..42d76e1dfe 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
@@ -277,6 +277,10 @@ describe("VaultPopupItemsService", () => {
   });
 
   describe("remainingCiphers$", () => {
+    beforeEach(() => {
+      searchService.isSearchable.mockImplementation(async (text) => text.length > 2);
+    });
+
     it("should exclude autofill and favorite ciphers", (done) => {
       service.remainingCiphers$.subscribe((ciphers) => {
         // 2 autofill ciphers, 2 favorite ciphers = 6 remaining ciphers to show
@@ -285,13 +289,21 @@ describe("VaultPopupItemsService", () => {
       });
     });
 
-    it("should sort by last used then by name", (done) => {
-      service.remainingCiphers$.subscribe((ciphers) => {
+    it("should sort by last used then by name by default", (done) => {
+      service.remainingCiphers$.subscribe(() => {
         expect(cipherServiceMock.getLocaleSortingFunction).toHaveBeenCalled();
         done();
       });
     });
 
+    it("should NOT sort by last used then by name when search text is applied", (done) => {
+      service.applyFilter("Login");
+      service.remainingCiphers$.subscribe(() => {
+        expect(cipherServiceMock.getLocaleSortingFunction).not.toHaveBeenCalled();
+        done();
+      });
+    });
+
     it("should filter remainingCiphers$ down to search term", (done) => {
       const cipherList = Object.values(allCiphers);
       const searchText = "Login";
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 414bf02875..09c7d5fb0d 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
@@ -6,7 +6,6 @@ import {
   distinctUntilChanged,
   distinctUntilKeyChanged,
   filter,
-  from,
   map,
   merge,
   MonoTypeOperatorFunction,
@@ -111,6 +110,14 @@ export class VaultPopupItemsService {
     ),
   );
 
+  /**
+   * Observable that indicates whether there is search text present that is searchable.
+   * @private
+   */
+  private _hasSearchText$ = this._searchText$.pipe(
+    switchMap((searchText) => this.searchService.isSearchable(searchText)),
+  );
+
   private _filteredCipherList$: Observable<PopupCipherView[]> = combineLatest([
     this._activeCipherList$,
     this._searchText$,
@@ -179,7 +186,11 @@ export class VaultPopupItemsService {
         (cipher) => !autoFillCiphers.includes(cipher) && !favoriteCiphers.includes(cipher),
       ),
     ),
-    map((ciphers) => ciphers.sort(this.cipherService.getLocaleSortingFunction())),
+    withLatestFrom(this._hasSearchText$),
+    map(([ciphers, hasSearchText]) =>
+      // Do not sort alphabetically when there is search text, default to the search service scoring
+      hasSearchText ? ciphers : ciphers.sort(this.cipherService.getLocaleSortingFunction()),
+    ),
     shareReplay({ refCount: false, bufferSize: 1 }),
   );
 
@@ -192,19 +203,14 @@ export class VaultPopupItemsService {
   ).pipe(startWith(true), distinctUntilChanged(), shareReplay({ refCount: false, bufferSize: 1 }));
 
   /**
-   * Observable that indicates whether a filter is currently applied to the ciphers.
+   * Observable that indicates whether a filter or search text is currently applied to the ciphers.
    */
   hasFilterApplied$ = combineLatest([
-    this._searchText$,
+    this._hasSearchText$,
     this.vaultPopupListFiltersService.filters$,
   ]).pipe(
-    switchMap(([searchText, filters]) => {
-      return from(this.searchService.isSearchable(searchText)).pipe(
-        map(
-          (isSearchable) =>
-            isSearchable || Object.values(filters).some((filter) => filter !== null),
-        ),
-      );
+    map(([hasSearchText, filters]) => {
+      return hasSearchText || Object.values(filters).some((filter) => filter !== null);
     }),
   );