Skip to content

Commit 9b6f6fb

Browse files
committed
android notif: Parse pm_users in the "crunchy shell".
When we pass this on to our JS code after the user opens a notification, that code will go and try to parse it. Ensure up front, here in our crunchy shell, that it will actually parse successfully. A further good step would be to actually pass JS a list of ints, rather than re-encode as a string. That can be separate, though.
1 parent 1df6fb6 commit 9b6f6fb

File tree

3 files changed

+12
-8
lines changed

3 files changed

+12
-8
lines changed

android/app/src/main/java/com/zulipmobile/notifications/FcmMessage.kt

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,11 @@ internal sealed class Recipient {
4949
/**
5050
* A group PM.
5151
*
52-
* pmUsers: the user IDs of all users in the conversation, sorted,
53-
* in ASCII decimal, comma-separated.
52+
* pmUsers: the user IDs of all users in the conversation.
5453
*/
55-
data class GroupPm(val pmUsers: String) : Recipient()
54+
data class GroupPm(val pmUsers: Set<Int>) : Recipient() {
55+
fun getPmUsersString() = pmUsers.sorted().joinToString { toString() }
56+
}
5657

5758
/** A stream message. */
5859
data class Stream(val stream: String, val topic: String) : Recipient()
@@ -120,7 +121,7 @@ internal data class MessageFcmMessage(
120121
}
121122
is Recipient.GroupPm -> {
122123
bundle.putString("recipient_type", "private")
123-
bundle.putString("pm_users", recipient.pmUsers)
124+
bundle.putString("pm_users", recipient.getPmUsersString())
124125
}
125126
is Recipient.Pm -> {
126127
bundle.putString("recipient_type", "private")
@@ -157,8 +158,8 @@ internal data class MessageFcmMessage(
157158
data.require("stream"),
158159
data.require("topic"))
159160
"private" ->
160-
data["pm_users"]?.let {
161-
Recipient.GroupPm(it)
161+
data["pm_users"]?.parseCommaSeparatedInts("pm_users")?.let {
162+
Recipient.GroupPm(it.toSet())
162163
} ?: Recipient.Pm
163164
else -> throw FcmMessageParseException("unexpected recipient_type: $recipientType")
164165
}

android/app/src/main/java/com/zulipmobile/notifications/NotificationHelper.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ private static String buildKeyString(MessageFcmMessage fcmMessage) {
9696
((Recipient.Stream) recipient).getStream());
9797
else if (recipient instanceof Recipient.GroupPm) {
9898
return String.format("%s:%s:group", fcmMessage.getSender().getFullName(),
99-
((Recipient.GroupPm) recipient).getPmUsers());
99+
((Recipient.GroupPm) recipient).getPmUsersString());
100100
} else {
101101
return String.format("%s:%s:private", fcmMessage.getSender().getFullName(),
102102
fcmMessage.getSender().getEmail());

android/app/src/test/java/com/zulipmobile/notifications/FcmMessageTest.kt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ class MessageFcmMessageTest : FcmMessageTestBase() {
121121
)
122122
)
123123
expect.that(parse(Example.groupPm).recipient).isEqualTo(
124-
Recipient.GroupPm(pmUsers = Example.groupPm["pm_users"]!!)
124+
Recipient.GroupPm(pmUsers = setOf(123, 234, 345))
125125
)
126126
expect.that(parse(Example.pm).recipient).isEqualTo(
127127
Recipient.Pm
@@ -151,6 +151,9 @@ class MessageFcmMessageTest : FcmMessageTestBase() {
151151
assertParseFails(Example.stream.minus("stream"))
152152
assertParseFails(Example.stream.minus("topic"))
153153
assertParseFails(Example.groupPm.minus("recipient_type"))
154+
assertParseFails(Example.groupPm.plus("pm_users" to "abc,34"))
155+
assertParseFails(Example.groupPm.plus("pm_users" to "12,abc"))
156+
assertParseFails(Example.groupPm.plus("pm_users" to "12,"))
154157
assertParseFails(Example.pm.minus("recipient_type"))
155158
assertParseFails(Example.pm.plus("recipient_type" to "nonsense"))
156159

0 commit comments

Comments
 (0)