Page MenuHomePhabricator

[web] Add device ID to redux
ClosedPublic

Authored by inka on Aug 18 2022, 4:30 AM.
Tags
None
Referenced Files
F3351638: D4868.diff
Sat, Nov 23, 2:56 AM
Unknown Object (File)
Mon, Nov 11, 8:34 AM
Unknown Object (File)
Tue, Nov 5, 11:32 AM
Unknown Object (File)
Tue, Nov 5, 11:32 AM
Unknown Object (File)
Tue, Nov 5, 11:20 AM
Unknown Object (File)
Tue, Nov 5, 11:04 AM
Unknown Object (File)
Tue, Oct 29, 5:15 PM
Unknown Object (File)
Thu, Oct 24, 12:34 PM
Subscribers

Details

Summary

Add device ID to redux on web so it can be persisted.

Test Plan

Check with redux tools that the ID is indeed present in the redux store.

Diff Detail

Repository
rCOMM Comm
Branch
inka/redux_device_id
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

inka requested review of this revision.Aug 18 2022, 4:41 AM

I gave this a quick review since we talked about it in our 1:1 earlier this week... it looks good to me :)

web/redux/device-id-reducer.js
7–9

I wonder, should this be somewhere in lib? Will we ever need to use it somewhere else?

(Maybe the answer is "no"... we only need it in C++ / Rust on native)

This revision is now accepted and ready to land.Aug 18 2022, 6:52 AM
web/redux/device-id-reducer.js
7–9

I thought about this and my conclusion was exactly that - since on natvie we only use it in rust/cpp it'd probably be unnecessarily complicated to share this constant between the languages.
It is also used in tunnelbroker (Constants.h), but in cpp...
Unless I put it in an XML file or something like that ?

This revision now requires review to proceed.Aug 18 2022, 7:23 AM
web/redux/device-id-reducer.js
7–9

Makes sense! I wouldn't worry about an XML file, seems like too much work...

Maybe just add a comment here to make sure things are kept "in sync", eg. like this other case:

services/tunnelbroker/src/Constants.h:// DEVICEID_CHAR_LENGTH has to be kept in sync with deviceIDCharLength
web/utils/device-id.js:// deviceIDCharLength has to be kept in sync with DEVICEID_CHAR_LENGTH
This revision is now accepted and ready to land.Aug 18 2022, 10:47 AM
web/redux/device-id-reducer.js
7–9

I actually had this defined in two places on web, so I created a new diff tidying it all up D4883