diff --git a/src/api/messages/getMessages.js b/src/api/messages/getMessages.js index 3e0ebc93589..7e5d114b03a 100644 --- a/src/api/messages/getMessages.js +++ b/src/api/messages/getMessages.js @@ -1,7 +1,7 @@ /* @flow strict-local */ import type { Auth, ApiResponseSuccess } from '../transportTypes'; import type { Identity } from '../../types'; -import type { Message, Narrow } from '../apiTypes'; +import type { Message, ApiNarrow } from '../apiTypes'; import type { Reaction } from '../modelTypes'; import { apiGet } from '../apiFetch'; import { identityOfAuth } from '../../account/accountMisc'; @@ -86,7 +86,7 @@ const migrateResponse = (response, identity: Identity) => { export default async ( auth: Auth, args: {| - narrow: Narrow, + narrow: ApiNarrow, anchor: number, numBefore: number, numAfter: number, diff --git a/src/api/modelTypes.js b/src/api/modelTypes.js index 638c6195e3d..7e9076882e4 100644 --- a/src/api/modelTypes.js +++ b/src/api/modelTypes.js @@ -312,7 +312,13 @@ export type NarrowElement = $ReadOnly<{| operator: NarrowOperator, |}>; -export type Narrow = $ReadOnlyArray; +/** + * A narrow, in the form used in the Zulip API at get-messages. + * + * See also `Narrow` in the non-API app code, which describes how we + * represent narrows within the app. + */ +export type ApiNarrow = $ReadOnlyArray; // // diff --git a/src/api/registerForEvents.js b/src/api/registerForEvents.js index 4f23f32b1f7..655849892d0 100644 --- a/src/api/registerForEvents.js +++ b/src/api/registerForEvents.js @@ -1,7 +1,7 @@ /* @flow strict-local */ import type { RawInitialData, InitialData } from './initialDataTypes'; import type { Auth } from './transportTypes'; -import type { Narrow } from './apiTypes'; +import type { ApiNarrow } from './apiTypes'; import type { CrossRealmBot, User } from './modelTypes'; import { apiPost } from './apiFetch'; import { AvatarURL } from '../utils/avatar'; @@ -13,7 +13,7 @@ type RegisterForEventsParams = {| event_types?: string[], fetch_event_types?: string[], include_subscribers?: boolean, - narrow?: Narrow, + narrow?: ApiNarrow, queue_lifespan_secs?: number, client_capabilities?: {| notification_settings_null: boolean, diff --git a/src/caughtup/__tests__/caughtUpReducer-test.js b/src/caughtup/__tests__/caughtUpReducer-test.js index 83a54d811cb..dd07f4ba47b 100644 --- a/src/caughtup/__tests__/caughtUpReducer-test.js +++ b/src/caughtup/__tests__/caughtUpReducer-test.js @@ -10,6 +10,7 @@ import { HOME_NARROW_STR, ALL_PRIVATE_NARROW, ALL_PRIVATE_NARROW_STR, + SEARCH_NARROW, } from '../../utils/narrow'; describe('caughtUpReducer', () => { @@ -42,7 +43,7 @@ describe('caughtUpReducer', () => { const action = deepFreeze({ ...eg.action.message_fetch_start, - narrow: [{ operator: 'search', operand: 'some query' }], + narrow: SEARCH_NARROW('some query'), }); const newState = caughtUpReducer(initialState, action); @@ -126,7 +127,7 @@ describe('caughtUpReducer', () => { const action = deepFreeze({ ...eg.action.message_fetch_complete, - narrow: [{ operator: 'search', operand: 'some query' }], + narrow: SEARCH_NARROW('some query'), }); const newState = caughtUpReducer(initialState, action); diff --git a/src/chat/MarkUnreadButton.js b/src/chat/MarkUnreadButton.js index d5f6abe15ed..bdf6a04fb3a 100644 --- a/src/chat/MarkUnreadButton.js +++ b/src/chat/MarkUnreadButton.js @@ -8,7 +8,13 @@ import { connect } from '../react-redux'; import { ZulipButton } from '../common'; import * as api from '../api'; import { getAuth, getStreams } from '../selectors'; -import { isHomeNarrow, isStreamNarrow, isTopicNarrow } from '../utils/narrow'; +import { + isHomeNarrow, + isStreamNarrow, + isTopicNarrow, + streamNameOfNarrow, + topicOfNarrow, +} from '../utils/narrow'; const styles = createStyleSheet({ button: { @@ -34,7 +40,8 @@ class MarkUnreadButton extends PureComponent { markStreamAsRead = () => { const { auth, narrow, streams } = this.props; - const stream = streams.find(s => s.name === narrow[0].operand); + const streamName = streamNameOfNarrow(narrow); + const stream = streams.find(s => s.name === streamName); if (stream) { api.markStreamAsRead(auth, stream.stream_id); } @@ -42,9 +49,10 @@ class MarkUnreadButton extends PureComponent { markTopicAsRead = () => { const { auth, narrow, streams } = this.props; - const stream = streams.find(s => s.name === narrow[0].operand); + const streamName = streamNameOfNarrow(narrow); + const stream = streams.find(s => s.name === streamName); if (stream) { - api.markTopicAsRead(auth, stream.stream_id, narrow[1].operand); + api.markTopicAsRead(auth, stream.stream_id, topicOfNarrow(narrow)); } }; diff --git a/src/chat/__tests__/fetchingReducer-test.js b/src/chat/__tests__/fetchingReducer-test.js index 64093bd808b..85294e84f73 100644 --- a/src/chat/__tests__/fetchingReducer-test.js +++ b/src/chat/__tests__/fetchingReducer-test.js @@ -3,7 +3,13 @@ import deepFreeze from 'deep-freeze'; import * as eg from '../../__tests__/lib/exampleData'; import fetchingReducer from '../fetchingReducer'; -import { HOME_NARROW, HOME_NARROW_STR, streamNarrow, keyFromNarrow } from '../../utils/narrow'; +import { + HOME_NARROW, + HOME_NARROW_STR, + streamNarrow, + keyFromNarrow, + SEARCH_NARROW, +} from '../../utils/narrow'; import { MESSAGE_FETCH_START, MESSAGE_FETCH_ERROR } from '../../actionConstants'; import { DEFAULT_FETCHING } from '../fetchingSelectors'; @@ -16,7 +22,7 @@ describe('fetchingReducer', () => { const action = deepFreeze({ type: MESSAGE_FETCH_START, - narrow: [], + narrow: HOME_NARROW, numBefore: 10, numAfter: 10, }); @@ -62,7 +68,7 @@ describe('fetchingReducer', () => { const action = deepFreeze({ ...eg.action.message_fetch_start, - narrow: [{ operator: 'search', operand: 'some query' }], + narrow: SEARCH_NARROW('some query'), }); const newState = fetchingReducer(initialState, action); @@ -108,7 +114,7 @@ describe('fetchingReducer', () => { const action = { ...eg.action.message_fetch_complete, - narrow: [], + narrow: HOME_NARROW, numBefore: 10, numAfter: 0, }; @@ -130,7 +136,7 @@ describe('fetchingReducer', () => { const action = deepFreeze({ ...eg.action.message_fetch_complete, - narrow: [{ operator: 'search', operand: 'some query' }], + narrow: SEARCH_NARROW('some query'), }); const newState = fetchingReducer(initialState, action); diff --git a/src/chat/__tests__/narrowsReducer-test.js b/src/chat/__tests__/narrowsReducer-test.js index f661b6dd717..bd7d05b0fc3 100644 --- a/src/chat/__tests__/narrowsReducer-test.js +++ b/src/chat/__tests__/narrowsReducer-test.js @@ -13,6 +13,7 @@ import { topicNarrow, STARRED_NARROW_STR, keyFromNarrow, + SEARCH_NARROW, } from '../../utils/narrow'; import { MESSAGE_FETCH_ERROR, @@ -324,7 +325,7 @@ describe('narrowsReducer', () => { const action = deepFreeze({ ...eg.action.message_fetch_start, - narrow: [{ operator: 'search', operand: 'some query' }], + narrow: SEARCH_NARROW('some query'), }); const newState = narrowsReducer(initialState, action); @@ -396,7 +397,7 @@ describe('narrowsReducer', () => { const action = deepFreeze({ type: MESSAGE_FETCH_COMPLETE, anchor: 2, - narrow: [], + narrow: HOME_NARROW, messages: [ eg.streamMessage({ id: 2 }), eg.streamMessage({ id: 3 }), @@ -424,7 +425,7 @@ describe('narrowsReducer', () => { const action = deepFreeze({ type: MESSAGE_FETCH_COMPLETE, anchor: 2, - narrow: [], + narrow: HOME_NARROW, messages: [ eg.streamMessage({ id: 3, timestamp: 2 }), eg.streamMessage({ id: 4, timestamp: 1 }), @@ -451,7 +452,7 @@ describe('narrowsReducer', () => { const action = deepFreeze({ anchor: FIRST_UNREAD_ANCHOR, type: MESSAGE_FETCH_COMPLETE, - narrow: [], + narrow: HOME_NARROW, messages: [ eg.streamMessage({ id: 3, timestamp: 2 }), eg.streamMessage({ id: 4, timestamp: 1 }), @@ -477,7 +478,7 @@ describe('narrowsReducer', () => { const action = deepFreeze({ anchor: LAST_MESSAGE_ANCHOR, type: MESSAGE_FETCH_COMPLETE, - narrow: [], + narrow: HOME_NARROW, messages: [ eg.streamMessage({ id: 3, timestamp: 2 }), eg.streamMessage({ id: 4, timestamp: 1 }), @@ -502,7 +503,7 @@ describe('narrowsReducer', () => { const action = deepFreeze({ ...eg.action.message_fetch_complete, - narrow: [{ operator: 'search', operand: 'some query' }], + narrow: SEARCH_NARROW('some query'), }); const newState = narrowsReducer(initialState, action); diff --git a/src/chat/narrowsSelectors.js b/src/chat/narrowsSelectors.js index dc1ffb53660..b62d402011f 100644 --- a/src/chat/narrowsSelectors.js +++ b/src/chat/narrowsSelectors.js @@ -29,6 +29,7 @@ import { isMessageInNarrow, caseNarrowDefault, keyFromNarrow, + streamNameOfNarrow, } from '../utils/narrow'; import { shouldBeMuted } from '../utils/message'; import { NULL_ARRAY, NULL_SUBSCRIPTION } from '../nullObjects'; @@ -125,13 +126,14 @@ export const getStreamInNarrow: Selector x.name === narrow[0].operand); + const subscription = subscriptions.find(x => x.name === streamName); if (subscription) { return subscription; } - const stream = streams.find(x => x.name === narrow[0].operand); + const stream = streams.find(x => x.name === streamName); if (stream) { return { ...stream, diff --git a/src/compose/ComposeBox.js b/src/compose/ComposeBox.js index 7945354cc76..6fcbc82cdb8 100644 --- a/src/compose/ComposeBox.js +++ b/src/compose/ComposeBox.js @@ -33,7 +33,12 @@ import * as api from '../api'; import { FloatingActionButton, Input } from '../common'; import { showErrorAlert } from '../utils/info'; import { IconDone, IconSend } from '../common/Icons'; -import { isStreamNarrow, isStreamOrTopicNarrow, topicNarrow } from '../utils/narrow'; +import { + isStreamNarrow, + isStreamOrTopicNarrow, + streamNameOfNarrow, + topicNarrow, +} from '../utils/narrow'; import ComposeMenu from './ComposeMenu'; import getComposeInputPlaceholder from './getComposeInputPlaceholder'; import NotSubscribed from '../message/NotSubscribed'; @@ -307,8 +312,12 @@ class ComposeBox extends PureComponent { getDestinationNarrow = (): Narrow => { const { narrow } = this.props; - const topic = this.state.topic.trim(); - return isStreamNarrow(narrow) ? topicNarrow(narrow[0].operand, topic || '(no topic)') : narrow; + if (isStreamNarrow(narrow)) { + const streamName = streamNameOfNarrow(narrow); + const topic = this.state.topic.trim(); + return topicNarrow(streamName, topic || '(no topic)'); + } + return narrow; }; handleSend = () => { diff --git a/src/compose/getComposeInputPlaceholder.js b/src/compose/getComposeInputPlaceholder.js index ec24a0d1f55..ff7a5563633 100644 --- a/src/compose/getComposeInputPlaceholder.js +++ b/src/compose/getComposeInputPlaceholder.js @@ -1,42 +1,40 @@ /* @flow strict-local */ import type { Narrow, UserOrBot, LocalizableText } from '../types'; -import { isStreamNarrow, isTopicNarrow, isGroupPmNarrow, is1to1PmNarrow } from '../utils/narrow'; +import { caseNarrowDefault } from '../utils/narrow'; export default ( narrow: Narrow, ownEmail: string, usersByEmail: Map, -): LocalizableText => { - if (isGroupPmNarrow(narrow)) { - return { text: 'Message group' }; - } +): LocalizableText => + caseNarrowDefault( + narrow, + { + pm: emails => { + if (emails.length > 1) { + return { text: 'Message group' }; + } + const email = emails[0]; - if (is1to1PmNarrow(narrow)) { - if (ownEmail && narrow[0].operand === ownEmail) { - return { text: 'Jot down something' }; - } + if (email === ownEmail) { + return { text: 'Jot down something' }; + } - if (!usersByEmail) { - return { text: 'Type a message' }; - } + const user = usersByEmail.get(email); + if (!user) { + return { text: 'Type a message' }; + } - const user = usersByEmail.get(narrow[0].operand) || {}; - return { - text: 'Message {recipient}', - values: { recipient: `@${user.full_name}` }, - }; - } - - if (isStreamNarrow(narrow)) { - return { - text: 'Message {recipient}', - values: { recipient: `#${narrow[0].operand}` }, - }; - } - - if (isTopicNarrow(narrow)) { - return { text: 'Reply' }; - } - - return { text: 'Type a message' }; -}; + return { + text: 'Message {recipient}', + values: { recipient: `@${user.full_name}` }, + }; + }, + stream: name => ({ + text: 'Message {recipient}', + values: { recipient: `#${name}` }, + }), + topic: () => ({ text: 'Reply' }), + }, + () => ({ text: 'Type a message' }), + ); diff --git a/src/message/__tests__/messageActionSheet-test.js b/src/message/__tests__/messageActionSheet-test.js index 5372a61585f..d6f6b9f463b 100644 --- a/src/message/__tests__/messageActionSheet-test.js +++ b/src/message/__tests__/messageActionSheet-test.js @@ -1,5 +1,6 @@ // @flow strict-local import deepFreeze from 'deep-freeze'; +import { HOME_NARROW } from '../../utils/narrow'; import * as eg from '../../__tests__/lib/exampleData'; import { constructMessageActionButtons, constructHeaderActionButtons } from '../messageActionSheet'; @@ -18,7 +19,7 @@ const baseBackgroundData = deepFreeze({ }); describe('constructActionButtons', () => { - const narrow = deepFreeze([]); + const narrow = deepFreeze(HOME_NARROW); test('show star message option if message is not starred', () => { const message = eg.streamMessage(); @@ -53,7 +54,7 @@ describe('constructActionButtons', () => { }); describe('constructHeaderActionButtons', () => { - const narrow = deepFreeze([]); + const narrow = deepFreeze(HOME_NARROW); test('show Unmute topic option if topic is muted', () => { const mute = deepFreeze([['electron issues', 'issue #556']]); diff --git a/src/message/__tests__/renderMessages-test.js b/src/message/__tests__/renderMessages-test.js index 110111f4f7f..079c14db968 100644 --- a/src/message/__tests__/renderMessages-test.js +++ b/src/message/__tests__/renderMessages-test.js @@ -4,7 +4,7 @@ import { HOME_NARROW } from '../../utils/narrow'; import renderMessages from '../renderMessages'; describe('renderMessages', () => { - const narrow = deepFreeze([]); + const narrow = deepFreeze(HOME_NARROW); test('empty messages results in a single empty section', () => { const messageList = renderMessages([], HOME_NARROW); diff --git a/src/message/fetchActions.js b/src/message/fetchActions.js index 7f1c2bcc410..e90c4196513 100644 --- a/src/message/fetchActions.js +++ b/src/message/fetchActions.js @@ -24,7 +24,7 @@ import { MESSAGE_FETCH_COMPLETE, } from '../actionConstants'; import { FIRST_UNREAD_ANCHOR, LAST_MESSAGE_ANCHOR } from '../anchor'; -import { ALL_PRIVATE_NARROW } from '../utils/narrow'; +import { ALL_PRIVATE_NARROW, apiNarrowOfNarrow } from '../utils/narrow'; import { BackoffMachine } from '../utils/async'; import { initNotifications } from '../notification/notificationActions'; import { addToOutbox, sendOutbox } from '../outbox/outboxActions'; @@ -88,6 +88,7 @@ export const fetchMessages = (fetchArgs: {| try { const { messages, found_newest, found_oldest } = await api.getMessages(getAuth(getState()), { ...fetchArgs, + narrow: apiNarrowOfNarrow(fetchArgs.narrow), useFirstUnread: fetchArgs.anchor === FIRST_UNREAD_ANCHOR, // TODO: don't use this; see #4203 }); dispatch( @@ -235,7 +236,7 @@ export const fetchMessagesInNarrow = ( const fetchPrivateMessages = () => async (dispatch: Dispatch, getState: GetState) => { const auth = getAuth(getState()); const { messages, found_newest, found_oldest } = await api.getMessages(auth, { - narrow: ALL_PRIVATE_NARROW, + narrow: apiNarrowOfNarrow(ALL_PRIVATE_NARROW), anchor: LAST_MESSAGE_ANCHOR, numBefore: 100, numAfter: 0, diff --git a/src/reduxTypes.js b/src/reduxTypes.js index 5a1a42d2255..2926af269d6 100644 --- a/src/reduxTypes.js +++ b/src/reduxTypes.js @@ -21,7 +21,6 @@ import type { CrossRealmBot, RealmEmojiById, RealmFilter, - Narrow, Stream, StreamUnreadItem, Subscription, @@ -30,7 +29,7 @@ import type { UserPresence, UserStatusMapObject, } from './api/apiTypes'; - +import type { Narrow } from './utils/narrow'; import type { SessionState } from './session/sessionReducer'; export type * from './actionTypes'; diff --git a/src/search/SearchMessagesCard.js b/src/search/SearchMessagesCard.js index f38dcc67b28..640f7278ca5 100644 --- a/src/search/SearchMessagesCard.js +++ b/src/search/SearchMessagesCard.js @@ -45,7 +45,7 @@ export default class SearchMessagesCard extends PureComponent { return ; } - const renderedMessages = renderMessages(messages, []); + const renderedMessages = renderMessages(messages, HOME_NARROW); return ( diff --git a/src/subscriptions/subscriptionSelectors.js b/src/subscriptions/subscriptionSelectors.js index 4d7436867f4..deac6b6de23 100644 --- a/src/subscriptions/subscriptionSelectors.js +++ b/src/subscriptions/subscriptionSelectors.js @@ -2,7 +2,7 @@ import { createSelector } from 'reselect'; import type { GlobalState, Narrow, Selector, Stream, Subscription } from '../types'; -import { isStreamOrTopicNarrow } from '../utils/narrow'; +import { isStreamOrTopicNarrow, streamNameOfNarrow } from '../utils/narrow'; import { getSubscriptions, getStreams } from '../directSelectors'; /** @@ -39,8 +39,9 @@ export const getIsActiveStreamSubscribed: Selector = createSele if (!isStreamOrTopicNarrow(narrow)) { return true; } + const streamName = streamNameOfNarrow(narrow); - return subscriptions.find(sub => narrow[0].operand === sub.name) !== undefined; + return subscriptions.find(sub => streamName === sub.name) !== undefined; }, ); @@ -79,7 +80,9 @@ export const getIsActiveStreamAnnouncementOnly: Selector = crea if (!isStreamOrTopicNarrow(narrow)) { return false; } - const stream = streams.find(stream_ => narrow[0].operand === stream_.name); + const streamName = streamNameOfNarrow(narrow); + + const stream = streams.find(stream_ => streamName === stream_.name); return stream ? stream.is_announcement_only : false; }, ); diff --git a/src/title-buttons/ExtraNavButtonStream.js b/src/title-buttons/ExtraNavButtonStream.js index 94ee3da0be9..be44468be5d 100644 --- a/src/title-buttons/ExtraNavButtonStream.js +++ b/src/title-buttons/ExtraNavButtonStream.js @@ -8,6 +8,7 @@ import { connect } from '../react-redux'; import { getStreams } from '../selectors'; import NavButton from '../nav/NavButton'; import { navigateToTopicList } from '../actions'; +import { streamNameOfNarrow } from '../utils/narrow'; type Props = $ReadOnly<{| dispatch: Dispatch, @@ -19,7 +20,8 @@ type Props = $ReadOnly<{| class ExtraNavButtonStream extends PureComponent { handlePress = () => { const { narrow, streams } = this.props; - const stream = streams.find(x => x.name === narrow[0].operand); + const streamName = streamNameOfNarrow(narrow); + const stream = streams.find(x => x.name === streamName); if (stream) { NavigationService.dispatch(navigateToTopicList(stream.stream_id)); } diff --git a/src/title-buttons/ExtraNavButtonTopic.js b/src/title-buttons/ExtraNavButtonTopic.js index 8127e58488d..a0a7e2e8903 100644 --- a/src/title-buttons/ExtraNavButtonTopic.js +++ b/src/title-buttons/ExtraNavButtonTopic.js @@ -5,7 +5,7 @@ import React, { PureComponent } from 'react'; import type { Dispatch, Narrow, Stream } from '../types'; import { connect } from '../react-redux'; import { getStreams } from '../selectors'; -import { streamNarrow } from '../utils/narrow'; +import { streamNameOfNarrow, streamNarrow } from '../utils/narrow'; import NavButton from '../nav/NavButton'; import { doNarrow } from '../actions'; @@ -19,7 +19,8 @@ type Props = $ReadOnly<{| class ExtraNavButtonTopic extends PureComponent { handlePress = () => { const { dispatch, narrow, streams } = this.props; - const stream = streams.find(x => x.name === narrow[0].operand); + const streamName = streamNameOfNarrow(narrow); + const stream = streams.find(x => x.name === streamName); if (stream) { dispatch(doNarrow(streamNarrow(stream.name))); } diff --git a/src/title-buttons/InfoNavButtonPrivate.js b/src/title-buttons/InfoNavButtonPrivate.js index 1495200cfac..fcd70b7e076 100644 --- a/src/title-buttons/InfoNavButtonPrivate.js +++ b/src/title-buttons/InfoNavButtonPrivate.js @@ -8,6 +8,7 @@ import { connect } from '../react-redux'; import NavButton from '../nav/NavButton'; import { navigateToAccountDetails } from '../actions'; import { getUserForEmail } from '../users/userSelectors'; +import { emailOfPm1to1Narrow } from '../utils/narrow'; type SelectorProps = $ReadOnly<{| userId: number, @@ -34,5 +35,5 @@ class InfoNavButtonPrivate extends PureComponent { } export default connect((state, props) => ({ - userId: getUserForEmail(state, props.narrow[0].operand).user_id, + userId: getUserForEmail(state, emailOfPm1to1Narrow(props.narrow)).user_id, }))(InfoNavButtonPrivate); diff --git a/src/title-buttons/InfoNavButtonStream.js b/src/title-buttons/InfoNavButtonStream.js index 7f5db4d6baa..3f5426aff23 100644 --- a/src/title-buttons/InfoNavButtonStream.js +++ b/src/title-buttons/InfoNavButtonStream.js @@ -8,6 +8,7 @@ import { connect } from '../react-redux'; import { getStreams } from '../selectors'; import NavButton from '../nav/NavButton'; import { navigateToStream } from '../actions'; +import { streamNameOfNarrow } from '../utils/narrow'; type Props = $ReadOnly<{| dispatch: Dispatch, @@ -19,7 +20,8 @@ type Props = $ReadOnly<{| class InfoNavButtonStream extends PureComponent { handlePress = () => { const { narrow, streams } = this.props; - const stream = streams.find(x => x.name === narrow[0].operand); + const streamName = streamNameOfNarrow(narrow); + const stream = streams.find(x => x.name === streamName); if (stream) { NavigationService.dispatch(navigateToStream(stream.stream_id)); } diff --git a/src/title/TitleStream.js b/src/title/TitleStream.js index 7f4c5d5bdd2..25eefb43548 100644 --- a/src/title/TitleStream.js +++ b/src/title/TitleStream.js @@ -7,7 +7,7 @@ import type { Narrow, Stream, Subscription, Dispatch } from '../types'; import styles, { createStyleSheet } from '../styles'; import { connect } from '../react-redux'; import StreamIcon from '../streams/StreamIcon'; -import { isTopicNarrow } from '../utils/narrow'; +import { isTopicNarrow, topicOfNarrow } from '../utils/narrow'; import { getStreamInNarrow } from '../selectors'; import { showToast } from '../utils/info'; @@ -57,11 +57,11 @@ class TitleStream extends PureComponent { {isTopicNarrow(narrow) && ( { - showToast(narrow[1].operand); + showToast(topicOfNarrow(narrow)); }} > - {narrow[1].operand} + {topicOfNarrow(narrow)} )} diff --git a/src/title/titleSelectors.js b/src/title/titleSelectors.js index 2191d732487..b0db78670c6 100644 --- a/src/title/titleSelectors.js +++ b/src/title/titleSelectors.js @@ -1,6 +1,6 @@ /* @flow strict-local */ import type { Narrow, GlobalState } from '../types'; -import { isStreamOrTopicNarrow } from '../utils/narrow'; +import { isStreamOrTopicNarrow, streamNameOfNarrow } from '../utils/narrow'; import { getSubscriptionsByName } from '../subscriptions/subscriptionSelectors'; export const DEFAULT_TITLE_BACKGROUND_COLOR = 'transparent'; @@ -16,6 +16,6 @@ export const getTitleBackgroundColor = (state: GlobalState, narrow?: Narrow) => if (!narrow || !isStreamOrTopicNarrow(narrow)) { return DEFAULT_TITLE_BACKGROUND_COLOR; } - const streamName = narrow[0].operand; + const streamName = streamNameOfNarrow(narrow); return subscriptionsByName.get(streamName)?.color ?? 'gray'; }; diff --git a/src/topics/topicActions.js b/src/topics/topicActions.js index 255528d9219..0b7efe74bc3 100644 --- a/src/topics/topicActions.js +++ b/src/topics/topicActions.js @@ -2,7 +2,7 @@ import type { GetState, Dispatch, Narrow, Topic, Action, Outbox, Stream } from '../types'; import * as api from '../api'; import { INIT_TOPICS } from '../actionConstants'; -import { isStreamNarrow } from '../utils/narrow'; +import { isStreamNarrow, streamNameOfNarrow } from '../utils/narrow'; import { getAuth, getStreams } from '../selectors'; import { deleteOutboxMessage } from '../actions'; import { getOutbox } from '../directSelectors'; @@ -29,9 +29,10 @@ export const fetchTopicsForStream = (narrow: Narrow) => async ( if (!isStreamNarrow(narrow)) { return; } + const streamName = streamNameOfNarrow(narrow); const streams = getStreams(state); - const stream = streams.find(sub => narrow[0].operand === sub.name); + const stream = streams.find(sub => streamName === sub.name); if (!stream) { return; } diff --git a/src/topics/topicSelectors.js b/src/topics/topicSelectors.js index ab0e6268e6c..4d39beacbac 100644 --- a/src/topics/topicSelectors.js +++ b/src/topics/topicSelectors.js @@ -17,7 +17,7 @@ import { getMute, getStreams, getTopics, getUnreadStreams } from '../directSelec import { getShownMessagesForNarrow } from '../chat/narrowsSelectors'; import { getStreamsById } from '../subscriptions/subscriptionSelectors'; import { NULL_ARRAY } from '../nullObjects'; -import { isStreamNarrow } from '../utils/narrow'; +import { isStreamNarrow, streamNameOfNarrow } from '../utils/narrow'; export const getTopicsForNarrow: Selector = createSelector( (state, narrow) => narrow, @@ -27,8 +27,9 @@ export const getTopicsForNarrow: Selector = createSelector( if (!isStreamNarrow(narrow)) { return NULL_ARRAY; } - const stream = streams.find(x => x.name === narrow[0].operand); + const streamName = streamNameOfNarrow(narrow); + const stream = streams.find(x => x.name === streamName); if (!stream || !topics[stream.stream_id]) { return NULL_ARRAY; } diff --git a/src/types.js b/src/types.js index 9300b42fd1b..c0cb11b2b99 100644 --- a/src/types.js +++ b/src/types.js @@ -10,6 +10,7 @@ import type { PmKeyUsers } from './utils/recipient'; export type * from './generics'; export type * from './reduxTypes'; export type * from './api/apiTypes'; +export type { Narrow } from './utils/narrow'; export { ensureUnreachable } from './generics'; diff --git a/src/typing/typingSelectors.js b/src/typing/typingSelectors.js index 447c324109e..61b928822cc 100644 --- a/src/typing/typingSelectors.js +++ b/src/typing/typingSelectors.js @@ -3,7 +3,7 @@ import { createSelector } from 'reselect'; import type { Narrow, Selector, UserOrBot } from '../types'; import { getTyping } from '../directSelectors'; -import { isPmNarrow } from '../utils/narrow'; +import { emailsOfPmNarrow, isPmNarrow } from '../utils/narrow'; import { normalizeRecipientsAsUserIds } from '../utils/recipient'; import { NULL_ARRAY, NULL_USER } from '../nullObjects'; import { getAllUsersById, getAllUsersByEmail } from '../users/userSelectors'; @@ -18,7 +18,7 @@ export const getCurrentTypingUsers: Selector<$ReadOnlyArray, Narrow> return NULL_ARRAY; } - const recipients = narrow[0].operand.split(',').map(email => { + const recipients = emailsOfPmNarrow(narrow).map(email => { const userId = allUsersByEmail.get(email)?.user_id; if (userId === undefined) { throw new Error(`Narrow contains email '${email}' that does not map to any user.`); diff --git a/src/unread/unreadSelectors.js b/src/unread/unreadSelectors.js index 1b623973117..da982d94c89 100644 --- a/src/unread/unreadSelectors.js +++ b/src/unread/unreadSelectors.js @@ -14,13 +14,7 @@ import { import { getOwnEmail, getAllUsersByEmail } from '../users/userSelectors'; import { getSubscriptionsById } from '../subscriptions/subscriptionSelectors'; import { isTopicMuted } from '../utils/message'; -import { - isHomeNarrow, - isStreamNarrow, - isTopicNarrow, - isGroupPmNarrow, - is1to1PmNarrow, -} from '../utils/narrow'; +import { caseNarrow } from '../utils/narrow'; import { NULL_SUBSCRIPTION, NULL_USER } from '../nullObjects'; /** The number of unreads in each stream, excluding muted topics, by stream ID. */ @@ -219,7 +213,8 @@ export const getUnreadByHuddlesMentionsAndPMs: Selector = createSelector /** * Total number of unreads in the given narrow... mostly. * - * For the @-mention narrow and search narrows, just returns 0. + * For the @-mention narrow, all-PMs narrow, and search narrows, + * just returns 0. * * For the all-messages narrow, the caveats on `getUnreadTotal` apply. */ @@ -244,64 +239,66 @@ export const getUnreadCountForNarrow: Selector = createSelector( unreadPms, mute, ) => { - if (isHomeNarrow(narrow)) { - return unreadTotal; - } - - if (isStreamNarrow(narrow)) { - const stream = streams.find(s => s.name === narrow[0].operand); + const sumLengths = unreads => unreads.reduce((sum, x) => sum + x.unread_message_ids.length, 0); - if (!stream) { - return 0; - } + return caseNarrow(narrow, { + home: () => unreadTotal, - return unreadStreams - .filter(x => x.stream_id === stream.stream_id) - .reduce( - (sum, x) => - sum + (isTopicMuted(stream.name, x.topic, mute) ? 0 : x.unread_message_ids.length), - 0, + stream: name => { + const stream = streams.find(s => s.name === name); + if (!stream) { + return 0; + } + return sumLengths( + unreadStreams.filter( + x => x.stream_id === stream.stream_id && !isTopicMuted(name, x.topic, mute), + ), ); - } - - if (isTopicNarrow(narrow)) { - const stream = streams.find(s => s.name === narrow[0].operand); - - if (!stream) { - return 0; - } + }, - return unreadStreams - .filter(x => x.stream_id === stream.stream_id && x.topic === narrow[1].operand) - .reduce((sum, x) => sum + x.unread_message_ids.length, 0); - } + topic: (streamName, topic) => { + const stream = streams.find(s => s.name === streamName); + if (!stream) { + return 0; + } + return sumLengths( + unreadStreams.filter(x => x.stream_id === stream.stream_id && x.topic === topic), + ); + }, - if (isGroupPmNarrow(narrow)) { - const userIds = [...narrow[0].operand.split(','), ownEmail] - .map(email => (allUsersByEmail.get(email) || NULL_USER).user_id) - .sort((a, b) => a - b) - .join(','); - const unread = unreadHuddles.find(x => x.user_ids_string === userIds); - return unread ? unread.unread_message_ids.length : 0; - } + pm: emails => { + if (emails.length > 1) { + const userIds = [...emails, ownEmail] + .map(email => (allUsersByEmail.get(email) || NULL_USER).user_id) + .sort((a, b) => a - b) + .join(','); + const unread = unreadHuddles.find(x => x.user_ids_string === userIds); + return unread ? unread.unread_message_ids.length : 0; + } else { + const sender = allUsersByEmail.get(emails[0]); + if (!sender) { + return 0; + } + const unread = unreadPms.find(x => x.sender_id === sender.user_id); + return unread ? unread.unread_message_ids.length : 0; + } + }, - if (is1to1PmNarrow(narrow)) { - const sender = allUsersByEmail.get(narrow[0].operand); - if (!sender) { - return 0; - } - const unread = unreadPms.find(x => x.sender_id === sender.user_id); - return unread ? unread.unread_message_ids.length : 0; - } + // Unread starred messages are impossible, so 0 is correct for the + // starred-messages narrow. + // TODO: fact-check that. + starred: () => 0, - // Unread starred messages are impossible, so 0 is correct for the - // starred-messages narrow. - // TODO: fact-check that. + // TODO: give a correct answer for the @-mentions narrow. + mentioned: () => 0, - // TODO: give a correct answer for the @-mentions narrow. + // For search narrows, shrug, a bogus answer of 0 is fine. + search: () => 0, - // For search narrows, shrug, a bogus answer of 0 is fine. - - return 0; + // For the all-PMs narrow, a bogus answer of 0 is perfectly fine + // because we never use this selector for that narrow (because we + // don't expose it as one you can narrow to in the UI.) + allPrivate: () => 0, + }); }, ); diff --git a/src/users/usersActions.js b/src/users/usersActions.js index 74e5cfbe39e..3d44f4abdd3 100644 --- a/src/users/usersActions.js +++ b/src/users/usersActions.js @@ -5,7 +5,7 @@ import type { Auth, Dispatch, GetState, GlobalState, Narrow } from '../types'; import * as api from '../api'; import { PRESENCE_RESPONSE } from '../actionConstants'; import { getAuth, tryGetAuth, getServerVersion } from '../selectors'; -import { isPmNarrow, caseNarrowPartial } from '../utils/narrow'; +import { isPmNarrow, emailsOfPmNarrow } from '../utils/narrow'; import { getAllUsersByEmail, getUserForId } from './userSelectors'; import { ZulipVersion } from '../utils/zulipVersion'; @@ -65,9 +65,7 @@ export const sendTypingStart = (narrow: Narrow) => async ( } const allUsersByEmail = getAllUsersByEmail(getState()); - const recipientIds = caseNarrowPartial(narrow, { - pm: emails => emails, - }).map(email => { + const recipientIds = emailsOfPmNarrow(narrow).map(email => { const user = allUsersByEmail.get(email); if (!user) { throw new Error('unknown user'); diff --git a/src/utils/__tests__/narrow-test.js b/src/utils/__tests__/narrow-test.js index 05f34029074..d11d60e4620 100644 --- a/src/utils/__tests__/narrow-test.js +++ b/src/utils/__tests__/narrow-test.js @@ -26,41 +26,29 @@ import { MENTIONED_NARROW, pm1to1NarrowFromUser, keyFromNarrow, + streamNameOfNarrow, + topicOfNarrow, + emailOfPm1to1Narrow, } from '../narrow'; import type { Narrow, Message } from '../../types'; import * as eg from '../../__tests__/lib/exampleData'; describe('HOME_NARROW', () => { - test('produces an empty list', () => { - expect(HOME_NARROW).toEqual([]); - }); - - test('empty list is a home narrow', () => { - expect(isHomeNarrow([])).toBe(true); + test('is a home narrow', () => { + expect(isHomeNarrow(HOME_NARROW)).toBe(true); }); }); describe('pmNarrowFromEmail', () => { - test('produces an one item list, pm-with operator and single email', () => { - expect(pmNarrowFromEmail(eg.otherUser.email)).toEqual([ - { - operator: 'pm-with', - operand: eg.otherUser.email, - }, - ]); + test('produces a 1:1 narrow', () => { + const narrow = pmNarrowFromEmail(eg.otherUser.email); + expect(is1to1PmNarrow(narrow)).toBeTrue(); + expect(emailOfPm1to1Narrow(narrow)).toEqual(eg.otherUser.email); }); test('if operator is "pm-with" and only one email, then it is a private narrow', () => { expect(is1to1PmNarrow(HOME_NARROW)).toBe(false); expect(is1to1PmNarrow(pmNarrowFromEmail(eg.otherUser.email))).toBe(true); - expect( - is1to1PmNarrow([ - { - operator: 'pm-with', - operand: eg.otherUser.email, - }, - ]), - ).toBe(true); }); }); @@ -70,22 +58,6 @@ describe('isPmNarrow', () => { expect(isPmNarrow(HOME_NARROW)).toBe(false); expect(isPmNarrow(pmNarrowFromEmail(eg.otherUser.email))).toBe(true); expect(isPmNarrow(pmNarrowFromUsersUnsafe([eg.otherUser, eg.thirdUser]))).toBe(true); - expect( - isPmNarrow([ - { - operator: 'pm-with', - operand: eg.otherUser.email, - }, - ]), - ).toBe(true); - expect( - isPmNarrow([ - { - operator: 'pm-with', - operand: [eg.otherUser.email, eg.thirdUser.email].join(','), - }, - ]), - ).toBe(true); }); }); @@ -104,80 +76,44 @@ describe('isStreamOrTopicNarrow', () => { }); describe('specialNarrow', () => { - test('produces a narrow with "is" operator', () => { - expect(STARRED_NARROW).toEqual([ - { - operator: 'is', - operand: 'starred', - }, - ]); - }); - test('only narrowing with the "is" operator is special narrow', () => { expect(isSpecialNarrow(undefined)).toBe(false); expect(isSpecialNarrow(HOME_NARROW)).toBe(false); expect(isSpecialNarrow(streamNarrow('some stream'))).toBe(false); expect(isSpecialNarrow(STARRED_NARROW)).toBe(true); - expect(isSpecialNarrow([{ operator: 'stream', operand: 'some stream' }])).toBe(false); - expect(isSpecialNarrow([{ operator: 'is', operand: 'starred' }])).toBe(true); }); }); describe('streamNarrow', () => { test('narrows to messages from a specific stream', () => { - expect(streamNarrow('some stream')).toEqual([ - { - operator: 'stream', - operand: 'some stream', - }, - ]); + const narrow = streamNarrow('some stream'); + expect(isStreamNarrow(narrow)).toBeTrue(); + expect(streamNameOfNarrow(narrow)).toEqual('some stream'); }); test('only narrow with operator of "stream" is a stream narrow', () => { expect(isStreamNarrow(undefined)).toBe(false); expect(isStreamNarrow(HOME_NARROW)).toBe(false); expect(isStreamNarrow(streamNarrow('some stream'))).toBe(true); - expect(isStreamNarrow([{ operator: 'stream', operand: 'some stream' }])).toBe(true); }); }); describe('topicNarrow', () => { test('narrows to a specific topic within a specified stream', () => { - expect(topicNarrow('some stream', 'some topic')).toEqual([ - { operator: 'stream', operand: 'some stream' }, - { operator: 'topic', operand: 'some topic' }, - ]); + const narrow = topicNarrow('some stream', 'some topic'); + expect(isTopicNarrow(narrow)).toBeTrue(); + expect(streamNameOfNarrow(narrow)).toEqual('some stream'); + expect(topicOfNarrow(narrow)).toEqual('some topic'); }); test('only narrow with two items, one for stream, one for topic is a topic narrow', () => { expect(isTopicNarrow(undefined)).toBe(false); expect(isTopicNarrow(HOME_NARROW)).toBe(false); expect(isTopicNarrow(topicNarrow('some stream', 'some topic'))).toBe(true); - expect( - isTopicNarrow([ - { - operator: 'stream', - operand: 'some stream', - }, - { - operator: 'topic', - operand: 'some topic', - }, - ]), - ).toBe(true); }); }); describe('SEARCH_NARROW', () => { - test('produces a narrow for a search query', () => { - expect(SEARCH_NARROW('some query')).toEqual([ - { - operator: 'search', - operand: 'some query', - }, - ]); - }); - test('narrow with "search" operand is a search narrow', () => { expect(isSearchNarrow(undefined)).toBe(false); expect(isSearchNarrow(HOME_NARROW)).toBe(false); diff --git a/src/utils/message.js b/src/utils/message.js index ba0bce5ef65..a5e3be9cead 100644 --- a/src/utils/message.js +++ b/src/utils/message.js @@ -1,6 +1,6 @@ /* @flow strict-local */ import type { Narrow, Message, MuteState, Outbox, Subscription } from '../types'; -import { isTopicNarrow } from './narrow'; +import { isHomeNarrow, isTopicNarrow } from './narrow'; import { streamNameOfStreamMessage } from './recipient'; export const isTopicMuted = (stream: string, topic: string, mute: MuteState = []): boolean => @@ -22,7 +22,7 @@ export const shouldBeMuted = ( const streamName = streamNameOfStreamMessage(message); - if (narrow.length === 0) { + if (isHomeNarrow(narrow)) { const sub = subscriptions.find(x => x.name === streamName); if (!sub || !sub.in_home_view) { return true; diff --git a/src/utils/narrow.js b/src/utils/narrow.js index a40e1f5a019..5ffed03fe2b 100644 --- a/src/utils/narrow.js +++ b/src/utils/narrow.js @@ -1,7 +1,8 @@ /* @flow strict-local */ +import invariant from 'invariant'; import isEqual from 'lodash.isequal'; -import type { Narrow, Message, Outbox, User, UserOrBot } from '../types'; +import type { ApiNarrow, Message, Outbox, User, UserOrBot } from '../types'; import { normalizeRecipientsSansMe, pmKeyRecipientsFromMessage, @@ -13,6 +14,25 @@ import { /* eslint-disable no-use-before-define */ +/** + * A narrow. + * + * A narrow is the navigational property that defines what to show in the + * message list UI: it's a subset of messages which might be a conversation, + * a whole stream, a search, or a few other varieties. + * + * Much of the data we fetch is to support the message list UI, and so we + * keep much of it in data structures indexed by narrow. + * + * See also: + * * `keyFromNarrow`, for converting into a string key good for indexing + * into a data structure; + * * `caseNarrow` and its relatives, for pattern-matching or destructuring; + * * `ApiNarrow` for the form we put a narrow in when talking to the + * server, and `apiNarrowOfNarrow` for converting to it. + */ +export opaque type Narrow = ApiNarrow; + export const isSameNarrow = (narrow1: Narrow, narrow2: Narrow): boolean => Array.isArray(narrow1) && Array.isArray(narrow2) && isEqual(narrow1, narrow2); @@ -365,6 +385,54 @@ export const emailsOfGroupPmNarrow = (narrow: Narrow): string[] => }, }); +/** + * The "other" user's email for a 1:1 PM narrow; else error. + * + * With "other" in scare-quotes because for the self-PM narrow, this is the + * self user. + * + * Any caller of this probably should be getting a whole user object instead + * of a Narrow in the first place. And then that wrinkle about the self-PM + * narrow vs. other 1:1 narrows is a UI decision that can get made + * explicitly at the appropriate spot. + */ +export const emailOfPm1to1Narrow = (narrow: Narrow): string => + caseNarrowPartial(narrow, { + pm: emails => { + invariant(emails.length === 1, 'emailOfPm1to1Narrow: got group-PM narrow'); + return emails[0]; + }, + }); + +/** + * The "PM key recipients" emails for a PM narrow; else error. + * + * This is the same list of users that can appear in a `PmKeyRecipients` or + * `PmKeyUsers`, but contains only their emails. + */ +export const emailsOfPmNarrow = (narrow: Narrow): string[] => + caseNarrowPartial(narrow, { pm: emails => emails }); + +/** + * The stream name for a stream or topic narrow; else error. + * + * Most callers of this should probably be getting passed a stream name + * instead of a Narrow in the first place; or if they do handle other kinds + * of narrows, should be using `caseNarrow`. + */ +export const streamNameOfNarrow = (narrow: Narrow): string => + caseNarrowPartial(narrow, { stream: name => name, topic: streamName => streamName }); + +/** + * The topic for a topic narrow; else error. + * + * Most callers of this should probably be getting passed a topic (and a + * stream name) instead of a Narrow in the first place; or if they do handle + * other kinds of narrows, should be using `caseNarrow`. + */ +export const topicOfNarrow = (narrow: Narrow): string => + caseNarrowPartial(narrow, { topic: (streamName, topic) => topic }); + export const isPmNarrow = (narrow?: Narrow): boolean => !!narrow && caseNarrowDefault(narrow, { pm: () => true }, () => false); @@ -391,6 +459,11 @@ export const isStreamOrTopicNarrow = (narrow?: Narrow): boolean => export const isSearchNarrow = (narrow?: Narrow): boolean => !!narrow && caseNarrowDefault(narrow, { search: () => true }, () => false); +/** + * Convert the narrow into the form used in the Zulip API at get-messages. + */ +export const apiNarrowOfNarrow = (narrow: Narrow): ApiNarrow => narrow; + /** * True just if the given message is part of the given narrow. *