From 3bb8d4944bf1bedb9ea84340339b3bb1cf529c22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Tue, 1 Oct 2024 22:19:28 +0100 Subject: [PATCH 1/9] [PM-12695] Add to password history updates of hidden fields --- .../Extensions/CipherView+Update.swift | 64 +++++++++++++++---- 1 file changed, 51 insertions(+), 13 deletions(-) diff --git a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift index 1398013058..aaf3c8d898 100644 --- a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift +++ b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift @@ -128,10 +128,10 @@ extension CipherView { // Update the password updated date and the password history if the password has changed. var passwordHistory = passwordHistory + let lastUsedDate = timeProvider.presentTime if addEditState.type == .login, let previousPassword = login?.password, addEditState.loginState.password != previousPassword { - let lastUsedDate = timeProvider.presentTime loginState.passwordUpdatedDate = lastUsedDate // Update the password history list. @@ -141,11 +141,21 @@ extension CipherView { } else { passwordHistory!.append(newPasswordHistoryView) } - - // Cap the size of the password history list to 5. - passwordHistory = passwordHistory?.suffix(5) } + // Map new fields + let newFields = mapFields(addEditState) + + updatePasswordHistoryWithHiddenFields( + passwordHistory: &passwordHistory, + oldFields: addEditState.cipher.fields, + newFields: newFields, + lastUsedDate: lastUsedDate + ) + + // Cap the size of the password history list to 5. + passwordHistory = passwordHistory?.suffix(5) + // Return the updated cipher. return CipherView( id: id, @@ -167,21 +177,49 @@ extension CipherView { viewPassword: viewPassword, localData: localData, attachments: attachments, - fields: addEditState.customFieldsState.customFields.isEmpty ? - nil : addEditState.customFieldsState.customFields.map { customField in - FieldView( - name: customField.name, - value: customField.value, - type: .init(fieldType: customField.type), - linkedId: customField.linkedIdType?.rawValue - ) - }, + fields: newFields, passwordHistory: passwordHistory, creationDate: creationDate, deletedDate: deletedDate, revisionDate: revisionDate ) } + + private func updatePasswordHistoryWithHiddenFields( + passwordHistory: inout [PasswordHistoryView]?, + oldFields: [FieldView]?, + newFields: [FieldView]?, + lastUsedDate: Date + ) { + if let oldFields { + oldFields.filter { field in + FieldType(fieldType: field.type) == FieldType.hidden && + !(newFields?.contains(field) ?? false) + }.forEach { hiddenField in + let passHistoryHiddenField = PasswordHistoryView( + password: "\(hiddenField.name ?? ""): \(hiddenField.value ?? "")", + lastUsedDate: lastUsedDate + ) + if passwordHistory == nil { + passwordHistory = [passHistoryHiddenField] + } else if !passwordHistory!.contains(passHistoryHiddenField) { + passwordHistory!.append(passHistoryHiddenField) + } + } + } + } + + private func mapFields(_ addEditState: AddEditItemState) -> [FieldView]? { + addEditState.customFieldsState.customFields.isEmpty ? + nil : addEditState.customFieldsState.customFields.map { customField in + FieldView( + name: customField.name, + value: customField.value, + type: .init(fieldType: customField.type), + linkedId: customField.linkedIdType?.rawValue + ) + } + } } extension CipherView { From 8eb491d4af6b7e48b5fbed78e9c4f366f0eeb411 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Fri, 4 Oct 2024 11:14:23 +0100 Subject: [PATCH 2/9] [PM-12695] Add tests for hidden fields updating password history --- .../BitwardenSdk+VaultFixtures.swift | 25 +++++++- .../Extensions/CipherViewUpdateTests.swift | 61 ++++++++++++++++++- 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/BitwardenShared/UI/Vault/PreviewContent/BitwardenSdk+VaultFixtures.swift b/BitwardenShared/UI/Vault/PreviewContent/BitwardenSdk+VaultFixtures.swift index 8d927c62ef..89e68e977c 100644 --- a/BitwardenShared/UI/Vault/PreviewContent/BitwardenSdk+VaultFixtures.swift +++ b/BitwardenShared/UI/Vault/PreviewContent/BitwardenSdk+VaultFixtures.swift @@ -193,7 +193,14 @@ extension CipherView { deletedDate: Date? = nil, edit: Bool = true, favorite: Bool = false, - fields: [FieldView]? = nil, + fields: [FieldView]? = [ + FieldView( + name: "Name", + value: "1", + type: BitwardenSdk.FieldType.hidden, + linkedId: nil + ), + ], folderId: String? = nil, id: String = "8675", key: String? = nil, @@ -383,6 +390,22 @@ extension Fido2CredentialView { } } +extension BitwardenSdk.FieldView { + static func fixture( + name: String? = "Name", + value: String? = "1", + type: BitwardenSdk.FieldType = BitwardenSdk.FieldType.hidden, + linkedId: BitwardenSdk.LinkedIdType? = nil + ) -> BitwardenSdk.FieldView { + BitwardenSdk.FieldView( + name: name, + value: value, + type: type, + linkedId: linkedId + ) + } +} + extension BitwardenSdk.Login { static func fixture( autofillOnPageLoad: Bool? = nil, diff --git a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherViewUpdateTests.swift b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherViewUpdateTests.swift index f8e00c620f..433b6e503f 100644 --- a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherViewUpdateTests.swift +++ b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherViewUpdateTests.swift @@ -3,7 +3,7 @@ import XCTest @testable import BitwardenShared -final class CipherViewUpdateTests: BitwardenTestCase { +final class CipherViewUpdateTests: BitwardenTestCase { // swiftlint:disable:this type_body_length // MARK: Properties var cipherItemState: CipherItemState! @@ -178,6 +178,7 @@ final class CipherViewUpdateTests: BitwardenTestCase { /// Tests that the update succeeds with new properties. func test_update_login_edits_succeeds() { + subject = CipherView.loginFixture(fields: nil) cipherItemState.type = .login cipherItemState.notes = "I have a note" cipherItemState.loginState.username = "PASTA" @@ -219,7 +220,7 @@ final class CipherViewUpdateTests: BitwardenTestCase { /// Tests that the update succeeds with a new password updating the password history. func test_update_login_passwordHistory_succeeds() { - subject = CipherView.loginFixture(login: .fixture(password: "Old password")) + subject = CipherView.loginFixture(fields: nil, login: .fixture(password: "Old password")) cipherItemState.loginState.password = "New password" let comparison = subject.updatedView(with: cipherItemState) @@ -234,6 +235,62 @@ final class CipherViewUpdateTests: BitwardenTestCase { XCTAssertEqual(newerPasswordHistory?.last?.password, "New password") } + /// Tests that the update succeeds when a hidden field change modifies the password history.. + func test_update_login_passwordHistory_hiddenField_succeeds() { + cipherItemState.customFieldsState.customFields = [ + CustomFieldState(fieldView: .fixture(value: "2")), + ] + + let comparison = subject.updatedView(with: cipherItemState) + let newPasswordHistory = comparison.passwordHistory + + XCTAssertEqual(newPasswordHistory?.last?.password, "Name: 1") + + cipherItemState.customFieldsState.customFields = [ + CustomFieldState(fieldView: .fixture(value: "3")), + ] + + let secondComparison = comparison.updatedView(with: cipherItemState) + let newerPasswordHistory = secondComparison.passwordHistory + + XCTAssertEqual(newerPasswordHistory?.last?.password, "Name: 2") + } + + /// Tests that the update succeeds when a hidden field is deleted modifies the password history.. + func test_update_login_passwordHistory_deleteHiddenField_succeeds() { + cipherItemState.customFieldsState.customFields = [ + CustomFieldState(fieldView: .fixture()), + CustomFieldState(fieldView: .fixture(name: "NewField", value: "1")), + ] + + let comparison = subject.updatedView(with: cipherItemState) + let newPasswordHistory = comparison.passwordHistory + + XCTAssertNil(newPasswordHistory) + + cipherItemState.customFieldsState.customFields = [ + CustomFieldState(fieldView: .fixture()), + ] + + let secondComparison = comparison.updatedView(with: cipherItemState) + let newerPasswordHistory = secondComparison.passwordHistory + + XCTAssertEqual(newerPasswordHistory?.last?.password, "NewField: 1") + } + + /// Tests a new hidden field doesn't update the password history. + func test_update_login_passwordHistory_newHiddenField_succeeds() { + cipherItemState.customFieldsState.customFields = [ + CustomFieldState(fieldView: .fixture()), + CustomFieldState(fieldView: .fixture(name: "NewField", value: "1")), + ] + + let comparison = subject.updatedView(with: cipherItemState) + let newPasswordHistory = comparison.passwordHistory + + XCTAssertNil(newPasswordHistory) + } + /// Tests that the update succeeds with a new password updating the password revision date. func test_update_login_passwordRevisionDate_succeeds() throws { subject = CipherView.loginFixture( From f1022883546c2416c77314085e021c1cf8662bbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Fri, 4 Oct 2024 11:14:55 +0100 Subject: [PATCH 3/9] [PM-12695] Code refactor --- .../Extensions/CipherView+Update.swift | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift index aaf3c8d898..392ce5a9a7 100644 --- a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift +++ b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift @@ -146,9 +146,9 @@ extension CipherView { // Map new fields let newFields = mapFields(addEditState) - updatePasswordHistoryWithHiddenFields( - passwordHistory: &passwordHistory, - oldFields: addEditState.cipher.fields, + passwordHistory = updatePasswordHistoryWithHiddenFields( + passwordHistory: passwordHistory, + oldFields: fields, newFields: newFields, lastUsedDate: lastUsedDate ) @@ -186,11 +186,12 @@ extension CipherView { } private func updatePasswordHistoryWithHiddenFields( - passwordHistory: inout [PasswordHistoryView]?, + passwordHistory: [PasswordHistoryView]?, oldFields: [FieldView]?, newFields: [FieldView]?, lastUsedDate: Date - ) { + ) -> [PasswordHistoryView]? { + var newHistory = passwordHistory if let oldFields { oldFields.filter { field in FieldType(fieldType: field.type) == FieldType.hidden && @@ -200,13 +201,14 @@ extension CipherView { password: "\(hiddenField.name ?? ""): \(hiddenField.value ?? "")", lastUsedDate: lastUsedDate ) - if passwordHistory == nil { - passwordHistory = [passHistoryHiddenField] + if newHistory == nil { + newHistory = [passHistoryHiddenField] } else if !passwordHistory!.contains(passHistoryHiddenField) { - passwordHistory!.append(passHistoryHiddenField) + newHistory!.append(passHistoryHiddenField) } } } + return newHistory } private func mapFields(_ addEditState: AddEditItemState) -> [FieldView]? { From eb695dc6bcf6e1994b7f2a2cfdc814324eae1ac9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Fri, 4 Oct 2024 21:08:27 +0100 Subject: [PATCH 4/9] [PM-12695] Fix comment typo --- .../ViewLoginItem/Extensions/CipherViewUpdateTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherViewUpdateTests.swift b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherViewUpdateTests.swift index 433b6e503f..6244e72749 100644 --- a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherViewUpdateTests.swift +++ b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherViewUpdateTests.swift @@ -235,7 +235,7 @@ final class CipherViewUpdateTests: BitwardenTestCase { // swiftlint:disable:this XCTAssertEqual(newerPasswordHistory?.last?.password, "New password") } - /// Tests that the update succeeds when a hidden field change modifies the password history.. + /// Tests that the update succeeds when a hidden field change modifies the password history. func test_update_login_passwordHistory_hiddenField_succeeds() { cipherItemState.customFieldsState.customFields = [ CustomFieldState(fieldView: .fixture(value: "2")), From a4dde62e7add84c498efe053867b136676b57255 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Fri, 4 Oct 2024 22:01:50 +0100 Subject: [PATCH 5/9] [PM-12695] Fix tests --- .../Alert/TestHelpers/Alert+TestHelpers.swift | 2 +- .../BitwardenSdk+VaultFixtures.swift | 9 +----- .../Extensions/CipherViewUpdateTests.swift | 28 +++++++++++++++++-- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/BitwardenShared/UI/Platform/Application/Utilities/Alert/Alert/TestHelpers/Alert+TestHelpers.swift b/BitwardenShared/UI/Platform/Application/Utilities/Alert/Alert/TestHelpers/Alert+TestHelpers.swift index b2d4474853..b29e4376ea 100644 --- a/BitwardenShared/UI/Platform/Application/Utilities/Alert/Alert/TestHelpers/Alert+TestHelpers.swift +++ b/BitwardenShared/UI/Platform/Application/Utilities/Alert/Alert/TestHelpers/Alert+TestHelpers.swift @@ -18,7 +18,7 @@ enum AlertError: LocalizedError { extension Alert { /// Simulates tapping the cancel button of the alert. - func tapCancel() async throws{ + func tapCancel() async throws { try await tapAction(title: Localizations.cancel) } diff --git a/BitwardenShared/UI/Vault/PreviewContent/BitwardenSdk+VaultFixtures.swift b/BitwardenShared/UI/Vault/PreviewContent/BitwardenSdk+VaultFixtures.swift index 89e68e977c..a2b0ecccda 100644 --- a/BitwardenShared/UI/Vault/PreviewContent/BitwardenSdk+VaultFixtures.swift +++ b/BitwardenShared/UI/Vault/PreviewContent/BitwardenSdk+VaultFixtures.swift @@ -193,14 +193,7 @@ extension CipherView { deletedDate: Date? = nil, edit: Bool = true, favorite: Bool = false, - fields: [FieldView]? = [ - FieldView( - name: "Name", - value: "1", - type: BitwardenSdk.FieldType.hidden, - linkedId: nil - ), - ], + fields: [FieldView]? = nil, folderId: String? = nil, id: String = "8675", key: String? = nil, diff --git a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherViewUpdateTests.swift b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherViewUpdateTests.swift index 6244e72749..28c782b43e 100644 --- a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherViewUpdateTests.swift +++ b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherViewUpdateTests.swift @@ -178,7 +178,7 @@ final class CipherViewUpdateTests: BitwardenTestCase { // swiftlint:disable:this /// Tests that the update succeeds with new properties. func test_update_login_edits_succeeds() { - subject = CipherView.loginFixture(fields: nil) + subject = CipherView.loginFixture() cipherItemState.type = .login cipherItemState.notes = "I have a note" cipherItemState.loginState.username = "PASTA" @@ -220,7 +220,7 @@ final class CipherViewUpdateTests: BitwardenTestCase { // swiftlint:disable:this /// Tests that the update succeeds with a new password updating the password history. func test_update_login_passwordHistory_succeeds() { - subject = CipherView.loginFixture(fields: nil, login: .fixture(password: "Old password")) + subject = CipherView.loginFixture(login: .fixture(password: "Old password")) cipherItemState.loginState.password = "New password" let comparison = subject.updatedView(with: cipherItemState) @@ -237,6 +237,14 @@ final class CipherViewUpdateTests: BitwardenTestCase { // swiftlint:disable:this /// Tests that the update succeeds when a hidden field change modifies the password history. func test_update_login_passwordHistory_hiddenField_succeeds() { + subject = CipherView.loginFixture(fields: [ + FieldView( + name: "Name", + value: "1", + type: BitwardenSdk.FieldType.hidden, + linkedId: nil + ), + ]) cipherItemState.customFieldsState.customFields = [ CustomFieldState(fieldView: .fixture(value: "2")), ] @@ -258,6 +266,14 @@ final class CipherViewUpdateTests: BitwardenTestCase { // swiftlint:disable:this /// Tests that the update succeeds when a hidden field is deleted modifies the password history.. func test_update_login_passwordHistory_deleteHiddenField_succeeds() { + subject = CipherView.loginFixture(fields: [ + FieldView( + name: "Name", + value: "1", + type: BitwardenSdk.FieldType.hidden, + linkedId: nil + ), + ]) cipherItemState.customFieldsState.customFields = [ CustomFieldState(fieldView: .fixture()), CustomFieldState(fieldView: .fixture(name: "NewField", value: "1")), @@ -280,6 +296,14 @@ final class CipherViewUpdateTests: BitwardenTestCase { // swiftlint:disable:this /// Tests a new hidden field doesn't update the password history. func test_update_login_passwordHistory_newHiddenField_succeeds() { + subject = CipherView.loginFixture(fields: [ + FieldView( + name: "Name", + value: "1", + type: BitwardenSdk.FieldType.hidden, + linkedId: nil + ), + ]) cipherItemState.customFieldsState.customFields = [ CustomFieldState(fieldView: .fixture()), CustomFieldState(fieldView: .fixture(name: "NewField", value: "1")), From 3312dfa5c01bf629a03ecb6fca5491f26a774189 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Wed, 9 Oct 2024 12:02:29 +0100 Subject: [PATCH 6/9] [PM-12695] Update with PR suggestions --- .../Extensions/CipherView+Update.swift | 55 ++++++++++--------- .../Extensions/CipherViewUpdateTests.swift | 2 +- .../Extensions/FieldView+Extensions.swift | 18 ++++++ 3 files changed, 49 insertions(+), 26 deletions(-) create mode 100644 BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/FieldView+Extensions.swift diff --git a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift index ae65796b72..e687717f11 100644 --- a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift +++ b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift @@ -161,7 +161,7 @@ extension CipherView { } // Map new fields - let newFields = mapFields(addEditState) + let newFields = mapToCustomFieldViews(from: addEditState.customFieldsState.customFields) passwordHistory = updatePasswordHistoryWithHiddenFields( passwordHistory: passwordHistory, @@ -208,36 +208,41 @@ extension CipherView { newFields: [FieldView]?, lastUsedDate: Date ) -> [PasswordHistoryView]? { + guard let oldFields else { + return passwordHistory + } + var newHistory = passwordHistory - if let oldFields { - oldFields.filter { field in - FieldType(fieldType: field.type) == FieldType.hidden && - !(newFields?.contains(field) ?? false) - }.forEach { hiddenField in - let passHistoryHiddenField = PasswordHistoryView( - password: "\(hiddenField.name ?? ""): \(hiddenField.value ?? "")", - lastUsedDate: lastUsedDate - ) - if newHistory == nil { - newHistory = [passHistoryHiddenField] - } else if !passwordHistory!.contains(passHistoryHiddenField) { - newHistory!.append(passHistoryHiddenField) - } + oldFields.filter { field in + FieldType(fieldType: field.type) == FieldType.hidden && + !field.name.isEmptyOrNil && + !field.value.isEmptyOrNil && + !(newFields?.contains(field) ?? false) + }.forEach { hiddenField in + guard !hiddenField.name.isEmptyOrNil else { + return + } + + let passHistoryHiddenField = PasswordHistoryView( + password: "\(hiddenField.name ?? ""): \(hiddenField.value ?? "")", + lastUsedDate: lastUsedDate + ) + if newHistory == nil { + newHistory = [passHistoryHiddenField] + } else { + newHistory?.append(passHistoryHiddenField) } } return newHistory } - private func mapFields(_ addEditState: AddEditItemState) -> [FieldView]? { - addEditState.customFieldsState.customFields.isEmpty ? - nil : addEditState.customFieldsState.customFields.map { customField in - FieldView( - name: customField.name, - value: customField.value, - type: .init(fieldType: customField.type), - linkedId: customField.linkedIdType?.rawValue - ) - } + /// Maps the array of `CustomFieldState` into the an array of `FieldView`. + private func mapToCustomFieldViews(from customFields: [CustomFieldState]) -> [FieldView]? { + guard !customFields.isEmpty else { + return nil + } + + return customFields.map { FieldView(customFieldState: $0) } } } diff --git a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherViewUpdateTests.swift b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherViewUpdateTests.swift index 28c782b43e..b72c7fb8a8 100644 --- a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherViewUpdateTests.swift +++ b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherViewUpdateTests.swift @@ -399,4 +399,4 @@ final class CipherViewUpdateTests: BitwardenTestCase { // swiftlint:disable:this XCTAssertNil(identity.postalCode) XCTAssertNil(identity.country) } -} +} // swiftlint:disable:this file_length diff --git a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/FieldView+Extensions.swift b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/FieldView+Extensions.swift new file mode 100644 index 0000000000..683baa63d9 --- /dev/null +++ b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/FieldView+Extensions.swift @@ -0,0 +1,18 @@ +import BitwardenSdk // swiftlint:disable:this file_name + +// MARK: - FieldView+Update + +extension BitwardenSdk.FieldView { + /// initializes a new LoginView with updated properties + /// + /// - Parameter customFieldState: The `CustomFieldState` used to create or update the field view. + /// + init(customFieldState: CustomFieldState) { + self.init( + name: customFieldState.name, + value: customFieldState.value, + type: BitwardenSdk.FieldType(fieldType: customFieldState.type), + linkedId: customFieldState.linkedIdType?.rawValue + ) + } +} From d70e830f217ceaf868a20d4cc4ec39cc8c0bbe43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Tue, 22 Oct 2024 10:34:19 +0100 Subject: [PATCH 7/9] [PM-12695] Fix comment typo --- .../ViewLoginItem/Extensions/FieldView+Extensions.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/FieldView+Extensions.swift b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/FieldView+Extensions.swift index 683baa63d9..958e918e64 100644 --- a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/FieldView+Extensions.swift +++ b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/FieldView+Extensions.swift @@ -3,7 +3,7 @@ import BitwardenSdk // swiftlint:disable:this file_name // MARK: - FieldView+Update extension BitwardenSdk.FieldView { - /// initializes a new LoginView with updated properties + /// initializes a new FieldView with updated properties /// /// - Parameter customFieldState: The `CustomFieldState` used to create or update the field view. /// From c355dcc8c054d36eafc771f7d98a8d54184ac3bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Tue, 22 Oct 2024 17:24:46 +0100 Subject: [PATCH 8/9] [PM-12695] Refactor method logic with PR suggestion --- .../Extensions/CipherView+Update.swift | 41 +++++++++---------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift index e687717f11..b75cfe2fe2 100644 --- a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift +++ b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift @@ -211,29 +211,28 @@ extension CipherView { guard let oldFields else { return passwordHistory } - - var newHistory = passwordHistory - oldFields.filter { field in - FieldType(fieldType: field.type) == FieldType.hidden && - !field.name.isEmptyOrNil && - !field.value.isEmptyOrNil && - !(newFields?.contains(field) ?? false) - }.forEach { hiddenField in - guard !hiddenField.name.isEmptyOrNil else { - return - } - - let passHistoryHiddenField = PasswordHistoryView( - password: "\(hiddenField.name ?? ""): \(hiddenField.value ?? "")", - lastUsedDate: lastUsedDate - ) - if newHistory == nil { - newHistory = [passHistoryHiddenField] - } else { - newHistory?.append(passHistoryHiddenField) + let newPasswordHistoryFields: [PasswordHistoryView] = oldFields + .filter { field in + FieldType(fieldType: field.type) == FieldType.hidden && + !field.name.isEmptyOrNil && + !field.value.isEmptyOrNil && + !(newFields?.contains(field) ?? false) + }.compactMap { hiddenField in + guard !hiddenField.name.isEmptyOrNil else { + return nil + } + return PasswordHistoryView( + password: "\(hiddenField.name ?? ""): \(hiddenField.value ?? "")", + lastUsedDate: lastUsedDate + ) } + guard !newPasswordHistoryFields.isEmpty else { + return passwordHistory + } + guard let passwordHistory else { + return newPasswordHistoryFields } - return newHistory + return passwordHistory + newPasswordHistoryFields } /// Maps the array of `CustomFieldState` into the an array of `FieldView`. From 2dfd7a385277bf250030f677172328aa9ce8d42a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Thu, 24 Oct 2024 15:41:06 +0100 Subject: [PATCH 9/9] [PM-12695] Remove unnecessary check for nil name --- .../ViewLoginItem/Extensions/CipherView+Update.swift | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift index b75cfe2fe2..1cb6f3f17d 100644 --- a/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift +++ b/BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewLoginItem/Extensions/CipherView+Update.swift @@ -218,10 +218,7 @@ extension CipherView { !field.value.isEmptyOrNil && !(newFields?.contains(field) ?? false) }.compactMap { hiddenField in - guard !hiddenField.name.isEmptyOrNil else { - return nil - } - return PasswordHistoryView( + PasswordHistoryView( password: "\(hiddenField.name ?? ""): \(hiddenField.value ?? "")", lastUsedDate: lastUsedDate )