Skip to content

RG-T132 Push linking fix #473

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
Aug 19, 2026
Merged

RG-T132 Push linking fix #473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ef61977
RG-T132 Push linking fix
ucswift Aug 19, 2026
70df4cc
RG-T132 PR#473 fixes
ucswift Aug 19, 2026
495bca7
RG-T132 PR#473 fixes
ucswift Aug 19, 2026
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
10 changes: 5 additions & 5 deletions Core/Resgrid.Services/CommunicationService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,16 +712,16 @@ public async Task<bool> SendChat(string chatId, int departmentId, string sending
if (recipients.Count == 1)
{
var sendingTo = recipients.FirstOrDefault();
spm.Id = $"T{sendingTo}";

if (sendingTo == null)
return false;

spm.Id = $"T{sendingTo.UserId}";

if (!await CanSendToUser(sendingTo.UserId, departmentId))
Comment on lines 714 to 721

@Resgrid-Bot Resgrid-Bot Aug 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Bug high

NullReferenceException risk in Core/Resgrid.Services/CommunicationService.cs: the single-recipient chat path dereferences sendingTo.UserId before the null check, and a missing profile from GetProfileByUserIdAsync can produce recipients.FirstOrDefault() == null from a one-item [null] recipients list. Move the sendingTo null guard before assigning spm.Id and before calling CanSendToUser so SendChat can return false instead of throwing.

var sendingTo = recipients.FirstOrDefault();
if (sendingTo == null)
	return false;

spm.Id = $"T{sendingTo.UserId}";

if (!await CanSendToUser(sendingTo.UserId, departmentId))
	return false;

await _pushService.PushChat(spm, sendingTo.UserId, sendingTo);
Prompt for LLM 

File Core/Resgrid.Services/CommunicationService.cs:

Line 714 to 718:

NullReferenceException risk in Core/Resgrid.Services/CommunicationService.cs: the single-recipient chat path dereferences sendingTo.UserId before the null check, and a missing profile from GetProfileByUserIdAsync can produce recipients.FirstOrDefault() == null from a one-item [null] recipients list. Move the sendingTo null guard before assigning spm.Id and before calling CanSendToUser so SendChat can return false instead of throwing.

Suggested Code:

var sendingTo = recipients.FirstOrDefault();
if (sendingTo == null)
	return false;

spm.Id = $"T{sendingTo.UserId}";

if (!await CanSendToUser(sendingTo.UserId, departmentId))
	return false;

await _pushService.PushChat(spm, sendingTo.UserId, sendingTo);

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

​

​

return false;

if (sendingTo != null)
{
await _pushService.PushChat(spm, sendingTo.UserId, sendingTo);
}
await _pushService.PushChat(spm, sendingTo.UserId, sendingTo);

@Resgrid-Bot Resgrid-Bot Aug 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Kody Rules high

Unhandled exception risk in Core/Resgrid.Services/CommunicationService.cs: awaiting _pushService.PushChat(spm, sendingTo.UserId, sendingTo) without error handling allows rejected push notifications to escape without an application-level outcome. Wrap the awaited call in try/catch so the method logs context and returns false instead of propagating the exception.

Kody rule violation: Handle async operations with proper error handling

try
{
	await _pushService.PushChat(spm, sendingTo.UserId, sendingTo);
}
catch (Exception ex)
{
	_logger.LogError(ex, "PushChat failed for user {UserId} in department {DepartmentId}", sendingTo.UserId, departmentId);
	return false;
}
Prompt for LLM 

File Core/Resgrid.Services/CommunicationService.cs:

Line 724:

Unhandled exception risk in Core/Resgrid.Services/CommunicationService.cs: awaiting _pushService.PushChat(spm, sendingTo.UserId, sendingTo) without error handling allows rejected push notifications to escape without an application-level outcome. Wrap the awaited call in try/catch so the method logs context and returns false instead of propagating the exception.

Suggested Code:

					try
					{
						await _pushService.PushChat(spm, sendingTo.UserId, sendingTo);
					}
					catch (Exception ex)
					{
						_logger.LogError(ex, "PushChat failed for user {UserId} in department {DepartmentId}", sendingTo.UserId, departmentId);
						return false;
					}

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

​

​

@Resgrid-Bot Resgrid-Bot Aug 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Kody Rules high

Unhandled external service exception in Core/Resgrid.Services/CommunicationService.cs: _pushService.PushChat(spm, sendingTo.UserId, sendingTo) performs an external/service call that can throw transport or service errors without contextual handling. Wrap the awaited call in try/catch so the method logs user and department context and returns false instead of bubbling the exception.

Kody rule violation: Add try-catch blocks for external calls

try
{
	await _pushService.PushChat(spm, sendingTo.UserId, sendingTo);
}
catch (Exception ex)
{
	_logger.LogError(ex, "External push call failed for user {UserId} in department {DepartmentId}", sendingTo.UserId, departmentId);
	return false;
}
Prompt for LLM 

File Core/Resgrid.Services/CommunicationService.cs:

Line 724:

Unhandled external service exception in Core/Resgrid.Services/CommunicationService.cs: _pushService.PushChat(spm, sendingTo.UserId, sendingTo) performs an external/service call that can throw transport or service errors without contextual handling. Wrap the awaited call in try/catch so the method logs user and department context and returns false instead of bubbling the exception.

Suggested Code:

					try
					{
						await _pushService.PushChat(spm, sendingTo.UserId, sendingTo);
					}
					catch (Exception ex)
					{
						_logger.LogError(ex, "External push call failed for user {UserId} in department {DepartmentId}", sendingTo.UserId, departmentId);
						return false;
					}

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

​

​

}
else
{
Expand Down
2 changes: 1 addition & 1 deletion Core/Resgrid.Services/PushService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ public async Task<bool> PushCall(StandardPushCall call, string userId, UserProfi
// Legacy Push Notifications (Azure)
try
{
await _notificationProvider.SendAllNotifications(call.SubTitle, call.Title, userId, string.Format("C{0}", call.CallId), soundType, true, call.ActiveCallCount, color);
await _notificationProvider.SendAllNotifications(call.Title, call.SubTitle, userId, string.Format("C{0}", call.CallId), soundType, true, call.ActiveCallCount, color);
}
catch (Exception ex)
{
Expand Down
11 changes: 11 additions & 0 deletions Providers/Resgrid.Providers.Bus/Models/APNSPayload.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,16 @@ public class ApnsPayload
public ApnsHeader aps { get; set; }
public string eventCode { get; set; }
public string type { get; set; }
public ApnsCustomData body { get; set; }
}

/// <summary>
/// Serialized as the top-level `body` custom key, which is the only key
/// expo-notifications on iOS surfaces to the app as content.data.
/// </summary>
public class ApnsCustomData
{
public string eventCode { get; set; }
public string type { get; set; }
}
}
7 changes: 6 additions & 1 deletion Providers/Resgrid.Providers.Bus/NotificationProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,12 @@ public async Task<NotificationOutcomeState> SendAppleNotification(string title,
}
},
eventCode = eventCode,
type = type
type = type,
body = new ApnsCustomData
{
eventCode = eventCode,
type = type
}
};

appleNotification = JsonConvert.SerializeObject(apnsPayload);
Expand Down
7 changes: 6 additions & 1 deletion Providers/Resgrid.Providers.Bus/UnitNotificationProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,12 @@ public async Task<NotificationOutcomeState> SendAppleNotification(string title,
}
},
eventCode = eventCode,
type = type
type = type,
body = new ApnsCustomData
{
eventCode = eventCode,
type = type
}
};

appleNotification = JsonConvert.SerializeObject(apnsPayload);
Expand Down
33 changes: 31 additions & 2 deletions Providers/Resgrid.Providers.Messaging/NovuProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,17 @@ private async Task<bool> SendNotification(string title, string body, string reci
eventCode = eventCode,
customType = type
},
// expo-notifications on iOS only surfaces the top-level `body` custom key
// as content.data, and per the APNs spec custom keys belong beside `aps`,
// not inside it. The aps-nested eventCode/customType above stay for
// backward compatibility with already deployed apps.
body = new
{
eventCode = eventCode,
type = type
},
eventCode = eventCode,
type = type
},
},
},
Expand All @@ -431,7 +442,20 @@ private async Task<bool> SendNotification(string title, string body, string reci
["type"] = type,
["category"] = channelName,
["eventCode"] = eventCode,
["gcm.message_id"] = "123"
["gcm.message_id"] = "123",
// node-apn merges `payload` in as the custom data at the top level of the
// APNs JSON; the nested `body` key is the one expo-notifications on iOS
// exposes as content.data.
["payload"] = new Dictionary<string, object>
{
["body"] = new
{
eventCode = eventCode,
type = type
},
["eventCode"] = eventCode,
["type"] = type
},
},
},
to = new[]{ new
Expand Down Expand Up @@ -657,7 +681,12 @@ private string CreateAppleNotification(string title, string subTitle, string typ
}
},
eventCode = eventCode,
type = type
type = type,
body = new ApnsCustomData
{
eventCode = eventCode,
type = type
}
};

var appleNotification = JsonConvert.SerializeObject(apnsPayload);
Expand Down
4 changes: 2 additions & 2 deletions Tests/Resgrid.Tests/Services/PushServiceModernApplicationSoundTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Threading.Tasks;
using System.Threading.Tasks;
using Moq;
using NUnit.Framework;
using Resgrid.Config;
Expand Down Expand Up @@ -178,8 +178,8 @@ public async Task PushCall_uses_effective_department_or_user_sound_setting(
await _pushService.PushCall(call, UserId, profile);

_notificationProvider.Verify(x => x.SendAllNotifications(
call.SubTitle,
call.Title,
call.SubTitle,
UserId,
"C99",
((int)expectedSound).ToString(),
Expand Down
Loading