BIT-1212: Refactor vault group to display a section of items (#529)

bitwarden · Mar 19, 2024 · f755309 · f755309
1 parent 721cc15
commit f755309
Show file tree

Hide file tree

Showing 11 changed files with 334 additions and 204 deletions.
diff --git a/BitwardenShared/Core/Vault/Repositories/TestHelpers/MockVaultRepository.swift b/BitwardenShared/Core/Vault/Repositories/TestHelpers/MockVaultRepository.swift
@@ -92,7 +92,7 @@ class MockVaultRepository: VaultRepository {
     var updateCipherCollectionsResult: Result<Void, Error> = .success(())
 
     var vaultListSubject = CurrentValueSubject<[VaultListSection], Error>([])
-    var vaultListGroupSubject = CurrentValueSubject<[VaultListItem], Error>([])
+    var vaultListGroupSubject = CurrentValueSubject<[VaultListSection], Error>([])
     var vaultListFilter: VaultFilterType?
 
     // MARK: Computed Properties
@@ -262,7 +262,7 @@ class MockVaultRepository: VaultRepository {
     func vaultListPublisher(
         group _: BitwardenShared.VaultListGroup,
         filter _: VaultFilterType
-    ) async throws -> AsyncThrowingPublisher<AnyPublisher<[VaultListItem], Error>> {
+    ) async throws -> AsyncThrowingPublisher<AnyPublisher<[VaultListSection], Error>> {
         vaultListGroupSubject.eraseToAnyPublisher().values
     }
 }

diff --git a/BitwardenShared/Core/Vault/Repositories/VaultRepository.swift b/BitwardenShared/Core/Vault/Repositories/VaultRepository.swift
@@ -230,18 +230,18 @@ public protocol VaultRepository: AnyObject {
         filter: VaultFilterType
     ) async throws -> AsyncThrowingPublisher<AnyPublisher<[VaultListSection], Error>>
 
-    /// A publisher for a group of items within the vault list.
+    /// A publisher for the sections within a group of items in the vault list.
     ///
     /// - Parameters:
     ///   - group: The group of items within the vault list to subscribe to.
     ///   - filter: A filter to apply to the vault items.
-    /// - Returns: A publisher for a group of items within the vault list which will be notified as
-    ///     the data changes.
+    /// - Returns: A publisher for the sections within a group of items in the vault list which will
+    ///     be notified as the data changes.
     ///
     func vaultListPublisher(
         group: VaultListGroup,
         filter: VaultFilterType
-    ) async throws -> AsyncThrowingPublisher<AnyPublisher<[VaultListItem], Error>>
+    ) async throws -> AsyncThrowingPublisher<AnyPublisher<[VaultListSection], Error>>
 }
 
 extension VaultRepository {
@@ -499,7 +499,8 @@ class DefaultVaultRepository { // swiftlint:disable:this type_body_length
         )
     }
 
-    /// Returns a list of items that are grouped together in the vault list from a list of encrypted ciphers.
+    /// Returns a list of sections containing the items that are grouped together in the vault list
+    /// from a list of encrypted ciphers.
     ///
     /// - Parameters:
     ///   - group: The group of items to get.
