Page MenuHomePhabricator

Ensure SQLiteContextProvider doesn't query SQLite and modify redux if user is logged out.
ClosedPublic

Authored by marcin on Nov 8 2022, 7:09 AM.
Tags
None
Referenced Files
F3380246: D5555.diff
Wed, Nov 27, 10:26 PM
Unknown Object (File)
Sun, Nov 24, 8:40 AM
Unknown Object (File)
Sun, Nov 24, 7:43 AM
Unknown Object (File)
Sun, Nov 24, 7:39 AM
Unknown Object (File)
Fri, Nov 22, 7:59 AM
Unknown Object (File)
Fri, Nov 22, 7:59 AM
Unknown Object (File)
Fri, Nov 22, 7:59 AM
Unknown Object (File)
Fri, Nov 22, 7:59 AM
Subscribers

Details

Summary

This differential ensures no action is dispatched to modify MessageStore or ThreadsStore, when user is logged out.

Test Plan

Build the app, log-out, kill the app and start it again with flipper. Ensure no MessageStore or ThreadsStore action is dispatched.

Diff Detail

Repository
rCOMM Comm
Branch
marcin/eng-2130
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

marcin requested review of this revision.Nov 8 2022, 7:23 AM

Short-circuiting review here... I reviewed this approach extensively with @marcin during our 1:1 yesterday and in the comments of ENG-2133, so I think it's good to land.

@marcin, please make sure to update the code above to use isLoggedIn before landing!

native/data/sqlite-context-provider.js
34–36 ↗(On Diff #18197)

I think we should use isLoggedIn here. It also checks dataLoaded, which is important to make sure this immediately responds to a user log out or account deletion. currentUserInfo might take a while to update (waits for server to provide the new one), but dataLoaded updates immediately.

42 ↗(On Diff #18197)

We don't need to look at the whole userID... since we're only checking if the user is logged in or not, we can pass in a boolean, which will avoid recomputation in the hypothetical case of userID changing from one truthy string to another truthy string

This revision is now accepted and ready to land.Nov 8 2022, 7:58 AM

Use isLoggedIn selector to detect if user is logged out