add logOut to the IdentityServiceClient type and implementations. use the client logOut method in useLogOut
Depends on D11334
Details
Details
successfully logged out of identity on web and native. confirmed that access token and device were removed from ddb
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| lib/actions/user-actions.js | ||
|---|---|---|
| 87 ↗ | (On Diff #38108) | Should this be renamed to keyserverLogOut? |
| 87 ↗ | (On Diff #38108) | Maybe add a newline above here to avoid the implication that logOutActionTypes is tied to only this function |
| 110 ↗ | (On Diff #38108) | This seems broken... callKeyserverEndpoint appears to return { +[keyserverID: string]: any }. But we're assigning that to currentUserInfo here, which doesn't match that type. I recognize this diff doesn't change that stuff, so won't block the review here. But would love to get @inka's perspective on this, and to understand if there's a task tracking fixing this. (I suspect we should probably prioritize that task... it seems like this code is currently broken.) |
Comment Actions
after rebasing i started getting a flow error. updated web/grpc/identity-service-client-proxy.js to resolve it, but not really sure what this code does... going to add @michal as a reviewer
Comment Actions
Looks good on the identity-service-client-proxy part.
Short explanation is -> we are moving the identity client to worker so that multiple tabs can synchronize the identity calls between them and not conflict. identity-service-client-wrapper.js will run on web worker and identity-service-client-proxy.js will run in the tab JS context and pass the method calls to the worker (we didn't "flip the switch" yet so for now the identity-service-client-wrapper.js is still running in the tab JS context though).
| lib/actions/user-actions.js | ||
|---|---|---|
| 110 ↗ | (On Diff #38108) | This is a ternary operator - if response is truthy, then I assign loggedOutUserInfo (a constant) to currentUserInfo. Otherwise I assign null to currentUserInfo. |
| lib/actions/user-actions.js | ||
|---|---|---|
| 129 ↗ | (On Diff #38150) | I don't think it makes sense for this callback to take a list of keyserver ids anymore? I don't think we ever want to logout of identity and only a subset of keyservers... |
| 140 ↗ | (On Diff #38150) | Is this typically is enough time for identity logout to succeed? Of is this error silently thrown most of the time? |
| lib/actions/user-actions.js | ||
|---|---|---|
| 140 ↗ | (On Diff #38150) | i tested on staging and it typically succeeded in under 500 ms. the error was silently thrown a few times, though |
| lib/actions/user-actions.js | ||
|---|---|---|
| 110 ↗ | (On Diff #38108) | Sorry, I'm not sure what I was thinking! |
| lib/actions/user-actions.js | ||
|---|---|---|
| 129 ↗ | (On Diff #38150) | it it's alright with you i'd like to address this in a follow up diff. will link a linear task here before landing |
| lib/actions/user-actions.js | ||
|---|---|---|
| 129 ↗ | (On Diff #38150) | |