@@ -511,7 +512,7 @@ class DefaultVaultRepository { // swiftlint:disable:this type_body_length
         group: VaultListGroup,
         filter: VaultFilterType,
         from ciphers: [Cipher]
-    ) async throws -> [VaultListItem] {
+    ) async throws -> [VaultListSection] {
         let ciphers = try await ciphers.asyncMap { cipher in
             try await self.clientVault.ciphers().decrypt(cipher: cipher)
         }
@@ -521,30 +522,36 @@ class DefaultVaultRepository { // swiftlint:disable:this type_body_length
         let activeCiphers = ciphers.filter { $0.deletedDate == nil }
         let deletedCiphers = ciphers.filter { $0.deletedDate != nil }
 
+        let items: [VaultListItem]
         switch group {
         case .card:
-            return activeCiphers.filter { $0.type == .card }.compactMap(VaultListItem.init)
+            items = activeCiphers.filter { $0.type == .card }.compactMap(VaultListItem.init)
         case let .collection(id, _, _):
-            return activeCiphers.filter { $0.collectionIds.contains(id) }.compactMap(VaultListItem.init)
+            items = activeCiphers.filter { $0.collectionIds.contains(id) }.compactMap(VaultListItem.init)
         case let .folder(id, _):
-            return activeCiphers.filter { $0.folderId == id }.compactMap(VaultListItem.init)
+            items = activeCiphers.filter { $0.folderId == id }.compactMap(VaultListItem.init)
         case .identity:
-            return activeCiphers.filter { $0.type == .identity }.compactMap(VaultListItem.init)
+            items = activeCiphers.filter { $0.type == .identity }.compactMap(VaultListItem.init)
         case .login:
-            return activeCiphers.filter { $0.type == .login }.compactMap(VaultListItem.init)
+            items = activeCiphers.filter { $0.type == .login }.compactMap(VaultListItem.init)
         case .noFolder:
-            return activeCiphers.filter { $0.folderId == nil }.compactMap(VaultListItem.init)
+            items = activeCiphers.filter { $0.folderId == nil }.compactMap(VaultListItem.init)
         case .secureNote:
-            return activeCiphers.filter { $0.type == .secureNote }.compactMap(VaultListItem.init)
+            items = activeCiphers.filter { $0.type == .secureNote }.compactMap(VaultListItem.init)
         case .totp:
             // Ensure the user has premium features access
             guard try await doesActiveAccountHavePremium() else {
-                return []
+                items = []
+                break
             }
-            return await totpListItems(from: ciphers, filter: filter)
+            items = await totpListItems(from: ciphers, filter: filter)
         case .trash:
-            return deletedCiphers.compactMap(VaultListItem.init)
+            items = deletedCiphers.compactMap(VaultListItem.init)
         }
+
+        return [
+            VaultListSection(id: "Items", items: items, name: Localizations.items),
+        ]
     }
 
     /// Returns a list of the sections in the vault list from a sync response.
@@ -1038,7 +1045,7 @@ extension DefaultVaultRepository: VaultRepository {
     func vaultListPublisher(
         group: VaultListGroup,
         filter: VaultFilterType
-    ) async throws -> AsyncThrowingPublisher<AnyPublisher<[VaultListItem], Error>> {
+    ) async throws -> AsyncThrowingPublisher<AnyPublisher<[VaultListSection], Error>> {
         try await cipherService.ciphersPublisher()
             .asyncTryMap { ciphers in
                 try await self.vaultListItems(group: group, filter: filter, from: ciphers)

diff --git a/BitwardenShared/Core/Vault/Repositories/VaultRepositoryTests.swift b/BitwardenShared/Core/Vault/Repositories/VaultRepositoryTests.swift
@@ -1214,9 +1214,18 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
         cipherService.ciphersSubject.send([cipher])
 
         var iterator = try await subject.vaultListPublisher(group: .card, filter: .allVaults).makeAsyncIterator()
-        let vaultListItems = try await iterator.next()
+        let vaultListSections = try await iterator.next()
 
-        XCTAssertEqual(vaultListItems, [.fixture(cipherView: .init(cipher: cipher))])
+        XCTAssertEqual(
+            vaultListSections,
+            [
+                VaultListSection(
+                    id: "Items",
+                    items: [.fixture(cipherView: .init(cipher: cipher))],
+                    name: Localizations.items
+                ),
+            ]
+        )
     }
 
     /// `vaultListPublisher(group:filter:)` returns a publisher for the vault list items.
@@ -1229,9 +1238,18 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
             filter: .allVaults
         )
         .makeAsyncIterator()
-        let vaultListItems = try await iterator.next()
+        let vaultListSections = try await iterator.next()
 
-        XCTAssertEqual(vaultListItems, [.fixture(cipherView: .init(cipher: cipher))])
+        XCTAssertEqual(
+            vaultListSections,
+            [
+                VaultListSection(
+                    id: "Items",
+                    items: [.fixture(cipherView: .init(cipher: cipher))],
+                    name: Localizations.items
+                ),
+            ]
+        )
     }
 
     /// `vaultListPublisher(group:filter:)` returns a publisher for the vault list items.
@@ -1241,9 +1259,18 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
 
         var iterator = try await subject.vaultListPublisher(group: .folder(id: "1", name: ""), filter: .allVaults)
             .makeAsyncIterator()
-        let vaultListItems = try await iterator.next()
+        let vaultListSections = try await iterator.next()
 
-        XCTAssertEqual(vaultListItems, [.fixture(cipherView: .init(cipher: cipher))])
+        XCTAssertEqual(
+            vaultListSections,
+            [
+                VaultListSection(
+                    id: "Items",
+                    items: [.fixture(cipherView: .init(cipher: cipher))],
+                    name: Localizations.items
+                ),
+            ]
+        )
     }
 
     /// `vaultListPublisher(group:filter:)` returns a publisher for the vault list items.
