diff --git a/src/api/eventTypes.js b/src/api/eventTypes.js index cc2b85f0377..d9f552d53ba 100644 --- a/src/api/eventTypes.js +++ b/src/api/eventTypes.js @@ -13,12 +13,26 @@ import type { Message, Stream, UserPresence } from './modelTypes'; export class EventTypes { + static alert_words: 'alert_words' = 'alert_words'; + static delete_message: 'delete_message' = 'delete_message'; static heartbeat: 'heartbeat' = 'heartbeat'; static message: 'message' = 'message'; + static muted_topics: 'muted_topics' = 'muted_topics'; static presence: 'presence' = 'presence'; + static reaction: 'reaction' = 'reaction'; + static realm_bot: 'realm_bot' = 'realm_bot'; + static realm_emoji: 'realm_emoji' = 'realm_emoji'; + static realm_filters: 'realm_filters' = 'realm_filters'; + static realm_user: 'realm_user' = 'realm_user'; static stream: 'stream' = 'stream'; static submessage: 'submessage' = 'submessage'; + static subscription: 'subscription' = 'subscription'; + static typing: 'typing' = 'typing'; + static update_display_settings: 'update_display_settings' = 'update_display_settings'; + static update_global_notifications: 'update_global_notifications' = 'update_global_notifications'; + static update_message: 'update_message' = 'update_message'; static update_message_flags: 'update_message_flags' = 'update_message_flags'; + static user_group: 'user_group' = 'user_group'; static user_status: 'user_status' = 'user_status'; } diff --git a/src/api/messages/__tests__/migrateMessages-test.js b/src/api/messages/__tests__/migrateMessages-test.js index 9f6339f679f..83d60bc9e5c 100644 --- a/src/api/messages/__tests__/migrateMessages-test.js +++ b/src/api/messages/__tests__/migrateMessages-test.js @@ -2,6 +2,7 @@ import omit from 'lodash.omit'; import { migrateMessages } from '../getMessages'; +import { identityOfAuth } from '../../../account/accountMisc'; import * as eg from '../../../__tests__/lib/exampleData'; import type { ServerMessage, ServerReaction } from '../getMessages'; import type { Message } from '../../modelTypes'; @@ -20,11 +21,13 @@ describe('migrateMessages', () => { }, }; + type CommonFields = $Diff; + const serverMessage: ServerMessage = { // The `omit` shouldn't be necessary with Flow v0.111: "Spreads // now overwrite properties instead of inferring unions" // (https://medium.com/flow-type/spreads-common-errors-fixes-9701012e9d58). - ...(omit(eg.streamMessage(), 'reactions'): $Diff), + ...(omit(eg.streamMessage(), 'reactions'): CommonFields), reactions: [serverReaction], }; @@ -33,7 +36,7 @@ describe('migrateMessages', () => { const expectedOutput: Message[] = [ { // The `omit` shouldn't be necessary with Flow v0.111. - ...(omit(serverMessage, 'reactions'): $Diff), + ...(omit(serverMessage, 'reactions'): CommonFields), reactions: [ { user_id: reactingUser.user_id, @@ -45,7 +48,7 @@ describe('migrateMessages', () => { }, ]; - const actualOutput: Message[] = migrateMessages(input); + const actualOutput: Message[] = migrateMessages(input, identityOfAuth(eg.selfAuth)); test('Replace user object with `user_id`', () => { expect(actualOutput.map(m => m.reactions)).toEqual(expectedOutput.map(m => m.reactions)); diff --git a/src/api/messages/getMessages.js b/src/api/messages/getMessages.js index ae81c76fb74..9af24c7cae8 100644 --- a/src/api/messages/getMessages.js +++ b/src/api/messages/getMessages.js @@ -1,8 +1,10 @@ /* @flow strict-local */ import type { Auth, ApiResponseSuccess } from '../transportTypes'; +import type { Identity } from '../../types'; import type { Message, Narrow } from '../apiTypes'; import type { Reaction } from '../modelTypes'; import { apiGet } from '../apiFetch'; +import { identityOfAuth } from '../../account/accountMisc'; type ApiResponseMessages = {| ...ApiResponseSuccess, @@ -41,7 +43,7 @@ type ServerApiResponseMessages = {| |}; /** Exported for tests only. */ -export const migrateMessages = (messages: ServerMessage[]): Message[] => +export const migrateMessages = (messages: ServerMessage[], identity: Identity): Message[] => messages.map(message => { const { reactions, ...restMessage } = message; return { @@ -56,11 +58,11 @@ export const migrateMessages = (messages: ServerMessage[]): Message[] => }; }); -const migrateResponse = response => { +const migrateResponse = (response, identity: Identity) => { const { messages, ...restResponse } = response; return { ...restResponse, - messages: migrateMessages(messages), + messages: migrateMessages(messages, identity), }; }; @@ -91,5 +93,5 @@ export default async ( apply_markdown: true, use_first_unread_anchor: useFirstUnread, }); - return migrateResponse(response); + return migrateResponse(response, identityOfAuth(auth)); }; diff --git a/src/events/__tests__/eventActions--test.js b/src/events/__tests__/eventActions--test.js index df2854e31aa..ee5d1106d0b 100644 --- a/src/events/__tests__/eventActions--test.js +++ b/src/events/__tests__/eventActions--test.js @@ -1,18 +1,18 @@ -import { responseToActions } from '../eventActions'; +import { eventsToActions } from '../eventActions'; console.log = () => {}; // eslint-disable-line -describe('responseToActions', () => { +describe('eventsToActions', () => { test('empty response return no actions', () => { const events = []; - const actions = responseToActions({}, events); + const actions = eventsToActions({}, events); expect(actions).toBeEmpty(); }); test('filter out unknown event types and some known ones', () => { const events = [{ type: 'some unknown type' }, { type: 'heartbeat' }]; - const actions = responseToActions({}, events); + const actions = eventsToActions({}, events); expect(actions).toBeEmpty(); }); @@ -20,7 +20,7 @@ describe('responseToActions', () => { test('when known events process and return actions', () => { const event = { type: 'presence' }; const events = [event]; - const actions = responseToActions({}, events); + const actions = eventsToActions({}, events); expect(actions).toHaveLength(1); }); diff --git a/src/events/__tests__/eventMiddleware-test.js b/src/events/__tests__/eventMiddleware-test.js deleted file mode 100644 index 4ec315c6e17..00000000000 --- a/src/events/__tests__/eventMiddleware-test.js +++ /dev/null @@ -1,27 +0,0 @@ -import eventMiddleware from '../eventMiddleware'; -import { NULL_ARRAY } from '../../nullObjects'; - -describe('eventMiddleware', () => { - test('if `event.flags` key exists, move it to `event.message.flags`', () => { - const state = { session: {} }; - const event = { - type: 'message', - flags: ['mentioned'], - message: {}, - }; - eventMiddleware(state, event); - - expect(event.flags).not.toBeDefined(); - expect(event.message.flags).toEqual(['mentioned']); - }); - - test('if `event.flags` does not exist, set message.flags to an empty array', () => { - const state = { session: {} }; - const event = { - type: 'message', - message: {}, - }; - eventMiddleware(state, event); - expect(event.message.flags).toEqual(NULL_ARRAY); - }); -}); diff --git a/src/events/doEventActionSideEffects.js b/src/events/doEventActionSideEffects.js new file mode 100644 index 00000000000..51578c63f96 --- /dev/null +++ b/src/events/doEventActionSideEffects.js @@ -0,0 +1,57 @@ +/* @flow strict-local */ +// import { Vibration } from 'react-native'; + +import { AppState } from 'react-native'; +import type { GlobalState, GetState, Dispatch, Message } from '../types'; +import type { EventAction } from '../actionTypes'; +import { EVENT_NEW_MESSAGE } from '../actionConstants'; +import { isHomeNarrow, isMessageInNarrow } from '../utils/narrow'; +import { getActiveAccount, getChatScreenParams, getOwnEmail } from '../selectors'; +import { playMessageSound } from '../utils/sound'; +import { NULL_ARRAY } from '../nullObjects'; + +/** + * React to incoming `MessageEvent`s. + */ +const messageEvent = (state: GlobalState, message: Message): void => { + const flags = message.flags ?? NULL_ARRAY; + + if (AppState.currentState !== 'active') { + return; + } + + const isPrivateMessage = Array.isArray(message.display_recipient); + const isMentioned = flags.includes('mentioned') || flags.includes('wildcard_mentioned'); + if (!(isPrivateMessage || isMentioned)) { + return; + } + + const activeAccount = getActiveAccount(state); + const { narrow } = getChatScreenParams(state); + const isUserInSameNarrow = + activeAccount + && narrow !== undefined // chat screen is not at top + && !isHomeNarrow(narrow) + && isMessageInNarrow(message, narrow, activeAccount.email); + const isSenderSelf = getOwnEmail(state) === message.sender_email; + if (!isUserInSameNarrow && !isSenderSelf) { + playMessageSound(); + // Vibration.vibrate(); + } +}; + +/** + * React to actions dispatched for Zulip server events. + * + * To be dispatched after the event actions are dispatched. + */ +export default (action: EventAction) => async (dispatch: Dispatch, getState: GetState) => { + const state = getState(); + switch (action.type) { + case EVENT_NEW_MESSAGE: { + messageEvent(state, action.message); + break; + } + default: + } +}; diff --git a/src/events/eventActions.js b/src/events/eventActions.js index 09b4f49c345..bce0efc25b5 100644 --- a/src/events/eventActions.js +++ b/src/events/eventActions.js @@ -1,34 +1,32 @@ /* @flow strict-local */ import { batchActions } from 'redux-batched-actions'; +import type { EventAction } from '../actionTypes'; import type { Action, Dispatch, GeneralEvent, GetState, GlobalState } from '../types'; import * as api from '../api'; import { logout } from '../account/accountActions'; import { deadQueue } from '../session/sessionActions'; import eventToAction from './eventToAction'; -import eventMiddleware from './eventMiddleware'; +import doEventActionSideEffects from './doEventActionSideEffects'; import { tryGetAuth } from '../selectors'; import actionCreator from '../actionCreator'; import { BackoffMachine } from '../utils/async'; import { ApiError } from '../api/apiErrors'; /** Convert an `/events` response into a sequence of our Redux actions. */ -export const responseToActions = ( +export const eventsToActions = ( state: GlobalState, events: $ReadOnlyArray, -): Action[] => +): EventAction[] => events - .map(event => { - eventMiddleware(state, event); - return eventToAction(state, event); - }) + .map(event => eventToAction(state, event)) .filter(action => { if (action.type === 'ignore') { return false; } - if (!action || !action.type || action.type === 'unknown') { - console.log('Can not handle event', action.event); // eslint-disable-line + if (action.type === 'unknown') { + console.log('Cannot handle event', action.event); // eslint-disable-line return false; } @@ -73,11 +71,18 @@ export const startEventPolling = (queueId: number, eventId: number) => async ( break; } - const actions = responseToActions(getState(), events); + const actions = eventsToActions(getState(), events); actionCreator(dispatch, actions, getState()); dispatchOrBatch(dispatch, actions); + actions.forEach(action => { + // These side effects should not be moved to reducers, which + // are explicitly not the place for side effects (see + // https://redux.js.org/faq/actions). + dispatch(doEventActionSideEffects(action)); + }); + lastEventId = Math.max.apply(null, [lastEventId, ...events.map(x => x.id)]); } catch (e) { if (e.httpStatus === 401) { diff --git a/src/events/eventMiddleware.js b/src/events/eventMiddleware.js deleted file mode 100644 index 24801f91763..00000000000 --- a/src/events/eventMiddleware.js +++ /dev/null @@ -1,63 +0,0 @@ -/* @flow strict-local */ -// import { Vibration } from 'react-native'; - -import { AppState } from 'react-native'; -import type { GeneralEvent, GlobalState, MessageEvent } from '../types'; -import { isHomeNarrow, isMessageInNarrow } from '../utils/narrow'; -import { getActiveAccount, getChatScreenParams, getOwnEmail } from '../selectors'; -import { playMessageSound } from '../utils/sound'; -import { NULL_ARRAY } from '../nullObjects'; - -/** - * React to incoming `MessageEvent`s. - * - * Beware: this function modifies its `event` argument, and has other side - * effects besides! - */ -const messageEvent = (state: GlobalState, event: MessageEvent): void => { - // Move `flags` key from `event` to `event.message` for consistency, and - // default to an empty array if `event.flags` is not set. - const flags = event.message.flags ?? event.flags ?? NULL_ARRAY; - if (!event.message.flags) { - // $FlowFixMe: Message is readonly to serve our use of it in Redux. - event.message.flags = flags; - delete event.flags; - } - - if (AppState.currentState !== 'active') { - return; - } - - const isPrivateMessage = Array.isArray(event.message.display_recipient); - const isMentioned = flags.includes('mentioned') || flags.includes('wildcard_mentioned'); - if (!(isPrivateMessage || isMentioned)) { - return; - } - - const activeAccount = getActiveAccount(state); - const { narrow } = getChatScreenParams(state); - const isUserInSameNarrow = - activeAccount - && (narrow !== undefined // chat screen is not at top - && (!isHomeNarrow(narrow) && isMessageInNarrow(event.message, narrow, activeAccount.email))); - const isSenderSelf = getOwnEmail(state) === event.message.sender_email; - if (!isUserInSameNarrow && !isSenderSelf) { - playMessageSound(); - // Vibration.vibrate(); - } -}; - -/** - * React to, and possibly normalize, incoming Zulip server events (from - * `/events` polling). - */ -export default (state: GlobalState, event_: GeneralEvent) => { - switch (event_.type) { - case 'message': { - // $FlowFixMe This expresses our unchecked assumptions about `message` events. - messageEvent(state, (event_: MessageEvent)); - break; - } - default: - } -}; diff --git a/src/events/eventToAction.js b/src/events/eventToAction.js index 3ad7c616911..43480262a2e 100644 --- a/src/events/eventToAction.js +++ b/src/events/eventToAction.js @@ -82,8 +82,15 @@ export default (state: GlobalState, event: $FlowFixMe): EventAction => { case 'message': return { - ...event, type: EVENT_NEW_MESSAGE, + id: event.id, + message: { + ...event.message, + // Move `flags` key from `event` to `event.message` for consistency, and + // default to an empty array if `event.flags` is not set. + flags: event.message.flags ?? event.flags ?? [], + }, + local_message_id: event.local_message_id, caughtUp: state.caughtUp, ownEmail: getOwnEmail(state), }; diff --git a/src/message/__tests__/fetchActions-test.js b/src/message/__tests__/fetchActions-test.js index 96f2f2926e8..5c19ba49116 100644 --- a/src/message/__tests__/fetchActions-test.js +++ b/src/message/__tests__/fetchActions-test.js @@ -1,11 +1,14 @@ /* @flow strict-local */ import configureStore from 'redux-mock-store'; import thunk from 'redux-thunk'; +import omit from 'lodash.omit'; import type { GlobalState } from '../../reduxTypes'; import type { Action } from '../../actionTypes'; import { isFetchNeededAtAnchor, fetchMessages, fetchOlder, fetchNewer } from '../fetchActions'; import { FIRST_UNREAD_ANCHOR } from '../../anchor'; +import type { Message } from '../../api/modelTypes'; +import type { ServerMessage } from '../../api/messages/getMessages'; import { streamNarrow, HOME_NARROW, HOME_NARROW_STR } from '../../utils/narrow'; import { navStateWithNarrow } from '../../utils/testHelpers'; import * as eg from '../../__tests__/lib/exampleData'; @@ -73,8 +76,6 @@ describe('fetchActions', () => { describe('fetchMessages', () => { const message1 = eg.streamMessage({ id: 1 }); - const message2 = eg.streamMessage({ id: 2 }); - const message3 = eg.streamMessage({ id: 3 }); const baseState = eg.reduxState({ ...navStateWithNarrow(HOME_NARROW), @@ -86,8 +87,20 @@ describe('fetchActions', () => { describe('success', () => { beforeEach(() => { + type CommonFields = $Diff; + + // A message exactly as we receive it from the server, before + // our own transformations. + // + // TODO: Deduplicate this logic with similar logic in + // migrateMessages-test.js. + const serverMessage1: ServerMessage = { + ...(omit(message1, 'reactions'): CommonFields), + reactions: [], + }; + const response = { - messages: [message1, message2, message3], + messages: [serverMessage1], result: 'success', }; fetch.mockResponseSuccess(JSON.stringify(response)); @@ -133,7 +146,7 @@ describe('fetchActions', () => { expect(actions[1].type).toBe('MESSAGE_FETCH_COMPLETE'); - expect(returnValue).toEqual([message1, message2, message3]); + expect(returnValue).toEqual([message1]); }); });