This differential implements function that can encrypt Android notification. Additionally it introduces type for Android notification. The type is very flexible (so is Android notification payload) but its main purpose is to
protect against uninformed developer that might not know that olm doesn't accept anything else than strings. Additionally it protects against attempts to encrypt notification id.
Details
Details
Use this funcion in prepareAndroidNotification. Log the result, examine its content.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- marcin/eng-3026
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| keyserver/src/push/types.js | ||
|---|---|---|
| 7 | I think so, but the code might be a bit more complicated. | |
| keyserver/src/push/types.js | ||
|---|---|---|
| 7 | I think it's probably worth it, but it's hard for me to say since I haven't seen the details | |
| keyserver/src/push/crypto.js | ||
|---|---|---|
| 101–110 ↗ | (On Diff #26866) | Personally I think this is more clear: const { id, badgeOnly, ...unencryptedPayload } = notification.data; const encryptedNotification = { data: { id, badgeOnly } }; |
| 126–130 ↗ | (On Diff #26866) | Personally I prefer to always have the existing data first when doing this pattern |
| keyserver/src/push/types.js | ||
| 7 ↗ | (On Diff #26866) | Still isn't read-only |
| keyserver/src/push/crypto.js | ||
|---|---|---|
| 126–130 ↗ | (On Diff #26866) | By changing the AndroidNotification type to be purely readonly, we can no longer introduce this change due to the flow error. It even explicitly advises to change the order of spreads: Cannot spread object literal because Flow cannot determine a type for object literal [1]. rest of object pattern [2]
cannot be spread because the indexer string [3] may overwrite properties with explicit keys in a way that Flow cannot
track. Try spreading rest of object pattern [2] first or remove the indexer. [cannot-spread-indexer]
src/push/crypto.js
[2] 73│ const { id, badgeOnly, ...unencryptedPayload } = notification.data;
:
87│ } catch (e) {
88│ console.log('Notification encryption failed: ' + e);
89│
[1] 90│ encryptedNotification.data = {
91│ ...encryptedNotification.data,
92│ ...unencryptedPayload,
93│ encryptionFailed: '1',
94│ };
95│ return encryptedNotification;
96│ }
97│
src/push/types.js
[3] 7│ +[string]: string,If it is a really important convention we can introduce it but we would probably have to refactor this function significantly. |
Comment Actions
- Make AndroidNotification type read-only
- Change order of assignments to increase readability