Page MenuHomePhabricator

[lib][web][native] Refactor actions in user-actions.js pt.3 - login
ClosedPublic

Authored by inka on Oct 31 2023, 5:12 AM.
Tags
None
Referenced Files
F3539307: D9648.id32616.diff
Thu, Dec 26, 12:40 AM
F3535576: D9648.id.diff
Wed, Dec 25, 4:05 PM
Unknown Object (File)
Wed, Dec 25, 5:55 AM
Unknown Object (File)
Fri, Dec 20, 3:47 PM
Unknown Object (File)
Sat, Dec 14, 11:49 PM
Unknown Object (File)
Sat, Dec 14, 11:49 PM
Unknown Object (File)
Sat, Dec 14, 10:58 PM
Unknown Object (File)
Fri, Dec 6, 3:42 AM

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

inka requested review of this revision.Oct 31 2023, 5:38 AM
inka planned changes to this revision.Oct 31 2023, 6:06 AM

Fix call to login action in resolveInvalidatedCookie

lib/utils/action-utils.js
241–257 ↗(On Diff #32616)

resolveInvalidatedCookie used to call boundCallServerEndpoint defined right above. Now, that login action takes CallKeyserverEndpoint instead of CallServerEndpoint, we need to create a CallKeyserverEndpoint function and pass it to resolveInvalidatedCookie which calls login. But here, we are in the context of one keyserver (this function is called for one, specific keyserver) , so this CallKeyserverEndpoint is just a wrapper to be able to use the login action underneath. For reference see callKeyserverEndpoint in keyserver-call.js in D9217
Calling only one keyserver with login action may seem odd, but this actually makes sense, because one keyserver may return a session change, triggering the call to fetchNewCookieFromNativeCredentials which calls login underneath.

native/account/resolve-invalidated-cookie.js
47 ↗(On Diff #32616)

In this case, we know which keyserver we want to be called, so we pass it into the action to avoid unnecessary calls to identity service.

lib/utils/action-utils.js
273–274 ↗(On Diff #32616)

Why are we passing both of these in? Is the idea that we will have both of them going forward? Or are you going to remove boundCallServerEndpoint in a subsequent diff?

native/account/resolve-invalidated-cookie.js
36 ↗(On Diff #32616)

Will this get updated in a subsequent diff? Is there a task tracking it?

inka added inline comments.
lib/utils/action-utils.js
273–274 ↗(On Diff #32616)

boundCallServerEndpoint is passed in resolveInvalidatedCookie to getInitialNotificationsEncryptedMessage. In the codebase today the only used getInitialNotificationsEncryptedMessage is the function returned from useInitialNotificationsEncryptedMessage. This function uses callServerEndpoint to call getOlmSessionInitializationData action, that indeed takes CallServerEndpoint - it wasn't refactored as discussed here
Eventually boundCallServerEndpoint should be removed, once getOlmSessionInitializationData is refactored to call the identity service

native/account/resolve-invalidated-cookie.js
36 ↗(On Diff #32616)

Since getOlmSessionInitializationData will be refactored as part of the identity service work, this will have to be refactored as a result (since the only reason for passing here callServerEndpoint is that it is being passed down to getOlmSessionInitializationData action)
@varun do you need me to create any tasks for this, or is will this follow naturally from your other changes?

native/account/resolve-invalidated-cookie.js
36 ↗(On Diff #32616)

Please link a task before landing

native/account/resolve-invalidated-cookie.js
36 ↗(On Diff #32616)

would you mind creating a task under this parent? https://linear.app/comm/issue/ENG-3262/new-keyserver-endpoint-for-logging-in-using-identity-service-based

will probably get covered in my work but i don't want to miss it by mistake

native/account/resolve-invalidated-cookie.js
36 ↗(On Diff #32616)

Here is the task: ENG-5658

lib/utils/action-utils.js
243 ↗(On Diff #32616)

Can we change this Object?

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

Undo accidental changes

This revision was landed with ongoing or failed builds.Nov 7 2023, 6:35 AM
This revision was automatically updated to reflect the committed changes.