diff --git a/src/__tests__/lib/exampleData.js b/src/__tests__/lib/exampleData.js index eb52da4c7c8..3253039e51c 100644 --- a/src/__tests__/lib/exampleData.js +++ b/src/__tests__/lib/exampleData.js @@ -39,6 +39,7 @@ import rootReducer from '../../boot/reducers'; import { authOfAccount } from '../../account/accountMisc'; import { HOME_NARROW } from '../../utils/narrow'; import type { BackgroundData } from '../../webview/MessageList'; +import { getStreamsById, getStreamsByName } from '../../selectors'; /* ======================================================================== * Utilities @@ -755,7 +756,10 @@ export const backgroundData: BackgroundData = deepFreeze({ mute: [], mutedUsers: Immutable.Map(), ownUser: selfUser, + streams: getStreamsById(baseReduxState), + streamsByName: getStreamsByName(baseReduxState), subscriptions: [], + unread: baseReduxState.unread, theme: 'default', twentyFourHourTime: false, }); diff --git a/src/message/__tests__/messageActionSheet-test.js b/src/message/__tests__/messageActionSheet-test.js index 87970c4f754..8b4c7fa5c26 100644 --- a/src/message/__tests__/messageActionSheet-test.js +++ b/src/message/__tests__/messageActionSheet-test.js @@ -1,10 +1,11 @@ // @flow strict-local import deepFreeze from 'deep-freeze'; import { HOME_NARROW } from '../../utils/narrow'; -import { streamNameOfStreamMessage } from '../../utils/recipient'; import * as eg from '../../__tests__/lib/exampleData'; -import { constructMessageActionButtons, constructHeaderActionButtons } from '../messageActionSheet'; +import { constructMessageActionButtons, constructTopicActionButtons } from '../messageActionSheet'; +import { reducer } from '../../unread/unreadModel'; +import { initialState } from '../../unread/__tests__/unread-testlib'; const buttonTitles = buttons => buttons.map(button => button.title); @@ -43,61 +44,93 @@ describe('constructActionButtons', () => { }); }); -describe('constructHeaderActionButtons', () => { +describe('constructTopicActionButtons', () => { + const stream = eg.makeStream(); + const streamMessage = eg.streamMessage({ stream }); + const topic = streamMessage.subject; + const streamId = streamMessage.stream_id; + const streams = deepFreeze(new Map([[stream.stream_id, stream]])); + + const baseState = (() => { + const r = (state, action) => reducer(state, action, eg.plusReduxState); + let state = initialState; + state = r(state, eg.mkActionEventNewMessage(streamMessage)); + return state; + })(); + + test('show mark as read if topic is unread', () => { + const unread = baseState; + const buttons = constructTopicActionButtons({ + backgroundData: { ...eg.backgroundData, streams, unread }, + streamId, + topic, + }); + expect(buttonTitles(buttons)).toContain('Mark topic as read'); + }); + + test('do not show mark as read if topic is read', () => { + const buttons = constructTopicActionButtons({ + backgroundData: { ...eg.backgroundData, streams }, + streamId, + topic, + }); + expect(buttonTitles(buttons)).not.toContain('Mark topic as read'); + }); + test('show Unmute topic option if topic is muted', () => { - const mute = deepFreeze([['electron issues', 'issue #556']]); - const buttons = constructHeaderActionButtons({ - backgroundData: { ...eg.backgroundData, mute }, - stream: 'electron issues', - topic: 'issue #556', + const mute = deepFreeze([[stream.name, topic]]); + const buttons = constructTopicActionButtons({ + backgroundData: { ...eg.backgroundData, streams, mute }, + streamId, + topic, }); expect(buttonTitles(buttons)).toContain('Unmute topic'); }); test('show mute topic option if topic is not muted', () => { - const buttons = constructHeaderActionButtons({ - backgroundData: { ...eg.backgroundData, mute: [] }, - stream: streamNameOfStreamMessage(eg.streamMessage()), - topic: eg.streamMessage().subject, + const buttons = constructTopicActionButtons({ + backgroundData: { ...eg.backgroundData, streams, mute: [] }, + streamId, + topic, }); expect(buttonTitles(buttons)).toContain('Mute topic'); }); test('show Unmute stream option if stream is not in home view', () => { - const subscriptions = [{ ...eg.subscription, in_home_view: false }]; - const buttons = constructHeaderActionButtons({ - backgroundData: { ...eg.backgroundData, subscriptions }, - stream: streamNameOfStreamMessage(eg.streamMessage()), - topic: eg.streamMessage().subject, + const subscriptions = [{ ...eg.subscription, in_home_view: false, ...stream }]; + const buttons = constructTopicActionButtons({ + backgroundData: { ...eg.backgroundData, subscriptions, streams }, + streamId, + topic, }); expect(buttonTitles(buttons)).toContain('Unmute stream'); }); test('show mute stream option if stream is in home view', () => { - const subscriptions = [{ ...eg.subscription, in_home_view: true }]; - const buttons = constructHeaderActionButtons({ - backgroundData: { ...eg.backgroundData, subscriptions }, - stream: streamNameOfStreamMessage(eg.streamMessage()), - topic: eg.streamMessage().subject, + const subscriptions = [{ ...eg.subscription, in_home_view: true, ...stream }]; + const buttons = constructTopicActionButtons({ + backgroundData: { ...eg.backgroundData, subscriptions, streams }, + streamId, + topic, }); expect(buttonTitles(buttons)).toContain('Mute stream'); }); test('show delete topic option if current user is an admin', () => { const ownUser = { ...eg.selfUser, is_admin: true }; - const buttons = constructHeaderActionButtons({ - backgroundData: { ...eg.backgroundData, ownUser }, - stream: streamNameOfStreamMessage(eg.streamMessage()), - topic: eg.streamMessage().subject, + const buttons = constructTopicActionButtons({ + backgroundData: { ...eg.backgroundData, ownUser, streams }, + streamId, + topic, }); expect(buttonTitles(buttons)).toContain('Delete topic'); }); test('do not show delete topic option if current user is not an admin', () => { - const buttons = constructHeaderActionButtons({ - backgroundData: eg.backgroundData, - stream: streamNameOfStreamMessage(eg.streamMessage()), - topic: eg.streamMessage().subject, + const buttons = constructTopicActionButtons({ + backgroundData: { ...eg.backgroundData, streams }, + streamId, + topic, }); expect(buttonTitles(buttons)).not.toContain('Delete topic'); }); diff --git a/src/message/messageActionSheet.js b/src/message/messageActionSheet.js index 7c2e508d7bd..6374a341f87 100644 --- a/src/message/messageActionSheet.js +++ b/src/message/messageActionSheet.js @@ -1,5 +1,6 @@ /* @flow strict-local */ import { Clipboard, Share, Alert } from 'react-native'; +import invariant from 'invariant'; import * as NavigationService from '../nav/NavigationService'; import type { @@ -14,7 +15,9 @@ import type { Subscription, User, EditMessage, + Stream, } from '../types'; +import type { UnreadState } from '../unread/unreadModelTypes'; import { getNarrowForReply, isPmNarrow, @@ -28,6 +31,7 @@ import { doNarrow, deleteOutboxMessage, navigateToEmojiPicker, navigateToStream import { navigateToMessageReactionScreen } from '../nav/navActions'; import { deleteMessagesForTopic } from '../topics/topicActions'; import * as logging from '../utils/logging'; +import { getUnreadCountForTopic } from '../unread/unreadModel'; // TODO really this belongs in a libdef. export type ShowActionSheetWithOptions = ( @@ -35,11 +39,12 @@ export type ShowActionSheetWithOptions = ( (number) => void, ) => void; -type HeaderArgs = { +type TopicArgs = { auth: Auth, - stream: string, + streamId: number, topic: string, subscriptions: Subscription[], + streams: Map, dispatch: Dispatch, _: GetText, ... @@ -55,7 +60,7 @@ type MessageArgs = { ... }; -type Button = {| +type Button = {| (Args): void | Promise, /** The label for the button. */ @@ -118,19 +123,29 @@ const deleteMessage = async ({ auth, message, dispatch }) => { deleteMessage.title = 'Delete message'; deleteMessage.errorMessage = 'Failed to delete message'; -const unmuteTopic = async ({ auth, stream, topic }) => { - await api.setTopicMute(auth, stream, topic, false); +const markTopicAsRead = async ({ auth, streamId, topic }) => { + await api.markTopicAsRead(auth, streamId, topic); +}; +markTopicAsRead.title = 'Mark topic as read'; +markTopicAsRead.errorMessage = 'Failed to mark topic as read'; + +const unmuteTopic = async ({ auth, streamId, topic, streams }) => { + const stream = streams.get(streamId); + invariant(stream !== undefined, 'Stream with provided streamId must exist.'); + await api.setTopicMute(auth, stream.name, topic, false); }; unmuteTopic.title = 'Unmute topic'; unmuteTopic.errorMessage = 'Failed to unmute topic'; -const muteTopic = async ({ auth, stream, topic }) => { - await api.setTopicMute(auth, stream, topic, true); +const muteTopic = async ({ auth, streamId, topic, streams }) => { + const stream = streams.get(streamId); + invariant(stream !== undefined, 'Stream with provided streamId must exist.'); + await api.setTopicMute(auth, stream.name, topic, true); }; muteTopic.title = 'Mute topic'; muteTopic.errorMessage = 'Failed to mute topic'; -const deleteTopic = async ({ auth, stream, topic, dispatch, _ }) => { +const deleteTopic = async ({ auth, streamId, topic, dispatch, _ }) => { const alertTitle = _('Are you sure you want to delete the topic “{topic}”?', { topic }); const AsyncAlert = async (): Promise => new Promise((resolve, reject) => { @@ -157,35 +172,26 @@ const deleteTopic = async ({ auth, stream, topic, dispatch, _ }) => { ); }); if (await AsyncAlert()) { - await dispatch(deleteMessagesForTopic(stream, topic)); + await dispatch(deleteMessagesForTopic(streamId, topic)); } }; deleteTopic.title = 'Delete topic'; deleteTopic.errorMessage = 'Failed to delete topic'; -const unmuteStream = async ({ auth, stream, subscriptions }) => { - const sub = subscriptions.find(x => x.name === stream); - if (sub) { - await api.setSubscriptionProperty(auth, sub.stream_id, 'is_muted', false); - } +const unmuteStream = async ({ auth, streamId, subscriptions }) => { + await api.setSubscriptionProperty(auth, streamId, 'is_muted', false); }; unmuteStream.title = 'Unmute stream'; unmuteStream.errorMessage = 'Failed to unmute stream'; -const muteStream = async ({ auth, stream, subscriptions }) => { - const sub = subscriptions.find(x => x.name === stream); - if (sub) { - await api.setSubscriptionProperty(auth, sub.stream_id, 'is_muted', true); - } +const muteStream = async ({ auth, streamId, subscriptions }) => { + await api.setSubscriptionProperty(auth, streamId, 'is_muted', true); }; muteStream.title = 'Mute stream'; muteStream.errorMessage = 'Failed to mute stream'; -const showStreamSettings = ({ stream, subscriptions }) => { - const sub = subscriptions.find(x => x.name === stream); - if (sub) { - NavigationService.dispatch(navigateToStream(sub.stream_id)); - } +const showStreamSettings = ({ streamId, subscriptions }) => { + NavigationService.dispatch(navigateToStream(streamId)); }; showStreamSettings.title = 'Stream settings'; showStreamSettings.errorMessage = 'Failed to show stream settings'; @@ -226,30 +232,38 @@ const cancel = params => {}; cancel.title = 'Cancel'; cancel.errorMessage = 'Failed to hide menu'; -export const constructHeaderActionButtons = ({ - backgroundData: { mute, subscriptions, ownUser }, - stream, +export const constructTopicActionButtons = ({ + backgroundData: { mute, ownUser, streams, subscriptions, unread }, + streamId, topic, }: {| backgroundData: $ReadOnly<{ mute: MuteState, + streams: Map, subscriptions: Subscription[], + unread: UnreadState, ownUser: User, ... }>, - stream: string, + streamId: number, topic: string, -|}): Button[] => { +|}): Button[] => { const buttons = []; if (ownUser.is_admin) { buttons.push(deleteTopic); } - if (isTopicMuted(stream, topic, mute)) { + const unreadCount = getUnreadCountForTopic(unread, streamId, topic); + if (unreadCount > 0) { + buttons.push(markTopicAsRead); + } + const stream = streams.get(streamId); + invariant(stream !== undefined, 'Stream with provided streamId not found.'); + if (isTopicMuted(stream.name, topic, mute)) { buttons.push(unmuteTopic); } else { buttons.push(muteTopic); } - const sub = subscriptions.find(x => x.name === stream); + const sub = subscriptions.find(x => x.stream_id === streamId); if (sub && !sub.in_home_view) { buttons.push(unmuteStream); } else { @@ -338,10 +352,7 @@ export const constructNonHeaderActionButtons = ({ } }; -function makeButtonCallback( - buttonList: Button[], - args: Args, -) { +function makeButtonCallback(buttonList: Button[], args: Args) { return buttonIndex => { (async () => { const pressedButton: Button = buttonList[buttonIndex]; @@ -392,12 +403,12 @@ export const showMessageActionSheet = ({ ); }; -export const showHeaderActionSheet = ({ +export const showTopicActionSheet = ({ showActionSheetWithOptions, callbacks, backgroundData, topic, - stream, + streamId, }: {| showActionSheetWithOptions: ShowActionSheetWithOptions, callbacks: {| @@ -407,29 +418,33 @@ export const showHeaderActionSheet = ({ backgroundData: $ReadOnly<{ auth: Auth, mute: MuteState, + streams: Map, subscriptions: Subscription[], + unread: UnreadState, ownUser: User, flags: FlagsState, ... }>, - stream: string, + streamId: number, topic: string, |}): void => { - const buttonList = constructHeaderActionButtons({ + const buttonList = constructTopicActionButtons({ backgroundData, - stream, + streamId, topic, }); + const stream = backgroundData.streams.get(streamId); + invariant(stream !== undefined, 'Stream with provided streamId not found.'); showActionSheetWithOptions( { - title: `#${stream} > ${topic}`, + title: `#${stream.name} > ${topic}`, options: buttonList.map(button => callbacks._(button.title)), cancelButtonIndex: buttonList.length - 1, }, makeButtonCallback(buttonList, { ...backgroundData, ...callbacks, - stream, + streamId, topic, }), ); diff --git a/src/streams/TopicItem.js b/src/streams/TopicItem.js index 69f2dc83ddf..13ad1cad8f1 100644 --- a/src/streams/TopicItem.js +++ b/src/streams/TopicItem.js @@ -3,14 +3,24 @@ import React, { useContext } from 'react'; import { View } from 'react-native'; // $FlowFixMe[untyped-import] import { useActionSheet } from '@expo/react-native-action-sheet'; +import invariant from 'invariant'; import styles, { BRAND_COLOR, createStyleSheet } from '../styles'; import { RawLabel, Touchable, UnreadCount } from '../common'; -import { showHeaderActionSheet } from '../message/messageActionSheet'; +import { showTopicActionSheet } from '../message/messageActionSheet'; import type { ShowActionSheetWithOptions } from '../message/messageActionSheet'; import { TranslationContext } from '../boot/TranslationProvider'; import { useDispatch, useSelector } from '../react-redux'; -import { getAuth, getMute, getFlags, getSubscriptions, getOwnUser } from '../selectors'; +import { + getAuth, + getMute, + getFlags, + getSubscriptions, + getStreamsById, + getStreamsByName, + getOwnUser, +} from '../selectors'; +import { getUnread } from '../unread/unreadModel'; const componentStyles = createStyleSheet({ selectedRow: { @@ -28,7 +38,7 @@ const componentStyles = createStyleSheet({ }); type Props = $ReadOnly<{| - stream: string, + streamName: string, name: string, isMuted?: boolean, isSelected?: boolean, @@ -37,7 +47,7 @@ type Props = $ReadOnly<{| |}>; export default function TopicItem(props: Props) { - const { name, stream, isMuted = false, isSelected = false, unreadCount = 0, onPress } = props; + const { name, streamName, isMuted = false, isSelected = false, unreadCount = 0, onPress } = props; const showActionSheetWithOptions: ShowActionSheetWithOptions = useActionSheet() .showActionSheetWithOptions; @@ -46,20 +56,26 @@ export default function TopicItem(props: Props) { const backgroundData = useSelector(state => ({ auth: getAuth(state), mute: getMute(state), + streams: getStreamsById(state), + streamsByName: getStreamsByName(state), subscriptions: getSubscriptions(state), + unread: getUnread(state), ownUser: getOwnUser(state), flags: getFlags(state), })); + const stream = backgroundData.streamsByName.get(streamName); + invariant(stream !== undefined, 'No stream with provided stream name was found.'); + return ( onPress(stream, name)} + onPress={() => onPress(streamName, name)} onLongPress={() => { - showHeaderActionSheet({ + showTopicActionSheet({ showActionSheetWithOptions, callbacks: { dispatch, _ }, backgroundData, - stream, + streamId: stream.stream_id, topic: name, }); }} diff --git a/src/subscriptions/subscriptionSelectors.js b/src/subscriptions/subscriptionSelectors.js index 78948f0db1c..1c3e8b4815f 100644 --- a/src/subscriptions/subscriptionSelectors.js +++ b/src/subscriptions/subscriptionSelectors.js @@ -21,6 +21,11 @@ export const getStreamsById: Selector> = createSelector( streams => new Map(streams.map(stream => [stream.stream_id, stream])), ); +export const getStreamsByName: Selector> = createSelector( + getStreams, + streams => new Map(streams.map(stream => [stream.name, stream])), +); + export const getSubscriptionsById: Selector> = createSelector( getSubscriptions, subscriptions => diff --git a/src/title/TitleStream.js b/src/title/TitleStream.js index aa9204bd165..f358657761b 100644 --- a/src/title/TitleStream.js +++ b/src/title/TitleStream.js @@ -25,18 +25,23 @@ import { getMute, getFlags, getSubscriptions, + getStreamsById, getOwnUser, getStreamInNarrow, } from '../selectors'; -import { showHeaderActionSheet } from '../message/messageActionSheet'; +import { showTopicActionSheet } from '../message/messageActionSheet'; import type { ShowActionSheetWithOptions } from '../message/messageActionSheet'; +import type { UnreadState } from '../unread/unreadModelTypes'; +import { getUnread } from '../unread/unreadModel'; type SelectorProps = {| stream: Subscription | {| ...Stream, in_home_view: boolean |}, backgroundData: {| auth: Auth, mute: MuteState, + streams: Map, subscriptions: Subscription[], + unread: UnreadState, ownUser: User, flags: FlagsState, |}, @@ -74,11 +79,11 @@ const TitleStream = (props: Props) => { onLongPress={ isTopicNarrow(narrow) ? () => { - showHeaderActionSheet({ + showTopicActionSheet({ showActionSheetWithOptions, callbacks: { dispatch, _ }, backgroundData, - stream: stream.name, + streamId: stream.stream_id, topic: topicOfNarrow(narrow), }); } @@ -113,7 +118,9 @@ export default connect((state, props) => ({ backgroundData: { auth: getAuth(state), mute: getMute(state), + streams: getStreamsById(state), subscriptions: getSubscriptions(state), + unread: getUnread(state), ownUser: getOwnUser(state), flags: getFlags(state), }, diff --git a/src/topics/TopicList.js b/src/topics/TopicList.js index 733610da13d..d7750974a33 100644 --- a/src/topics/TopicList.js +++ b/src/topics/TopicList.js @@ -41,7 +41,7 @@ export default class TopicList extends PureComponent { renderItem={({ item }) => ( async ( dispatch(fetchTopics(stream.stream_id)); }; -export const deleteMessagesForTopic = (streamName: string, topic: string) => async ( +export const deleteMessagesForTopic = (streamId: number, topic: string) => async ( dispatch: Dispatch, getState: GetState, ) => { const state = getState(); const outbox = getOutbox(state); + const stream = getStreamsById(state).get(streamId); + outbox.forEach((outboxMessage: Outbox) => { if ( outboxMessage.type === 'stream' - && streamNameOfStreamMessage(outboxMessage) === streamName + // TODO: Use StreamId here (see #M3918) when `Outbox` starts + // having that property (see #M3998). + && streamNameOfStreamMessage(outboxMessage) === stream?.name && outboxMessage.subject === topic ) { dispatch(deleteOutboxMessage(outboxMessage.id)); } }); - const currentStream: Stream | void = getStreams(state).find( - (stream: Stream) => stream.name === streamName, - ); - if (currentStream) { - await api.deleteTopic(getAuth(state), currentStream.stream_id, topic); - } + await api.deleteTopic(getAuth(state), streamId, topic); }; diff --git a/src/unread/UnreadCards.js b/src/unread/UnreadCards.js index 03d3b0fbd43..a5c329bbf82 100644 --- a/src/unread/UnreadCards.js +++ b/src/unread/UnreadCards.js @@ -64,7 +64,7 @@ export default function UnreadCards(props: Props) { ) : ( , + streamsByName: Map, subscriptions: Subscription[], + unread: UnreadState, theme: ThemeName, twentyFourHourTime: boolean, |}>; @@ -307,7 +315,10 @@ const MessageList: ComponentType = connect( mute: getMute(state), mutedUsers: getMutedUsers(state), ownUser: getOwnUser(state), + streams: getStreamsById(state), + streamsByName: getStreamsByName(state), subscriptions: getSubscriptions(state), + unread: getUnread(state), theme: getSettings(state).theme, twentyFourHourTime: getRealm(state).twentyFourHourTime, }; diff --git a/src/webview/handleOutboundEvents.js b/src/webview/handleOutboundEvents.js index 5264ca7ba44..b84ea950d65 100644 --- a/src/webview/handleOutboundEvents.js +++ b/src/webview/handleOutboundEvents.js @@ -1,6 +1,7 @@ /* @flow strict-local */ import { Clipboard, Alert } from 'react-native'; +import invariant from 'invariant'; import * as NavigationService from '../nav/NavigationService'; import * as api from '../api'; import config from '../config'; @@ -23,7 +24,7 @@ import { navigateToLightbox, messageLinkPress, } from '../actions'; -import { showHeaderActionSheet, showMessageActionSheet } from '../message/messageActionSheet'; +import { showTopicActionSheet, showMessageActionSheet } from '../message/messageActionSheet'; import { ensureUnreachable } from '../types'; import { base64Utf8Decode } from '../utils/encoding'; @@ -212,11 +213,14 @@ const handleLongPress = ( const { dispatch, showActionSheetWithOptions, backgroundData, narrow, startEditMessage } = props; if (target === 'header') { if (message.type === 'stream') { - showHeaderActionSheet({ + const streamName = streamNameOfStreamMessage(message); + const stream = backgroundData.streamsByName.get(streamName); + invariant(stream !== undefined, 'No stream with provided stream name was found.'); + showTopicActionSheet({ showActionSheetWithOptions, callbacks: { dispatch, _ }, backgroundData, - stream: streamNameOfStreamMessage(message), + streamId: stream.stream_id, topic: message.subject, }); } else if (message.type === 'private') { diff --git a/static/translations/messages_en.json b/static/translations/messages_en.json index 5766aa2538d..b4867b2eeee 100644 --- a/static/translations/messages_en.json +++ b/static/translations/messages_en.json @@ -247,5 +247,6 @@ "Uploading {fileName} failed.": "Uploading {fileName} failed.", "Attachment": "Attachment", "Attachment {num}": "Attachment {num}", + "Failed to mark topic as read": "Failed to mark topic as read", "Remove account": "Remove account" }