Page MenuHomePhabricator

[keyserver][lib] Remove id from LoggedOutUserInfo
ClosedPublic

Authored by inka on Oct 31 2023, 12:57 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 19, 5:49 PM
Unknown Object (File)
Tue, Nov 12, 3:37 AM
Unknown Object (File)
Tue, Nov 12, 3:18 AM
Unknown Object (File)
Sun, Oct 27, 9:21 PM
Unknown Object (File)
Sun, Oct 27, 9:21 PM
Unknown Object (File)
Sun, Oct 27, 9:13 PM
Unknown Object (File)
Sep 29 2024, 3:38 AM
Unknown Object (File)
Sep 29 2024, 2:45 AM
Subscribers

Details

Summary

issue: https://linear.app/comm/issue/ENG-5281/refactor-logout-and-deleteaccount-actions
We need to remove the id field from LoggedOutUserInfo, because it doesn't make sense in multikeyserver world for the keyserver to set some user id for the logged out user. It wasn't really being used anyway - right after the app is freshly built, the whole currentUserInfo is null. I couldn't just make it null though, because the app was behaving incorrectly then (see the linear issue's comments)

Test Plan

Tested that it is possible to log out and back in. No errors show up.
Ran yarn flow-all.

TESTED OLD CLIENTS:

  1. Built the client on master, without this diff applied
  2. Ran metro, opened the app
  3. Killed metro
  4. Ran the keyserver on this branch, with this diff applied
  5. Logged out
  6. Logged in

No errors appeared on keyserver console, checked in redux devtools that logout and login actions were successful. The client didn't show any errors.
The currentUserInfo after logout was {anonymous: true}, but this didn't cause any problems - I clicked around on the login/register screens and no errors showed up. I was able to register a new user (I used the new registration flow). This is consistent with the theory this diff is based on - that the id wasn't begin used when not logged in

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

inka requested review of this revision.Oct 31 2023, 1:14 AM

Will this break logging out on older clients?

Will this break logging out on older clients?

Testing this should be added to this diff's test plan. I think it should be okay, since clients typically log out immediately rather than waiting for the server's response, but I'm not sure if there will be some weird side effects that cause the app to crash if the id is expected but missing.

I amended the test plan, it seems this doesn't break anything

This revision is now accepted and ready to land.Nov 7 2023, 2:12 AM