If we want the system to launch NotificationService when notification is received it muse have flag mutable-content set to true (https://developer.apple.com/documentation/usernotifications/unnotificationserviceextension?language=objc)
Details
This change was already used to test previous differentials in this stack. The only additional testing needed is to check whether it doesn't break previous versions of the app (no NotificationService).
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
From the docs it looks like this is safe, but we should be careful about modifying all existing notifs. It's probably okay, but if we have any doubts we could hide this behind a codeVersion check
Just making sure – is there any danger in setting this flag when sending a notif to an old client that doesn't support the Notification Service Extension? It might be smart to either:
- Test what happens when an old client (with no Notification Service Extension support) receives this notif, OR
- Gate this on codeVersion that supports Notification Service Extension
@ashoat I have already realized this concern and attempted to reach @palys-swm regarding this matter. Now that you also brought this matter into discussion I think it is best to add codeVersion if statement to this diff.
Add codeVersion flag to determine whether notification should be visible for NotificationService
@ashoat I noticed that codeVersion is optional parameter so we need an additional check to satisfy flow constraints. However I noticed that the only place this function (prepareIOSNotification) gets called is sendIOSNotification where codeVersion is specified. Can we then make this parameter obligatory and simplify if statement?
When I opened XCode I found out that the current code version is 136 - this is why I check codeVersion against 136.
Good question – I can't tell why it's optional either! I think it's okay to make it required.
When I opened XCode I found out that the current code version is 136 - this is why I check codeVersion against 136.
I used to do it this way too, but then realized that sometimes we accidentally increment code version with the engineer who is working on it knowing if it is ready. So I think instead it's better to use a high number here, and then submit another diff later when you're ready to launch the feature.
keyserver/src/push/send.js | ||
---|---|---|
495 ↗ | (On Diff #14510) | Before landing, can you change this to something high, eg. 1000? |
Make codeVersion required parameter. Set threshold for mutableContent flag to high number.