getTitle: Return nothing instead of an invalid phone number

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);
   }
 

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<keyof StorageAccessType> = [
@@ -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'] },
+          group_update: { joined: [bob?.get('uuid')] },
-        }).getNotificationData(),
+        }),
         {
           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'] },
+          group_update: {
-        }).getNotificationData(),
+            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'] },
+          group_update: {
-        }).getNotificationData(),
+            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' },
+          group_update: { joined: [bob?.get('uuid')], name: 'blerg' },
-        }).getNotificationData(),
+        }),
         {
           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: [] }
       );
     });

ts/util/getTitle.ts

@@ -108,7 +108,7 @@ export function getSystemName(
 
 export function getNumber(
   attributes: Pick<ConversationAttributesType, 'e164' | 'type'>
-): 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;
   }
 }