[native] Add calling setDeviceID from js
ClosedPublic
Actions

Authored by inka on Oct 20 2022, 10:01 AM.

Details

Reviewers

marcin
tomek

Commits

rCOMMea7addf88f6f: [native] Add calling setDeviceID from js

Summary

Device id has to be set whenerver a user is logged in. It has to be removed when a user logs out/deletes accout.
With this approach a new id is generated when a new database gets created, or app launches and there is no id in the database.
This translates to these events:

user logs out/deletes their account
app starts/reloads and there is no id in the database for some reason
SQLite database gets cleared in response to currentUserInfo.anonymous changing to true or currentUserInfo.id changing to a different value for other reasons

So the id is always present in the database, apart from very short periods of time after database creation.

Test Plan

Disable database encryption.
See that when app launches the id gets written to the database (using breakpoints and looking on the database). See that when user logs out/deletes their accout the id changes.
Run the app whithout these changes, add these changes to the code, reload the app and see that the id gets created even if a user has been logged in before the changes to the code (this imitates
what would happen for users updating their app versions when this code gets pushed to production).

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

inka created this revision.Oct 20 2022, 10:01 AM

Herald added subscribers: • abosh, atul, ashoat. · View Herald TranscriptOct 20 2022, 10:01 AM

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 20 2022, 10:11 AM

Harbormaster failed remote builds in B12940: Diff 17741!

CI fails because we haven't yet made rust work on android on CI. This is tracked in ENG-1852

This diff looks good provided changes requested in D5341 are introduced. Once D5341 is acted upon I will return back to this differential and probably accept it. In the meantime - @inka could you please add this differential to the stack of D5341 so that it is easily accessible from it?

inka added a parent revision: D5341: [native] Add JSI functions for creating/setting and fetching device ID.Oct 21 2022, 3:33 AM

As in D5341 - please make sure you test it on both Android and iOS physical devices before landing if you haven't done it already.

tomek requested changes to this revision.Oct 24 2022, 2:42 AM

tomek added inline comments.

native/data/sensitive-data-cleaner.react.js
9 ↗	(On Diff #17741)	This component now has more responsibility than just cleaning. We should somehow communicate that in its name, e.g. `SensitiveDataHandler`?
26–29 ↗	(On Diff #17741)	It is important to set this value before we set the current user id. In current approach, if the app is closed after current user id is set, but before device id is set, it would end up with a state where we have db with old device id and no way to detect that.

This revision now requires changes to proceed.Oct 24 2022, 2:42 AM

inka added inline comments.Oct 27 2022, 2:18 AM

native/data/sensitive-data-cleaner.react.js
26–29 ↗	(On Diff #17741)	The id is only set when there is no id in the database (`if (!databaseDeviceID)`) so I don't think this is possible. The id is set when the database gets created, and then when user logs in it doesn't change - just like on web. On the other hand when user logs out the database gets removed entirely, so current user id and device id are both removed or none of them is. We could end up with app having current user id set and no device id (in the case that app closes between calling `setCurrentUseriD` and `setDeviceID`), but then when user opens the app again, this effect would be re-run, and the device id would get set.

Rename SensitiveDataCleaner to SensitiveDataHandler

Harbormaster completed remote builds in B13063: Diff 17938.Oct 27 2022, 3:55 AM

tomek accepted this revision.Oct 27 2022, 5:33 AM

tomek added inline comments.

native/data/sensitive-data-cleaner.react.js
26–29 ↗	(On Diff #17741)	You're right! Sorry for the confusion.

This revision is now accepted and ready to land.Oct 27 2022, 5:33 AM

Fix calling SensitiveDataHandler twice in a row when logging out

When logging out currentLoggedInUserID would first change to undefined as state.currentUserInfo would change to null, but then state.currentUserInfo would change to some value having state.currentUserInfo.anonymous = true, so currentLoggedInUserID would change to null. This caused the effect hook to be run twice and sometimes this resulted in device id being set twice. I don't think we need to call this hook twice in a row when logging out.