Skip to content

Add configurable notification content for FCM v1 pushkins#432

Open
wcjord wants to merge 1 commit intoelement-hq:mainfrom
pangeachat:feat/fcm-notification-content
Open

Add configurable notification content for FCM v1 pushkins#432
wcjord wants to merge 1 commit intoelement-hq:mainfrom
pangeachat:feat/fcm-notification-content

Conversation

@wcjord
Copy link
Copy Markdown

@wcjord wcjord commented Mar 2, 2026

Summary

Data-only FCM messages are throttled/dropped by iOS in the background, causing unreliable push delivery. This adds an opt-in include_notification_content config option for GCM/FCM v1 pushkins that injects a visible notification payload into FCM v1 requests, ensuring immediate display on iOS devices.

Addresses #366.

Changes

When include_notification_content: true is set in the pushkin config:

  • title: room_name > sender_display_name > sender > "New Message"
  • body: content.body or "New Message"

The notification payload is only added for FCM v1 API requests (not legacy GCM). When disabled (the default), behavior is unchanged.

New config option

com.example.gcm:
  type: gcm
  api_version: 1
  project_id: my-project
  service_account_file: /path/to/sa.json
  include_notification_content: true   # opt-in

Tests

  • test_include_notification_content_api_v1 — verifies title/body from push data
  • test_include_notification_content_fallback_title — verifies fallback when room_name/sender_display_name absent
  • test_notification_content_disabled_by_default — verifies no notification key when config absent

Changelog

changelog.d/gcm-notification-content.feature included.

Signed-off-by: wcjord 32568597+wcjord@users.noreply.github.com

Add an opt-in `include_notification_content` config option for GCM/FCM
v1 pushkins. When enabled, a visible `notification` payload is injected
into FCM v1 requests with:
- title: room_name > sender_display_name > sender > "New Message"
- body: content.body or "New Message"

Data-only FCM messages are throttled/dropped by iOS in the background,
causing unreliable push delivery. Including a visible notification
payload ensures immediate display on iOS devices.

Addresses element-hq#366.

Signed-off-by: wcjord <32568597+wcjord@users.noreply.github.com>
@wcjord wcjord requested a review from a team as a code owner March 2, 2026 19:02
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 2, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an opt-in configuration flag to the FCM v1 (GCM) pushkin to inject a visible message.notification payload derived from Sygnal’s push data, improving reliability of iOS delivery for background scenarios.

Changes:

  • Introduces include_notification_content config option for FCM v1 pushkins and uses it to add message.notification.title/body.
  • Adds unit tests covering enabled behavior, title fallback, and the default-disabled behavior.
  • Adds a changelog entry documenting the new opt-in feature.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
sygnal/gcmpushkin.py Adds the include_notification_content config and injects an FCM v1 message.notification payload derived from message data.
tests/test_gcm.py Adds coverage for the new opt-in notification content behavior and default behavior.
changelog.d/gcm-notification-content.feature Documents the new feature and rationale.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +601 to +605
body["message"]["notification"] = {
"title": title,
"body": notification_body,
}

Copy link
Copy Markdown
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Most apps use the event_id_only format these days for privacy, so I'm just going to focus on that use case. Shout if your use case makes use of the full event body!)

I believe how this is supposed to work is the iOS app will define a default_payload in the data dictionary which includes the fields it would like to appear in the notification.

For instance, ElementX iOS sets the following for default_payload:

  "default_payload": {
    "aps": {
      "mutable-content": 1,
      "alert": {"loc-key": "Notification", "loc-args": []}
    }
  }

aps.alert.loc-{key,args} allow for an app to define localisable strings. This means that apps for non-English markets can have local generic message translations, instead of "New Message".

This payload is then merged into the payload sent to APNs:

def _get_payload_event_id_only(
self,
n: Notification,
default_payload: Dict[str, Any],
send_badge_counts: bool,
) -> Dict[str, Any]:
"""
Constructs a payload for a notification where we know only the event ID.
Args:
n: The notification to construct a payload for.
device: Device information to which the constructed payload
will be sent.
send_badge_counts: if `True`, the `unread_count` and `missed_calls` fields will be included.
Returns:
The APNs payload as a nested dicts.
"""
payload = {}
payload.update(default_payload)
if n.room_id:
payload["room_id"] = n.room_id
if n.event_id:
payload["event_id"] = n.event_id
if send_badge_counts:
if n.counts.unread is not None:
payload["unread_count"] = n.counts.unread
if n.counts.missed_calls is not None:
payload["missed_calls"] = n.counts.missed_calls
return payload

I would expect the FCM pusher to do a similar thing - taking the default_payload into account. That's in contrast to this PR, which just sets body.message.notification to hard-coded English terms (when event_id_only format is used).

We advise iOS applications to set default_payload in our application documentation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should be renamed to 432.feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants