Skip to content

Commit 9e03ed5

Browse files
fix: push notifications not delivered when message length is too large (#37852)
1 parent a979ceb commit 9e03ed5

File tree

5 files changed

+149
-2
lines changed

5 files changed

+149
-2
lines changed

.changeset/rotten-pugs-trade.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@rocket.chat/meteor": patch
3+
"@rocket.chat/tools": patch
4+
---
5+
6+
Adds improvements to the push notifications logic; the logic now truncates messages and titles larger than 240, and 65 characters respectively.

apps/meteor/app/push/server/push.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { IAppsTokens, RequiredField, Optional, IPushNotificationConfig } from '@rocket.chat/core-typings';
22
import { AppsTokens } from '@rocket.chat/models';
33
import { serverFetch as fetch } from '@rocket.chat/server-fetch';
4-
import { pick } from '@rocket.chat/tools';
4+
import { pick, truncateString } from '@rocket.chat/tools';
55
import Ajv from 'ajv';
66
import { JWT } from 'google-auth-library';
77
import { Match, check } from 'meteor/check';
@@ -15,6 +15,9 @@ import { settings } from '../../settings/server';
1515

1616
export const _matchToken = Match.OneOf({ apn: String }, { gcm: String });
1717

18+
const PUSH_TITLE_LIMIT = 65;
19+
const PUSH_MESSAGE_BODY_LIMIT = 240;
20+
1821
const ajv = new Ajv({
1922
coerceTypes: true,
2023
});
@@ -459,8 +462,10 @@ class PushClass {
459462
createdBy: '<SERVER>',
460463
sent: false,
461464
sending: 0,
465+
title: truncateString(options.title, PUSH_TITLE_LIMIT),
466+
text: truncateString(options.text, PUSH_MESSAGE_BODY_LIMIT),
462467

463-
...pick(options, 'from', 'title', 'text', 'userId', 'payload', 'badge', 'sound', 'notId', 'priority'),
468+
...pick(options, 'from', 'userId', 'payload', 'badge', 'sound', 'notId', 'priority'),
464469

465470
...(this.hasApnOptions(options)
466471
? {
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import type { IPushNotificationConfig } from '@rocket.chat/core-typings/src/IPushNotificationConfig';
2+
import { pick, truncateString } from '@rocket.chat/tools';
3+
import { expect } from 'chai';
4+
import proxyquire from 'proxyquire';
5+
import sinon from 'sinon';
6+
7+
const loggerStub = { debug: sinon.stub(), warn: sinon.stub(), error: sinon.stub(), info: sinon.stub(), log: sinon.stub() };
8+
const settingsStub = { get: sinon.stub().returns('') };
9+
10+
const { Push } = proxyquire.noCallThru().load('../../../../app/push/server/push', {
11+
'./logger': { logger: loggerStub },
12+
'../../settings/server': { settings: settingsStub },
13+
'@rocket.chat/tools': { pick, truncateString },
14+
'meteor/check': {
15+
check: sinon.stub(),
16+
Match: {
17+
Optional: () => sinon.stub(),
18+
Integer: Number,
19+
OneOf: () => sinon.stub(),
20+
test: sinon.stub().returns(true),
21+
},
22+
},
23+
'meteor/meteor': {
24+
Meteor: {
25+
absoluteUrl: sinon.stub().returns('http://localhost'),
26+
},
27+
},
28+
});
29+
30+
describe('Push Notifications [PushClass]', () => {
31+
afterEach(() => {
32+
sinon.restore();
33+
});
34+
35+
describe('send()', () => {
36+
let sendNotificationStub: sinon.SinonStub;
37+
beforeEach(() => {
38+
sendNotificationStub = sinon.stub(Push, 'sendNotification').resolves({ apn: [], gcm: [] });
39+
});
40+
41+
it('should call sendNotification with required fields', async () => {
42+
const options: IPushNotificationConfig = {
43+
from: 'test',
44+
title: 'title',
45+
text: 'body',
46+
userId: 'user1',
47+
apn: { category: 'MESSAGE' },
48+
gcm: { style: 'inbox', image: 'url' },
49+
};
50+
51+
await Push.send(options);
52+
53+
expect(sendNotificationStub.calledOnce).to.be.true;
54+
55+
const notification = sendNotificationStub.firstCall.args[0];
56+
expect(notification.from).to.equal('test');
57+
expect(notification.title).to.equal('title');
58+
expect(notification.text).to.equal('body');
59+
expect(notification.userId).to.equal('user1');
60+
});
61+
62+
it('should truncate text if longer than 240 chars', async () => {
63+
const longText = 'a'.repeat(300);
64+
const options: IPushNotificationConfig = {
65+
from: 'test',
66+
title: 'title',
67+
text: longText,
68+
userId: 'user1',
69+
apn: { category: 'MESSAGE' },
70+
gcm: { style: 'inbox', image: 'url' },
71+
};
72+
73+
await Push.send(options);
74+
75+
const notification = sendNotificationStub.firstCall.args[0];
76+
77+
expect(notification.text.length).to.equal(240);
78+
});
79+
80+
it('should truncate title if longer than 65 chars', async () => {
81+
const longTitle = 'a'.repeat(100);
82+
const options: IPushNotificationConfig = {
83+
from: 'test',
84+
title: longTitle,
85+
text: 'bpdu',
86+
userId: 'user1',
87+
apn: { category: 'MESSAGE' },
88+
gcm: { style: 'inbox', image: 'url' },
89+
};
90+
91+
await Push.send(options);
92+
93+
const notification = sendNotificationStub.firstCall.args[0];
94+
95+
expect(notification.title.length).to.equal(65);
96+
});
97+
98+
it('should throw if userId is missing', async () => {
99+
const options = {
100+
from: 'test',
101+
title: 'title',
102+
text: 'body',
103+
apn: { category: 'MESSAGE' },
104+
gcm: { style: 'inbox', image: 'url' },
105+
};
106+
107+
await expect(Push.send(options)).to.be.rejectedWith('No userId found');
108+
109+
expect(sendNotificationStub.called).to.be.false;
110+
});
111+
});
112+
});

packages/tools/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,4 @@ export * from './removeEmpty';
1313
export * from './isObject';
1414
export * from './isRecord';
1515
export * from './validateEmail';
16+
export * from './truncateString';
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/**
2+
* Truncates a string to a specified maximum length, optionally adding ellipses.
3+
* @param str
4+
* @param maxLength
5+
* @param shouldAddEllipses
6+
* @return {string}
7+
*/
8+
export function truncateString(str: string, maxLength: number, shouldAddEllipses = true): string {
9+
const ellipsis = '...';
10+
if (str.length <= maxLength) {
11+
return str;
12+
}
13+
14+
if (shouldAddEllipses && str.length > maxLength) {
15+
if (maxLength <= ellipsis.length) {
16+
return str.slice(0, maxLength);
17+
}
18+
19+
return `${str.slice(0, maxLength - ellipsis.length)}${ellipsis}`;
20+
}
21+
22+
return str.slice(0, maxLength);
23+
}

0 commit comments

Comments
 (0)