3591 detect push changes

[Lakoja]

Probably fixes #3591, but is a huge step forward regardless.

Draft because the TODO count is still too high: Architecture decisions, feature decisions or at least "this is fine for now" decisions.

This improves the following for push notifications (ordered by importance):

• Possibility to change the distributor (detects that the previously used one is missing, unregisters and registers another one on the server)
• Only update (push) notification settings on server if needed
• Show currently used distributor as (unchangeable) setting
• Allow to only fetch notifications for one account (the one for which a push message was received)

Maybe main problem with also this code: All of this is (still) very edge-casey. For example there are a number of different paths and intentions in which the various methods of the new PushNotificationManager can be called.
It is rather involved testing this and it is still not possible for the user to "change anything" if something is wrong.
And: if there are few people using this the effort in programming here is also a problem.

[nikclayton]

NotificationPreferencesFragment is for older versions of Android that don't provide per-notification group settings (API versions <= 25).

You can see this in AccountPreferencesFragment.openNotificationSystemPrefs().

Unfortunately the term "filter" is overloaded and depends on context. In one context it's filtering the types of Android notifications you will receive.

In the other context it's filtering the types of notifications that are displayed in the notifications timeline.

This allows you to, e.g,. disable Android notifications for new followers, but still see the new follower notifications in the Notifications timeline.

[Lakoja]

ok, removed the comment

[nikclayton]

Suggested change

	isUnifiedPushAvailable() && !account.unifiedDistributorName.isNullOrEmpty()
	isUnifiedPushAvailable() && account.unifiedDistributorName.isNotBlank()

[Lakoja]

unifiedDistributorName is nullable

[nikclayton]

Why is a method called enablePushNotifications possibly disabling push notifications?

[Lakoja]

(it's moved old code)

Could be called doYourStuff or manage instead - as it is in PushNotificationManager.

Maybe enableOrDisablePerAccount?

[nikclayton]

Draft because the TODO count is still too high: Architecture decisions, feature decisions or at least "this is fine for now" decisions.

Can you describe those separately to the code? E.g., a high level architecture sketch that describes what you think this should do?

That's helpful for several reasons:

1. I don't need to try and reverse engineer the architecture from the code
2. It's easier to see if the architecture you intended matches the architecture in the code
3. It's possible to have a conversation about the larger architecture without needing to get in to the specifics of the implementation

[Lakoja]

Draft because the TODO count is still too high: Architecture decisions, feature decisions or at least "this is fine for now" decisions.

Can you describe those separately to the code? E.g., a high level architecture sketch that describes what you think this should do?

There is no larger architecture here, yet. Maybe not needed (and there was none).

It's basically "do push notification registration if needed on start".

Most questions make sense at their specific location. Extracting them would loose valuable information.

Thus they can and should be discussed in-line here.

Most of them are in PushNotificationManager.

The biggest there on the top: Should we join pull and push notification code more?

[Lakoja]

Maybe one general question still:

The linked issue has be closed (by author). So it probably makes sense to add another one "improve push notifications" and change the references here (?)

[Lakoja]

Updating / rebasing this another time.

As people's main problem with Tusky is missing notifications.