Skip to content

Commit

Permalink
Sync: Fire accountRemoved KV store reason when no account (#1189)
Browse files Browse the repository at this point in the history
<!--
Note: This checklist is a reminder of our shared engineering
expectations.
-->

Please review the release process for BrowserServicesKit
[here](https://app.asana.com/0/1200194497630846/1200837094583426).

**Required**:

Task/Issue URL:
https://app.asana.com/0/1202926619870900/1209014391063056/f
iOS PR: duckduckgo/iOS#3875
macOS PR: duckduckgo/macos-browser#3782
What kind of version bump will this require?: Minor

**Description**:

As part of [✓ Send pixels for account removal + decoding
issues](https://app.asana.com/0/1199230911884351/1208723035104886/f),
pixels were added indicating what the reason for the Sync account being
removed from the Keychain was.

However, the pixel
`sync_account_removed_reason_not-set-on-key-value-store` is being sent
even when there was no account stored in the keychain in the first
place. This could be obscuring a problem.

To make this Pixel more useful, we should update it so that it's only
sent when there was an account stored in the keychain.

**Steps to test this PR**:
Hard to test the pixel itself as reporting on a broken state, but just
do a quick smoke test of Sync on both platforms.

<!--
Before submitting a PR, please ensure you have tested the combinations
you expect the reviewer to test, then delete configurations you *know*
do not need explicit testing.

Using a simulator where a physical device is unavailable is acceptable.
-->

**OS Testing**:

* [ ] iOS 14
* [ ] iOS 15
* [ ] iOS 16
* [ ] macOS 10.15
* [ ] macOS 11
* [ ] macOS 12

---
###### Internal references:
[Software Engineering
Expectations](https://app.asana.com/0/59792373528535/199064865822552)
[Technical Design
Template](https://app.asana.com/0/59792373528535/184709971311943)
  • Loading branch information
graeme authored Jan 27, 2025
1 parent 86ba43b commit 5a37e3a
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion Sources/DDGSync/DDGSync.swift
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,10 @@ public class DDGSync: DDGSyncing {

let syncEnabled = dependencies.keyValueStore.object(forKey: Constants.syncEnabledKey) != nil
guard syncEnabled else {
if account != nil {
dependencies.errorEvents.fire(.accountRemoved(.syncEnabledNotSetOnKeyValueStore))
}
try? dependencies.secureStore.removeAccount()
dependencies.errorEvents.fire(.accountRemoved(.syncEnabledNotSetOnKeyValueStore))
authState = .inactive
return
}
Expand Down

0 comments on commit 5a37e3a

Please sign in to comment.