From 62e648da2741b83953ce063a96ce66311db1ba7b Mon Sep 17 00:00:00 2001 From: Scott Nonnenberg Date: Fri, 9 Jun 2023 10:46:59 -0700 Subject: [PATCH] getTitle: Return nothing instead of an invalid phone number --- ts/models/conversations.ts | 2 +- ts/test-electron/models/messages_test.ts | 139 +++++++++++++++-------- ts/util/getTitle.ts | 6 +- 3 files changed, 93 insertions(+), 54 deletions(-) diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index 5b79ef8fcd32..18cb6ade8c84 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -4908,7 +4908,7 @@ export class ConversationModel extends window.Backbone return getProfileName(this.attributes); } - getNumber(): string { + getNumber(): string | undefined { return getNumber(this.attributes); } diff --git a/ts/test-electron/models/messages_test.ts b/ts/test-electron/models/messages_test.ts index efc729f2ebef..d6d7dea0639a 100644 --- a/ts/test-electron/models/messages_test.ts +++ b/ts/test-electron/models/messages_test.ts @@ -13,6 +13,7 @@ import type { StorageAccessType } from '../../types/Storage.d'; import { UUID } from '../../types/UUID'; import { SignalService as Proto } from '../../protobuf'; import { getContact } from '../../messages/helpers'; +import type { ConversationModel } from '../../models/conversations'; describe('Message', () => { const STORAGE_KEYS_TO_RESTORE: Array = [ @@ -44,6 +45,12 @@ describe('Message', () => { }); } + function createMessageAndGetNotificationData(attrs: { + [key: string]: unknown; + }) { + return createMessage(attrs).getNotificationData(); + } + before(async () => { window.ConversationController.reset(); await window.ConversationController.load(); @@ -228,29 +235,52 @@ describe('Message', () => { // - Profile changes // - Stickers describe('getNotificationData', () => { + let alice: ConversationModel | undefined; + let bob: ConversationModel | undefined; + let eve: ConversationModel | undefined; + before(() => { + alice = window.ConversationController.getOrCreate( + UUID.generate().toString(), + 'private' + ); + alice.set({ systemGivenName: 'Alice' }); + + bob = window.ConversationController.getOrCreate( + UUID.generate().toString(), + 'private' + ); + bob.set({ systemGivenName: 'Bob' }); + + eve = window.ConversationController.getOrCreate( + UUID.generate().toString(), + 'private' + ); + eve.set({ systemGivenName: 'Eve' }); + }); + it('handles unsupported messages', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ supportedVersionAtReceive: 0, requiredProtocolVersion: Infinity, - }).getNotificationData(), + }), { text: 'Unsupported message' } ); }); it('handles erased tap-to-view messages', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ isViewOnce: true, isErased: true, - }).getNotificationData(), + }), { text: 'View-once Media' } ); }); it('handles tap-to-view photos', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ isViewOnce: true, isErased: false, attachments: [ @@ -258,14 +288,14 @@ describe('Message', () => { contentType: 'image/png', }, ], - }).getNotificationData(), + }), { text: 'View-once Photo', emoji: '📷' } ); }); it('handles tap-to-view videos', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ isViewOnce: true, isErased: false, attachments: [ @@ -273,14 +303,14 @@ describe('Message', () => { contentType: 'video/mp4', }, ], - }).getNotificationData(), + }), { text: 'View-once Video', emoji: '🎥' } ); }); it('handles non-media tap-to-view file types', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ isViewOnce: true, isErased: false, attachments: [ @@ -288,53 +318,53 @@ describe('Message', () => { contentType: 'text/plain', }, ], - }).getNotificationData(), + }), { text: 'Media Message', emoji: '📎' } ); }); it('handles group updates where you left the group', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ group_update: { left: 'You', }, - }).getNotificationData(), + }), { text: 'You are no longer a member of the group.' } ); }); it('handles group updates where someone left the group', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ type: 'incoming', source, group_update: { - left: 'Alice', + left: alice?.get('uuid'), }, - }).getNotificationData(), + }), { text: 'Alice left the group.' } ); }); it('handles empty group updates with a generic message', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ type: 'incoming', - source: 'Alice', + source: alice?.get('uuid'), group_update: {}, - }).getNotificationData(), + }), { text: 'Alice updated the group.' } ); }); it('handles group name updates by you', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ type: 'incoming', source: me, group_update: { name: 'blerg' }, - }).getNotificationData(), + }), { text: "You updated the group. Group name is now 'blerg'.", } @@ -343,11 +373,11 @@ describe('Message', () => { it('handles group name updates by someone else', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ type: 'incoming', source, group_update: { name: 'blerg' }, - }).getNotificationData(), + }), { text: "+1 415-555-5555 updated the group. Group name is now 'blerg'.", } @@ -356,11 +386,11 @@ describe('Message', () => { it('handles group avatar updates', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ type: 'incoming', source, group_update: { avatarUpdated: true }, - }).getNotificationData(), + }), { text: '+1 415-555-5555 updated the group. Group avatar was updated.', } @@ -369,11 +399,11 @@ describe('Message', () => { it('handles you joining the group', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ type: 'incoming', source, group_update: { joined: [me] }, - }).getNotificationData(), + }), { text: '+1 415-555-5555 updated the group. You joined the group.', } @@ -382,11 +412,11 @@ describe('Message', () => { it('handles someone else joining the group', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ type: 'incoming', source, - group_update: { joined: ['Bob'] }, - }).getNotificationData(), + group_update: { joined: [bob?.get('uuid')] }, + }), { text: '+1 415-555-5555 updated the group. Bob joined the group.', } @@ -395,11 +425,13 @@ describe('Message', () => { it('handles multiple people joining the group', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ type: 'incoming', source, - group_update: { joined: ['Bob', 'Alice', 'Eve'] }, - }).getNotificationData(), + group_update: { + joined: [bob?.get('uuid'), alice?.get('uuid'), eve?.get('uuid')], + }, + }), { text: '+1 415-555-5555 updated the group. Bob, Alice, Eve joined the group.', } @@ -408,11 +440,18 @@ describe('Message', () => { it('handles multiple people joining the group, including you', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ type: 'incoming', source, - group_update: { joined: ['Bob', me, 'Alice', 'Eve'] }, - }).getNotificationData(), + group_update: { + joined: [ + bob?.get('uuid'), + me, + alice?.get('uuid'), + eve?.get('uuid'), + ], + }, + }), { text: '+1 415-555-5555 updated the group. Bob, Alice, Eve joined the group. You joined the group.', } @@ -421,11 +460,11 @@ describe('Message', () => { it('handles multiple changes to group properties', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ type: 'incoming', source, - group_update: { joined: ['Bob'], name: 'blerg' }, - }).getNotificationData(), + group_update: { joined: [bob?.get('uuid')], name: 'blerg' }, + }), { text: "+1 415-555-5555 updated the group. Bob joined the group. Group name is now 'blerg'.", } @@ -434,22 +473,22 @@ describe('Message', () => { it('handles a session ending', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ type: 'incoming', source, flags: true, - }).getNotificationData(), + }), { text: i18n('icu:sessionEnded') } ); }); it('handles incoming message errors', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ type: 'incoming', source, errors: [{}], - }).getNotificationData(), + }), { text: i18n('icu:incomingError') } ); }); @@ -538,18 +577,18 @@ describe('Message', () => { attachmentTestCases.forEach(({ title, attachment, expectedResult }) => { it(`handles single ${title} attachments`, () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ type: 'incoming', source, attachments: [attachment], - }).getNotificationData(), + }), expectedResult ); }); it(`handles multiple attachments where the first is a ${title}`, () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ type: 'incoming', source, attachments: [ @@ -558,19 +597,19 @@ describe('Message', () => { contentType: 'text/html', }, ], - }).getNotificationData(), + }), expectedResult ); }); it(`respects the caption for ${title} attachments`, () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ type: 'incoming', source, attachments: [attachment], body: 'hello world', - }).getNotificationData(), + }), { ...expectedResult, text: 'hello world' } ); }); @@ -578,11 +617,11 @@ describe('Message', () => { it('handles a "plain" message', () => { assert.deepEqual( - createMessage({ + createMessageAndGetNotificationData({ type: 'incoming', source, body: 'hello world', - }).getNotificationData(), + }), { text: 'hello world', bodyRanges: [] } ); }); diff --git a/ts/util/getTitle.ts b/ts/util/getTitle.ts index 13e2c0ed917b..4b629b735375 100644 --- a/ts/util/getTitle.ts +++ b/ts/util/getTitle.ts @@ -108,7 +108,7 @@ export function getSystemName( export function getNumber( attributes: Pick -): string { +): string | undefined { if (!isDirectConversation(attributes)) { return ''; } @@ -121,7 +121,7 @@ export function getNumber( return renderNumber(e164); } -export function renderNumber(e164: string): string { +export function renderNumber(e164: string): string | undefined { try { const parsedNumber = window.libphonenumberInstance.parse(e164); const regionCode = getRegionCodeForNumber(e164); @@ -136,6 +136,6 @@ export function renderNumber(e164: string): string { window.libphonenumberFormat.INTERNATIONAL ); } catch (e) { - return e164; + return undefined; } }