From 94e30ff62061c354f8f10dd00f88d5b6e441b765 Mon Sep 17 00:00:00 2001 From: Graeme Arthur Date: Mon, 11 Nov 2024 14:50:03 +0100 Subject: [PATCH] Send success pixel on successful data import (#3437) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task/Issue URL: https://app.asana.com/0/1203822806345703/1207768516988587/f **Description**: During [✓ Postmortem: Deduplicate passwords on import](https://app.asana.com/0/0/1207598056251596) it was mentioned that having insight on the rate of import success vs failure would be useful. We already have Pixels for failures but cannot calculate a success rate from that alone. This adds import success Pixels with source and make sure the failure Pixels and their source parameter are being sent properly. **Steps to test this PR**: 1. Run this with the debugger attached 2. Perform password / bookmark / favicon imports from all the import sources (or at least a sensible subset) 3. Check that, on successful import, the pixel: m_mac_data-import-succeeded_{action}_{source} is sent **Definition of Done**: * [ ] Does this PR satisfy our [Definition of Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)? --- ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) [Pull Request Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f) --- .../Bookmarks/Safari/SafariDataImporter.swift | 4 +++- .../Logins/Chromium/ChromiumDataImporter.swift | 5 +++-- .../Logins/Firefox/FirefoxDataImporter.swift | 4 +++- .../DataImport/Model/DataImportViewModel.swift | 4 +++- DuckDuckGo/Statistics/GeneralPixel.swift | 12 ++++++++++++ 5 files changed, 24 insertions(+), 5 deletions(-) diff --git a/DuckDuckGo/DataImport/Bookmarks/Safari/SafariDataImporter.swift b/DuckDuckGo/DataImport/Bookmarks/Safari/SafariDataImporter.swift index 0f3daaef5d..ddf719b5f1 100644 --- a/DuckDuckGo/DataImport/Bookmarks/Safari/SafariDataImporter.swift +++ b/DuckDuckGo/DataImport/Bookmarks/Safari/SafariDataImporter.swift @@ -96,6 +96,7 @@ final class SafariDataImporter: DataImporter { private func importFavicons(from dataDirectoryURL: URL) async { let faviconsReader = SafariFaviconsReader(safariDataDirectoryURL: dataDirectoryURL) let faviconsResult = faviconsReader.readFavicons() + let sourceVersion = profile.installedAppsMajorVersionDescription() switch faviconsResult { case .success(let faviconsByURL): @@ -112,9 +113,10 @@ final class SafariDataImporter: DataImporter { result[pageURL] = favicons } await faviconManager.handleFaviconsByDocumentUrl(faviconsByDocument) + PixelKit.fire(GeneralPixel.dataImportSucceeded(action: .favicons, source: source, sourceVersion: sourceVersion)) case .failure(let error): - PixelKit.fire(GeneralPixel.dataImportFailed(source: source, sourceVersion: profile.installedAppsMajorVersionDescription(), error: error)) + PixelKit.fire(GeneralPixel.dataImportFailed(source: source, sourceVersion: sourceVersion, error: error)) } } diff --git a/DuckDuckGo/DataImport/Logins/Chromium/ChromiumDataImporter.swift b/DuckDuckGo/DataImport/Logins/Chromium/ChromiumDataImporter.swift index 1f920dd809..8db3ac1878 100644 --- a/DuckDuckGo/DataImport/Logins/Chromium/ChromiumDataImporter.swift +++ b/DuckDuckGo/DataImport/Logins/Chromium/ChromiumDataImporter.swift @@ -122,6 +122,7 @@ internal class ChromiumDataImporter: DataImporter { private func importFavicons() async { let faviconsReader = ChromiumFaviconsReader(chromiumDataDirectoryURL: profile.profileURL) let faviconsResult = faviconsReader.readFavicons() + let sourceVersion = profile.installedAppsMajorVersionDescription() switch faviconsResult { case .success(let faviconsByURL): @@ -138,9 +139,9 @@ internal class ChromiumDataImporter: DataImporter { result[pageURL] = favicons } await faviconManager.handleFaviconsByDocumentUrl(faviconsByDocument) - + PixelKit.fire(GeneralPixel.dataImportSucceeded(action: .favicons, source: source, sourceVersion: sourceVersion)) case .failure(let error): - PixelKit.fire(GeneralPixel.dataImportFailed(source: source, sourceVersion: profile.installedAppsMajorVersionDescription(), error: error)) + PixelKit.fire(GeneralPixel.dataImportFailed(source: source, sourceVersion: sourceVersion, error: error)) } } diff --git a/DuckDuckGo/DataImport/Logins/Firefox/FirefoxDataImporter.swift b/DuckDuckGo/DataImport/Logins/Firefox/FirefoxDataImporter.swift index 5eb7ca6353..ed42e8740e 100644 --- a/DuckDuckGo/DataImport/Logins/Firefox/FirefoxDataImporter.swift +++ b/DuckDuckGo/DataImport/Logins/Firefox/FirefoxDataImporter.swift @@ -122,6 +122,7 @@ internal class FirefoxDataImporter: DataImporter { private func importFavicons() async { let faviconsReader = FirefoxFaviconsReader(firefoxDataDirectoryURL: profile.profileURL) let faviconsResult = faviconsReader.readFavicons() + let sourceVersion = profile.installedAppsMajorVersionDescription() switch faviconsResult { case .success(let faviconsByURL): @@ -138,9 +139,10 @@ internal class FirefoxDataImporter: DataImporter { result[pageURL] = favicons } await faviconManager.handleFaviconsByDocumentUrl(faviconsByDocument) + PixelKit.fire(GeneralPixel.dataImportSucceeded(action: .favicons, source: source, sourceVersion: sourceVersion)) case .failure(let error): - PixelKit.fire(GeneralPixel.dataImportFailed(source: source, sourceVersion: profile.installedAppsMajorVersionDescription(), error: error)) + PixelKit.fire(GeneralPixel.dataImportFailed(source: source, sourceVersion: sourceVersion, error: error)) } } diff --git a/DuckDuckGo/DataImport/Model/DataImportViewModel.swift b/DuckDuckGo/DataImport/Model/DataImportViewModel.swift index 3660f02777..254295f6d5 100644 --- a/DuckDuckGo/DataImport/Model/DataImportViewModel.swift +++ b/DuckDuckGo/DataImport/Model/DataImportViewModel.swift @@ -245,12 +245,14 @@ struct DataImportViewModel { var nextScreen: Screen? // merge new import results into the model import summary keeping the original DataType sorting order for (dataType, result) in DataType.allCases.compactMap({ dataType in summary[dataType].map { (dataType, $0) } }) { + let sourceVersion = importSource.installedAppsMajorVersionDescription(selectedProfile: selectedProfile) switch result { case .success(let dataTypeSummary): // if a data type can‘t be imported (Yandex/Passwords) - switch to its file import displaying successful import results if dataTypeSummary.isEmpty, !(screen.isFileImport && screen.fileImportDataType == dataType), nextScreen == nil { nextScreen = .fileImport(dataType: dataType, summary: Set(summary.filter({ $0.value.isSuccess }).keys)) } + PixelKit.fire(GeneralPixel.dataImportSucceeded(action: .init(dataType), source: importSource, sourceVersion: sourceVersion)) case .failure(let error): // successful imports are appended above self.summary.append( .init(dataType, result) ) @@ -260,7 +262,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)) } - PixelKit.fire(GeneralPixel.dataImportFailed(source: importSource, sourceVersion: importSource.installedAppsMajorVersionDescription(selectedProfile: selectedProfile), error: error)) + PixelKit.fire(GeneralPixel.dataImportFailed(source: importSource, sourceVersion: sourceVersion, error: error)) } } diff --git a/DuckDuckGo/Statistics/GeneralPixel.swift b/DuckDuckGo/Statistics/GeneralPixel.swift index d7861dd042..2f5a897199 100644 --- a/DuckDuckGo/Statistics/GeneralPixel.swift +++ b/DuckDuckGo/Statistics/GeneralPixel.swift @@ -37,6 +37,7 @@ enum GeneralPixel: PixelKitEventV2 { case dailyOsVersionCounter case dataImportFailed(source: DataImport.Source, sourceVersion: String?, error: any DataImportError) + case dataImportSucceeded(action: DataImportAction, source: DataImport.Source, sourceVersion: String?) case formAutofilled(kind: FormAutofillKind) case autofillItemSaved(kind: FormAutofillKind) @@ -474,6 +475,9 @@ enum GeneralPixel: PixelKitEventV2 { case .dataImportFailed(source: let source, sourceVersion: _, error: let error): return "m_mac_data-import-failed_\(error.action)_\(source)" + case .dataImportSucceeded(action: let action, source: let source, sourceVersion: _): + return "m_mac_data-import-succeeded_\(action)_\(source)" + case .formAutofilled(kind: let kind): return "m_mac_autofill_\(kind)" @@ -1134,6 +1138,14 @@ enum GeneralPixel: PixelKitEventV2 { } return params + case .dataImportSucceeded(action: _, source: _, sourceVersion: let version): + var params = [String: String]() + + if let version { + params[PixelKit.Parameters.sourceBrowserVersion] = version + } + return params + case .launchInitial(let cohort): return [PixelKit.Parameters.experimentCohort: cohort]