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
F3359342: D5555.id18250.diff
Sun, Nov 24, 8:40 AM
F3359181: D5555.id18197.diff
Sun, Nov 24, 7:43 AM
F3359174: D5555.id18214.diff
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
Unknown Object (File)
Fri, Nov 22, 7:52 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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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