From 92bee0ed42f4c76889327acb7126ce50b0d3a389 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 21 Feb 2020 13:40:13 -0800 Subject: [PATCH] notif: Separate ids in pm_users for group PMs with ',' instead of ', '. ', ' would have been handled correctly, but seemingly by accident; in getNarrowFromNotificationData, pm_users was split on ',' to give ['1', ' 2', ' 3'] (note the spaces), then + was used on elements of that array, as in +idStrs[i], to convert to a number. Also, replace the confusing + syntax with parseInt. --- .../main/java/com/zulipmobile/notifications/FcmMessage.kt | 2 +- .../java/com/zulipmobile/notifications/FcmMessageTest.kt | 8 ++++---- src/notification/__tests__/notification-test.js | 4 ++-- src/notification/index.js | 3 ++- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/android/app/src/main/java/com/zulipmobile/notifications/FcmMessage.kt b/android/app/src/main/java/com/zulipmobile/notifications/FcmMessage.kt index aa7c21458e0..49b9674011e 100644 --- a/android/app/src/main/java/com/zulipmobile/notifications/FcmMessage.kt +++ b/android/app/src/main/java/com/zulipmobile/notifications/FcmMessage.kt @@ -51,7 +51,7 @@ sealed class Recipient { * pmUsers: the user IDs of all users in the conversation. */ data class GroupPm(val pmUsers: Set) : Recipient() { - fun getPmUsersString() = pmUsers.sorted().joinToString { it.toString() } + fun getPmUsersString() = pmUsers.sorted().joinToString(separator=",") { it.toString() } } /** A stream message. */ diff --git a/android/app/src/test/java/com/zulipmobile/notifications/FcmMessageTest.kt b/android/app/src/test/java/com/zulipmobile/notifications/FcmMessageTest.kt index 43ba125ad4d..37253c87544 100644 --- a/android/app/src/test/java/com/zulipmobile/notifications/FcmMessageTest.kt +++ b/android/app/src/test/java/com/zulipmobile/notifications/FcmMessageTest.kt @@ -55,13 +55,13 @@ class FcmMessageTest : FcmMessageTestBase() { class RecipientTest : FcmMessageTestBase() { @Test fun `GroupPm#getPmUsersString gives the correct string`() { - expect.that(Recipient.GroupPm(pmUsers = setOf(123, 234, 345)).getPmUsersString()) - .isEqualTo("123, 234, 345") + expect.that(Recipient.GroupPm(pmUsers = setOf(123,234,345)).getPmUsersString()) + .isEqualTo("123,234,345") } @Test fun `GroupPm#getPmUsersString correctly reorders user ids`() { - expect.that(Recipient.GroupPm(pmUsers = setOf(234, 123, 23, 345)).getPmUsersString()) - .isEqualTo("23, 123, 234, 345") + expect.that(Recipient.GroupPm(pmUsers = setOf(234,123,23,345)).getPmUsersString()) + .isEqualTo("23,123,234,345") } } diff --git a/src/notification/__tests__/notification-test.js b/src/notification/__tests__/notification-test.js index 8daba25744c..2e9acc28828 100644 --- a/src/notification/__tests__/notification-test.js +++ b/src/notification/__tests__/notification-test.js @@ -30,7 +30,7 @@ describe('getNarrowFromNotificationData', () => { test('on notification for a group message returns a group narrow', () => { const notification = { recipient_type: 'private', - pm_users: '1, 2, 4', + pm_users: '1,2,4', }; const usersById = new Map([ [1, { email: 'me@example.com' }], @@ -47,7 +47,7 @@ describe('getNarrowFromNotificationData', () => { test('do not throw when users are not found; return null', () => { const notification = { recipient_type: 'private', - pm_users: '1, 2, 4', + pm_users: '1,2,4', }; const usersById = new Map(); diff --git a/src/notification/index.js b/src/notification/index.js index 7b346204005..c2392afbca3 100644 --- a/src/notification/index.js +++ b/src/notification/index.js @@ -121,7 +121,8 @@ export const getNarrowFromNotificationData = ( const emails = []; const idStrs = data.pm_users.split(','); for (let i = 0; i < idStrs.length; ++i) { - const user = usersById.get(+idStrs[i]); + const id = parseInt(idStrs[i], 10); + const user = usersById.get(id); if (!user) { return null; }