-
Improvement
-
Resolution: Fixed
-
Minor
-
4.2
-
MOODLE_402_STABLE
-
MOODLE_402_STABLE
-
MDL-77893-master -
Including here some additional improvements in the existing features discovered by dpalou while testing the MOBILE side in MOBILE-4214
- Along with the new WebService core_user_update_user_device_public_key, I think it would be nice to also modify the existing WS core_user_add_user_device to add a new optional parameter: the publickey. This way for new sites the app can supply the publickey in the same request used to add the new device and we can avoid performing an extra request just to store the key.
- We use a customised version of airnotifier to deliver push notifications to the app. One of the improvements we did, to decrease the payload size, is that we only send a field named "body" to the app and we remove smallmessage, fullmessage and fullmessagehtml from the notification payload:
message = extra.get('smallmessage', None)
if not message:
message = extra.get('fullmessage', None)
extra["smallmessage"] = None
extra["fullmessage"] = None
extra["fullmessagehtml"] = None
Maybe we can do something similar in LMS if the payload size is bigger than 4000 to decrease the chances of having to display "Tap to view". E.g. we can put the first valid text in smallmessage and get rid of the others, as long as this doesn't cause problems with the public Airnotifier branch (not our private fork). (fullmessage and fullmessagehtml are not currently used by the Moodle app at all, so it is safe to avoid sending them). This will also improve performance in large sites sending lots of notifications (a considerable smallest payload). Please notice that this is also compatible to existing community Airnotifier instances because the code retrieving the notification from the small message has been there since the beginning. - If I understood it right, if a device doesn't have a publickey then the notification is sent without being encrypted. It can happen that a site enables encryption but some devices don't have a public key because the app hasn't been updated yet or the device hasn't used the app in a while. In this case the sensitive data will still be sent to the push servers but the admin might think it isn't because he enabled encryption. Maybe we shouldn't send the notification to that device, or maybe we should create a new setting so the admin can decide whether to send them or not. For devices not supporting encryption: [Do not send notifications at all | Send notifications without encryption](encryption will be supported only in Android 6.0 and iOS 13 and up)
- The code that controls the size of the payload (and try to reduce it when needed) was inside the function that does the encryption, I think it would be good to have this also for non-encrypted push
- There was a bug in the way $extra->encrypted was set, it has to be set/reset for every device cause the user can have multiple devices and some of them support notifications while others not
