Page MenuHomePhabricator

[native] update action types and reducer for setting comm access token
ClosedPublic

Authored by varun on Sep 8 2023, 12:54 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 22, 6:34 PM
Unknown Object (File)
Fri, Nov 22, 6:25 PM
Unknown Object (File)
Fri, Nov 22, 6:25 PM
Unknown Object (File)
Fri, Nov 22, 6:23 PM
Unknown Object (File)
Fri, Nov 22, 2:47 PM
Unknown Object (File)
Sun, Nov 10, 12:36 AM
Unknown Object (File)
Wed, Nov 6, 4:27 AM
Unknown Object (File)
Wed, Nov 6, 4:27 AM
Subscribers

Details

Summary

Added a new action type and updated the reducer so we can set the token string in redux state

Depends on D9113

Test Plan

how i tested the SET_ACCESS_TOKEN action: https://gist.github.com/vdhanan/c1a70adc6125268a8f9a085f0b1fcea2
tested the other cases in reduceServicesAccessToken by logging out and deleting account on iOS simulator and dispatching a SET_NEW_SESSION action manually from the redux devtools (confirmed that the token was set to null in redux state in all these cases)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun published this revision for review.Sep 8 2023, 12:58 PM
This revision is now accepted and ready to land.Sep 11 2023, 5:12 AM
native/redux/action-types.js
26 ↗(On Diff #30877)

Nit: Each type in this file ends with ActionType suffix so we should follow the convention here as well and rename this to setAccessTokenActionType.

ashoat requested changes to this revision.Sep 12 2023, 10:28 AM
  1. I think we should implement a separate reducer for this. We should expect that the access token will need to be cleared for a log out, account deletion, and session invalidation. For an example, see here
  2. I think the reducer should be in lib, since this action will be shared with web. I think it should be possible to submit this diff purely on lib, without touching native or web
  3. Please keep in mind that we will also need this info to be in CommSecureStore for @marcin's purposes, since he needs to be able to access it from native code (and NSE in particular)
This revision now requires changes to proceed.Sep 12 2023, 10:28 AM
native/redux/action-types.js
26 ↗(On Diff #30877)

Agree with this. Try to make sure you maintain consistency with the existing codebase... we can't rely on reviewers to always spot things like this

address feedback (CommSecureStore changes will come later, but wanted to acknowledge the comment)

Nice, forgot I had already done some of this work!

This revision is now accepted and ready to land.Sep 13 2023, 4:54 AM
This revision was landed with ongoing or failed builds.Sep 13 2023, 10:09 AM
This revision was automatically updated to reflect the committed changes.