Skip to content
This repository was archived by the owner on Feb 24, 2025. It is now read-only.

Conversation

@brindy
Copy link
Contributor

@brindy brindy commented May 17, 2023

Task/Issue URL: https://app.asana.com/0/0/1204469803916196/f
Tech Design URL:
CC:

Description:
Shows real connected device name.

Steps to test this PR:

  1. See sync device connected names BrowserServicesKit#357

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

brindy added 4 commits May 15, 2023 17:46
# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
Copy link
Collaborator

@ayoy ayoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, it seems it’s broken on both iOS and macOS when adding a third device. See the comment.

let knownDevices = Set(self.devices.map { $0.id })
let devices = try await syncService.login(recoveryKey, deviceName: device.name, deviceType: device.type)
mapDevices(devices)
let syncedDevices = self.devices.filter { !knownDevices.contains($0.id) && !$0.isCurrent }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There may be something wrong with this check, but if knownDevices really is a set of all previously known devices, that should all be fine.

Anyway, when adding a third device to sync, both iOS and macOS display either 2 devices or 0 devices in the “Device Synced!” popup. I tried with adding iOS to macOS and vice versa.

@ayoy ayoy assigned brindy and unassigned ayoy May 17, 2023
brindy added 3 commits May 17, 2023 17:30
# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
@brindy brindy assigned ayoy and unassigned brindy May 17, 2023
@brindy brindy requested a review from ayoy May 17, 2023 16:39
Copy link
Collaborator

@ayoy ayoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👏

@ayoy ayoy assigned brindy and unassigned ayoy May 17, 2023
@brindy brindy merged commit 26b86a8 into develop May 17, 2023
@brindy brindy deleted the brindy/sync-device-connected-names branch May 17, 2023 19:01
samsymons added a commit that referenced this pull request May 23, 2023
* upstream_develop:
  sync device connected names (#1205)
  Autofill/subdomain matching (#1122)
  add-windows-browser-download-link-privacy-config (#1198)
  Update metadata
  Fix creating DMG from release workflow (#1200)
  Unit tests to cover sensitive areas of Burner Window (#1169)
  [Hackdays] Share via QR Code (#1177)
  Fix sync connect flow  (#1195)
  Update to BSK adding new remote message image (#1194)
  Set version to 1.39.0
  Update embedded files
  Update old Dax asset (#1193)
  WebKit find in page (#1188)
  modify manage bookmarks button (#1175)
  sync update device model / poll for devices (#1191)
samsymons added a commit that referenced this pull request May 27, 2023
* release/1.42.0: (26 commits)
  Bump version to 1.42.0 (28)
  Update embedded files
  Network Protection (#1211)
  Update BSK with autofill 7.1.0 (#1225)
  Autofill password generation support for iOS (#1212)
  Sync Engine with support for syncing bookmarks (#1203)
  Bump BSK for messaging updates (#1213)
  add newTab pixel, fire pixels only for new users first time (#1219)
  Update tds endpoint (#1218)
  prevent devices from being fetch when sync not visible (#1214)
  License and contributor guidelines for open-sourcing (#1189)
  Add a unique Bitwarden decryption failure pixel (#1197)
  Bump version to 1.41.0 (27)
  Update embedded files
  Check that the source and target frame security origins are equal (#1207)
  Add activation points pixels (#1206)
  Add Login export links to settings and Autofill 3 dot menu (#1183)
  sync device connected names (#1205)
  Autofill/subdomain matching (#1122)
  add-windows-browser-download-link-privacy-config (#1198)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants