Page MenuHomePhabricator

[Spike]: Notification Receipt Questions
Closed, ResolvedPublicSpike

Description

This spike is for the iOS engineers to dig on some questions around the receipt of notifications:

  1. Confirm how the server manipulates notifications before delivering. Root job notifications are deduplicated with only the latest instance showing, but what types of notifications are root? Confirm other notifications are batched and sent individually at a random time.
    • Sidenote: will we do any OS-supported grouping and is that possible? (this seems doubtful, but just to confirm).

Answer: applicable comments are here, here and here. Sounds like we don't need to worry about root details, but we will not be able to assume a 1 to 1 mapping between the push notification received and objects in the notifications API response.
Sidenote Answer: Actually I think we could via the Notification Service extension. When constructing the UNMutableNotificationContent in didReceive(_ request: UNNotificationRequest), we should be able to assign a threadIdentifier at this point.

  1. Will deduping always match the echo read notifications API? I.e. push notification sends one deduped notificaation, to represent for example “7 posts were made to your talk page”. When then fetching the push notification read API, do we get 7 different notification objects? Or are they grouped there into one as well?

Answer: No, we can not assume we'll receive 4 push notifications and read 4 new push notifications from the notifications API. We need to be able to handle a push notification with no new content to read, as well as a push notification with multiple new notification types to read. (spun off https://phabricator.wikimedia.org/T285353 and https://phabricator.wikimedia.org/T285348 for further discussion on these).

  1. How do we determine the unread push notification content to display in a push?
    • Should we do coalescing on our end, i.e. get all new notifications from the API and combine them into one notification to read? This runs the risk of there being a followup push without anything new to read because we already combined it in a previous notification display. We’d need generic text for this case since we can't suppress the OS notification. We would also need design input on the wording and quick actions for a combined notification.
    • OR do we pull only the first new notification from the API and display that? There’s a risk here in that we’re relying on the push notification service to send follow up pushes for every notification that returns from the notifications API.

