Skip to content

Commit

Permalink
Send success pixel on successful data import (#3437)
Browse files Browse the repository at this point in the history
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

<!--
Tagging instructions
If this PR isn't ready to be merged for whatever reason it should be
marked with the `DO NOT MERGE` label (particularly if it's a draft)
If it's pending Product Review/PFR, please add the `Pending Product
Review` label.

If at any point it isn't actively being worked on/ready for
review/otherwise moving forward (besides the above PR/PFR exception)
strongly consider closing it (or not opening it in the first place). If
you decide not to close it, make sure it's labelled to make it clear the
PRs state and comment with more information.
-->

**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)
  • Loading branch information
graeme committed Nov 11, 2024
1 parent d56d266 commit 94e30ff
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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))
}
}

Expand Down
4 changes: 3 additions & 1 deletion DuckDuckGo/DataImport/Model/DataImportViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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) )
Expand All @@ -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))
}
}

Expand Down
12 changes: 12 additions & 0 deletions DuckDuckGo/Statistics/GeneralPixel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)"

Expand Down Expand Up @@ -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]

Expand Down

0 comments on commit 94e30ff

Please sign in to comment.