Page MenuHomePhabricator
Differential D11294

[native] Update device list on QR code scaned
ClosedPublic
Actions

Authored by bartek on Mar 11 2024, 6:17 AM.
Tags
None
Referenced Files
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
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
View All 78 Files
Subscribers
ashoat
tomek

Details

Reviewers
kamil
michal
Commits
rCOMM9342eac75a6f: [native] Update device list on QR code scaned
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 created this revision.Mar 11 2024, 6:17 AM
bartek held this revision as a draft.
Herald added subscribers: tomek, ashoat. · View Herald TranscriptMar 11 2024, 6:17 AM
bartek added a child revision: D11295: [native] Upload secondary device keys when added to list.Mar 11 2024, 6:18 AM
Harbormaster failed remote builds in B27424: Diff 37972!Mar 11 2024, 6:31 AM
bartek updated this revision to Diff 37985.Mar 11 2024, 6:48 AM

Rebase

Harbormaster failed remote builds in B27437: Diff 37985!Mar 11 2024, 7:03 AM
bartek edited the summary of this revision. (Show Details)Mar 11 2024, 7:09 AM
bartek edited the test plan for this revision. (Show Details)
bartek updated this revision to Diff 37991.Mar 11 2024, 7:20 AM

Rebase

Harbormaster failed remote builds in B27443: Diff 37991!Mar 11 2024, 7:33 AM
bartek updated this revision to Diff 38013.Mar 12 2024, 1:48 AM

Rebase

Harbormaster completed remote builds in B27464: Diff 38013.Mar 12 2024, 2:04 AM
bartek published this revision for review.Mar 12 2024, 2:19 AM
ashoat added inline comments.Mar 12 2024, 3:21 PM
native/profile/secondary-device-qr-code-scanner.react.js
56

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

70

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

kamil accepted this revision.Mar 13 2024, 5:36 AM
kamil added inline comments.
native/profile/secondary-device-qr-code-scanner.react.js
40

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

62–73
111

I think this make is easier to understand

113

Why not make onConnect async function?

This revision is now accepted and ready to land.Mar 13 2024, 5:36 AM
bartek added inline comments.Mar 18 2024, 4:55 AM
native/profile/secondary-device-qr-code-scanner.react.js
40

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

56

Good call

70

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

bartek updated this revision to Diff 38124.Mar 18 2024, 4:57 AM

Address review feedback

Harbormaster completed remote builds in B27553: Diff 38124.Mar 18 2024, 5:12 AM
Closed by commit rCOMM9342eac75a6f: [native] Update device list on QR code scaned (authored by bartek). · Explain WhyMar 21 2024, 4:29 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMM9342eac75a6f: [native] Update device list on QR code scaned.

Revision Contents
Changeset List

PathSize
native/
profile/
116 lines

Diff 38013

View Options

native/profile/secondary-device-qr-code-scanner.react.js

Loading...