From 26dfe3f172902e6b1ef4d6237ea4e91f348b6a30 Mon Sep 17 00:00:00 2001 From: Pete Smith Date: Thu, 4 Apr 2024 10:54:17 +0100 Subject: [PATCH] Improve Handling of noData Import Errors (#2494) Task/Issue URL: https://app.asana.com/0/0/1206886138223455/f **Description**: Stop prompting the user for feedback when we are handling a noData error. When no data is found initially, and user chooses to skip manual import, no summary screen is displayed (the import dialog is dismissed) --- .../Images/Error.imageset/Contents.json | 2 +- ...ror.pdf => Exclamation-Recolorable-16.pdf} | Bin 1801 -> 1697 bytes .../Images/Skipped.imageset/Contents.json | 12 ++ .../Skipped.imageset/Jump-Recolorable-16.pdf | Bin 0 -> 2069 bytes .../Check-Recolorable-16.pdf | Bin 0 -> 1560 bytes .../SuccessCheckmark.imageset/Contents.json | 2 +- .../SuccessCheckmark.pdf | Bin 1507 -> 0 bytes DuckDuckGo/Common/Localizables/UserText.swift | 7 + .../Firefox/FirefoxBookmarksReader.swift | 8 +- .../Logins/Firefox/FirefoxLoginReader.swift | 8 +- .../Model/DataImportViewModel.swift | 19 ++- .../View/DataImportNoDataView.swift | 6 +- .../View/DataImportSummaryView.swift | 38 +++++- DuckDuckGo/Localizable.xcstrings | 126 +++++++++++++++++- .../DataImport/DataImportViewModelTests.swift | 102 ++++++++++---- .../FirefoxBookmarksReaderTests.swift | 17 +++ .../DataImport/FirefoxLoginReaderTests.swift | 22 ++- 17 files changed, 322 insertions(+), 47 deletions(-) rename DuckDuckGo/Assets.xcassets/Images/Error.imageset/{Error.pdf => Exclamation-Recolorable-16.pdf} (50%) create mode 100644 DuckDuckGo/Assets.xcassets/Images/Skipped.imageset/Contents.json create mode 100644 DuckDuckGo/Assets.xcassets/Images/Skipped.imageset/Jump-Recolorable-16.pdf create mode 100644 DuckDuckGo/Assets.xcassets/Images/SuccessCheckmark.imageset/Check-Recolorable-16.pdf delete mode 100644 DuckDuckGo/Assets.xcassets/Images/SuccessCheckmark.imageset/SuccessCheckmark.pdf diff --git a/DuckDuckGo/Assets.xcassets/Images/Error.imageset/Contents.json b/DuckDuckGo/Assets.xcassets/Images/Error.imageset/Contents.json index c8ff07440c..98276ca193 100644 --- a/DuckDuckGo/Assets.xcassets/Images/Error.imageset/Contents.json +++ b/DuckDuckGo/Assets.xcassets/Images/Error.imageset/Contents.json @@ -1,7 +1,7 @@ { "images" : [ { - "filename" : "Error.pdf", + "filename" : "Exclamation-Recolorable-16.pdf", "idiom" : "universal" } ], diff --git a/DuckDuckGo/Assets.xcassets/Images/Error.imageset/Error.pdf b/DuckDuckGo/Assets.xcassets/Images/Error.imageset/Exclamation-Recolorable-16.pdf similarity index 50% rename from DuckDuckGo/Assets.xcassets/Images/Error.imageset/Error.pdf rename to DuckDuckGo/Assets.xcassets/Images/Error.imageset/Exclamation-Recolorable-16.pdf index f8efeca609c4e025751136c335010f42a7e3f66c..4f4f2518c48d40f216f48ff3983c2fe1073b1e57 100644 GIT binary patch literal 1697 zcma)7O>fjN5WV|X_)@7ws^hQtL#is#Eky_rWy`JN5VCH&Xg7gPiVDA;nQSsnw;b?c z4nRy=19$jBvohsoOQ(W@)+jqwK*%?1SXQ^4?C)0Dh_>h|W{(&0+muS`XyHssi zahZOt>$H4(!5264uexJDnG$kXTuz%)+PyrZqk7Avh#Fnd)2U!1Y?OD}Mc&m73lkst zY?;!+s-S(y(UhC)j3|ThTJsDMN6j*rNoG4@A(!k>vwu)iR+)q#;;5m>Q68e84IPm= zAvJqocduB(KL4-6P6S_~m}UwH3X}dXH>j`_UY1fR@Ub9VR2(k_5aUe7q%g)t;{hps z3>w-45O8yV_+YFu9P3ngo0>=Alv7}}K`eNwJYYdPXB~Q$&M3PN2w7%lEu@V)#2f(;YoyJ2AX|h6i>S?*FOo&9Gv=z`p+KE>R=EqrUtui4D!%;wk$7=k-KP=wm{#lRqJ84G`MFa9`FZBu#I#IZ`?#x9$8qp2N4iDZ z^r?Y}IQ1s*y4nH8%MqH*Mucc)7Vp<9NR?v+OFqR}w2$DcUgX~aGGl_ipD|&99R7x9SkUIV zm4$acLH({;Z_dkq5WV|X%w?sdQVa%cgA_$-cAKhdt8PheQ4f@LHdMRN0;$sc`i{Yj*^*WB8_zFvk5i%#ayOBM-h04I7;S0PcpNJg+hEl$zGtQsyqon#8G0CZahRm23?Um zAtifY_peyZKI4Q#u0t(C4LDe5Ee*U=I>jF-9wk{8C4eKQ1uP9aV#=p^oQgD7V7KB5 z)?8;Qlp&5fj-^f&q+G;<7p9li%OfSrc=Q)`KuoD560!FaOZqwFs=#fLpW>O)3b#sR zimR~4;{^u?jsyHMM7_fip0t+{6*Xf}TQ!^f?pj3prKjzB3GE>Jk!iLuTT zNT3KTAdPD}^-2<~^%Rnk5A55bI+&K(?Y-OJIL^A}XwKoR+MSO>3G$}t!8SY$IJm8z W4OTz=X%%fhv?HX>*lP9W)6E}+`gn-| diff --git a/DuckDuckGo/Assets.xcassets/Images/Skipped.imageset/Contents.json b/DuckDuckGo/Assets.xcassets/Images/Skipped.imageset/Contents.json new file mode 100644 index 0000000000..89a905405a --- /dev/null +++ b/DuckDuckGo/Assets.xcassets/Images/Skipped.imageset/Contents.json @@ -0,0 +1,12 @@ +{ + "images" : [ + { + "filename" : "Jump-Recolorable-16.pdf", + "idiom" : "universal" + } + ], + "info" : { + "author" : "xcode", + "version" : 1 + } +} diff --git a/DuckDuckGo/Assets.xcassets/Images/Skipped.imageset/Jump-Recolorable-16.pdf b/DuckDuckGo/Assets.xcassets/Images/Skipped.imageset/Jump-Recolorable-16.pdf new file mode 100644 index 0000000000000000000000000000000000000000..f0c7598cc801ef4ffc5c628f626836798ccbd1fb GIT binary patch literal 2069 zcma)7O^*{f5WV|X_%af#gyXVZ{*VyTG6M*~YKIIrw1;d@Gh~(NX43%yzn)i}j=N`u z16CBdpX>8gxqNnc@$w0oq!ekZ`+xsaT0ecNpFLA;cP*dFE%DXcw!1le(jMSiq=v)3 z-Q23xMf-KzwCh*T_4(EEs~Ob)ii{nXwx`XLx_fw5kD6O$4SQM8yuTTmPUVxms&lKz znLyd4JbR;flJkZ$CFk@ERU3_(!P&@SSFKc)S)ozCFkPdXjZ~=GXprPYPc>0D^{9$h zq*0&L$3Il3-u*7a&cx__AstL{!Bf;=xbpvSBMg%@-bcwNqIw=qqlm8;zQAs*I+9#QPvsN zm2iNR;5iWx7ej`>U|cFeQdSB5ky7x;h?so{2*wpAD@Y2(=kZ8#wLl8nz#e)qN;$BD zI5BZx?I6z36sf?eBuysU%svD0iHZ%-vnD69HX;Uy6ra$T&O4|g&Rjw%MOu}-6G+QKDAcTjc!=bXS1^b-yCX5Ea{{+W&_>4Xf_RGJ zCLni@pKMBYXa>j-Xb@u9WQPnbFph}DgIV|hH9*9jwK_?|IMm3aYBoSv3RsXGIt=`S zKTasKkddS!7EOwZWD}5|lSI86+fn?LI5uR(cEBQy4OuZ%?%43S5Mk`?idcjFs~mwz2GOjrY+ z7Fj*+bXOsdIAv7`g9Pu?Jt%;kyHKJBiYd>Ehy7-|Yxhd3_xb{KJR17j_OX7ny?%To kWwq}6gO-Fxf>&4DA1$IEAMnloFs6gymekqVi@#s~1XXse`Tzg` literal 0 HcmV?d00001 diff --git a/DuckDuckGo/Assets.xcassets/Images/SuccessCheckmark.imageset/Check-Recolorable-16.pdf b/DuckDuckGo/Assets.xcassets/Images/SuccessCheckmark.imageset/Check-Recolorable-16.pdf new file mode 100644 index 0000000000000000000000000000000000000000..719cb643de9f549679735432b0e111415fa25614 GIT binary patch literal 1560 zcma)+O>Yx15Qgvm6?3W7BGo(NZ%b7r+ERo7QBrOdhh^ioLCG#;x2W*z8E+GB+Hk;! z$a(B}J@b0U2Ui!D$0WTF1PI-??*icD1Wr#yYUlZ{P`kYPklMT99yG&Q-qjE5RIfyJ zk$x?jG<$mn=hwUcnqK@AByk*o+sSd6?XQ)kW+hOPiiS7`>201(5`EK(u+=LuD?v&c z3d)CRE4l4HEo~sL6ildSG-862$j+>&l#nT)5&w`bSxq7Z6^(|A+}Kkj%A_on?S(Yr zUfjMCt@!-E4jVSp@=;)9KqODV4j2FTC)r^yts(C$SQ*LM7-7ZC$B5{dxnPZU;AK!4 zt!JVlYVu5ML?3e;YK`7Oc1W9%p;TdHOJ<59QobxQE_w}GqR|2Km7~XK6et-SDHhGd zL~mJ3N#}#pnaH~=x@b&Ra>^qy(UCtN%S59=6FDv)!_4Jl?8ufPe~jL%U>Ise z3N>V;SvN59WTMN5D;1ToOrqA-(gu7?^F@8uJ;DuF{Ia>H zu9#+9rCI8`hjo*B$nVW;BX{1lLu&cN{Mc;aRlQ~ge0?L@d6F_JPgV7PG3QhvS1^O^ zIp^I2$I@Q@J(C?JzOWsoTwq@goC-g$hkDuFJr8|cr(1#G#Z&qD7o4@)0^`jfgmDIK zL7MZ-qaeb)U4)vAc)m*^SN#I{SkSPp7t6F3{B~FGF4&I4zFVauI9$w+o=mA`T{l23 ccr>`WUi?X%{rHC0>tQU1v<~9n;LWGYzh{9;VgLXD literal 0 HcmV?d00001 diff --git a/DuckDuckGo/Assets.xcassets/Images/SuccessCheckmark.imageset/Contents.json b/DuckDuckGo/Assets.xcassets/Images/SuccessCheckmark.imageset/Contents.json index 09a818b90b..07d8227e97 100644 --- a/DuckDuckGo/Assets.xcassets/Images/SuccessCheckmark.imageset/Contents.json +++ b/DuckDuckGo/Assets.xcassets/Images/SuccessCheckmark.imageset/Contents.json @@ -1,7 +1,7 @@ { "images" : [ { - "filename" : "SuccessCheckmark.pdf", + "filename" : "Check-Recolorable-16.pdf", "idiom" : "universal" } ], diff --git a/DuckDuckGo/Assets.xcassets/Images/SuccessCheckmark.imageset/SuccessCheckmark.pdf b/DuckDuckGo/Assets.xcassets/Images/SuccessCheckmark.imageset/SuccessCheckmark.pdf deleted file mode 100644 index bb50d381fa0c14409d842a68bcf87bdd05ab8ac1..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 1507 zcma)6O>fjN5WV|X%%xI`RL5TvJ5p7NZYe^5Shn0M4k7Edi*^%8QdIiu8OMo}QV#gA za^8C0ym{mC(fac0RC3Fh1Od&r?+oDV49?G)ueafosU@C$@bz7P51PPDIMwy7FAprc z^uKnM-@Ltm)%EhL>ex>vxfm9RX>*!(i~sUGM`7p?ZHhucbzp)^RTxPcxW%xWDY~j= z(uA{COR!vPEwzKg0@PeNE$CkL-gpMV~ zRLUSEuBbAUHUxy6GgPrz+>FkJ&3rSq$a~LjSM7CrrAcKoMGt z&I$*LJ1GPX&7KgnXo)n%1g_Bo#sh8<_H} zng@!NFZ6eWESTtSESO@MtFkZm&D|Ipithd;;<5F&On_x^hQlx9h;*w79+#y185YzD z(xme_RG@tU(OrCgN=hVeo&`#!Nd5@*ZMoa~meDhuy}Kkkj=SdIPvCgBJ&C4do2Ka@ d6g(K5UGJVesUM!>vh9a*q;klPj^2E_`U8SYL7@Nu diff --git a/DuckDuckGo/Common/Localizables/UserText.swift b/DuckDuckGo/Common/Localizables/UserText.swift index 5f17e7d9b2..e78ee81e96 100644 --- a/DuckDuckGo/Common/Localizables/UserText.swift +++ b/DuckDuckGo/Common/Localizables/UserText.swift @@ -636,6 +636,13 @@ struct UserText { String(format: NSLocalizedString("import.logins.select-csv-file.source", value: "Select %@ CSV File…", comment: "Button text for selecting a CSV file exported from (LastPass or Bitwarden or 1Password - %@)"), source.importSourceName) } + static func importNoDataBookmarksSubtitle(from source: DataImport.Source) -> String { + String(format: NSLocalizedString("import.nodata.bookmarks.subtitle", value: "If you have %@ bookmarks, try importing them manually instead.", comment: "Data import error subtitle: suggestion to import Bookmarks manually by selecting a CSV or HTML file."), source.importSourceName) + } + static func importNoDataPasswordsSubtitle(from source: DataImport.Source) -> String { + String(format: NSLocalizedString("import.nodata.passwords.subtitle", value: "If you have %@ passwords, try importing them manually instead.", comment: "Data import error subtitle: suggestion to import passwords manually by selecting a CSV or HTML file. The placeholder here represents the source browser, e.g Firefox"), source.importSourceName) + } + static let importLoginsPasswords = NSLocalizedString("import.logins.passwords", value: "Passwords", comment: "Title text for the Passwords import option") static let importBookmarksButtonTitle = NSLocalizedString("bookmarks.import.button.title", value: "Import", comment: "Button text to open bookmark import dialog") diff --git a/DuckDuckGo/DataImport/Bookmarks/Firefox/FirefoxBookmarksReader.swift b/DuckDuckGo/DataImport/Bookmarks/Firefox/FirefoxBookmarksReader.swift index fdf0029709..afef50085b 100644 --- a/DuckDuckGo/DataImport/Bookmarks/Firefox/FirefoxBookmarksReader.swift +++ b/DuckDuckGo/DataImport/Bookmarks/Firefox/FirefoxBookmarksReader.swift @@ -45,6 +45,8 @@ final class FirefoxBookmarksReader { case fetchAllFolders case copyTemporaryFile + + case couldNotFindBookmarksFile } var action: DataImportAction { .bookmarks } @@ -53,7 +55,7 @@ final class FirefoxBookmarksReader { var errorType: DataImport.ErrorType { switch type { - case .dbOpen: .noData + case .dbOpen, .couldNotFindBookmarksFile: .noData case .fetchRootEntries, .noRootEntries, .fetchTopLevelFolders, .fetchAllBookmarks, .fetchAllFolders: .dataCorrupted case .copyTemporaryFile: .other } @@ -72,6 +74,10 @@ final class FirefoxBookmarksReader { } func readBookmarks() -> DataImportResult { + guard FileManager.default.fileExists(atPath: firefoxPlacesDatabaseURL.path) else { + return .failure(ImportError(type: .couldNotFindBookmarksFile, underlyingError: nil)) + } + do { currentOperationType = .copyTemporaryFile return try firefoxPlacesDatabaseURL.withTemporaryFile { temporaryDatabaseURL in diff --git a/DuckDuckGo/DataImport/Logins/Firefox/FirefoxLoginReader.swift b/DuckDuckGo/DataImport/Logins/Firefox/FirefoxLoginReader.swift index 3c354886cc..af702efd16 100644 --- a/DuckDuckGo/DataImport/Logins/Firefox/FirefoxLoginReader.swift +++ b/DuckDuckGo/DataImport/Logins/Firefox/FirefoxLoginReader.swift @@ -41,6 +41,8 @@ final class FirefoxLoginReader { case decryptUsername case decryptPassword + + case couldNotFindKeyDB } var action: DataImportAction { .passwords } @@ -49,7 +51,7 @@ final class FirefoxLoginReader { var errorType: DataImport.ErrorType { switch type { - case .couldNotFindLoginsFile, .couldNotReadLoginsFile: .noData + case .couldNotFindLoginsFile, .couldNotFindKeyDB, .couldNotReadLoginsFile: .noData case .key3readerStage1, .key3readerStage2, .key3readerStage3, .key4readerStage1, .key4readerStage2, .key4readerStage3, .decryptUsername, .decryptPassword: .decryptionError case .couldNotDetermineFormat: .dataCorrupted case .requiresPrimaryPassword: .other @@ -94,7 +96,7 @@ final class FirefoxLoginReader { func readLogins(dataFormat: DataFormat?) -> DataImportResult<[ImportedLoginCredential]> { var currentOperationType: ImportError.OperationType = .couldNotFindLoginsFile do { - let dataFormat = try dataFormat ?? detectLoginFormat() ?? { throw ImportError(type: .couldNotDetermineFormat, underlyingError: nil) }() + let dataFormat = try dataFormat ?? detectLoginFormat() ?? { throw ImportError(type: .couldNotFindKeyDB, underlyingError: nil) }() let keyData = try getEncryptionKey(dataFormat: dataFormat) let result = try reallyReadLogins(dataFormat: dataFormat, keyData: keyData, currentOperationType: ¤tOperationType) return .success(result) @@ -106,7 +108,7 @@ final class FirefoxLoginReader { } func getEncryptionKey() throws -> Data { - let dataFormat = try detectLoginFormat() ?? { throw ImportError(type: .couldNotDetermineFormat, underlyingError: nil) }() + let dataFormat = try detectLoginFormat() ?? { throw ImportError(type: .couldNotFindKeyDB, underlyingError: nil) }() return try getEncryptionKey(dataFormat: dataFormat) } diff --git a/DuckDuckGo/DataImport/Model/DataImportViewModel.swift b/DuckDuckGo/DataImport/Model/DataImportViewModel.swift index 1a2d1eef7b..8757194031 100644 --- a/DuckDuckGo/DataImport/Model/DataImportViewModel.swift +++ b/DuckDuckGo/DataImport/Model/DataImportViewModel.swift @@ -107,6 +107,11 @@ struct DataImportViewModel { self.dataType = dataType self.result = result } + + static func == (lhs: DataTypeImportResult, rhs: DataTypeImportResult) -> Bool { + lhs.dataType == rhs.dataType && + lhs.result.description == rhs.result.description + } } /// collected import summary for current import operation per selected import source @@ -248,6 +253,7 @@ struct DataImportViewModel { // switch to file import of the failed data type displaying successful import results nextScreen = .fileImport(dataType: dataType, summary: Set(summary.filter({ $0.value.isSuccess }).keys)) } + Pixel.fire(.dataImportFailed(source: importSource, sourceVersion: importSource.installedAppsMajorVersionDescription(selectedProfile: selectedProfile), error: error)) } } @@ -322,16 +328,19 @@ struct DataImportViewModel { } /// Skip button press - @MainActor mutating func skipImport() { + @MainActor mutating func skipImportOrDismiss(using dismiss: @escaping () -> Void) { if let screen = screenForNextDataTypeRemainingToImport(after: screen.fileImportDataType) { // skip to next non-imported data type self.screen = screen - } else if selectedDataTypes.first(where: { error(for: $0) != nil }) != nil { + } else if selectedDataTypes.first(where: { + let error = error(for: $0) + return error != nil && error?.errorType != .noData + }) != nil { // errors occurred during import: show feedback screen self.screen = .feedback } else { - // display total summary - self.screen = .summary(selectedDataTypes) + // When we skip a manual import, and there are no next non-imported data types, we dismiss + self.dismiss(using: dismiss) } } @@ -671,7 +680,7 @@ extension DataImportViewModel { initiateImport() case .skip: - skipImport() + skipImportOrDismiss(using: dismiss) case .cancel: importTask?.cancel() diff --git a/DuckDuckGo/DataImport/View/DataImportNoDataView.swift b/DuckDuckGo/DataImport/View/DataImportNoDataView.swift index 84fb50a583..889daccbab 100644 --- a/DuckDuckGo/DataImport/View/DataImportNoDataView.swift +++ b/DuckDuckGo/DataImport/View/DataImportNoDataView.swift @@ -31,15 +31,13 @@ struct DataImportNoDataView: View { Text("We couldn‘t find any bookmarks.", comment: "Data import error message: Bookmarks weren‘t found.") .bold() - Text("Try importing bookmarks manually instead.", - comment: "Data import error subtitle: suggestion to import Bookmarks manually by selecting a CSV or HTML file.") + Text(UserText.importNoDataBookmarksSubtitle(from: source)) case .passwords: Text("We couldn‘t find any passwords.", comment: "Data import error message: Passwords weren‘t found.") .bold() - Text("Try importing passwords manually instead.", - comment: "Data import error subtitle: suggestion to import Passwords manually by selecting a CSV or HTML file.") + Text(UserText.importNoDataPasswordsSubtitle(from: source)) } } } diff --git a/DuckDuckGo/DataImport/View/DataImportSummaryView.swift b/DuckDuckGo/DataImport/View/DataImportSummaryView.swift index f9eddeecfb..b385a90c66 100644 --- a/DuckDuckGo/DataImport/View/DataImportSummaryView.swift +++ b/DuckDuckGo/DataImport/View/DataImportSummaryView.swift @@ -33,6 +33,8 @@ struct DataImportSummaryView: View { self.model = model } + private let zeroString = "0" + var body: some View { VStack(alignment: .leading, spacing: 8) { { @@ -61,7 +63,7 @@ struct DataImportSummaryView: View { } if summary.duplicate > 0 { HStack { - failureImage() + skippedImage() Text("Duplicate Bookmarks Skipped:", comment: "Data import summary format of how many duplicate bookmarks (%lld) were skipped during import.") + Text(" " as String) @@ -78,6 +80,15 @@ struct DataImportSummaryView: View { } } + case (.bookmarks, .failure(let error)) where error.errorType == .noData: + HStack { + skippedImage() + Text("Bookmarks:", + comment: "Data import summary format of how many bookmarks were successfully imported.") + + Text(" " as String) + + Text(zeroString).bold() + } + case (.bookmarks, .failure): HStack { failureImage() @@ -85,11 +96,21 @@ struct DataImportSummaryView: View { comment: "Data import summary message of failed bookmarks import.") } - case (.passwords, .failure): - HStack { - failureImage() - Text("Password import failed.", - comment: "Data import summary message of failed passwords import.") + case (.passwords, .failure(let error)): + if error.errorType == .noData { + HStack { + skippedImage() + Text("Passwords:", + comment: "Data import summary format of how many passwords were successfully imported.") + + Text(" " as String) + + Text(zeroString).bold() + } + } else { + HStack { + failureImage() + Text("Password import failed.", + comment: "Data import summary message of failed passwords import.") + } } case (.passwords, .success(let summary)): @@ -126,6 +147,11 @@ private func failureImage() -> some View { .frame(width: 16, height: 16) } +private func skippedImage() -> some View { + Image(.skipped) + .frame(width: 16, height: 16) +} + #if DEBUG #Preview { VStack { diff --git a/DuckDuckGo/Localizable.xcstrings b/DuckDuckGo/Localizable.xcstrings index bcd4fd7fac..288c5a5bc5 100644 --- a/DuckDuckGo/Localizable.xcstrings +++ b/DuckDuckGo/Localizable.xcstrings @@ -24263,6 +24263,126 @@ } } }, + "import.nodata.bookmarks.subtitle" : { + "comment" : "Data import error subtitle: suggestion to import Bookmarks manually by selecting a CSV or HTML file. The placeholder here represents the source browser, e.g Firefox.", + "extractionState" : "extracted_with_value", + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Wenn du %@ Lesezeichen hast, versuche stattdessen, sie manuell zu importieren." + } + }, + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "If you have %@ bookmarks, try importing them manually instead." + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Si tienes %@ marcadores, intenta importarlos manualmente." + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Si vous avez %@ signets, essayez plutôt de les importer manuellement." + } + }, + "it" : { + "stringUnit" : { + "state" : "translated", + "value" : "Se hai %@ segnalibri, prova a importarli manualmente." + } + }, + "nl" : { + "stringUnit" : { + "state" : "translated", + "value" : "Als je %@ bladwijzers hebt, probeer ze dan handmatig te importeren." + } + }, + "pl" : { + "stringUnit" : { + "state" : "translated", + "value" : "Jeśli masz zakładki %@, spróbuj zaimportować je ręcznie." + } + }, + "pt" : { + "stringUnit" : { + "state" : "translated", + "value" : "Se tens favoritos do %@, tenta importá-los manualmente." + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Если у вас есть закладки в %@, попробуйте импортировать их вручную." + } + } + } + }, + "import.nodata.passwords.subtitle" : { + "comment" : "Data import error subtitle: suggestion to import passwords manually by selecting a CSV or HTML file. The placeholder here represents the source browser, e.g Firefox.", + "extractionState" : "extracted_with_value", + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Wenn du %@ Passwörter hast, versuche stattdessen, sie manuell zu importieren." + } + }, + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "If you have %@ passwords, try importing them manually instead." + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Si tiene %@ contraseñas, intenta importarlas manualmente." + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Si vous avez %@ mots de passe, essayez plutôt de les importer manuellement." + } + }, + "it" : { + "stringUnit" : { + "state" : "translated", + "value" : "Se hai %@ password, prova a importarle manualmente." + } + }, + "nl" : { + "stringUnit" : { + "state" : "translated", + "value" : "Als je %@ wachtwoorden hebt, probeer ze dan handmatig te importeren." + } + }, + "pl" : { + "stringUnit" : { + "state" : "translated", + "value" : "Jeśli masz hasła %@, spróbuj zaimportować je ręcznie." + } + }, + "pt" : { + "stringUnit" : { + "state" : "translated", + "value" : "Se tens palavras-passe do %@, tenta importá-las manualmente." + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Если у вас есть пароли в %@, попробуйте импортировать их вручную." + } + } + } + }, "import.onePassword.app.version.info" : { "comment" : "Instructions how to find an installed 1Password password manager app version.\n%1$s, %2$s - app name (1Password)", "extractionState" : "extracted_with_value", @@ -34811,7 +34931,7 @@ } }, "Passwords:" : { - "comment" : "Data import summary format of how many passwords (%lld) were successfully imported.", + "comment" : "Data import summary format of how many passwords were successfully imported.", "localizations" : { "de" : { "stringUnit" : { @@ -50946,6 +51066,7 @@ }, "Try importing bookmarks manually instead." : { "comment" : "Data import error subtitle: suggestion to import Bookmarks manually by selecting a CSV or HTML file.", + "extractionState" : "stale", "localizations" : { "de" : { "stringUnit" : { @@ -50999,6 +51120,7 @@ }, "Try importing passwords manually instead." : { "comment" : "Data import error subtitle: suggestion to import Passwords manually by selecting a CSV or HTML file.", + "extractionState" : "stale", "localizations" : { "de" : { "stringUnit" : { @@ -52129,4 +52251,4 @@ } }, "version" : "1.0" -} \ No newline at end of file +} diff --git a/UnitTests/DataImport/DataImportViewModelTests.swift b/UnitTests/DataImport/DataImportViewModelTests.swift index 6d698185e4..81f5fca4a7 100644 --- a/UnitTests/DataImport/DataImportViewModelTests.swift +++ b/UnitTests/DataImport/DataImportViewModelTests.swift @@ -1067,9 +1067,9 @@ import XCTest // MARK: - File Import after failure (or nil result for a data type) // all possible import summaries for combining - let bookmarksSummaries: [DataImportViewModel.DataTypeImportResult?] = [ + var bookmarksSummaries: [DataImportViewModel.DataTypeImportResult?] { // bookmarks import didn‘t happen (or skipped) - nil, + [nil, // bookmarks import succeeded .init(.bookmarks, .success(.init(successful: 42, duplicate: 3, failed: 1))), // bookmarks import succeeded with no bookmarks imported @@ -1077,8 +1077,10 @@ import XCTest // bookmarks import failed with error .init(.bookmarks, .failure(Failure(.bookmarks, .dataCorrupted))), // bookmarks import failed with file not found - .init(.bookmarks, .failure(Failure(.bookmarks, .noData))), - ] + bookmarkSummaryNoData] + } + + private let bookmarkSummaryNoData: DataImportViewModel.DataTypeImportResult? = .init(.bookmarks, .failure(Failure(.bookmarks, .noData))) let bookmarksResults: [DataImportResult?] = [ .failure(Failure(.bookmarks, .dataCorrupted)), @@ -1087,9 +1089,9 @@ import XCTest nil, // skip ] - let passwordsSummaries: [DataImportViewModel.DataTypeImportResult?] = [ + var passwordsSummaries: [DataImportViewModel.DataTypeImportResult?] { // passwords import didn‘t happen (or skipped) - nil, + [nil, // passwords import succeeded .init(.passwords, .success(.init(successful: 99, duplicate: 4, failed: 2))), // passwords import succeeded with no passwords imported @@ -1097,8 +1099,10 @@ import XCTest // passwords import failed with error .init(.passwords, .failure(Failure(.passwords, .keychainError))), // passwords import failed with file not found - .init(.passwords, .failure(Failure(.passwords, .noData))), - ] + passwordSummaryNoData] + } + + private let passwordSummaryNoData: DataImportViewModel.DataTypeImportResult? = .init(.passwords, .failure(Failure(.passwords, .noData))) let passwordsResults: [DataImportResult?] = [ .failure(Failure(.passwords, .dataCorrupted)), @@ -1128,24 +1132,19 @@ import XCTest summary: [bookmarksSummary, passwordsSummary].compactMap { $0 }) var xctDescr = "bookmarksSummary: \(bookmarksSummary?.description ?? "") passwordsSummary: \(passwordsSummary?.description ?? "") result: \(result?.description ?? ".skip")" - // run File Import (or skip) + // run File Import let expectation: DataImportViewModel if let result { try await initiateImport(of: [.bookmarks], fromFile: .testHTML, resultingWith: [.bookmarks: result], xctDescr) // expect Final Summary expectation = DataImportViewModel(importSource: source, screen: .summary([.bookmarks], isFileImport: true), summary: [bookmarksSummary, passwordsSummary, .init(.bookmarks, result)].compactMap { $0 }) - } else { - XCTAssertEqual(model.actionButton, .skip) - model.performAction(.skip) - // expect Final Summary - expectation = DataImportViewModel(importSource: source, screen: .summary([.bookmarks, .passwords]), summary: [bookmarksSummary, passwordsSummary].compactMap { $0 }) - } - xctDescr = "\(source): " + xctDescr + xctDescr = "\(source): " + xctDescr - XCTAssertEqual(model.description, expectation.description, xctDescr) - XCTAssertEqual(model.actionButton, .done, xctDescr) - XCTAssertNil(model.secondaryButton, xctDescr) + XCTAssertEqual(model.description, expectation.description, xctDescr) + XCTAssertEqual(model.actionButton, .done, xctDescr) + XCTAssertNil(model.secondaryButton, xctDescr) + } } } } @@ -1192,8 +1191,9 @@ import XCTest func testWhenBrowsersBookmarksFileImportFailsAndNoPasswordsFileImportNeeded_feedbackShown() async throws { for source in Source.allCases where source.initialScreen == .profileAndDataTypesPicker && source.supportedDataTypes.contains(.passwords) { for bookmarksSummary in bookmarksSummaries - // initial bookmark import failed or ended with empty result - where bookmarksSummary?.result.isSuccess == false || (try? bookmarksSummary?.result.get())?.isEmpty == true { + // initial bookmark import failed and is not a `noData` error, or empty + where (bookmarksSummary?.result.isSuccess == false && bookmarkSummaryNoData != bookmarksSummary) + || (try? bookmarksSummary?.result.get())?.isEmpty == true { for passwordsSummary in passwordsSummaries // passwords successfully imported @@ -1231,6 +1231,31 @@ import XCTest } } + func testWhenBrowsersBookmarksImportFailsNoDataAndFileImportSkippedAndNoPasswordsFileImportNeeded_dialogDismissed() throws { + for source in Source.allCases where source.initialScreen == .profileAndDataTypesPicker && source.supportedDataTypes.contains(.passwords) { + + let bookmarksSummary = bookmarkSummaryNoData + + for passwordsSummary in passwordsSummaries + // passwords successfully imported + where (try? passwordsSummary?.result.get().isEmpty) == false { + + // setup model with pre-failed bookmarks import + setupModel(with: source, + profiles: [BrowserProfile.test, BrowserProfile.default, BrowserProfile.test2], + screen: .fileImport(dataType: .bookmarks, summary: []), + summary: [bookmarksSummary, passwordsSummary].compactMap { $0 }) + + let expectation = expectation(description: "dismissed") + model.performAction(for: .skip) { + expectation.fulfill() + } + + waitForExpectations(timeout: 0) + } + } + } + func testWhenBrowsersOnlySelectedBookmarksFileImportFails_feedbackShown() async throws { for source in Source.allCases where source.initialScreen == .profileAndDataTypesPicker { for bookmarksSummary in bookmarksSummaries @@ -1434,8 +1459,9 @@ import XCTest for bookmarksSummary in bookmarksSummaries { for passwordsSummary in passwordsSummaries - // passwords import failed or empty - where passwordsSummary?.result.isSuccess == false || (try? passwordsSummary?.result.get().isEmpty) == false { + // passwords import failed and is not a `noData` error, or empty + where (passwordsSummary?.result.isSuccess == false && passwordSummaryNoData != passwordsSummary) + || (try? passwordsSummary?.result.get().isEmpty) == false { for bookmarksFileImportSummary in bookmarksSummaries // if bookmarks result was failure - append successful bookmarks file import result @@ -1477,6 +1503,36 @@ import XCTest } } + func testWhenBrowsersPasswordsImportFailNoDataAndFileImportSkipped_dialogDismissed() throws { + for source in Source.allCases where source.initialScreen == .profileAndDataTypesPicker && source.supportedDataTypes.contains(.passwords) { + for bookmarksSummary in bookmarksSummaries { + + let passwordsSummary = passwordSummaryNoData + + for bookmarksFileImportSummary in bookmarksSummaries + // if bookmarks result was failure - append successful bookmarks file import result + where (bookmarksSummary?.result.isSuccess == false && bookmarksFileImportSummary?.result.isSuccess == true) + // if bookmarks file import summary was successful and non empty - don‘t append bookmarks file import result + // or if bookmarks file import was empty - and bookmarks file import skipped + || (bookmarksSummary?.result.isSuccess == true && bookmarksFileImportSummary == nil) { + + // setup model with pre-failed passwords import + setupModel(with: source, + profiles: [BrowserProfile.test, BrowserProfile.default, BrowserProfile.test2], + screen: .fileImport(dataType: .passwords, summary: []), + summary: [bookmarksSummary, passwordsSummary, bookmarksFileImportSummary].compactMap { $0 }) + + let expectation = expectation(description: "dismissed") + model.performAction(for: .skip) { + expectation.fulfill() + } + + waitForExpectations(timeout: 0) + } + } + } + } + func testWhenBrowsersOnlySelectedPasswordsFileImportFails_feedbackShown() async throws { for source in Source.allCases where source.initialScreen == .profileAndDataTypesPicker && source.supportedDataTypes.contains(.passwords) { for passwordsSummary in passwordsSummaries diff --git a/UnitTests/DataImport/FirefoxBookmarksReaderTests.swift b/UnitTests/DataImport/FirefoxBookmarksReaderTests.swift index 0a4ffdc2ee..f175f59f1b 100644 --- a/UnitTests/DataImport/FirefoxBookmarksReaderTests.swift +++ b/UnitTests/DataImport/FirefoxBookmarksReaderTests.swift @@ -39,9 +39,26 @@ class FirefoxBookmarksReaderTests: XCTestCase { }), true) } + func testFileNotFoundReturnsFailureWithDbOpenError() { + // Given + let bookmarksReader = FirefoxBookmarksReader(firefoxDataDirectoryURL: invalidResourceURL()) + let expected: DataImportResult = .failure(FirefoxBookmarksReader.ImportError(type: .couldNotFindBookmarksFile, underlyingError: nil)) + + // When + let result = bookmarksReader.readBookmarks() + + // Then + XCTAssertEqual(expected, result) + } + private func resourceURL() -> URL { let bundle = Bundle(for: FirefoxBookmarksReaderTests.self) return bundle.resourceURL!.appendingPathComponent("DataImportResources/TestFirefoxData") } + private func invalidResourceURL() -> URL { + let bundle = Bundle(for: FirefoxBookmarksReaderTests.self) + return bundle.resourceURL!.appendingPathComponent("Nothing/Here") + } + } diff --git a/UnitTests/DataImport/FirefoxLoginReaderTests.swift b/UnitTests/DataImport/FirefoxLoginReaderTests.swift index 83abe67b66..cdbaa31662 100644 --- a/UnitTests/DataImport/FirefoxLoginReaderTests.swift +++ b/UnitTests/DataImport/FirefoxLoginReaderTests.swift @@ -132,7 +132,7 @@ class FirefoxLoginReaderTests: XCTestCase { switch result { case .failure(let error as FirefoxLoginReader.ImportError): - XCTAssertEqual(error.type, .couldNotDetermineFormat) + XCTAssertEqual(error.type, .couldNotFindKeyDB) default: XCTFail("Received unexpected \(result)") } @@ -274,6 +274,26 @@ class FirefoxLoginReaderTests: XCTestCase { } } + func testWhenImportingLogins_AndNoKeysDBExists_ThenImportFailsWithNoDBError() throws { + // Given + let logins = resourcesURLWithoutPassword().appendingPathComponent("logins.json") + + let structure = FileSystem(rootDirectoryName: rootDirectoryName) { + File("logins.json", contents: .copy(logins)) + } + + try structure.writeToTemporaryDirectory() + let profileDirectoryURL = FileManager.default.temporaryDirectory.appendingPathComponent(rootDirectoryName) + + let firefoxLoginReader = FirefoxLoginReader(firefoxProfileURL: profileDirectoryURL) + + // When + let result = firefoxLoginReader.readLogins(dataFormat: nil) + + // Then + XCTAssertEqual(result, .failure(FirefoxLoginReader.ImportError(type: .couldNotFindKeyDB, underlyingError: nil))) + } + private func resourcesURLWithPassword() -> URL { let bundle = Bundle(for: FirefoxLoginReaderTests.self) return bundle.resourceURL!.appendingPathComponent("DataImportResources/TestFirefoxData/Primary Password")