[native] Skip onInitialAppLoad if usingCommServicesAccessToken
ClosedPublic
Actions

Authored by ashoat on Jan 8 2024, 6:17 AM.

Details

Reviewers

inka
varun
tomek

Commits

rCOMMcfb0033da3a5: [native] Skip onInitialAppLoad if usingCommServicesAccessToken

Summary

This handles part of ENG-5940 (appStartReduxLoggedInButInvalidCookie and appStartCookieLoggedInButInvalidRedux).

In the old format, currentUserInfo and the cookie from my keyserver should always be in sync. In the multi-keyserver world, currentUserInfo is sourced from Comm services while cookie is keyserver-specific, so there is no issue when the two are out-of-sync.

If a keyserver is logged out when we have a CSAT, the keyserver connection handlers will handle it by logged the user into the keyserver using the Olm auth responder (assuming there isn't a permanent connectionIssue).

We'll have a somewhat similar issue with currentUserInfo and the CSAT potentially being out-of-sync. This will be handled separately; will make a comment on ENG-5798 to discuss this further, but it looks like it's meant to be part of ENG-4332.

Test Plan

Not sure how to test this one. Flow checks out

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ashoat created this revision.Jan 8 2024, 6:17 AM

ashoat edited the summary of this revision. (Show Details)Jan 8 2024, 6:27 AM

Harbormaster returned this revision to the author for changes because remote builds failed.Jan 8 2024, 6:45 AM

Harbormaster failed remote builds in B25600: Diff 35369!

ashoat requested review of this revision.Jan 8 2024, 6:57 AM

ashoat added a child revision: D10576: [lib][native] Rename fetchNewCookieFromNativeCredentials to resolveKeyserverSessionInvalidation.Jan 8 2024, 7:25 AM

Should we consider making tasks for removing the old code when we are ready to deprecate not using CSAT?

This revision is now accepted and ready to land.Jan 9 2024, 1:36 AM

Should we consider making tasks for removing the old code when we are ready to deprecate not using CSAT?

My understanding is that you would be handling this as part of ENG-5798. From the description of ENG-5940:

Screenshot 2024-01-09 at 8.54.12 AM.png (538×1 px, 224 KB)

Let me know if I'm misinterpreting! Perhaps that is the wrong place in the task tree, and we should have some sort of supertask tracking all the things that will need to be removed.

In D10575#306130, @ashoat wrote:

Should we consider making tasks for removing the old code when we are ready to deprecate not using CSAT?

My understanding is that you would be handling this as part of ENG-5798. From the description of ENG-5940:

Let me know if I'm misinterpreting! Perhaps that is the wrong place in the task tree, and we should have some sort of supertask tracking all the things that will need to be removed.

Talked through this with @inka in our 1:1 earlier today. We decided that we would close ENG-5798 once this diff lands, and instead track deprecating usingCommServicesAccessToken in ENG-6415

This revision was landed with ongoing or failed builds.Jan 10 2024, 5:43 AM

Closed by commit rCOMMcfb0033da3a5: [native] Skip onInitialAppLoad if usingCommServicesAccessToken (authored by ashoat). · Explain Why

This revision was automatically updated to reflect the committed changes.

ashoat added a commit: rCOMMcfb0033da3a5: [native] Skip onInitialAppLoad if usingCommServicesAccessToken.