I can make this change if a Growth engineer will sign off on the proposed solution and review the patch.

In T287909#7264759, @LGoto wrote:

Hi @Catrope wrt: the conversation with apps on notifications, can you comment on what the next step here might be? Thanks!

@LGoto it's on our backlog of tasks for Growth-Team to discuss (cc @DMburugu). Given it's marked as high priority, it sounds like you'd like this to be acted on in the next week or two?

@kostajh Yes, next week or two would be great, thank you! Please let me know if you need more info from the Apps.

In T287909#7273972, @LGoto wrote:

@kostajh Yes, next week or two would be great, thank you! Please let me know if you need more info from the Apps.

OK. @Mholloway if you write the patch we will make time to review it. Thank you!

In T287909#7275508, @kostajh wrote:

In T287909#7273972, @LGoto wrote:

@kostajh Yes, next week or two would be great, thank you! Please let me know if you need more info from the Apps.

OK. @Mholloway if you write the patch we will make time to review it. Thank you!

Awesome, thanks @kostajh. @Tsevener & @Dbrant, this would mean that the apps would need to add a query argument like &notnotifiertype=push to the API request for notification retrieval. Does that sound good?

thanks @Mholloway, that sounds good to me! And just to confirm, once this is in this means the desktop web preference checkboxes would not affect the output of the notifications API if we send in that parameter, right? And the desktop apps preference checkboxes will continue to not affect the output of the notifications API?

Cool, thanks @Tsevener! Yes, so right now, the API always sends notifications for only those notification types which are enabled for web. That will continue to be the default behavior if notifications are requested without any additional arguments. The patch will add a notnotifiertype parameter to allow a caller to specifiy the notifier type (currently email, web, or push) or types (using a pipe-separated multivalued argument, e.g., web|push). When the notifier type push is specified, notifications of types which are enabled for push (labeled "Apps" in the preference checkboxes) will be sent. (These preferences can be set by API, and do not require app users to adjust their preferences in the web UI.)

I'll aim to get this in on Friday.

Sounds good to me as well. Thanks @Mholloway

@Mholloway ah okay I think I get it - for some reason I was imagining the apps checkmark column to only be referenced by the push service to serve as a gatekeeper on which type of push notification is sent to APNS, but the user's notification center in the app can show all notifications without filters. But it probably makes more sense to the user that these line up.

Tagging @JMinor as a heads up in case he has feedback - this means the apps notification center will only show notifications the user has enabled in the apps checkbox column. I'm unsure of how this works with global preferences.

@Mholloway One last side question to confirm I'm thinking of this right - the push service does consider these apps column preferences before sending out a push to APNS, correct? Just want to make sure that wasn't an assumption I had wrong.

Assuming this is code review and not too complex thereof, the Growth team will review next week

In T287909#7276438, @Tsevener wrote:

@Mholloway ah okay I think I get it - for some reason I was imagining the apps checkmark column to only be referenced by the push service to serve as a gatekeeper on which type of push notification is sent to APNS, but the user's notification center in the app can show all notifications without filters. But it probably makes more sense to the user that these line up.

Yeah, I actually didn't realize this filtering was being applied in the notifications API code, and had also assumed that it was returning all available notifications by default.

@Mholloway One last side question to confirm I'm thinking of this right - the push service does consider these apps column preferences before sending out a push to APNS, correct? Just want to make sure that wasn't an assumption I had wrong.

Yes, that's right.

In T287909#7276555, @MBinder_WMF wrote:

Assuming this is code review and not too complex thereof, the Growth team will review next week

This should be a fairly simple change for an experienced MediaWiki dev to review.

This all sounds ok to me, though we should be sure to document this limitation in FAQs or other user facing info, as this may be counter-intuitive for users.

But, we now also need to switch the default state for apps to "on". Otherwise the user will never have any messages when first opting into the system as a whole, and it adds a significant barrier by having to find and check/tap each type.

Because the user still has to opt in at the application/OS level I don't think there's any danger of user annoyance or unexpected messages.

Do you want a subtask to set those to "on" by default, or can you wrap it in here?

@JMinor Changing that default will only take a config change. I'd say we can do it as part of this task.

The auto-generated API docs will document the new parameter, and I'll also make a note on the relevant mediawiki.org help page.

Change 712980 had a related patch set uploaded (by Mholloway; author: Michael Holloway):

[mediawiki/extensions/Echo@master] Add notifiertypes parameter to ApiEchoNotifications

https://gerrit.wikimedia.org/r/712980

About the defaulting behavior, after some code investigation and further thinking on the matter: it does not appear to be possible at present to set an entire notifier type to be opt-out. The recommended default, and the assumption throughout the notification preferences code, is for notifications to be opt-in, and the code for determining a user's eligibilty for receiving a notification is already complex enough that I would hesitate to add conditional user preference defaults by notifier type to the mix.

