From 240f15018c1a30583b8456e18a92f327b93778d4 Mon Sep 17 00:00:00 2001 From: Ray Kraesig Date: Thu, 19 Dec 2019 17:52:36 -0800 Subject: [PATCH 1/3] api.sendMessage [nfc]: Accept a params-object. ... rather than having a seven-parameter-long signature. --- src/api/messages/sendMessage.js | 26 ++++++++++++++------------ src/outbox/outboxActions.js | 17 ++++++++--------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/api/messages/sendMessage.js b/src/api/messages/sendMessage.js index 47d88c5e769..67d73924c07 100644 --- a/src/api/messages/sendMessage.js +++ b/src/api/messages/sendMessage.js @@ -6,18 +6,20 @@ import { apiPost } from '../apiFetch'; /** See https://zulipchat.com/api/send-message */ export default async ( auth: Auth, - type: 'private' | 'stream', - to: string, - subject: string, - content: string, - localId: number, - eventQueueId: number, + params: {| + type: 'private' | 'stream', + to: string, + subject: string, + content: string, + localId: number, + eventQueueId: number, + |}, ): Promise => apiPost(auth, 'messages', { - type, - to, - subject, - content, - local_id: localId, - queue_id: eventQueueId, + type: params.type, + to: params.to, + subject: params.subject, + content: params.content, + local_id: params.localId, + queue_id: params.eventQueueId, }); diff --git a/src/outbox/outboxActions.js b/src/outbox/outboxActions.js index a6d9248c2e0..a95fa860656 100644 --- a/src/outbox/outboxActions.js +++ b/src/outbox/outboxActions.js @@ -52,15 +52,14 @@ export const trySendMessages = (dispatch: Dispatch, getState: GetState): boolean const outboxToSend = state.outbox.filter(outbox => !outbox.isSent); try { outboxToSend.forEach(async item => { - await api.sendMessage( - auth, - item.type, - isPrivateOrGroupNarrow(item.narrow) ? item.narrow[0].operand : item.display_recipient, - item.subject, - item.markdownContent, - item.timestamp, - state.session.eventQueueId, - ); + await api.sendMessage(auth, { + type: item.type, + to: isPrivateOrGroupNarrow(item.narrow) ? item.narrow[0].operand : item.display_recipient, + subject: item.subject, + content: item.markdownContent, + localId: item.timestamp, + eventQueueId: state.session.eventQueueId, + }); dispatch(messageSendComplete(item.timestamp)); }); return true; From 8158dae7fd844879f7ad2d9e992cfca757809962 Mon Sep 17 00:00:00 2001 From: Ray Kraesig Date: Tue, 7 Jan 2020 16:34:13 -0800 Subject: [PATCH 2/3] outbox: Discard messages unsendable for more than a week. ... at which point, they're more likely to generate confusion than to be useful. (This isn't a great solution, since it *does* discard data -- but it's probably the lesser evil.) --- src/outbox/outboxActions.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/outbox/outboxActions.js b/src/outbox/outboxActions.js index a95fa860656..4d8322d001a 100644 --- a/src/outbox/outboxActions.js +++ b/src/outbox/outboxActions.js @@ -50,8 +50,20 @@ export const trySendMessages = (dispatch: Dispatch, getState: GetState): boolean const state = getState(); const auth = getAuth(state); const outboxToSend = state.outbox.filter(outbox => !outbox.isSent); + const oneWeekAgoTimestamp = Date.now() / 1000 - 60 * 60 * 24 * 7; try { outboxToSend.forEach(async item => { + // If a message has spent over a week in the outbox, it's probably too + // stale to try sending it. + // + // TODO: instead of just throwing these away, create an "unsendable" state + // (including a reason for unsendability), and transition old messages to + // that instead. + if (item.timestamp < oneWeekAgoTimestamp) { + dispatch(deleteOutboxMessage(item.id)); + return; // i.e., continue + } + await api.sendMessage(auth, { type: item.type, to: isPrivateOrGroupNarrow(item.narrow) ? item.narrow[0].operand : item.display_recipient, From e4a83be68794ae6f249367597afc52f27af30f8a Mon Sep 17 00:00:00 2001 From: Ray Kraesig Date: Thu, 19 Dec 2019 17:25:55 -0800 Subject: [PATCH 3/3] sending messages: Always use JSON for `to` field for streams. ... to avoid its potential server-side misinterpretation as CSV when the stream name has commas. The Zulip server appears to have accepted JSON in this field at least as far back as 2013, and possibly farther -- the relevant code dates to zulip/zulip@97d7d31b684aae36d1406b9157fc230d6d5f8e93 or thereabouts in its current form, but JSON appears to have been accepted even then. Fixes #3729. --- src/outbox/outboxActions.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/outbox/outboxActions.js b/src/outbox/outboxActions.js index 4d8322d001a..a422b692639 100644 --- a/src/outbox/outboxActions.js +++ b/src/outbox/outboxActions.js @@ -64,9 +64,21 @@ export const trySendMessages = (dispatch: Dispatch, getState: GetState): boolean return; // i.e., continue } + const to = ((): string => { + const { narrow } = item; + // TODO: can this test be `if (item.type === private)`? + if (isPrivateOrGroupNarrow(narrow)) { + return narrow[0].operand; + } else { + // HACK: the server attempts to interpret this argument as JSON, then + // CSV, then a literal. To avoid misparsing, always use JSON. + return JSON.stringify([item.display_recipient]); + } + })(); + await api.sendMessage(auth, { type: item.type, - to: isPrivateOrGroupNarrow(item.narrow) ? item.narrow[0].operand : item.display_recipient, + to, subject: item.subject, content: item.markdownContent, localId: item.timestamp,