Skip to content

Commit

Permalink
android notif: Require realm_uri in payload, present since server 1.9.
Browse files Browse the repository at this point in the history
  • Loading branch information
gnprice committed Sep 13, 2021
1 parent 144dbb1 commit 9620629
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ data class Identity(
/// the base for all URLs a client uses with this realm. It corresponds
/// to `realm_uri` in the `server_settings` API response:
/// https://zulip.com/api/get-server-settings
val realmUri: URL?,
val realmUri: URL,

/// This user's ID within the server. Useful mainly in the case where
/// the user has multiple accounts in the same org.
Expand Down Expand Up @@ -111,7 +111,7 @@ data class MessageFcmMessage(
*/
fun dataForOpen(): Bundle = Bundle().apply {
// NOTE: Keep the JS-side type definition in sync with this code.
identity.realmUri?.let { putString("realm_uri", it.toString()) }
identity.realmUri.let { putString("realm_uri", it.toString()) }
when (recipient) {
is Recipient.Stream -> {
putString("recipient_type", "stream")
Expand Down Expand Up @@ -190,10 +190,7 @@ private fun extractIdentity(data: Map<String, String>): Identity =
Identity(
serverHost = data.require("server"),
realmId = data.require("realm_id").parseInt("realm_id"),

// `realm_uri` was added in server version 1.9.0
// (released 2018-11-06; commit 5f8d193bb).
realmUri = data["realm_uri"]?.parseUrl("realm_uri"),
realmUri = data.require("realm_uri").parseUrl("realm_uri"),

// Server versions from 1.6.0 through 2.0.0 (and possibly earlier
// and later) send the user's email address, as `user`. We *could*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,6 @@ class MessageFcmMessageTest : FcmMessageTestBase() {

@Test
fun `optional fields missing cause no error`() {
expect.that(parse(Example.pm.minus("realm_uri")).identity.serverHost)
.isEqualTo(Example.stream["server"]!!)
expect.that(parse(Example.pm.minus("realm_uri")).identity.realmUri).isNull()
expect.that(parse(Example.pm.minus("user_id")).identity.userId).isNull()
}

Expand All @@ -164,6 +161,7 @@ class MessageFcmMessageTest : FcmMessageTestBase() {
assertParseFails(Example.pm.minus("realm_id"))
assertParseFails(Example.pm.plus("realm_id" to "12,34"))
assertParseFails(Example.pm.plus("realm_id" to "abc"))
assertParseFails(Example.pm.minus("realm_uri"))
assertParseFails(Example.pm.plus("realm_uri" to "zulip.example.com"))
assertParseFails(Example.pm.plus("realm_uri" to "/examplecorp"))

Expand Down Expand Up @@ -252,9 +250,6 @@ class RemoveFcmMessageTest : FcmMessageTestBase() {

@Test
fun `optional fields missing cause no error`() {
expect.that(parse(Example.hybrid.minus("realm_uri")).identity.serverHost)
.isEqualTo(Example.hybrid["server"]!!)
expect.that(parse(Example.hybrid.minus("realm_uri")).identity.realmUri).isNull()
expect.that(parse(Example.hybrid.minus("user_id")).identity.userId).isNull()
}

Expand All @@ -264,6 +259,7 @@ class RemoveFcmMessageTest : FcmMessageTestBase() {
assertParseFails(Example.hybrid.minus("realm_id"))
assertParseFails(Example.hybrid.plus("realm_id" to "abc"))
assertParseFails(Example.hybrid.plus("realm_id" to "12,34"))
assertParseFails(Example.hybrid.minus("realm_uri"))
assertParseFails(Example.hybrid.plus("realm_uri" to "zulip.example.com"))
assertParseFails(Example.hybrid.plus("realm_uri" to "/examplecorp"))

Expand Down
3 changes: 3 additions & 0 deletions src/notification/__tests__/notification-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ describe('getNarrowFromNotificationData', () => {

test('recognizes stream notifications and returns topic narrow', () => {
const notification = {
realm_uri,
recipient_type: 'stream',
stream: 'some stream',
topic: 'some topic',
Expand All @@ -37,6 +38,7 @@ describe('getNarrowFromNotificationData', () => {
const users = [eg.selfUser, eg.otherUser];
const allUsersByEmail: Map<string, UserOrBot> = new Map(users.map(u => [u.email, u]));
const notification = {
realm_uri,
recipient_type: 'private',
sender_email: eg.otherUser.email,
};
Expand All @@ -49,6 +51,7 @@ describe('getNarrowFromNotificationData', () => {
const allUsersByEmail: Map<string, UserOrBot> = new Map(users.map(u => [u.email, u]));

const notification = {
realm_uri,
recipient_type: 'private',
pm_users: users.map(u => u.user_id).join(','),
};
Expand Down
6 changes: 3 additions & 3 deletions src/notification/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
*/
// NOTE: Keep the Android-side code in sync with this type definition.
export type Notification =
| {| recipient_type: 'stream', stream: string, topic: string, realm_uri?: string |}
| {| recipient_type: 'stream', stream: string, topic: string, realm_uri: string |}
// Group PM messages have `pm_users`, which is sorted, comma-separated IDs.
| {| recipient_type: 'private', pm_users: string, realm_uri?: string |}
| {| recipient_type: 'private', pm_users: string, realm_uri: string |}
// 1:1 PM messages lack `pm_users`.
| {| recipient_type: 'private', sender_email: string, realm_uri?: string |};
| {| recipient_type: 'private', sender_email: string, realm_uri: string |};

0 comments on commit 9620629

Please sign in to comment.