Page MenuHomePhabricator

[native] Emit a token and user ID after setting the values
ClosedPublic

Authored by tomek on Jan 29 2024, 5:42 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 16, 6:49 PM
Unknown Object (File)
Sat, Nov 16, 6:49 PM
Unknown Object (File)
Sat, Nov 16, 6:48 PM
Unknown Object (File)
Sat, Nov 16, 6:48 PM
Unknown Object (File)
Sun, Nov 10, 1:04 PM
Unknown Object (File)
Tue, Nov 5, 8:41 PM
Unknown Object (File)
Tue, Nov 5, 7:21 PM
Unknown Object (File)
Tue, Nov 5, 5:38 PM
Subscribers

Details

Summary

The emitter currently isn't fed with new values. The new values could be provided in multiple ways: from JS, from C++ CommRustModule, or from Rust.

The C++ layer exposes an API that allows setting the token, but we need to make sure that emitter is also aware about this change. Currently, the only place that consumes the events will be AccessTokenHandler, which then updates Redux, but directly updating Redux when calling CommRustModule won't be maintainable - there could be more emitter consumers in the future.

We can also consider calling emitter directly from Rust, but it also has some maintainability cost because the JS logic can decide how to handle the result.

So overall, this solution makes data flow simple and achieves what is needed. There are some other options, but introducing them should also result in removing setCommServicesAuthMetadata from JS.

Test Plan

Tested in combination with the next diff. Call commCoreModule.setCommServicesAuthMetadata and check if the token in Redux is updated. Add a timeout with a call to commCoreModule.setCommServicesAuthMetadata and check if it results in the token being updated.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek requested review of this revision.Jan 29 2024, 6:01 AM

[...] So overall, this solution makes data flow simple and achieves what is needed.

Agree with that, this solution looks best to me.

This revision is now accepted and ready to land.Jan 29 2024, 7:44 AM