Improve logic for push notification tap routing

[tonisevener]

Phabricator:
https://phabricator.wikimedia.org/T287628

Notes

This makes the push notification tap routing a little bit smarter, taking into account the user's editing state and presenting a confirmation alert before routing. It also no longer pops to the main navigation controller's root before routing, so we're no longer wiping out their rabbit hole.

Test Steps

This is easiest to test by triggering fake notifications to a test device through our internal push utility tool. Reach out to me if you need help setting that up. Once you're able to send notifications to device, use these steps to test various configurations. If you think of any others, feel free to test those too.

1. Open up Settings modal. Trigger push. Tap push. Settings should dismiss, then Notifications Center pushed on from the right.
2. Open up Notifications Center from Explore. Trigger push. Tap push. Nothing should happen (no push), because the user is already on Notifications Center.
3. From History tab, create a deep deep rabbit hole by tapping various article links. Trigger push. Tap Push. Notifications Center should push on from the right, preserving the rabbit hole.
4. Set up a couple of presenting modals by setting up languages on Search (Settings > Search > Show languages on Search), then go to the Search tab > More > Add another language. Trigger push. Tap push. All modals should dismiss, then Notifications Center pushed on from the right.
5. Set up a smaller modal. Go to the Places tab. You should see one of our smaller modals asking for Location Services. Trigger push. Tap push. Smaller modal should dismiss and Notifications Center pushed on from the right.
6. Now try Editor flow. Tap the 2nd or later Edit pencil on an article. Tap through editor onboarding. While on editor screen, trigger push notification. Tap notification. You should see an alert confirmation. Tapping cancel dismisses confirmation and keeps the user in Editor. Tapping Yes dismisses all modals (including editing) and pushes on Notification Center from the right.
7. Repeat step 6, but do it from farther in editing flow (the edit Preview screen, or the Edit summary screen before the user publishes). You can also try tapping the Insert link keyboard toolbar button or Insert Media keyboard toolbar button and triggering it from the subsequent modals. In all these cases, the same alert view and it's behavior should happen from step 6.
8. Try editing article descriptions - Tap the 1st Edit pencil in an article, then choose "Edit article description". Go past onboarding modal if necessary. Trigger notification, tap notification. You should see the same alert and behavior from step 6 at this point.

[staykids]

Nice! As discusssed in planning, I tested all the routes but primary focused on editing related navigation and will rely on QA to do a deeper dive. Merges conflict to resolve, a note, and a question in review.

This is easiest to test by triggering fake notifications to a test device through our internal push utility tool.

You know I love the push utility! But by the way, there's another simple way to test this routing via the Simulator. The Simulator supports accepting simulated payloads. While I don't believe it goes through our service extension, it at least provides a quick debug cycle.

First you need to construct some kind of test payload.apns like this:

{
	"aps": {
		"alert": {
			"title": "Notification",
			"body": ""
		} 
	}
}

And via the Terminal, you can push the payload to the Simulator. This will work if you only have one Simulator booted:

xcrun simctl push booted org.wikimedia.wikipedia payload.apns

[staykids]

Should we also consider writing a User talk discussion reply as part of the editing flow as well? Currently, tapping a notification when replying to a User talk discussion dismisses that to show the Notifications Center.

[tonisevener]

Great catch! I added this work to my latest commit. The awkward thing is that the talk page new topic and reply screens push on from the right, so the user actually doesn't lose their changes unless they're somehow already in a modal, say, from visiting your talk page in Settings. So I'm only displaying an alert if they are in a talk page from Settings right now (or any other modal, but I think this is the only place it can happen). Let me know what you think - it still feels a bit weird to me.

[staykids]

Looks great - I'm glad the protocol approach felt like the right path forward to you too! Only a few tiny notes below and 2 merge conflicts to resolve.

[staykids]

Because all these interested objects are gonna inherit this property, I'd recommend we take a page out of Objective-C's book and be a bit more verbose about what we call this in an effort to 1. be more explicit about what it controls and 2. prevent unintended misuse of the property in future work in these controllers.

Just a couple ideas: shouldDisplayDiscardEditsAlert, shouldDisplayExitConfirmationAlert, something like that?

[tonisevener]

Good idea! I went with shouldDisplayExitConfirmationAlert for this

[staykids]

Is the where Self: UIViewController conditional conformance actually needed here?

[tonisevener]

@staykids I don't think it's needed to compile, but I liked constraining them to UIViewController because I am only considering UIKit view controllers when looking around in the navigation hierarchy for these types (within editingFlowViewControllerInHierarchy and notificationsCenterFlowViewController. I did initially try an option with no constraint and conformed the NotificationsCenterFilterView and NotificationsCenterInboxView SwiftUI structs to it, but I had a hard time checking for that conformance from WMFAppViewController. I may just be missing something with SwiftUI / UIKit interop though.

[staykids]

Same note as above re: conditional conformance.

[tonisevener]

@staykids thanks for the suggestions! This is ready for another look.