issue: https://linear.app/comm/issue/ENG-6635/have-ashoatkeyserverid-fetch-is-value-from-a-config
Replacing ashoatKeyserverID with authoritativeKeyserverID, which is fetched from config, based on a platform.
Because keyserver depends on web, authoritative-keyserver.js is run on the keyserver. On the keyserver config is not set, so we have to work aroud that. For now the keyserver shouldn't ever use authoritativeKeyserverID. It could in the future, but it's outside of the scope of this task.
jest tests also need authoritativeKeyserverID, and they also don't register configs. We use a default value for them.
Details
Tested that web and native can be run and no erros show up, tested that no errros show up on keyserver console.
Run jest tests and teset that they work and pass
Tested that it is possible to send messages, recieve them, create entries, fetch entries, log out, log in, and no errors show up
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
lib/socket/activity-handler.react.js | ||
---|---|---|
18 ↗ | (On Diff #36929) |
lib/socket/activity-handler.react.js | ||
---|---|---|
18 ↗ | (On Diff #36929) |
I think it's better to keep the scope well-defined - that makes e.g. code review easier. For the handlers, there are some tasks defined https://linear.app/comm/issue/ENG-5776/update-socket-handlers which cover updating these. |
lib/utils/authoritative-keyserver.js | ||
8–10 ↗ | (On Diff #36929) |
I don't think that's the case. getInitialReduxStateResponder uses ashoatKeyserverID which should become authoritativeKeyserverID |
Rebase - remove changes from Socket, because D11013 replaced ashoatKeyserverID with props.keyserverID
lib/utils/authoritative-keyserver.js | ||
---|---|---|
8–10 ↗ | (On Diff #36929) | Actually, that will be replaced by a call to fetchIdentityInfo in ENG-6633 |
Replace with function call - this is needed to ensure the value is read after the config is set.
lib/reducers/keyserver-reducer.test.js | ||
---|---|---|
43 ↗ | (On Diff #37347) |
lib/utils/authoritative-keyserver.js | ||
---|---|---|
9 ↗ | (On Diff #37347) | This is a little bit fragile - we don't enforce this relationship. Maybe we can add an invariant invariant(!process.env['KEYSERVER'], '...') Or maybe we can check this condition instead of just hasConfig if (process.env['KEYSERVER'] || !hasConfig()) |
lib/utils/authoritative-keyserver.js | ||
---|---|---|
14–16 | Does keyserver use this when it does server-side rendering of web? |
lib/utils/authoritative-keyserver.js | ||
---|---|---|
14–16 | Hmm I didn't experience the above invariant trowing, so I don't think so. And I was able to login to the app and send messages using this code. This comment made sense when we were calling this function top level, it doesn't anymore. |