Additionally, given the heightened user privacy and security concerns around push, I think the case for defaulting to opt-in is even stronger than in the general case. Catching a user off-guard by unexpectedly enabling push notifications for some on-wiki action could have grave results. (I recall discussing this with the privacy and security engineers during the design phase, but after looking back just now, I don't believe it was ultimately captured in the documents resulting from that assessment.)

In any case, if we want to change the default behavior of push notifications to opt-out, I do think that's a significant change that should have a dedicated ticket after all.

Thanks @Mholloway it does sound like an impactful enough change for its own task. Lani linked an initial ticket, and I will fill out with details.

I'm a bit confused as my understanding is that the "Web" ones were on by default? I think the idea was just to match the existing behavior so that notifications will show up in the app Notification Centers at all. Then the user would need to opt in to get the actual native pushes. Requiring users to opt in to even see messages in the Notification Center (not pushed) seems like an unessary step. No one is going to see push notification on their phones by accident or without clear opt-in. The issue is will they see any messages in the Notification Center if these are unchecked? Anyway, a conversation for that other task...

Having now had a chance to digest this change with the team, I wanted to clarify what the impact will be, and the solution we're considering on iOS.

I think this API change makes sense, and gives the preferences UI and API a cleaner mapping (ie. no hidden values). I support that.

The impact however, is that now you can only see a notification on the app's notification center if and only if you also agree to push that notification type. There is a conflation of a message being stored and available for the "app" to pull, and the user granting permission for that message type to be "pushed" to the native push infrastructure.

From a user story perspective this means that an app user can either either have the message type in their Notification Center AND get that message pushed to them or neither. This lack of distinction between "storage/availability to pull" and "push/availability to receive" did not matter for the web, which has no push (the messages are displayed when the user clicks on the web element), or for email which has no storage (the email is sent and our work is done). Apps have both. We let people see their messages on demand but also be able to receive them via push. By putting both options in one checkbox we've conflated two user stories into one option.

Additionally, Ive been told that only messages for users with "web" checked are permanently stored, whese publishers like "email" use transient storage. This means that a user might be pushed messages which are then lost by the time they view their Notification Center (ie. they have apps checked but not web). This issue was hidden before because "web" was always secretly hardcoded. Will this change also change the storage model or does web still mean "store these" as well?

One solution would be to treat the Echo Apps preference as "store/availability" opt-in. So users are agreeing to see the notification in the apps' notification center by setting the Apps preference, but push only happens when the user registers/opts in to native push from the device side. However that device registration opt in is global (device is enrolled or not) and so the user couldn't turn off push for a specific type in this approach, only allow all messages or none.

Seemingly for Android this has an easier soltuion as they can "quash" notifications before they are shown to the user. That still results in a lot of unessary network traffic for something the user doesn't even want. And on iOS this is not the case. On iOS we can't filter individual types on the client layer before they are shown. Additionally we are not given type info in the payload (ie. the pushes don't tell us what types they are) to be able to filter at delivery.

The alternative we are thinking about is to use the "Web" preference to mean "store these messages and make available in notification centers" and the Apps column to be "push these to me". We would expose both in the app, one as a general account preference the other as a push opt in for specific types. This is also likely to lead to some user confusion but it allows users to chose the two delivery options separately for any types and doesn't require any additional service changes.

In T287909#7293036, @JMinor wrote:

The impact however, is that now you can only see a notification on the app's notification center if and only if you also agree to push that notification type.

This is where I'm a bit confused myself: I don't believe this is true. After this change, the API will behave by default exactly as it does now. The only change is to add an optional notifier type parameter. Are you concerned that passing push as this parameter means that web notifications will not be available? One way to work around that on the client, as I suggested above, would be to request notifications available for both web and push, or indeed for all three currently defined types; the API will happily accept that.

I did test the other day with a new account on testwiki, and it does appear that several notification types are enabled for web by default, while none are enabled for apps/push by default. I believe that's set on a preference-by-preference basis and not by notifier type category. I don't know off the top of my head where the special defaults are being set for web, but that's something any engineer should be able to track down.

As for the broader questions about storage: I am genuinely sorry, but I only meant to fix an obvious API bug that I identified in conversation with @Tsevener. I simply don't have the capacity right now to fix all of Echo. But I don't think that's necessary, as I don't believe this change actually causes the issue that you describe. If I'm missing something, I apologize.

(As for the privacy/security stuff, I'm sorry I brought it up, because I think it's more distracting than helpful here given what your concerns seem to be.)

It's also an option for the client to opt in to push notifications for all supported types when the user first registers a token, or for a new type when support for a new type is added. (That gets into the surprise avoidance issues I mentioned earlier, but I'm done fighting that battle; do whatever you think is right.)

Hey @Mholloway first, thanks for the help here. My comments here are not meant to imply you are responsible for fixing or doing anything. You jumped back into an old project and there's no expectation you'll continue to engage or write changes for this. That said, it seems like we still have quite a bit of confusion, so I appreciate your willingness to help clarify what exists.

As for the solution, I think the "ask for both web and apps" is the second alternative I was making sure would make sense.

In T287909#7293188, @JMinor wrote:

As for the solution, I think the "ask for both web and apps" is the second alternative I was making sure would make sense.

OK. That makes sense. I know it's hacky, but if it takes care of the immediate problem, great.

I've been struggling with the notion of 1:1 correspondence with web, because that's not a concept that really translates to what's going on in the code. The only thing that isn't currently equivalent, as far as I can see, is some opt-in defaults which are set somewhere I've yet to identify. That said, my fear is that there are more hidden discrepancies of the kind that led to this ticket. The transient vs. permanent storage issue you mentioned above sounds like one of those.

Change 712980 merged by jenkins-bot:

[mediawiki/extensions/Echo@master] Add notifiertypes parameter to ApiEchoNotifications

https://gerrit.wikimedia.org/r/712980