Page MenuHomePhabricator
Differential D10404

[identity] Reorder device list when updated
ClosedPublic
Actions

Authored by bartek on Dec 19 2023, 6:22 AM.
Tags
None
Referenced Files
F2153280: D10404.id35574.diff
Sun, Jun 30, 4:47 PM
Unknown Object (File)
Tue, Jun 25, 11:29 AM
Unknown Object (File)
Tue, Jun 25, 11:29 AM
Unknown Object (File)
Tue, Jun 25, 11:29 AM
Unknown Object (File)
Tue, Jun 25, 11:29 AM
Unknown Object (File)
Tue, Jun 25, 11:29 AM
Unknown Object (File)
Tue, Jun 25, 11:29 AM
Unknown Object (File)
Fri, Jun 14, 4:42 PM
View All 37 Files
Subscribers
ashoat
tomek

Details

Reviewers
varun
michal
Commits
rCOMMfd3843532984: [identity] Reorder device list when updated
Summary

Resolves ENG-5841. Each time we update the device list (login/register/logout), we should determine primary device and reorder the list accordingly.

Depends on D10400

Test Plan

Added unit tests for the reordering function. Also tested manually by:

  1. Register a user (mobile device)
  2. Log in the user on a "web" device
  3. Check database (or call GetDeviceListForUser RPC) - last device list entry has the following order (simplified for brevity): ["ios", "web"]

Now for web-only users:

  1. Register a user (mobile device)
  2. Call logout to remove the device
  3. Login with web - a log is displayed: "No valid primary device found..."
  4. Log in with mobile again. Check database - last device list entry has the following order: ["ios", "web"]. Mobile was added BEFORE web

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek created this revision.Dec 19 2023, 6:22 AM
bartek held this revision as a draft.
Herald added subscribers: tomek, ashoat. · View Herald TranscriptDec 19 2023, 6:22 AM
bartek published this revision for review.Dec 19 2023, 7:17 AM
Harbormaster completed remote builds in B25221: Diff 34841.Dec 19 2023, 7:36 AM
bartek added a child revision: D10420: [commtest] Rename create_device to register_user_device.Dec 21 2023, 2:09 AM
bartek added inline comments.Jan 9 2024, 12:50 AM
services/identity/src/database/device_list.rs
1152 ↗(On Diff #34841)
bartek added a child revision: D10422: [commtest] Add tests for Identity device list.Jan 9 2024, 5:50 AM
bartek removed a child revision: D10420: [commtest] Rename create_device to register_user_device.
bartek added inline comments.Jan 12 2024, 7:05 AM
services/identity/src/database/device_list.rs
1103–1106 ↗(On Diff #34841)

I could've used HashSet here because we only use map keys below

bartek updated this revision to Diff 35574.Jan 12 2024, 7:11 AM

Use HashSet

Harbormaster completed remote builds in B25725: Diff 35574.Jan 12 2024, 7:30 AM
michal accepted this revision.Jan 15 2024, 3:49 AM
michal added inline comments.
services/identity/src/database/device_list.rs
780–782 ↗(On Diff #35574)

Can we update the comment?

1060–1080 ↗(On Diff #35574)

Possibly another option would be to make two HashSets and compare them

This revision is now accepted and ready to land.Jan 15 2024, 3:49 AM
bartek updated this revision to Diff 35615.Jan 15 2024, 6:30 AM

Update comment, use HashSet difference

Harbormaster completed remote builds in B25763: Diff 35615.Jan 15 2024, 6:49 AM
Closed by commit rCOMMfd3843532984: [identity] Reorder device list when updated (authored by bartek). · Explain WhyJan 15 2024, 6:53 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMMfd3843532984: [identity] Reorder device list when updated.

Revision Contents
Changeset List

PathSize
services/
identity/
src/
database/
143 lines

Diff 35629

View Options

services/identity/src/database/device_list.rs

Loading...