Details

Reviewers

tomek
atul

Commits

rCOMM9a1eee09a936: [keyserver] Factor out importJSON

Summary

I was using copy-pasted code in several places, which is an anti-pattern. This diff should be a pure refactor.

Test Plan

Here's my plan:

Test backup: create keyservers/facts/backups.json, update cronjob schedule to run right now, and make sure backup is created:
Test GeoIP update: create keyservers/secrets/geoip_license.json, and then run cd keyserver && yarn update-geoip
Test push notifs on both iOS and Android (see @palys-swm's instructions in a comment below)
Test URLs (make sure keyserver works)

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ashoat created this revision.May 12 2022, 6:41 PM

Herald added subscribers: • abosh, • adrian. · View Herald TranscriptMay 12 2022, 6:41 PM

ashoat added a child revision: D4025: [keyserver] Use importJSON to import olm_config.json.May 12 2022, 6:47 PM

Harbormaster completed remote builds in B9056: Diff 12601.May 12 2022, 6:48 PM

ashoat requested review of this revision.May 12 2022, 6:48 PM

Test push notifs (@palys-swm, any advice? I've never done this and I think you have)

On iOS we need to have comm_apn.p8 and comm_apn_config.json with "production": false. With these it should be possible to send notifications to a physical device in dev mode.
On Android, as far as I remember, the notifications work when comm_fcm_config.json is present, even on a simulator.

As to how test them, any scenario in which a notification is generated should work ok, e.g. when sending a message from user A to user B while the app is backgrounded.

keyserver/src/push/providers.js
30–54 ↗	(On Diff #12601)	It doesn't look like these two usages will work because in the previous version we were importing the files and then using `.default` when creating `Provider` / `cert`, but now `importJSON` takes the `default` and returns its value.
keyserver/src/utils/import-json.js
3–6 ↗	(On Diff #12601)	It is possible to avoid `any` by using a generic function, but I'm not sure if that's an improvement