@@ -1252,9 +1279,18 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
         cipherService.ciphersSubject.send([cipher])
 
         var iterator = try await subject.vaultListPublisher(group: .identity, filter: .allVaults).makeAsyncIterator()
-        let vaultListItems = try await iterator.next()
+        let vaultListSections = try await iterator.next()
 
-        XCTAssertEqual(vaultListItems, [.fixture(cipherView: .init(cipher: cipher))])
+        XCTAssertEqual(
+            vaultListSections,
+            [
+                VaultListSection(
+                    id: "Items",
+                    items: [.fixture(cipherView: .init(cipher: cipher))],
+                    name: Localizations.items
+                ),
+            ]
+        )
     }
 
     /// `vaultListPublisher(group:filter:)` returns a publisher for the vault list items.
@@ -1263,9 +1299,18 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
         cipherService.ciphersSubject.send([cipher])
 
         var iterator = try await subject.vaultListPublisher(group: .login, filter: .allVaults).makeAsyncIterator()
-        let vaultListItems = try await iterator.next()
+        let vaultListSections = try await iterator.next()
 
-        XCTAssertEqual(vaultListItems, [.fixture(cipherView: .init(cipher: cipher))])
+        XCTAssertEqual(
+            vaultListSections,
+            [
+                VaultListSection(
+                    id: "Items",
+                    items: [.fixture(cipherView: .init(cipher: cipher))],
+                    name: Localizations.items
+                ),
+            ]
+        )
     }
 
     /// `vaultListPublisher(group:filter:)` returns a publisher for the vault list items.
@@ -1274,9 +1319,18 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
         cipherService.ciphersSubject.send([cipher])
 
         var iterator = try await subject.vaultListPublisher(group: .secureNote, filter: .allVaults).makeAsyncIterator()
-        let vaultListItems = try await iterator.next()
+        let vaultListSections = try await iterator.next()
 
-        XCTAssertEqual(vaultListItems, [.fixture(cipherView: .init(cipher: cipher))])
+        XCTAssertEqual(
+            vaultListSections,
+            [
+                VaultListSection(
+                    id: "Items",
+                    items: [.fixture(cipherView: .init(cipher: cipher))],
+                    name: Localizations.items
+                ),
+            ]
+        )
     }
 
     /// `vaultListPublisher(group:filter:)` returns a publisher for the vault list items for premium accounts.
@@ -1286,9 +1340,10 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
         cipherService.ciphersSubject.send([cipher])
 
         var iterator = try await subject.vaultListPublisher(group: .totp, filter: .allVaults).makeAsyncIterator()
-        let vaultListItems = try await iterator.next()
+        let vaultListSections = try await iterator.next()
+        let vaultListItems = try XCTUnwrap(vaultListSections).flatMap(\.items)
 
-        let itemType = try XCTUnwrap(vaultListItems?.last?.itemType)
+        let itemType = try XCTUnwrap(vaultListItems.last?.itemType)
         if case let .totp(name, _) = itemType {
             XCTAssertEqual(name, "Bitwarden")
         } else {
@@ -1303,8 +1358,8 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
         cipherService.ciphersSubject.send([cipher])
 
         var iterator = try await subject.vaultListPublisher(group: .totp, filter: .allVaults).makeAsyncIterator()
-        let maybeItems = try await iterator.next()
-        let vaultListItems = try XCTUnwrap(maybeItems)
+        let vaultListSections = try await iterator.next()
+        let vaultListItems = try XCTUnwrap(vaultListSections).flatMap(\.items)
         XCTAssertTrue(vaultListItems.isEmpty)
     }
 
@@ -1318,9 +1373,9 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
         cipherService.ciphersSubject.send([cipher])
 
         var iterator = try await subject.vaultListPublisher(group: .totp, filter: .allVaults).makeAsyncIterator()
-        let vaultListItems = try await iterator.next()
+        let vaultListSections = try await iterator.next()
 