Answer: we need to be able to display a notification with no new data (https://phabricator.wikimedia.org/T285353), and we either combine the content of multiple notification types into one push content (without quick actions), or schedule followup local notifications for the additional unread notifications (to be explored in https://phabricator.wikimedia.org/T285348)

  1. There are 20 different notification types, all of which the user can enable and receive in the app via changing settings on Desktop. How do we handle undesigned notifications? Do we have a very plain version that asks them to go to desktop to view?

Answer: We either design for all notification types, or consider disabling the unsupported checkboxes in Desktop (see https://phabricator.wikimedia.org/T284238#7138541). Update: there's a 3rd option, which is that we add very default/generic handling for any notification type the app doesn't recognize, which should catch these extra types. (to be done in https://phabricator.wikimedia.org/T287033)

Event Timeline

Restricted Application changed the subtype of this task from "Task" to "Spike". · View Herald TranscriptJun 3 2021, 4:30 PM
Tsevener updated the task description. (Show Details)

@Jgiannelos @Mholloway I have a few of questions on how the push notifications service delivers notifications.

Background: On the iOS side we will not be able to suppress any OS notifications and have the non-suppressed notifications reliably display, so I'm trying to find the best logic for fetching OS notification content given this restriction.

In a nutshell I'm trying to confirm if we can expect a 1-to-1 mapping between the push notifications we will receive and the notifications objects returned from the notifications API, which we will query upon receipt of a push to get the notification content:

  1. I've heard and read about the service batching pushes and sending them at a random time with a generic "checkEchoV1" payload. This seems fine, but are these notifications ever combined at this layer? That is, could we be in a situation where we would only receive one push, but a fetch to the notifications API would indicate say, 3 new notifications, two of which the device would not receive a push notification for. I realize there may be connection hiccups or with device permissions where this situation could happen, but I mainly want to know if there's any explicit combining logic in the service layer.
  1. On a related note, I have heard that "root job" notification types are deduped, but I'm not sure which types these are, and if I can expect the deduping to also be reflected in notifications API. Is this related to the collapsed notifications mentioned here?
  1. We have notes that a user could opt-in to any notification type from Desktop, even if we don't provide that option in the app, so we need to be prepared to handle any notification type. But I just noticed in Desktop preferences some of the checkboxes are grayed out - just curious if this is something we could do in the Apps column, that is, disable the checkmarks on Desktop for any notification type the apps aren't prepared to handle?

Screen Shot 2021-06-04 at 1.28.01 PM.png (364×205 px, 42 KB)

Apologies if this was all already discussed in the past during the service development, or if I'm tagging the wrong folks. I'm brushing up on all of it again before we get into heavy development. Thanks in advance!

I'll let @Jgiannelos handle questions 1 & 2, but I can speak to 3. Yes, specific notification types can be disbled for push using $wgNotifyTypeAvailabilityByCategory[$type]['push'] = false;. The push notifier type is currently configured in CommonSettings.php in operations/mediawiki-config (so that it can be enabled conditionally per wiki), and you can see there that we're disabling push notifications for the notification types system and system-noemail. Other notification types could be handled in the same way.

If the apps don't intend to stay in sync with regard to supported notification types, then we may want to give each app its own notifier type for push notifications so that different notification types can be disabled for each; this wouldn't be terribly difficult to do.

OK, with respect to deduplication based on root job (question 2), after some email digging I think I've found what you're referring to. There is a pattern supported by ChangeProp and used for some job queue jobs whereby a "root" job is first created that serves only to, e.g., identify the scope of the effects of a change and then create a subsequent set of jobs to perform the required follow-up tasks. In this case the triggering root event is kept in a field in the subsequent "leaf" requests and used for deduplication. We're not doing anything like this, just firing off simple, self-contained jobs, so I don't think this deduplication is applicable to NotificationRequestJob jobs.

  1. In the service layer we do batching based on the combination of:
    • notification token
    • type
    • provider
    • message metadata.

In the scenario you describe, if echo extension sends 10 notifications using checkEchoV1 to the same token for the same provider (eg APNS) and the same metadata, push service will deduplicate them to 1 push notification to check echo for notifications. Echo API response then will return 10 notifications for that specific token.

Note about batching:

The way its implemented, batching in push service is time and size based and gets implemented per service runner instance, so you can't really rely on the deduplication of the tasks at that level. For example, the notification batch might be ready to be sent at a point in time when from a sum of 10 notifications 3 ended up on worker X and 7 on worker Y. That means 2 notifications are send to the device at different times.

  1. Regarding root jobs, jobqueue in mediawiki allows generating jobs (root) that can have other related jobs that can get deduplicated so eventually only one job is executed. Even though Echo extension supports job deduplication, I don't think we use it anywhere.

@Jgiannelos @Mholloway awesome, thanks for the info! I'll mention the checkmark disabling to the rest of the team and will let you know if we decide to move forward with that.

I have a couple of more questions -

  1. For the combination you list to drive the batching logic, can you tell me what the message metadata is that is used in the batching logic? Would it just be the notification "type" property ("edit-user-talk", "reverted", etc.) from the notifications API? Just want to make sure we're matching that logic as close as possible when pulling data to display to display from the notifications API (though I realize further batching could occur based on time, size and service level as you mention).
  1. Should we be limiting any logic to "checkEchoV1" on our side? Could a payload with "checkEchoV2" be pushed sometime in the future?

Thanks!

  1. type is not considered metadata in our message definition. Currently metadata is only the topic.
  2. What kind of logic do you expect on the app side? I guess having a new checkEchoV2 depends in future product needs. Also usage is pretty low at the moment so there is space to adapt to client needs.

@Jgiannelos thanks for the followup!

  1. Got it, do you know what topic represents in this context? I assume it's not the same as a talk page topic, seems like it must be something more general. That may be more of a question for a different team. I dug as far as https://github.com/wikimedia/mediawiki-extensions-Echo/blob/839b833033a5cc4286ca0618b12188b670b87562/includes/Push/NotificationServiceClient.php#L35 for a mention of topic, but I'm a bit stuck on what a topic is and if it maps to anything in the notifications API response (there's no mention of topic in that response from what I gather). This could be digging too far into the backend weeds, just trying to fill in the holes in my knowledge of this.
  2. Good question, I guess if there's some major change in the way the service will batch and send notifications, that might merit a version bump if we need to alter the client-side code to match. Or if we want two different ways of displaying notifications sent to the same device, we'd need the device to switch on that logic accordingly. I'll follow up with others on the iOS team for any other thoughts.

@Jgiannelos after some more digging, I've seen some references of topic being the app bundle identifier (like org.wikimedia.wikipedia), which would make sense. Ultimately I just needed to confirm that we could receive only one push that represents multiple types of notifications, for example, a talk page notification and an edit reverted notification, and that we need to handle that combined description in the push UI. Those would not be separated on the backend and sent as two different generic pushes simply because they are different notification types. But they could also be sent separately if the randomized timing, sizing or workers sussed it out that way.

Hopefully this is correct!

@Jgiannelos @Mholloway I have a couple of more asks/questions surrounding how to test notifications as we develop this (actually five questions, but I am cheating and bundling 3 under the first question). I can spin any of these off into separate tasks if needed, just let me know.

  1. In the apns sandbox environment, as we develop it would be nice to mimic the flow as it will be when we go live to trigger notifications, if that makes sense. For example, would I be able to add to my talk page via desktop, and have the notification pushed and appear on my device in the sandbox environment without any extra steps? And would that hit any sort of push notification service batching logic to test that functionality as well? Or would a flow like that only work in the apns production (TestFlight / App Store) environment?
    • 1a. It is okay if this isn't possible, because it looks like we can always lean on the https://pushnotifications.wmflabs.org/v1/message/apns call to manually trigger a notification in the sandbox environment. My only concern there is that it will bypass the batching logic so we wouldn't get as much testing with that functionality.
    • 1b. I had trouble getting the https://pushnotifications.wmflabs.org/v1/message/apns call to work with any app other than the Push Notification Utility app. @Dmantena noticed it may be because that app identifier is the only one called out in debug_topics in the config (see https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/services/push-notifications/+/HEAD/src/outgoing/apns/readme.md and search for debug_topics). Would it be possible to add the org.wikimedia.wikipedia.tfalpha and org.wikimedia.wikipedia identifiers to this field as well?
    • 1c. On a related note, is the production flow ready for us to test against? I have tried triggering a talk page notification in a TestFlight build but haven't had any luck so far. Here's an example of an echopushsubscription create command that I'm making after logging in. I do not see any push notifications when posting a topic or reply to my talk page after this call succeeds. Note the topic is our experimental build identifier, if this only works with something specific let me know.

POST https://en.wikipedia.org/w/api.php
Body action=echopushsubscriptions&command=create&format=json&provider=apns&providertoken=c3a82fe5f4cd5c3bae28bee36cede3f112900dc706085eea4358396555d6920c&token=09c3eb696da0df1acb24143e83bd509260d0fdd8%2B%5C&topic=org.wikimedia.wikipedia.tfalpha
Response

{
  "create": {
    "result": "Success"
  }
}
  1. Once we get into heavy duty development of these and we have a production-like test flow working, it would be nice if there were an easy way for us to trigger Echo notifications of any type listed here (https://www.mediawiki.org/wiki/Help:Notifications/Types). Is there any type of internal tool we could lean on for this? I'm not sure how we'd trigger something like a user rights change alert or an edit milestone notice, for example. This may be more of a question for another team.

Thanks for your patience with all of these!

@Jgiannelos @Mholloway I have a couple of more asks/questions surrounding how to test notifications as we develop this (actually five questions, but I am cheating and bundling 3 under the first question). I can spin any of these off into separate tasks if needed, just let me know.

  1. In the apns sandbox environment, as we develop it would be nice to mimic the flow as it will be when we go live to trigger notifications, if that makes sense. For example, would I be able to add to my talk page via desktop, and have the notification pushed and appear on my device in the sandbox environment without any extra steps? And would that hit any sort of push notification service batching logic to test that functionality as well? Or would a flow like that only work in the apns production (TestFlight / App Store) environment?

I think that (if not already configured) we can connect the mediawiki beta (on deployment-prep) to send echo notifications to push notification beta that is sandboxed.

  • 1a. It is okay if this isn't possible, because it looks like we can always lean on the https://pushnotifications.wmflabs.org/v1/message/apns call to manually trigger a notification in the sandbox environment. My only concern there is that it will bypass the batching logic so we wouldn't get as much testing with that functionality.

The same batching logic we have on prod we also have on beta so I am not expecting this to be a problem.

Its a simple config change to allow other app identifiers to send visible debug notifications through the beta push notifications service. That said I am not sure if its a good idea to allow any interactions from the beta deployment to org.wikimedia.wikipedia. Can you file a ticket for that so we can find a good way forward for your workflow?

  • 1c. On a related note, is the production flow ready for us to test against? I have tried triggering a talk page notification in a TestFlight build but haven't had any luck so far. Here's an example of an echopushsubscription create command that I'm making after logging in. I do not see any push notifications when posting a topic or reply to my talk page after this call succeeds. Note the topic is our experimental build identifier, if this only works with something specific let me know.

POST https://en.wikipedia.org/w/api.php
Body action=echopushsubscriptions&command=create&format=json&provider=apns&providertoken=c3a82fe5f4cd5c3bae28bee36cede3f112900dc706085eea4358396555d6920c&token=09c3eb696da0df1acb24143e83bd509260d0fdd8%2B%5C&topic=org.wikimedia.wikipedia.tfalpha
Response

{
  "create": {
    "result": "Success"
  }
}

I remember when we deployed the changes to production we tested a couple of wikis (probably test wiki?) via API access. I don't remember off the top of my head if we have enabled any actions to trigger actual push notifications.

  1. Once we get into heavy duty development of these and we have a production-like test flow working, it would be nice if there were an easy way for us to trigger Echo notifications of any type listed here (https://www.mediawiki.org/wiki/Help:Notifications/Types). Is there any type of internal tool we could lean on for this? I'm not sure how we'd trigger something like a user rights change alert or an edit milestone notice, for example. This may be more of a question for another team.

My mediawiki experience is not great so this needs a bit of research. I guess we can introduce a helper function that devs can trigger notifications manually on mediawiki/echo level. I think its better to start the discussion for that in a new phab ticket.

Thanks for your patience with all of these!

Its a simple config change to allow other app identifiers to send visible debug notifications through the beta push notifications service. That said I am not sure if its a good idea to allow any interactions from the beta deployment to org.wikimedia.wikipedia. Can you file a ticket for that so we can find a good way forward for your workflow?

Filed https://phabricator.wikimedia.org/T285417 for these workflows.

My mediawiki experience is not great so this needs a bit of research. I guess we can introduce a helper function that devs can trigger notifications manually on mediawiki/echo level. I think its better to start the discussion for that in a new phab ticket.

Filed https://phabricator.wikimedia.org/T285440 to begin these discussions.

Thanks @Jgiannelos!

All of my questions in this spike have been answered or have followup tasks spun off. Moving this into Done.

@Jgiannelos and/or @Mholloway (may be of interest to @Dbrant too)

I'm breathing life back into this spike because I thought of a few (hopefully last) straggler questions around the push service.

  1. Does push subscription with the device token (via https://test.wikipedia.org/w/api.php?action=help&modules=echopushsubscriptions ) happen globally? If I posted the subscription to test.wikipedia.org, would I get whatever notifications are set on every wiki Apps notification preferences checkboxes? Or would I only get pushes triggered from Test Wikipedia and not, say, EN because of where I posted the subscription to?
  2. Related, the notification type preferences (set via https://test.wikipedia.org/w/api.php?action=help&modules=options) are wiki-specific. Just confirming that this means if a user unchecks all Test Wikipedia Apps column checkmarks, they would no longer receive any push notifications triggered from Test Wikipedia, but they would receive notifications triggered from other wikis. If so, we might need to account for these preference settings when we populate the push notification content.
  3. On the desktop notification preferences screen, I noticed a side-effect with the API that we're supposed to call for populating the notification content. If I turn off certain checkmarks under the "Web" column, the notifications API begins to filter out those types. This could mean a user receives a push from the Apps column, but we would not be able to populate it's content because it's filtered out from the API due to the Web column. Just highlighting the weirdness - it may be too much of an edge case to react to.
  4. In the logic for populating our push notification content, I want to make sure I'm not populating it with some ancient unread notification text. Would it be reasonable for me to throw out any notification with a timestamp of greater than 10 minutes ago for content consideration? I think I read somewhere that this delay value in the service is configurable, so I wasn't sure if adding a hardcoded 10 minutes value in the app was a horrible idea or not.

@Jgiannelos and/or @Mholloway (may be of interest to @Dbrant too)

I'm breathing life back into this spike because I thought of a few (hopefully last) straggler questions around the push service.

  1. Does push subscription with the device token (via https://test.wikipedia.org/w/api.php?action=help&modules=echopushsubscriptions ) happen globally? If I posted the subscription to test.wikipedia.org, would I get whatever notifications are set on every wiki Apps notification preferences checkboxes? Or would I only get pushes triggered from Test Wikipedia and not, say, EN because of where I posted the subscription to?

Push subscriptions are associated with the user's central ID and are shared across all wikis.

  1. Related, the notification type preferences (set via https://test.wikipedia.org/w/api.php?action=help&modules=options) are wiki-specific. Just confirming that this means if a user unchecks all Test Wikipedia Apps column checkmarks, they would no longer receive any push notifications triggered from Test Wikipedia, but they would receive notifications triggered from other wikis. If so, we might need to account for these preference settings when we populate the push notification content.

Yes, that's right. I explored using global preferences for this as part of T251462, but we ultimately deemed it impractical for v1 given some of the assumptions baked into global preferences and how tightly coupled they are to the web UI.

  1. On the desktop notification preferences screen, I noticed a side-effect with the API that we're supposed to call for populating the notification content. If I turn off certain checkmarks under the "Web" column, the notifications API begins to filter out those types. This could mean a user receives a push from the Apps column, but we would not be able to populate it's content because it's filtered out from the API due to the Web column. Just highlighting the weirdness - it may be too much of an edge case to react to.

Hmm, this sounds like a bug. I don't think that notifications ought to be excluded from being returned by the API if the 'web' notification type is unchecked, but I wouldn't be shocked to find out that's what's happening, either.

  1. In the logic for populating our push notification content, I want to make sure I'm not populating it with some ancient unread notification text. Would it be reasonable for me to throw out any notification with a timestamp of greater than 10 minutes ago for content consideration? I think I read somewhere that this delay value in the service is configurable, so I wasn't sure if adding a hardcoded 10 minutes value in the app was a horrible idea or not.

That sounds reasonable to me. The delay value is currently randomly selected (on a per-worker basis) from between 60000 and 300000 ms (i.e., between 1 and 5 minutes). For reference, those settings can be found in the service's Helm chart.

Hmm, this sounds like a bug. I don't think that notifications ought to be excluded from being returned by the API if the 'web' notification type is unchecked, but I wouldn't be shocked to find out that's what's happening, either.

@Mholloway if you don't mind, can you file a bug to the right team for us when you get a chance? Here's the API call for quick testing, you can see the changes when unchecking various notification preferences types.

https://en.wikipedia.org/w/api.php?action=query&meta=notifications

@Mholloway if you don't mind, can you file a bug to the right team for us when you get a chance?

Done: T287909: Notifications API module should not hard-code the `web` notifier type

@Mholloway thanks for filing that! I have one more quick question if you don't mind. When a device subscribes to notifications, would they receive a one-time flurry of push notifications for every unread notification they have in their history across wikis? I assumed not, and that upon subscribing they would only receive pushes for new unread notifications triggered from that point in time on, but please correct me if I'm wrong.

@Tsevener Push notifications should only be created for new events occuring after the user subscribes, not for unread past notifications.