Skip to content

notif: Move helpers shared with iOS out of Android-specific display.dart #2384

Description

@chrisbobbe

lib/notifications/display.dart is essentially Android display mechanics: NotificationChannelManager and most of NotificationDisplayManager deal in notification channels, MessagingStyle accumulation, group summaries, and cancellation, with entry points that assert(defaultTargetPlatform == TargetPlatform.android).

But three of NotificationDisplayManager's statics don't touch any of that machinery: they just compute values from the push payload (NotifPayloadNewMessage), and they have consumers in iOS code (lib/notifications/ios_service.dart):

  • titleForNotifPayload (both platforms)
  • subtitleForNotifPayloadOnIos (iOS only; an "OnIos" method on a class that otherwise asserts Android)
  • notificationUrlForNotifPayload (both platforms)

Proposed reorganization:

  1. Move notificationUrlForNotifPayload into lib/notifications/open.dart, as a NotificationOpenPayload.fromNotifPayload constructor or a static helper (it's already just "construct a payload, call buildNotificationUrl"). Then the payload-to-URL mapping lives next to the URL parsing it has to round-trip with.
  2. Move titleForNotifPayload and subtitleForNotifPayloadOnIos to a shared module in lib/notifications/. They're localization-dependent presentation logic, so neither lib/api/notifications.dart nor the Android manager is the right home.
  3. With no iOS consumers left, rename display.dart (and NotificationDisplayManager) to say "Android", mirroring ios_service.dart and pigeon/android_notifications.dart, so the platform boundary is visible at import sites.

This gets more valuable with #2169 (iOS notification grouping): iOS's threadIdentifier will need the same conversation identity Android computes as the private _conversationKey. That would join the shared layer too, so the two platforms' grouping keys can't drift apart.

Possibly also worth folding in: logOutAccount (lib/model/actions.dart) clears notifications behind an Android-only platform check at the call site. A platform-neutral entry point would make the missing iOS-side cleanup an explicit gap rather than an absent else. (The delivered notifications already carry account identity, in notification_url in userInfo or in the legacy payload fields, so the cleanup itself mainly needs bindings for getDeliveredNotifications / removeDeliveredNotifications.)

Noticed while reviewing #2380, which threads a new parameter through notificationUrlForNotifPayload and illustrates the friction: the iOS call site must pass messageId: null to a helper on the Android display class. Related: #1565, #2169.

Metadata

Metadata

Type

No type

Projects

Status
No status

Relationships

None yet

Development

No branches or pull requests

Issue actions