diff --git a/.changeset/rotten-pugs-trade.md b/.changeset/rotten-pugs-trade.md new file mode 100644 index 0000000000000..37515336702d6 --- /dev/null +++ b/.changeset/rotten-pugs-trade.md @@ -0,0 +1,6 @@ +--- +"@rocket.chat/meteor": patch +"@rocket.chat/tools": patch +--- + +Adds improvements to the push notifications logic; the logic now truncates messages and titles larger than 240, and 65 characters respectively. diff --git a/apps/meteor/app/push/server/push.ts b/apps/meteor/app/push/server/push.ts index 95543aaa43e90..8382214a6b35a 100644 --- a/apps/meteor/app/push/server/push.ts +++ b/apps/meteor/app/push/server/push.ts @@ -1,7 +1,7 @@ import type { IAppsTokens, RequiredField, Optional, IPushNotificationConfig } from '@rocket.chat/core-typings'; import { AppsTokens } from '@rocket.chat/models'; import { serverFetch as fetch } from '@rocket.chat/server-fetch'; -import { pick } from '@rocket.chat/tools'; +import { pick, truncateString } from '@rocket.chat/tools'; import Ajv from 'ajv'; import { JWT } from 'google-auth-library'; import { Match, check } from 'meteor/check'; @@ -15,6 +15,9 @@ import { settings } from '../../settings/server'; export const _matchToken = Match.OneOf({ apn: String }, { gcm: String }); +const PUSH_TITLE_LIMIT = 65; +const PUSH_MESSAGE_BODY_LIMIT = 240; + const ajv = new Ajv({ coerceTypes: true, }); @@ -459,8 +462,10 @@ class PushClass { createdBy: '', sent: false, sending: 0, + title: truncateString(options.title, PUSH_TITLE_LIMIT), + text: truncateString(options.text, PUSH_MESSAGE_BODY_LIMIT), - ...pick(options, 'from', 'title', 'text', 'userId', 'payload', 'badge', 'sound', 'notId', 'priority'), + ...pick(options, 'from', 'userId', 'payload', 'badge', 'sound', 'notId', 'priority'), ...(this.hasApnOptions(options) ? { diff --git a/apps/meteor/tests/unit/app/push/push.spec.ts b/apps/meteor/tests/unit/app/push/push.spec.ts new file mode 100644 index 0000000000000..cfabd30d0291d --- /dev/null +++ b/apps/meteor/tests/unit/app/push/push.spec.ts @@ -0,0 +1,112 @@ +import type { IPushNotificationConfig } from '@rocket.chat/core-typings/src/IPushNotificationConfig'; +import { pick, truncateString } from '@rocket.chat/tools'; +import { expect } from 'chai'; +import proxyquire from 'proxyquire'; +import sinon from 'sinon'; + +const loggerStub = { debug: sinon.stub(), warn: sinon.stub(), error: sinon.stub(), info: sinon.stub(), log: sinon.stub() }; +const settingsStub = { get: sinon.stub().returns('') }; + +const { Push } = proxyquire.noCallThru().load('../../../../app/push/server/push', { + './logger': { logger: loggerStub }, + '../../settings/server': { settings: settingsStub }, + '@rocket.chat/tools': { pick, truncateString }, + 'meteor/check': { + check: sinon.stub(), + Match: { + Optional: () => sinon.stub(), + Integer: Number, + OneOf: () => sinon.stub(), + test: sinon.stub().returns(true), + }, + }, + 'meteor/meteor': { + Meteor: { + absoluteUrl: sinon.stub().returns('http://localhost'), + }, + }, +}); + +describe('Push Notifications [PushClass]', () => { + afterEach(() => { + sinon.restore(); + }); + + describe('send()', () => { + let sendNotificationStub: sinon.SinonStub; + beforeEach(() => { + sendNotificationStub = sinon.stub(Push, 'sendNotification').resolves({ apn: [], gcm: [] }); + }); + + it('should call sendNotification with required fields', async () => { + const options: IPushNotificationConfig = { + from: 'test', + title: 'title', + text: 'body', + userId: 'user1', + apn: { category: 'MESSAGE' }, + gcm: { style: 'inbox', image: 'url' }, + }; + + await Push.send(options); + + expect(sendNotificationStub.calledOnce).to.be.true; + + const notification = sendNotificationStub.firstCall.args[0]; + expect(notification.from).to.equal('test'); + expect(notification.title).to.equal('title'); + expect(notification.text).to.equal('body'); + expect(notification.userId).to.equal('user1'); + }); + + it('should truncate text if longer than 240 chars', async () => { + const longText = 'a'.repeat(300); + const options: IPushNotificationConfig = { + from: 'test', + title: 'title', + text: longText, + userId: 'user1', + apn: { category: 'MESSAGE' }, + gcm: { style: 'inbox', image: 'url' }, + }; + + await Push.send(options); + + const notification = sendNotificationStub.firstCall.args[0]; + + expect(notification.text.length).to.equal(240); + }); + + it('should truncate title if longer than 65 chars', async () => { + const longTitle = 'a'.repeat(100); + const options: IPushNotificationConfig = { + from: 'test', + title: longTitle, + text: 'bpdu', + userId: 'user1', + apn: { category: 'MESSAGE' }, + gcm: { style: 'inbox', image: 'url' }, + }; + + await Push.send(options); + + const notification = sendNotificationStub.firstCall.args[0]; + + expect(notification.title.length).to.equal(65); + }); + + it('should throw if userId is missing', async () => { + const options = { + from: 'test', + title: 'title', + text: 'body', + apn: { category: 'MESSAGE' }, + gcm: { style: 'inbox', image: 'url' }, + }; + + await expect(Push.send(options)).to.be.rejectedWith('No userId found'); + + expect(sendNotificationStub.called).to.be.false; + }); + }); +}); diff --git a/packages/tools/src/index.ts b/packages/tools/src/index.ts index 8734e9f4d8c1e..4549ff513a238 100644 --- a/packages/tools/src/index.ts +++ b/packages/tools/src/index.ts @@ -13,3 +13,4 @@ export * from './removeEmpty'; export * from './isObject'; export * from './isRecord'; export * from './validateEmail'; +export * from './truncateString'; diff --git a/packages/tools/src/truncateString.ts b/packages/tools/src/truncateString.ts new file mode 100644 index 0000000000000..6de03c7eaa9e5 --- /dev/null +++ b/packages/tools/src/truncateString.ts @@ -0,0 +1,23 @@ +/** + * Truncates a string to a specified maximum length, optionally adding ellipses. + * @param str + * @param maxLength + * @param shouldAddEllipses + * @return {string} + */ +export function truncateString(str: string, maxLength: number, shouldAddEllipses = true): string { + const ellipsis = '...'; + if (str.length <= maxLength) { + return str; + } + + if (shouldAddEllipses && str.length > maxLength) { + if (maxLength <= ellipsis.length) { + return str.slice(0, maxLength); + } + + return `${str.slice(0, maxLength - ellipsis.length)}${ellipsis}`; + } + + return str.slice(0, maxLength); +}