Page MenuHomePhabricator

[native] Update device list on QR code scaned
ClosedPublic

Authored by bartek on Mar 11 2024, 6:17 AM.
Tags
None
Referenced Files
F3396126: D11294.diff
Sun, Dec 1, 10:34 AM
Unknown Object (File)
Fri, Nov 29, 12:22 PM
Unknown Object (File)
Fri, Nov 29, 7:56 AM
Unknown Object (File)
Sun, Nov 10, 2:03 PM
Unknown Object (File)
Oct 27 2024, 8:20 AM
Unknown Object (File)
Oct 22 2024, 3:04 PM
Unknown Object (File)
Oct 22 2024, 3:04 PM
Unknown Object (File)
Oct 22 2024, 3:04 PM
Subscribers

Details

Summary

Part of QR code auth flow - primary device performs device list update after the QR code is scanned. Then it notifies the secondary device that it can continue.

Depends on D11293

Test Plan

Ran the app on both simulator and physical phone - scanned QR code and made sure that Identity Service receives updated device list (checked the DDB table)

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek held this revision as a draft.
bartek edited the test plan for this revision. (Show Details)
bartek published this revision for review.Mar 12 2024, 2:19 AM
native/profile/secondary-device-qr-code-scanner.react.js
56 ↗(On Diff #38013)

Should we add an invariant above this line that deviceLists.length > 0? Otherwise the error may not be very informative

70 ↗(On Diff #38013)

It doesn't affect like the return value is used in this diff. Is it really necessary?

kamil added inline comments.
native/profile/secondary-device-qr-code-scanner.react.js
40 ↗(On Diff #38013)

I think I saw this invariant in our codebase right after React.useContext but I might be wrong

62–73 ↗(On Diff #38013)
111 ↗(On Diff #38013)

I think this make is easier to understand

113 ↗(On Diff #38013)

Why not make onConnect async function?

This revision is now accepted and ready to land.Mar 13 2024, 5:36 AM
native/profile/secondary-device-qr-code-scanner.react.js
40 ↗(On Diff #38013)

Yeah it can be moved - we'll see the error sooner if it ever happens

56 ↗(On Diff #38013)

Good call

70 ↗(On Diff #38013)

It was mostly needed for debugging, to see if the device list was actually updated or not. I can get rid of the return value for now