Skip to content

RE1-T88 Looking at using FCM to forward APNS messages with Novu #264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ucswift merged 3 commits into from
Nov 9, 2025
Merged

RE1-T88 Looking at using FCM to forward APNS messages with Novu #264

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f157b83
RE1-T88 Looking at using FCM to forward APNS messages with Novu
ucswift Nov 9, 2025
2292217
RE1-T88 PR#264 fix
ucswift Nov 9, 2025
6306057
RE1-T88 PR#264 fix
ucswift Nov 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 78 additions & 23 deletions Providers/Resgrid.Providers.Messaging/NovuProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Resgrid.Model;
using Resgrid.Model.Providers;
using Resgrid.Providers.Bus.Models;
using SharpCompress.Common;
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 9, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Unused import: Remove if not needed.

The SharpCompress.Common namespace doesn't appear to be used anywhere in this file. Please remove it unless it's required for functionality outside the shown code.

-using SharpCompress.Common;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
using SharpCompress.Common;
// Line 10 and the using SharpCompress.Common; statement removed
🤖 Prompt for AI Agents 
In Providers/Resgrid.Providers.Messaging/NovuProvider.cs around line 10, the
using directive for SharpCompress.Common is unused; remove the 'using
SharpCompress.Common;' line to clean up imports and ensure no references to that
namespace remain in the file (if any are later added, re-add the using where
needed).

using System.Text;


Expand Down Expand Up @@ -132,7 +133,7 @@ private async Task<bool> UpdateSubscriberFcm(string id, string token, string fcm
}
}