-        XCTAssertNil(vaultListItems?.last?.itemType)
+        try XCTAssertTrue(XCTUnwrap(vaultListSections).allSatisfy(\.items.isEmpty))
     }
 
     /// `vaultListPublisher(group:filter:)` returns a publisher for the vault list items.
@@ -1329,9 +1384,18 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
         cipherService.ciphersSubject.send([cipher])
 
         var iterator = try await subject.vaultListPublisher(group: .trash, filter: .allVaults).makeAsyncIterator()
-        let vaultListItems = try await iterator.next()
+        let vaultListSections = try await iterator.next()
 
-        XCTAssertEqual(vaultListItems, [.fixture(cipherView: .init(cipher: cipher))])
+        XCTAssertEqual(
+            vaultListSections,
+            [
+                VaultListSection(
+                    id: "Items",
+                    items: [.fixture(cipherView: .init(cipher: cipher))],
+                    name: Localizations.items
+                ),
+            ]
+        )
     }
 
     /// `vaultListPublisher(filter:)` returns a publisher for the vault list sections.
@@ -1687,11 +1751,12 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
         var iterator = try await subject.vaultListPublisher(group: .login, filter: .allVaults).makeAsyncIterator()
         let items = try await iterator.next()
 
-        try assertInlineSnapshot(of: dumpVaultListItems(XCTUnwrap(items)), as: .lines) {
+        try assertInlineSnapshot(of: dumpVaultListSections(XCTUnwrap(items)), as: .lines) {
             """
-            - Cipher: Apple
-            - Cipher: Facebook
-            - Cipher: Figma
+            Section: Items
+              - Cipher: Apple
+              - Cipher: Facebook
+              - Cipher: Figma
             """
         }
     }
@@ -1708,9 +1773,10 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
         var iterator = try await subject.vaultListPublisher(group: .login, filter: .myVault).makeAsyncIterator()
         let items = try await iterator.next()
 
-        try assertInlineSnapshot(of: dumpVaultListItems(XCTUnwrap(items)), as: .lines) {
+        try assertInlineSnapshot(of: dumpVaultListSections(XCTUnwrap(items)), as: .lines) {
             """
-            - Cipher: Facebook
+            Section: Items
+              - Cipher: Facebook
             """
         }
     }
@@ -1730,10 +1796,11 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
         ).makeAsyncIterator()
         let items = try await iterator.next()
 
-        try assertInlineSnapshot(of: dumpVaultListItems(XCTUnwrap(items)), as: .lines) {
+        try assertInlineSnapshot(of: dumpVaultListSections(XCTUnwrap(items)), as: .lines) {
             """
-            - Cipher: Apple
-            - Cipher: Figma
+            Section: Items
+              - Cipher: Apple
+              - Cipher: Figma
             """
         }
     }
@@ -1757,9 +1824,10 @@ class VaultRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_b
         ).makeAsyncIterator()
         let items = try await iterator.next()
 
-        try assertInlineSnapshot(of: dumpVaultListItems(XCTUnwrap(items)), as: .lines) {
+        try assertInlineSnapshot(of: dumpVaultListSections(XCTUnwrap(items)), as: .lines) {
             """
-            - Cipher: Apple
+            Section: Items
+              - Cipher: Apple
             """
         }
     }

diff --git a/BitwardenShared/UI/Vault/Vault/VaultGroup/Extensions/Array+TOTPHelpers.swift b/BitwardenShared/UI/Vault/Vault/VaultGroup/Extensions/Array+TOTPHelpers.swift
@@ -32,3 +32,22 @@ extension [VaultListItem] {
             .sorted { $0.name.localizedStandardCompare($1.name) == .orderedAscending }
     }
 }
+
+extension [VaultListSection] {
+    /// Update the array of sections with a batch of possible item updates.
+    ///
+    /// - Parameters:
+    ///   - updatedValues: An array of updates to make the items within the sections.
+    ///   - includeNewValues: A flag for including new values not found in the current list. Default is `false`.
+    /// - Returns: An updated version of the array including the updated elements.
+    ///
+    func updated(
+        with updatedValues: [VaultListItem],
+        includeNewValues: Bool = false
+    ) -> [VaultListSection] {
+        map { section in
+            let updatedItems = section.items.updated(with: updatedValues, includeNewValues: includeNewValues)
+            return VaultListSection(id: section.id, items: updatedItems, name: section.name)
+        }
+    }
+}