private async Task<bool> UpdateSubscriberApns(string id, string token, string apnsId)
private async Task<bool> UpdateSubscriberApns(string id, string token, string apnsId, string fcmId)
{
try
{
Expand All @@ -144,16 +145,40 @@ private async Task<bool> UpdateSubscriberApns(string id, string token, string ap
request.Headers.Add("idempotency-key", Guid.NewGuid().ToString());
request.Headers.Add("Authorization", $"ApiKey {ChatConfig.NovuSecretKey}");

var payload = new
string jsonContent = string.Empty;
if (!string.IsNullOrWhiteSpace(apnsId))
{
providerId = "apns",
credentials = new
var payload = new
{
deviceTokens = new string[] { token }
},
integrationIdentifier = apnsId
};
string jsonContent = JsonConvert.SerializeObject(payload, new JsonSerializerSettings { NullValueHandling = NullValueHandling.Ignore });
providerId = "apns",
credentials = new
{
deviceTokens = new string[] { token }
},
integrationIdentifier = apnsId
};

jsonContent = JsonConvert.SerializeObject(payload, new JsonSerializerSettings { NullValueHandling = NullValueHandling.Ignore });
}
else if (!string.IsNullOrWhiteSpace(fcmId))
{
var payload = new
{
providerId = "fcm",
credentials = new
{
deviceTokens = new string[] { token }
},
integrationIdentifier = fcmId
};

jsonContent = JsonConvert.SerializeObject(payload, new JsonSerializerSettings { NullValueHandling = NullValueHandling.Ignore });
}

if (string.IsNullOrWhiteSpace(jsonContent))
{
return false;
}

request.Content = new StringContent(jsonContent, Encoding.UTF8, "application/json");
HttpResponseMessage response = await client.SendAsync(request);
Expand Down Expand Up @@ -197,7 +222,7 @@ public async Task<bool> UpdateUserSubscriberFcm(string userId, string code, stri

public async Task<bool> UpdateUserSubscriberApns(string userId, string code, string token)
{
return await UpdateSubscriberApns($"{code}_User_{userId}", token, ChatConfig.NovuResponderApnsProviderId);
return await UpdateSubscriberApns($"{code}_User_{userId}", token, ChatConfig.NovuResponderApnsProviderId, null);
}

public async Task<bool> UpdateUnitSubscriberFcm(int unitId, string code, string token)
Expand All @@ -207,7 +232,8 @@ public async Task<bool> UpdateUnitSubscriberFcm(int unitId, string code, string

public async Task<bool> UpdateUnitSubscriberApns(int unitId, string code, string token)
{
return await UpdateSubscriberApns($"{code}_Unit_{unitId}", token, ChatConfig.NovuUnitApnsProviderId);
//return await UpdateSubscriberApns($"{code}_Unit_{unitId}", token, ChatConfig.NovuUnitApnsProviderId);
return await UpdateSubscriberApns($"{code}_Unit_{unitId}", token, null, ChatConfig.NovuUnitFcmProviderId);
}
Comment on lines 233 to 237
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 9, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

iOS unit registrations are now pushed into the wrong provider

UpdateUnitSubscriberApns no longer stores iOS tokens under the APNS integration and instead pushes them into the FCM provider. APNS device tokens are 64‑char hex strings that must be registered against an APNS integration; feeding them to FCM results in INVALID_ARGUMENT/UNREGISTERED responses because FCM expects its own registration tokens (longer base64 strings) and rejects APNS tokens outright. The net effect is that unit notifications on iOS will stop delivering. Please keep iOS registrations on the APNS integration (as before) or ensure the caller passes real FCM registration tokens before switching providers. (docs.novu.co)

🤖 Prompt for AI Agents 
In Providers/Resgrid.Providers.Messaging/NovuProvider.cs around lines 232-236,
UpdateUnitSubscriberApns is registering iOS APNS tokens against the FCM
provider; change the call to register tokens with the APNS integration instead.
Replace the current call that passes the FCM provider id with the APNS provider
id (e.g. use ChatConfig.NovuUnitApnsProviderId) so iOS 64‑char hex device tokens
are stored under APNS, not FCM.


private async Task<bool> SendNotification(string title, string body, string recipientId, string eventCode,
Expand Down Expand Up @@ -259,21 +285,50 @@ private async Task<bool> SendNotification(string title, string body, string reci
type = type,
}
},
apns = new
{
badge = count,
sound = new
{
name = sound,
critical = channelName == "calls" ? 1 : 0,
volume = 1.0f
},
type = type,
category = channelName,
eventCode = eventCode,
payload = new
{
aps = new
{
badge = count,
sound = new
{
name = sound,
critical = channelName == "calls" ? 1 : 0,
volume = 1.0f
},
category = channelName,
eventCode = eventCode,
customType = type
},
},
},
},
apns = new Dictionary<string, object>
{
["badge"] = count,
["sound"] = new
apns = new Dictionary<string, object>
{
name = sound,
critical = channelName == "calls" ? 1 : 0,
volume = 1.0f
["badge"] = count,
["sound"] = new
{
name = sound,
critical = channelName == "calls" ? 1 : 0,
volume = 1.0f
},
["type"] = type,
["category"] = channelName,
["eventCode"] = eventCode,
["gcm.message_id"] = "123"
},
Comment on lines +318 to 331
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 9, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove or clarify gcm.message_id in APNS override.

Line 330 adds ["gcm.message_id"] = "123" to the APNS override dictionary. This is problematic:

  • gcm.message_id is a GCM (Google Cloud Messaging, FCM's predecessor) concept and doesn't belong in APNS payloads
  • The hardcoded value "123" suggests placeholder code
  • APNS has no concept of GCM message IDs and will likely ignore or reject this field

If this field is required for some FCM-APNS passthrough scenario, please:

  1. Document why it's needed
  2. Generate a meaningful value instead of hardcoding "123"
  3. Verify that Novu/APNS doesn't reject this unexpected field

Otherwise, remove it:

 							["type"] = type,
 							["category"] = channelName,
 							["eventCode"] = eventCode,
-							["gcm.message_id"] = "123"
 						},
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
apns = new Dictionary<string, object>
{
name = sound,
critical = channelName == "calls" ? 1 : 0,
volume = 1.0f
["badge"] = count,
["sound"] = new
{
name = sound,
critical = channelName == "calls" ? 1 : 0,
volume = 1.0f
},
["type"] = type,
["category"] = channelName,
["eventCode"] = eventCode,
["gcm.message_id"] = "123"
},
apns = new Dictionary<string, object>
{
["badge"] = count,
["sound"] = new
{
name = sound,
critical = channelName == "calls" ? 1 : 0,
volume = 1.0f
},
["type"] = type,
["category"] = channelName,
["eventCode"] = eventCode,
},
🤖 Prompt for AI Agents 
In Providers/Resgrid.Providers.Messaging/NovuProvider.cs around lines 318 to
331, the APNS override currently contains a hardcoded GCM field
["gcm.message_id"] = "123", which is inappropriate for APNS; remove that key
from the APNS dictionary, or if you truly need to pass an FCM/Android message
id, move it to the FCM/GCM-specific payload branch (not APNS), generate a
meaningful unique id (e.g., guid or provider message id) instead of "123", and
add a short comment documenting why and where that field is used and a check to
ensure Novu/APNS won’t reject it.

["type"] = type,
["category"] = channelName,
["eventCode"] = eventCode,
["gcm.message_id"] = "123"
},
},
to = new[]{ new
{
Expand Down
Loading